From 4566b34b5066a77962906e3e0072374798ef2591 Mon Sep 17 00:00:00 2001 From: Jerel Miller Date: Thu, 4 Jan 2024 11:55:27 -0700 Subject: [PATCH 01/16] Create a useLazyRef hook --- src/react/hooks/internal/index.ts | 1 + src/react/hooks/internal/useLazyRef.ts | 13 +++++++++++++ 2 files changed, 14 insertions(+) create mode 100644 src/react/hooks/internal/useLazyRef.ts diff --git a/src/react/hooks/internal/index.ts b/src/react/hooks/internal/index.ts index d1c90c41f4a..db13ed03b39 100644 --- a/src/react/hooks/internal/index.ts +++ b/src/react/hooks/internal/index.ts @@ -1,4 +1,5 @@ // These hooks are used internally and are not exported publicly by the library export { useDeepMemo } from "./useDeepMemo.js"; export { useIsomorphicLayoutEffect } from "./useIsomorphicLayoutEffect.js"; +export { useLazyRef } from "./useLazyRef.js"; export { __use } from "./__use.js"; diff --git a/src/react/hooks/internal/useLazyRef.ts b/src/react/hooks/internal/useLazyRef.ts new file mode 100644 index 00000000000..645872aa9bd --- /dev/null +++ b/src/react/hooks/internal/useLazyRef.ts @@ -0,0 +1,13 @@ +import * as React from "react"; + +const INIT = {}; + +export function useLazyRef(getInitialValue: () => T) { + const ref = React.useRef(INIT as unknown as T); + + if (ref.current === INIT) { + ref.current = getInitialValue(); + } + + return ref; +} From 6652d94a1a7b15d4fd8e1ac0c3a83626711e4424 Mon Sep 17 00:00:00 2001 From: Jerel Miller Date: Fri, 5 Jan 2024 10:03:46 -0700 Subject: [PATCH 02/16] Wrap diffOptions in useDeepMemo --- src/react/hooks/useFragment.ts | 25 +++++++++++++++++-------- 1 file changed, 17 insertions(+), 8 deletions(-) diff --git a/src/react/hooks/useFragment.ts b/src/react/hooks/useFragment.ts index 33b50fa2ded..84ac053b662 100644 --- a/src/react/hooks/useFragment.ts +++ b/src/react/hooks/useFragment.ts @@ -14,6 +14,7 @@ import { useApolloClient } from "./useApolloClient.js"; import { useSyncExternalStore } from "./useSyncExternalStore.js"; import type { OperationVariables } from "../../core/index.js"; import type { NoInfer } from "../types/types.js"; +import { useDeepMemo } from "./internal/useDeepMemo.js"; export interface UseFragmentOptions extends Omit< @@ -46,15 +47,23 @@ export function useFragment( ): UseFragmentResult { const { cache } = useApolloClient(); - const { fragment, fragmentName, from, optimistic = true, ...rest } = options; + const diffOptions: Cache.DiffOptions = useDeepMemo(() => { + const { + fragment, + fragmentName, + from, + optimistic = true, + ...rest + } = options; - const diffOptions: Cache.DiffOptions = { - ...rest, - returnPartialData: true, - id: typeof from === "string" ? from : cache.identify(from), - query: cache["getFragmentDoc"](fragment, fragmentName), - optimistic, - }; + return { + ...rest, + returnPartialData: true, + id: typeof from === "string" ? from : cache.identify(from), + query: cache["getFragmentDoc"](fragment, fragmentName), + optimistic, + }; + }, [options]); const resultRef = React.useRef>(); let latestDiff = cache.diff(diffOptions); From f1ea3a1ae4c7c6f32b79712dd5f95ebe8b4b7e37 Mon Sep 17 00:00:00 2001 From: Jerel Miller Date: Fri, 5 Jan 2024 10:05:21 -0700 Subject: [PATCH 03/16] Compare results in resultRef.current --- src/react/hooks/useFragment.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/react/hooks/useFragment.ts b/src/react/hooks/useFragment.ts index 84ac053b662..f606ce42ad1 100644 --- a/src/react/hooks/useFragment.ts +++ b/src/react/hooks/useFragment.ts @@ -86,7 +86,7 @@ export function useFragment( ...diffOptions, immediate: true, callback(diff) { - if (!equal(diff, latestDiff)) { + if (!equal(diff.result, resultRef.current?.data)) { resultRef.current = diffToResult((latestDiff = diff)); lastTimeout = setTimeout(forceUpdate) as any; } From 180fe818aaf1f7ccdb9a16676b3432fbd6ef2fda Mon Sep 17 00:00:00 2001 From: Jerel Miller Date: Fri, 5 Jan 2024 10:06:26 -0700 Subject: [PATCH 04/16] Use resultRef.current in getSnapshot --- src/react/hooks/useFragment.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/react/hooks/useFragment.ts b/src/react/hooks/useFragment.ts index f606ce42ad1..8c455deb11a 100644 --- a/src/react/hooks/useFragment.ts +++ b/src/react/hooks/useFragment.ts @@ -70,7 +70,7 @@ export function useFragment( // Used for both getSnapshot and getServerSnapshot const getSnapshot = () => { - const latestDiffToResult = diffToResult(latestDiff); + const latestDiffToResult = resultRef.current || diffToResult(latestDiff); return ( resultRef.current && equal(resultRef.current.data, latestDiffToResult.data) From 6a62bed494d2e46f27ed26dcb046f674120d005c Mon Sep 17 00:00:00 2001 From: Jerel Miller Date: Fri, 5 Jan 2024 10:07:24 -0700 Subject: [PATCH 05/16] Only call cache.diff when there is no result ref --- src/react/hooks/useFragment.ts | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/react/hooks/useFragment.ts b/src/react/hooks/useFragment.ts index 8c455deb11a..b6d064cb299 100644 --- a/src/react/hooks/useFragment.ts +++ b/src/react/hooks/useFragment.ts @@ -66,11 +66,12 @@ export function useFragment( }, [options]); const resultRef = React.useRef>(); - let latestDiff = cache.diff(diffOptions); // Used for both getSnapshot and getServerSnapshot const getSnapshot = () => { - const latestDiffToResult = resultRef.current || diffToResult(latestDiff); + const latestDiffToResult = + resultRef.current || diffToResult(cache.diff(diffOptions)); + return ( resultRef.current && equal(resultRef.current.data, latestDiffToResult.data) @@ -87,7 +88,7 @@ export function useFragment( immediate: true, callback(diff) { if (!equal(diff.result, resultRef.current?.data)) { - resultRef.current = diffToResult((latestDiff = diff)); + resultRef.current = diffToResult(diff); lastTimeout = setTimeout(forceUpdate) as any; } }, From c08a56cd7e2850b7c8ce7a47e48d55c79ca8bee2 Mon Sep 17 00:00:00 2001 From: Jerel Miller Date: Fri, 5 Jan 2024 10:10:01 -0700 Subject: [PATCH 06/16] useLazyRef to get initial diff result --- src/react/hooks/useFragment.ts | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/react/hooks/useFragment.ts b/src/react/hooks/useFragment.ts index b6d064cb299..0c133c8bc95 100644 --- a/src/react/hooks/useFragment.ts +++ b/src/react/hooks/useFragment.ts @@ -1,4 +1,3 @@ -import * as React from "react"; import { equal } from "@wry/equality"; import type { DeepPartial } from "../../utilities/index.js"; @@ -14,7 +13,7 @@ import { useApolloClient } from "./useApolloClient.js"; import { useSyncExternalStore } from "./useSyncExternalStore.js"; import type { OperationVariables } from "../../core/index.js"; import type { NoInfer } from "../types/types.js"; -import { useDeepMemo } from "./internal/useDeepMemo.js"; +import { useDeepMemo, useLazyRef } from "./internal/index.js"; export interface UseFragmentOptions extends Omit< @@ -65,12 +64,13 @@ export function useFragment( }; }, [options]); - const resultRef = React.useRef>(); + const resultRef = useLazyRef>(() => + diffToResult(cache.diff(diffOptions)) + ); // Used for both getSnapshot and getServerSnapshot const getSnapshot = () => { - const latestDiffToResult = - resultRef.current || diffToResult(cache.diff(diffOptions)); + const latestDiffToResult = resultRef.current; return ( resultRef.current && From 30aae502b071ee6f823d8f3bfa4f92bf3c4e9d25 Mon Sep 17 00:00:00 2001 From: Jerel Miller Date: Fri, 5 Jan 2024 10:14:29 -0700 Subject: [PATCH 07/16] Make getSnapshot a stable function reference --- src/react/hooks/useFragment.ts | 12 ++---------- 1 file changed, 2 insertions(+), 10 deletions(-) diff --git a/src/react/hooks/useFragment.ts b/src/react/hooks/useFragment.ts index 0c133c8bc95..fd0694dd980 100644 --- a/src/react/hooks/useFragment.ts +++ b/src/react/hooks/useFragment.ts @@ -1,3 +1,4 @@ +import * as React from "react"; import { equal } from "@wry/equality"; import type { DeepPartial } from "../../utilities/index.js"; @@ -69,16 +70,7 @@ export function useFragment( ); // Used for both getSnapshot and getServerSnapshot - const getSnapshot = () => { - const latestDiffToResult = resultRef.current; - - return ( - resultRef.current && - equal(resultRef.current.data, latestDiffToResult.data) - ) ? - resultRef.current - : (resultRef.current = latestDiffToResult); - }; + const getSnapshot = React.useCallback(() => resultRef.current, []); return useSyncExternalStore( (forceUpdate) => { From 5e1e8a2e868391a3d5f064436463de4d5f885276 Mon Sep 17 00:00:00 2001 From: Jerel Miller Date: Fri, 5 Jan 2024 10:18:03 -0700 Subject: [PATCH 08/16] Wrap useSyncExternalStore subscribe function in useCallback --- src/react/hooks/useFragment.ts | 37 ++++++++++++++++++---------------- 1 file changed, 20 insertions(+), 17 deletions(-) diff --git a/src/react/hooks/useFragment.ts b/src/react/hooks/useFragment.ts index fd0694dd980..c1dd2309d2d 100644 --- a/src/react/hooks/useFragment.ts +++ b/src/react/hooks/useFragment.ts @@ -73,23 +73,26 @@ export function useFragment( const getSnapshot = React.useCallback(() => resultRef.current, []); return useSyncExternalStore( - (forceUpdate) => { - let lastTimeout = 0; - const unsubcribe = cache.watch({ - ...diffOptions, - immediate: true, - callback(diff) { - if (!equal(diff.result, resultRef.current?.data)) { - resultRef.current = diffToResult(diff); - lastTimeout = setTimeout(forceUpdate) as any; - } - }, - }); - return () => { - unsubcribe(); - clearTimeout(lastTimeout); - }; - }, + React.useCallback( + (forceUpdate) => { + let lastTimeout = 0; + const unsubcribe = cache.watch({ + ...diffOptions, + immediate: true, + callback(diff) { + if (!equal(diff.result, resultRef.current?.data)) { + resultRef.current = diffToResult(diff); + lastTimeout = setTimeout(forceUpdate) as any; + } + }, + }); + return () => { + unsubcribe(); + clearTimeout(lastTimeout); + }; + }, + [cache, diffOptions] + ), getSnapshot, getSnapshot ); From c0b6166e951cf7db8b6424d6764ed38a698ca70c Mon Sep 17 00:00:00 2001 From: Jerel Miller Date: Fri, 5 Jan 2024 10:27:36 -0700 Subject: [PATCH 09/16] Move diff options type to useDeepMemo generic arg --- src/react/hooks/useFragment.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/react/hooks/useFragment.ts b/src/react/hooks/useFragment.ts index c1dd2309d2d..6288ad17bba 100644 --- a/src/react/hooks/useFragment.ts +++ b/src/react/hooks/useFragment.ts @@ -47,7 +47,7 @@ export function useFragment( ): UseFragmentResult { const { cache } = useApolloClient(); - const diffOptions: Cache.DiffOptions = useDeepMemo(() => { + const diffOptions = useDeepMemo>(() => { const { fragment, fragmentName, From 6c1372cbb3fc74abf122d2fde37accbeabae2bca Mon Sep 17 00:00:00 2001 From: Jerel Miller Date: Fri, 5 Jan 2024 13:45:35 -0700 Subject: [PATCH 10/16] Clear timeout in callback when trying to update again --- src/react/hooks/useFragment.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/src/react/hooks/useFragment.ts b/src/react/hooks/useFragment.ts index 6288ad17bba..3bce3be1788 100644 --- a/src/react/hooks/useFragment.ts +++ b/src/react/hooks/useFragment.ts @@ -82,6 +82,7 @@ export function useFragment( callback(diff) { if (!equal(diff.result, resultRef.current?.data)) { resultRef.current = diffToResult(diff); + clearTimeout(lastTimeout); lastTimeout = setTimeout(forceUpdate) as any; } }, From f51d7adbde60da2a0a20c032cb7a3aff74c0b9da Mon Sep 17 00:00:00 2001 From: Jerel Miller Date: Fri, 5 Jan 2024 13:46:26 -0700 Subject: [PATCH 11/16] Add comment about clearTimeout --- src/react/hooks/useFragment.ts | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/react/hooks/useFragment.ts b/src/react/hooks/useFragment.ts index 3bce3be1788..75635323a93 100644 --- a/src/react/hooks/useFragment.ts +++ b/src/react/hooks/useFragment.ts @@ -82,6 +82,10 @@ export function useFragment( callback(diff) { if (!equal(diff.result, resultRef.current?.data)) { resultRef.current = diffToResult(diff); + // 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 + // fixed in https://github.com/apollographql/apollo-client/pull/11083 clearTimeout(lastTimeout); lastTimeout = setTimeout(forceUpdate) as any; } From 087fc4c9e13d024af920851ad035c669233bd46f Mon Sep 17 00:00:00 2001 From: Jerel Miller Date: Fri, 5 Jan 2024 13:46:33 -0700 Subject: [PATCH 12/16] No need for optional chaining --- src/react/hooks/useFragment.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/react/hooks/useFragment.ts b/src/react/hooks/useFragment.ts index 75635323a93..9bfbf6bb5ba 100644 --- a/src/react/hooks/useFragment.ts +++ b/src/react/hooks/useFragment.ts @@ -80,7 +80,7 @@ export function useFragment( ...diffOptions, immediate: true, callback(diff) { - if (!equal(diff.result, resultRef.current?.data)) { + if (!equal(diff.result, resultRef.current.data)) { resultRef.current = diffToResult(diff); // If we get another update before we've re-rendered, bail out of // the update and try again. This ensures that the relative timing From ecf9427c70521e5b0c7bf9684db158a7a47b1508 Mon Sep 17 00:00:00 2001 From: Jerel Miller Date: Fri, 5 Jan 2024 13:54:22 -0700 Subject: [PATCH 13/16] Add changeset --- .changeset/great-badgers-jog.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/great-badgers-jog.md diff --git a/.changeset/great-badgers-jog.md b/.changeset/great-badgers-jog.md new file mode 100644 index 00000000000..0770fe221e0 --- /dev/null +++ b/.changeset/great-badgers-jog.md @@ -0,0 +1,5 @@ +--- +'@apollo/client': patch +--- + +Prevent `useFragment` from excessively unsubscribing and resubscribing the fragment with the cache on every render. From 0d889211a8a02b0e81330ff33de424768d6d01da Mon Sep 17 00:00:00 2001 From: Jerel Miller Date: Fri, 5 Jan 2024 13:54:48 -0700 Subject: [PATCH 14/16] Update size limits --- .size-limits.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.size-limits.json b/.size-limits.json index c38ad4443b0..a29559780f6 100644 --- a/.size-limits.json +++ b/.size-limits.json @@ -1,4 +1,4 @@ { - "dist/apollo-client.min.cjs": 37901, + "dist/apollo-client.min.cjs": 37932, "import { ApolloClient, InMemoryCache, HttpLink } from \"dist/index.js\" (production)": 31976 } From a10fc98545400d2b6b3d3e503e3b5b0dff83aa8c Mon Sep 17 00:00:00 2001 From: Jerel Miller Date: Fri, 5 Jan 2024 14:01:01 -0700 Subject: [PATCH 15/16] Remove unneeded extra check for React 17 in useFragment tests --- src/react/hooks/__tests__/useFragment.test.tsx | 6 ------ 1 file changed, 6 deletions(-) diff --git a/src/react/hooks/__tests__/useFragment.test.tsx b/src/react/hooks/__tests__/useFragment.test.tsx index 7dd032eb8c7..9184f18d13f 100644 --- a/src/react/hooks/__tests__/useFragment.test.tsx +++ b/src/react/hooks/__tests__/useFragment.test.tsx @@ -1513,12 +1513,6 @@ describe("has the same timing as `useQuery`", () => { cache.writeQuery({ query, data: { item: updatedItem } }); - if (React.version.startsWith("17.")) { - const { snapshot } = await ProfiledComponent.takeRender(); - expect(snapshot.queryData).toStrictEqual({ item: initialItem }); - expect(snapshot.fragmentData).toStrictEqual(updatedItem); - } - { const { snapshot } = await ProfiledComponent.takeRender(); expect(snapshot.queryData).toStrictEqual({ item: updatedItem }); From 58384c322fd065bd8b178f0c4d025aefca20099c Mon Sep 17 00:00:00 2001 From: Jerel Miller Date: Mon, 8 Jan 2024 23:25:40 -0700 Subject: [PATCH 16/16] Fix typo in variable name --- src/react/hooks/useFragment.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/react/hooks/useFragment.ts b/src/react/hooks/useFragment.ts index 9bfbf6bb5ba..b686689fc79 100644 --- a/src/react/hooks/useFragment.ts +++ b/src/react/hooks/useFragment.ts @@ -76,7 +76,7 @@ export function useFragment( React.useCallback( (forceUpdate) => { let lastTimeout = 0; - const unsubcribe = cache.watch({ + const unsubscribe = cache.watch({ ...diffOptions, immediate: true, callback(diff) { @@ -92,7 +92,7 @@ export function useFragment( }, }); return () => { - unsubcribe(); + unsubscribe(); clearTimeout(lastTimeout); }; },