Skip to content
This repository has been archived by the owner on Apr 13, 2023. It is now read-only.

Commit

Permalink
This commit helps address 2 main issues:
Browse files Browse the repository at this point in the history
1. It prevents the `onError` callback from unnecessarily being
called multiple times in a row.

2. It migrates all peer and dev deps to use `apollo-client`
2.6.4. Apollo Client 2.6.4 provides a fix for last result
error tracking
(apollographql/apollo-client@c44e821),
which fixes the long standing React Apollo issue of `refetch`
not setting `loading` state properly. Calling `refetch` will
now first set `loading` to `true`.

These fixes are bundled together, as verifying that `onError`
is working properly requires being able to make sure results
after a refetch that don't have an error, are handled properly.

Fixes #3331.
Fixes #3287.
Fixes #2559.
Fixes #321.

(and probably fixes others!)
  • Loading branch information
hwillson committed Aug 10, 2019
1 parent baf9a76 commit 6eae158
Showing 16 changed files with 111 additions and 51 deletions.
2 changes: 1 addition & 1 deletion examples/hooks/client/package.json
Original file line number Diff line number Diff line change
@@ -9,7 +9,7 @@
"@types/react": "16.8.24",
"@types/react-dom": "16.8.5",
"apollo-cache-inmemory": "^1.6.0",
"apollo-client": "^2.6.3",
"apollo-client": "^2.6.4",
"apollo-link-http": "^1.5.14",
"apollo-link-ws": "^1.0.17",
"graphql": "^14.3.1",
6 changes: 3 additions & 3 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
@@ -46,7 +46,7 @@
"@types/zen-observable": "0.8.0",
"apollo-cache": "1.3.2",
"apollo-cache-inmemory": "1.6.2",
"apollo-client": "2.6.3",
"apollo-client": "2.6.4",
"apollo-link": "1.2.12",
"bundlesize": "0.18.0",
"graphql": "14.4.2",
2 changes: 1 addition & 1 deletion packages/all/package.json
Original file line number Diff line number Diff line change
@@ -33,7 +33,7 @@
"deploy": "npm publish"
},
"peerDependencies": {
"apollo-client": "^2.6.3",
"apollo-client": "^2.6.4",
"graphql": "^14.3.1",
"react": "^16.8.0",
"react-dom": "^16.8.0"
2 changes: 1 addition & 1 deletion packages/common/package.json
Original file line number Diff line number Diff line change
@@ -32,7 +32,7 @@
"test:umd": "npm run build && npx jest --config ../../config/jest.umd.config.js --testPathPattern packages/common"
},
"peerDependencies": {
"apollo-client": "^2.6.3",
"apollo-client": "^2.6.4",
"apollo-utilities": "^1.3.2",
"graphql": "^14.3.1",
"react": "^16.8.0"
2 changes: 1 addition & 1 deletion packages/components/package.json
Original file line number Diff line number Diff line change
@@ -37,7 +37,7 @@
},
"peerDependencies": {
"apollo-cache": "^1.3.2",
"apollo-client": "^2.6.3",
"apollo-client": "^2.6.4",
"apollo-link": "^1.2.12",
"apollo-utilities": "^1.3.2",
"graphql": "^14.3.1",
11 changes: 4 additions & 7 deletions packages/components/src/__tests__/client/Query.test.tsx
Original file line number Diff line number Diff line change
@@ -1524,14 +1524,11 @@ describe('Query component', () => {
});
}, 0);
break;
// Further fix required in QueryManager, we should have an extra
// step for the loading status of the third result
// case 4:
// expect(result.loading).toBeTruthy();
// expect(result.error).toBeFalsy();
// break;
case 4:
// Third result's data is loaded
expect(result.loading).toBeTruthy();
expect(result.error).toBeFalsy();
break;
case 5:
expect(result.loading).toBeFalsy();
expect(result.error).toBeFalsy();
if (!result.data) {
2 changes: 1 addition & 1 deletion packages/hoc/package.json
Original file line number Diff line number Diff line change
@@ -36,7 +36,7 @@
"test:umd": "npm run build && npx jest --config ../../config/jest.umd.config.js --testPathPattern packages/hoc"
},
"peerDependencies": {
"apollo-client": "^2.6.3",
"apollo-client": "^2.6.4",
"graphql": "^14.3.1",
"react": "^16.8.0",
"react-dom": "^16.8.0"
Original file line number Diff line number Diff line change
@@ -24,16 +24,6 @@ exports[`General use errors if the query in the mock and component do not match
, variables: {"username":"mock_username"}]
`;

exports[`General use errors if the query in the mock and component do not match 2`] = `
[Error: Network error: No more mocked responses for the query: query GetUser($username: String!) {
user(username: $username) {
id
__typename
}
}
, variables: {"username":"mock_username"}]
`;

exports[`General use errors if the variables do not deep equal 1`] = `
[Error: Network error: No more mocked responses for the query: query GetUser($username: String!) {
user(username: $username) {
@@ -44,16 +34,6 @@ exports[`General use errors if the variables do not deep equal 1`] = `
, variables: {"username":"some_user","age":42}]
`;

exports[`General use errors if the variables do not deep equal 2`] = `
[Error: Network error: No more mocked responses for the query: query GetUser($username: String!) {
user(username: $username) {
id
__typename
}
}
, variables: {"username":"some_user","age":42}]
`;

exports[`General use errors if the variables in the mock and component do not match 1`] = `
[Error: Network error: No more mocked responses for the query: query GetUser($username: String!) {
user(username: $username) {
@@ -64,16 +44,6 @@ exports[`General use errors if the variables in the mock and component do not ma
, variables: {"username":"other_user"}]
`;

exports[`General use errors if the variables in the mock and component do not match 2`] = `
[Error: Network error: No more mocked responses for the query: query GetUser($username: String!) {
user(username: $username) {
id
__typename
}
}
, variables: {"username":"other_user"}]
`;

exports[`General use mocks the data 1`] = `
Object {
"__typename": "User",
7 changes: 7 additions & 0 deletions packages/hoc/src/__tests__/queries/errors.test.tsx
Original file line number Diff line number Diff line change
@@ -428,6 +428,9 @@ describe('[queries] errors', () => {
});
break;
case 3:
expect(props.data!.loading).toBeTruthy();
break;
case 4:
expect(props.data!.loading).toBeFalsy();
expect(props.data!.error).toBeFalsy();
expect(stripSymbols(props.data!.allPeople)).toEqual(
@@ -621,7 +624,11 @@ describe('[queries] errors', () => {
}, 50);
break;
case 2:
expect(this.props.data!.loading).toBe(true);
break;
case 3:
expect(this.props.data!.loading).toBe(false);
expect(this.props.data!.allPeople).toEqual(dataTwo.allPeople);
done();
break;
default:
2 changes: 1 addition & 1 deletion packages/hooks/package.json
Original file line number Diff line number Diff line change
@@ -36,7 +36,7 @@
"test:umd": "npm run build && npx jest --config ../../config/jest.umd.config.js --testPathPattern packages/hooks"
},
"peerDependencies": {
"apollo-client": "^2.6.3",
"apollo-client": "^2.6.4",
"graphql": "^14.3.1",
"react": "^16.8.0",
"react-dom": "^16.8.0"
82 changes: 81 additions & 1 deletion packages/hooks/src/__tests__/useQuery.test.tsx
Original file line number Diff line number Diff line change
@@ -3,7 +3,10 @@ import { DocumentNode, GraphQLError } from 'graphql';
import gql from 'graphql-tag';
import { MockedProvider, MockLink } from '@apollo/react-testing';
import { render, cleanup } from '@testing-library/react';
import { useQuery } from '@apollo/react-hooks';
import { useQuery, ApolloProvider } from '@apollo/react-hooks';
import { ApolloClient } from 'apollo-client';
import { ApolloLink, Observable } from 'apollo-link';
import { InMemoryCache } from 'apollo-cache-inmemory';

describe('useQuery Hook', () => {
const CAR_QUERY: DocumentNode = gql`
@@ -239,5 +242,82 @@ describe('useQuery Hook', () => {
</MockedProvider>
);
});

it('should only call onError callbacks once', done => {
const query = gql`
query SomeQuery {
stuff {
thing
}
}
`;

const resultData = { stuff: { thing: 'it!', __typename: 'Stuff' } };

let callCount = 0;
const link = new ApolloLink(() => {
if (!callCount) {
callCount += 1;
return new Observable(observer => {
observer.error(new Error('Oh no!'));
});
} else {
return Observable.of({ data: resultData });
}
});

const client = new ApolloClient({
link,
cache: new InMemoryCache()
});

const onErrorMock = jest.fn();

let renderCount = 0;
const Component = () => {
const { loading, error, refetch, data, networkStatus } = useQuery(
query,
{
onError: onErrorMock,
notifyOnNetworkStatusChange: true
}
);

console.log('ns', networkStatus);

switch (renderCount) {
case 0:
expect(loading).toBeTruthy();
break;
case 1:
expect(loading).toBeFalsy();
expect(error).toBeDefined();
expect(error!.message).toEqual('Network error: Oh no!');
setTimeout(() => {
expect(onErrorMock.mock.calls.length).toBe(1);
refetch();
});
break;
case 2:
expect(loading).toBeTruthy();
break;
case 3:
expect(loading).toBeFalsy();
expect(data).toEqual(resultData);
done();
break;
default: // Do nothing
}

renderCount += 1;
return null;
};

render(
<ApolloProvider client={client}>
<Component />
</ApolloProvider>
);
});
});
});
5 changes: 4 additions & 1 deletion packages/hooks/src/data/QueryData.ts
Original file line number Diff line number Diff line change
@@ -285,7 +285,10 @@ export class QueryData<TData, TVariables> extends OperationData {
error: error => {
this.resubscribeToQuery();
if (!error.hasOwnProperty('graphQLErrors')) throw error;
this.forceUpdate();
if (!isEqual(error, this.previousData.error)) {
this.previousData.error = error;
this.forceUpdate();
}
}
});
}
4 changes: 3 additions & 1 deletion packages/hooks/src/types.ts
Original file line number Diff line number Diff line change
@@ -2,7 +2,8 @@ import { ReactNode } from 'react';
import {
ApolloClient,
ApolloQueryResult,
ObservableQuery
ObservableQuery,
ApolloError
} from 'apollo-client';
import { Observable } from 'apollo-link';
import {
@@ -52,6 +53,7 @@ export interface QueryPreviousData<TData, TVariables> {
result?: ApolloQueryResult<TData> | null;
loading?: boolean;
options?: QueryOptions<TData, TVariables>;
error?: ApolloError;
}

export interface QueryCurrentObservable<TData, TVariables> {
1 change: 1 addition & 0 deletions packages/hooks/src/utils/useBaseQuery.ts
Original file line number Diff line number Diff line change
@@ -34,6 +34,7 @@ export function useBaseQuery<TData = any, TVariables = OperationVariables>(
context,
tick
};

const result = useDeepMemo(
() => (lazy ? queryData.executeLazy() : queryData.execute()),
memo
2 changes: 1 addition & 1 deletion packages/testing/package.json
Original file line number Diff line number Diff line change
@@ -33,7 +33,7 @@
},
"peerDependencies": {
"apollo-cache-inmemory": "^1.6.2",
"apollo-client": "^2.6.3",
"apollo-client": "^2.6.4",
"apollo-link": "^1.2.12",
"apollo-utilities": "^1.3.2",
"graphql": "^14.3.1",

0 comments on commit 6eae158

Please sign in to comment.