From 3fdc3816b18dc8fff7e6f966e399a625e4b7a9e7 Mon Sep 17 00:00:00 2001 From: Mike Ciesielka Date: Mon, 2 Sep 2024 15:28:51 -0400 Subject: [PATCH 1/2] Write failing test --- .../hooks/__tests__/useFragment.test.tsx | 41 ++++++++++++++++++- 1 file changed, 40 insertions(+), 1 deletion(-) diff --git a/src/react/hooks/__tests__/useFragment.test.tsx b/src/react/hooks/__tests__/useFragment.test.tsx index f58ef9aaa6d..046c1c538c9 100644 --- a/src/react/hooks/__tests__/useFragment.test.tsx +++ b/src/react/hooks/__tests__/useFragment.test.tsx @@ -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", @@ -268,7 +307,7 @@ describe("useFragment", () => { "Item:2": { __typename: "Item", id: 2, - text: "Item #2 updated", + text: "Item #2", }, "Item:3": { __typename: "Item", From 9c268927b1f8e5921b9440a53c9979a37f594e75 Mon Sep 17 00:00:00 2001 From: Lenz Weber-Tronic Date: Tue, 3 Sep 2024 11:00:35 +0200 Subject: [PATCH 2/2] `useFragment`: keep current result on a memoized mutable object --- .changeset/fuzzy-shrimps-call.md | 5 +++++ .size-limits.json | 4 ++-- src/react/hooks/useFragment.ts | 37 +++++++++++++++----------------- 3 files changed, 24 insertions(+), 22 deletions(-) create mode 100644 .changeset/fuzzy-shrimps-call.md diff --git a/.changeset/fuzzy-shrimps-call.md b/.changeset/fuzzy-shrimps-call.md new file mode 100644 index 00000000000..ad484ae08ba --- /dev/null +++ b/.changeset/fuzzy-shrimps-call.md @@ -0,0 +1,5 @@ +--- +"@apollo/client": patch +--- + +Fix a bug where `useFragment` did not re-render as expected diff --git a/.size-limits.json b/.size-limits.json index 8965193f2fe..7b0839e8f80 100644 --- a/.size-limits.json +++ b/.size-limits.json @@ -1,4 +1,4 @@ { - "dist/apollo-client.min.cjs": 40252, - "import { ApolloClient, InMemoryCache, HttpLink } from \"dist/index.js\" (production)": 33059 + "dist/apollo-client.min.cjs": 40242, + "import { ApolloClient, InMemoryCache, HttpLink } from \"dist/index.js\" (production)": 33058 } diff --git a/src/react/hooks/useFragment.ts b/src/react/hooks/useFragment.ts index 9cc56f05908..89ce1603a00 100644 --- a/src/react/hooks/useFragment.ts +++ b/src/react/hooks/useFragment.ts @@ -75,30 +75,28 @@ function _useFragment( [cache, from] ); - const resultRef = React.useRef>(); 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({ - ...stableOptions, - returnPartialData: true, - id: from, - query: cache["getFragmentDoc"](fragment, fragmentName), - optimistic, - }) - ); + return { + result: diffToResult( + cache.diff({ + ...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( @@ -109,9 +107,9 @@ function _useFragment( // 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 @@ -121,12 +119,11 @@ function _useFragment( }, }); return () => { - resultRef.current = void 0; subscription.unsubscribe(); clearTimeout(lastTimeout); }; }, - [cache, stableOptions, currentDiff] + [cache, stableOptions, diff] ), getSnapshot, getSnapshot