Skip to content

Commit

Permalink
Fix issue apollographql#9794 by using setTimeout to delay execution o…
Browse files Browse the repository at this point in the history
…f the callback functions.

Fix broken tests and add new test for the issue.
  • Loading branch information
dylanwulf committed Jun 10, 2022
1 parent 83935e8 commit 3f551fe
Show file tree
Hide file tree
Showing 2 changed files with 50 additions and 6 deletions.
51 changes: 48 additions & 3 deletions src/react/hooks/__tests__/useQuery.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2191,9 +2191,8 @@ describe('useQuery Hook', () => {
rerender({ variables: { first: 1 } });
expect(result.current.loading).toBe(false);
expect(result.current.data).toEqual(data1);
await waitFor(() => expect(onCompleted).toHaveBeenCalledTimes(3));
expect(onCompleted).toHaveBeenLastCalledWith(data1);

expect(onCompleted).toHaveBeenCalledTimes(3);
});
});

Expand Down Expand Up @@ -2918,7 +2917,7 @@ describe('useQuery Hook', () => {

expect(result.current.loading).toBe(false);
expect(result.current.data).toEqual({ hello: 'world' });
expect(onCompleted).toHaveBeenCalledTimes(1);
await waitFor(() => expect(onCompleted).toHaveBeenCalledTimes(1));
expect(onCompleted).toHaveBeenCalledWith({ hello: 'world' });
await expect(waitForNextUpdate({ timeout: 20 })).rejects.toThrow('Timed out');
expect(onCompleted).toHaveBeenCalledTimes(1);
Expand Down Expand Up @@ -3084,6 +3083,52 @@ describe('useQuery Hook', () => {
expect(result.current.data).toEqual({ hello: 'world 3' });
expect(onCompleted).toHaveBeenCalledTimes(3);
});

// This test was added for issue https://github.com/apollographql/apollo-client/issues/9794
it("onCompleted can set state without causing react errors", async () => {
const errorSpy = jest.spyOn(console, "error");
const query = gql`
{
hello
}
`;

const cache = new InMemoryCache();
cache.writeQuery({
query,
data: { hello: "world" },
});

const ChildComponent: React.FC<{
setOnCompletedCalled: React.Dispatch<React.SetStateAction<boolean>>;
}> = ({ setOnCompletedCalled }) => {
useQuery(query, {
fetchPolicy: "cache-only",
onCompleted: () => {
setOnCompletedCalled(true);
},
});

return null;
};

const ParentComponent: React.FC = () => {
const [onCompletedCalled, setOnCompletedCalled] = useState(false);
return (
<MockedProvider mocks={[]} cache={cache}>
<div>
<ChildComponent setOnCompletedCalled={setOnCompletedCalled} />
onCompletedCalled: {String(onCompletedCalled)}
</div>
</MockedProvider>
);
};

const { findByText } = render(<ParentComponent />);
await findByText("onCompletedCalled: true");
expect(errorSpy).not.toHaveBeenCalled();
errorSpy.mockRestore();
});
});

describe('Optimistic data', () => {
Expand Down
5 changes: 2 additions & 3 deletions src/react/hooks/useQuery.ts
Original file line number Diff line number Diff line change
Expand Up @@ -517,9 +517,8 @@ class InternalState<TData, TVariables> {
// the same (===) result object, unless state.setResult has been called, or
// we're doing server rendering and therefore override the result below.
if (!this.result) {
this.handleErrorOrCompleted(
this.result = this.observable.getCurrentResult()
);
const result = (this.result = this.observable.getCurrentResult());
setTimeout(() => this.handleErrorOrCompleted(result), 0);
}
return this.result;
}
Expand Down

0 comments on commit 3f551fe

Please sign in to comment.