Skip to content

Commit

Permalink
Merge remote-tracking branch 'origin/main' into fix-usesubscription-r…
Browse files Browse the repository at this point in the history
…estart-changed-every-render
  • Loading branch information
phryneas committed Sep 3, 2024
2 parents 7be36c8 + a67727e commit 5842753
Show file tree
Hide file tree
Showing 4 changed files with 63 additions and 22 deletions.
5 changes: 5 additions & 0 deletions .changeset/fuzzy-shrimps-call.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@apollo/client": patch
---

Fix a bug where `useFragment` did not re-render as expected
2 changes: 1 addition & 1 deletion .size-limits.json
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
{
"dist/apollo-client.min.cjs": 40258,
"dist/apollo-client.min.cjs": 40249,
"import { ApolloClient, InMemoryCache, HttpLink } from \"dist/index.js\" (production)": 33058
}
41 changes: 40 additions & 1 deletion src/react/hooks/__tests__/useFragment.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -260,6 +260,45 @@ describe("useFragment", () => {
"item 4",
]);

// set Item #2 back to its original value
act(() => {
cache.writeFragment({
fragment: ItemFragment,
data: {
__typename: "Item",
id: 2,
text: "Item #2",
},
});
});

await waitFor(() => {
expect(getItemTexts()).toEqual([
"Item #1",
"Item #2",
"Item #3 from cache.modify",
"Item #4 updated",
"Item #5",
]);
});

expect(renders).toEqual([
"list",
"item 1",
"item 2",
"item 5",
"item 2",
"list",
"item 1",
"item 2",
"item 3",
"item 4",
"item 5",
"item 4",
// Only the second item should have re-rendered.
"item 2",
]);

expect(cache.extract()).toEqual({
"Item:1": {
__typename: "Item",
Expand All @@ -268,7 +307,7 @@ describe("useFragment", () => {
"Item:2": {
__typename: "Item",
id: 2,
text: "Item #2 updated",
text: "Item #2",
},
"Item:3": {
__typename: "Item",
Expand Down
37 changes: 17 additions & 20 deletions src/react/hooks/useFragment.ts
Original file line number Diff line number Diff line change
Expand Up @@ -75,30 +75,28 @@ function _useFragment<TData = any, TVars = OperationVariables>(
[cache, from]
);

const resultRef = React.useRef<UseFragmentResult<TData>>();
const stableOptions = useDeepMemo(() => ({ ...rest, from: id! }), [rest, id]);

// Since .next is async, we need to make sure that we
// get the correct diff on the next render given new diffOptions
const currentDiff = React.useMemo(() => {
const diff = React.useMemo(() => {
const { fragment, fragmentName, from, optimistic = true } = stableOptions;

return diffToResult(
cache.diff<TData>({
...stableOptions,
returnPartialData: true,
id: from,
query: cache["getFragmentDoc"](fragment, fragmentName),
optimistic,
})
);
return {
result: diffToResult(
cache.diff<TData>({
...stableOptions,
returnPartialData: true,
id: from,
query: cache["getFragmentDoc"](fragment, fragmentName),
optimistic,
})
),
};
}, [stableOptions, cache]);

// Used for both getSnapshot and getServerSnapshot
const getSnapshot = React.useCallback(
() => resultRef.current || currentDiff,
[currentDiff]
);
const getSnapshot = React.useCallback(() => diff.result, [diff]);

return useSyncExternalStore(
React.useCallback(
Expand All @@ -109,9 +107,9 @@ function _useFragment<TData = any, TVars = OperationVariables>(
// Since `next` is called async by zen-observable, we want to avoid
// unnecessarily rerendering this hook for the initial result
// emitted from watchFragment which should be equal to
// `currentDiff`.
if (equal(result, currentDiff)) return;
resultRef.current = result;
// `diff.result`.
if (equal(result, diff.result)) return;
diff.result = result;
// If we get another update before we've re-rendered, bail out of
// the update and try again. This ensures that the relative timing
// between useQuery and useFragment stays roughly the same as
Expand All @@ -121,12 +119,11 @@ function _useFragment<TData = any, TVars = OperationVariables>(
},
});
return () => {
resultRef.current = void 0;
subscription.unsubscribe();
clearTimeout(lastTimeout);
};
},
[cache, stableOptions, currentDiff]
[cache, stableOptions, diff]
),
getSnapshot,
getSnapshot
Expand Down

0 comments on commit 5842753

Please sign in to comment.