From 5ff3a65cb0aefb8756ae7a4e250a28ec3992734d Mon Sep 17 00:00:00 2001 From: Jerel Miller Date: Tue, 17 Sep 2024 17:32:46 -0600 Subject: [PATCH 01/43] Add way to cache fragment promises --- src/react/internal/cache/SuspenseCache.ts | 41 ++++++++++++++++++++++- 1 file changed, 40 insertions(+), 1 deletion(-) diff --git a/src/react/internal/cache/SuspenseCache.ts b/src/react/internal/cache/SuspenseCache.ts index 8b1eba321b5..0f4632f06a7 100644 --- a/src/react/internal/cache/SuspenseCache.ts +++ b/src/react/internal/cache/SuspenseCache.ts @@ -1,6 +1,10 @@ import { Trie } from "@wry/trie"; import type { ObservableQuery } from "../../../core/index.js"; -import { canUseWeakMap } from "../../../utilities/index.js"; +import type { PromiseWithState } from "../../../utilities/index.js"; +import { + canUseWeakMap, + wrapPromiseWithState, +} from "../../../utilities/index.js"; import { InternalQueryReference } from "./QueryReference.js"; import type { CacheKey } from "./types.js"; @@ -22,6 +26,9 @@ export class SuspenseCache { private queryRefs = new Trie<{ current?: InternalQueryReference }>( canUseWeakMap ); + private fragmentPromises = new Trie<{ + current?: PromiseWithStateAndResolvers; + }>(canUseWeakMap); private options: SuspenseCacheOptions; constructor(options: SuspenseCacheOptions = Object.create(null)) { @@ -48,8 +55,40 @@ export class SuspenseCache { return ref.current; } + getPromiseWithResolvers(cacheKey: CacheKey) { + const ref = this.fragmentPromises.lookupArray(cacheKey) as { + current?: PromiseWithStateAndResolvers; + }; + + if (!ref.current) { + ref.current = withResolvers(); + } + + return ref.current; + } + add(cacheKey: CacheKey, queryRef: InternalQueryReference) { const ref = this.queryRefs.lookupArray(cacheKey); ref.current = queryRef; } } + +interface PromiseWithStateAndResolvers { + promise: PromiseWithState; + resolve: (value: T | PromiseLike) => void; + reject: (reason?: unknown) => void; +} + +function withResolvers(): PromiseWithStateAndResolvers { + let resolve!: (value: T | PromiseLike) => void; + let reject!: (reason?: unknown) => void; + + const promise = wrapPromiseWithState( + new Promise((res, rej) => { + resolve = res; + reject = rej; + }) + ); + + return { promise, resolve, reject }; +} From 8ecae2e8e4aa8eeb3d696b45202075a839a373d1 Mon Sep 17 00:00:00 2001 From: Jerel Miller Date: Tue, 17 Sep 2024 18:07:29 -0600 Subject: [PATCH 02/43] Add some code to handle suspenseful useFragment --- src/react/hooks/useSuspenseFragment.ts | 79 ++++++++++++ src/react/internal/cache/FragmentReference.ts | 117 ++++++++++++++++++ src/react/internal/cache/SuspenseCache.ts | 30 +++-- src/react/internal/cache/types.ts | 4 + 4 files changed, 221 insertions(+), 9 deletions(-) create mode 100644 src/react/hooks/useSuspenseFragment.ts create mode 100644 src/react/internal/cache/FragmentReference.ts diff --git a/src/react/hooks/useSuspenseFragment.ts b/src/react/hooks/useSuspenseFragment.ts new file mode 100644 index 00000000000..d10272a6a61 --- /dev/null +++ b/src/react/hooks/useSuspenseFragment.ts @@ -0,0 +1,79 @@ +import type { ApolloClient, Reference, StoreObject } from "../../core/index.js"; +import { canonicalStringify } from "../../cache/index.js"; +import type { Cache } from "../../cache/index.js"; +import { useApolloClient } from "./useApolloClient.js"; +import { getSuspenseCache } from "../internal/index.js"; +import type { CacheKey } from "../internal/index.js"; +import React from "rehackt"; +import type { FragmentKey } from "../internal/cache/types.js"; +import { __use } from "./internal/__use.js"; + +export interface UseSuspenseFragmentOptions + extends Omit< + Cache.DiffOptions, NoInfer>, + "id" | "query" | "optimistic" | "previousResult" | "returnPartialData" + >, + Omit< + Cache.ReadFragmentOptions, + "id" | "variables" | "returnPartialData" + > { + from: StoreObject | Reference | string; + // Override this field to make it optional (default: true). + optimistic?: boolean; + /** + * The instance of `ApolloClient` to use to look up the fragment. + * + * By default, the instance that's passed down via context is used, but you + * can provide a different instance here. + * + * @docGroup 1. Operation options + */ + client?: ApolloClient; +} + +export type UseSuspenseFragmentResult = { data: TData }; + +export function useSuspenseFragment( + options: UseSuspenseFragmentOptions +): UseSuspenseFragmentResult { + const client = useApolloClient(options.client); + + const cacheKey: CacheKey = [ + options.fragment, + canonicalStringify(options.variables), + ]; + + const fragmentRef = getSuspenseCache(client).getFragmentRef( + cacheKey, + () => client.watchFragment(options) + ); + + let [current, setPromise] = React.useState<[FragmentKey, Promise]>([ + fragmentRef.key, + fragmentRef.promise, + ]); + + if (current[0] !== fragmentRef.key) { + // eslint-disable-next-line react-compiler/react-compiler + current[0] = fragmentRef.key; + current[1] = fragmentRef.promise; + } + + React.useEffect(() => { + const dispose = fragmentRef.retain(); + const removeListener = fragmentRef.listen((promise) => { + setPromise([fragmentRef.key, promise]); + }); + + return () => { + dispose(); + removeListener(); + }; + }, [fragmentRef]); + + let promise = current[1]; + + const data = __use(promise); + + return { data }; +} diff --git a/src/react/internal/cache/FragmentReference.ts b/src/react/internal/cache/FragmentReference.ts new file mode 100644 index 00000000000..1770a2eaab7 --- /dev/null +++ b/src/react/internal/cache/FragmentReference.ts @@ -0,0 +1,117 @@ +import type { WatchFragmentResult } from "../../../cache/index.js"; +import { wrapPromiseWithState } from "../../../utilities/index.js"; +import type { + Observable, + ObservableSubscription, + PromiseWithState, +} from "../../../utilities/index.js"; +import type { FragmentKey } from "./types.js"; + +type FragmentRefPromise = PromiseWithState; +type Listener = (promise: FragmentRefPromise) => void; + +interface FragmentReferenceOptions { + onDispose?: () => void; +} + +export class FragmentReference { + public readonly observable: Observable>; + public readonly key: FragmentKey = {}; + public promise!: FragmentRefPromise; + + private resolve: ((result: TData) => void) | undefined; + private reject: ((error: unknown) => void) | undefined; + + private subscription!: ObservableSubscription; + private listeners = new Set>(); + + private references = 0; + + constructor( + observable: Observable>, + options: FragmentReferenceOptions + ) { + this.dispose = this.dispose.bind(this); + this.handleNext = this.handleNext.bind(this); + this.handleError = this.handleError.bind(this); + + this.observable = observable; + + if (options.onDispose) { + this.onDispose = options.onDispose; + } + + this.promise = wrapPromiseWithState( + new Promise((resolve, reject) => { + this.resolve = resolve; + this.reject = reject; + }) + ); + + this.subscribeToFragment(); + } + + listen(listener: Listener) { + this.listeners.add(listener); + + return () => { + this.listeners.delete(listener); + }; + } + + retain() { + this.references++; + let disposed = false; + + return () => { + if (disposed) { + return; + } + + disposed = true; + this.references--; + + setTimeout(() => { + if (!this.references) { + this.dispose(); + } + }); + }; + } + + private dispose() { + this.subscription.unsubscribe(); + this.onDispose(); + } + + private onDispose() { + // noop. overridable by options + } + + private subscribeToFragment() { + this.subscription = this.observable.subscribe( + this.handleNext.bind(this), + this.handleError.bind(this) + ); + } + + private handleNext(result: WatchFragmentResult) { + switch (this.promise.status) { + case "pending": { + if (result.complete) { + this.resolve?.(result.data); + } + + break; + } + } + } + + private handleError(error: unknown) { + this.reject?.(error); + } + + private deliver(promise: FragmentRefPromise) { + this.listeners.forEach((listener) => listener(promise)); + } +} diff --git a/src/react/internal/cache/SuspenseCache.ts b/src/react/internal/cache/SuspenseCache.ts index 0f4632f06a7..39c6f86fbd7 100644 --- a/src/react/internal/cache/SuspenseCache.ts +++ b/src/react/internal/cache/SuspenseCache.ts @@ -1,12 +1,16 @@ import { Trie } from "@wry/trie"; -import type { ObservableQuery } from "../../../core/index.js"; -import type { PromiseWithState } from "../../../utilities/index.js"; +import type { + ObservableQuery, + WatchFragmentResult, +} from "../../../core/index.js"; +import type { Observable, PromiseWithState } from "../../../utilities/index.js"; import { canUseWeakMap, wrapPromiseWithState, } from "../../../utilities/index.js"; import { InternalQueryReference } from "./QueryReference.js"; import type { CacheKey } from "./types.js"; +import { FragmentReference } from "./FragmentReference.js"; export interface SuspenseCacheOptions { /** @@ -26,9 +30,10 @@ export class SuspenseCache { private queryRefs = new Trie<{ current?: InternalQueryReference }>( canUseWeakMap ); - private fragmentPromises = new Trie<{ - current?: PromiseWithStateAndResolvers; - }>(canUseWeakMap); + private fragmentRefs = new Trie<{ current?: FragmentReference }>( + canUseWeakMap + ); + private options: SuspenseCacheOptions; constructor(options: SuspenseCacheOptions = Object.create(null)) { @@ -55,13 +60,20 @@ export class SuspenseCache { return ref.current; } - getPromiseWithResolvers(cacheKey: CacheKey) { - const ref = this.fragmentPromises.lookupArray(cacheKey) as { - current?: PromiseWithStateAndResolvers; + getFragmentRef( + cacheKey: CacheKey, + createObservable: () => Observable> + ) { + const ref = this.fragmentRefs.lookupArray(cacheKey) as { + current?: FragmentReference; }; if (!ref.current) { - ref.current = withResolvers(); + ref.current = new FragmentReference(createObservable(), { + onDispose: () => { + delete ref.current; + }, + }); } return ref.current; diff --git a/src/react/internal/cache/types.ts b/src/react/internal/cache/types.ts index 40f3c4cc8fc..f45a55eda8f 100644 --- a/src/react/internal/cache/types.ts +++ b/src/react/internal/cache/types.ts @@ -9,3 +9,7 @@ export type CacheKey = [ export interface QueryKey { __queryKey?: string; } + +export interface FragmentKey { + __fragmentKey?: string; +} From 531322888af8376a2323a920a8a4b260472e5b4b Mon Sep 17 00:00:00 2001 From: Jerel Miller Date: Tue, 17 Sep 2024 18:27:55 -0600 Subject: [PATCH 03/43] Add handling of promise state in FragmentReference --- src/react/internal/cache/FragmentReference.ts | 33 ++++++++++++++----- 1 file changed, 24 insertions(+), 9 deletions(-) diff --git a/src/react/internal/cache/FragmentReference.ts b/src/react/internal/cache/FragmentReference.ts index 1770a2eaab7..6e817036cc3 100644 --- a/src/react/internal/cache/FragmentReference.ts +++ b/src/react/internal/cache/FragmentReference.ts @@ -1,5 +1,8 @@ import type { WatchFragmentResult } from "../../../cache/index.js"; -import { wrapPromiseWithState } from "../../../utilities/index.js"; +import { + createFulfilledPromise, + wrapPromiseWithState, +} from "../../../utilities/index.js"; import type { Observable, ObservableSubscription, @@ -41,13 +44,7 @@ export class FragmentReference { this.onDispose = options.onDispose; } - this.promise = wrapPromiseWithState( - new Promise((resolve, reject) => { - this.resolve = resolve; - this.reject = reject; - }) - ); - + this.promise = this.createPendingPromise(); this.subscribeToFragment(); } @@ -99,11 +96,20 @@ export class FragmentReference { switch (this.promise.status) { case "pending": { if (result.complete) { - this.resolve?.(result.data); + return this.resolve?.(result.data); } + this.deliver(this.promise); break; } + case "fulfilled": { + this.promise = + result.complete ? + createFulfilledPromise(result.data) + : this.createPendingPromise(); + + this.deliver(this.promise); + } } } @@ -114,4 +120,13 @@ export class FragmentReference { private deliver(promise: FragmentRefPromise) { this.listeners.forEach((listener) => listener(promise)); } + + private createPendingPromise() { + return wrapPromiseWithState( + new Promise((resolve, reject) => { + this.resolve = resolve; + this.reject = reject; + }) + ); + } } From c07aaa46e822881ef8f3ddbb2def7120d85b6a01 Mon Sep 17 00:00:00 2001 From: Jerel Miller Date: Tue, 17 Sep 2024 18:46:32 -0600 Subject: [PATCH 04/43] Add some basic tests for useSuspenseFragment --- .../__tests__/useSuspenseFragment.test.tsx | 319 ++++++++++++++++++ 1 file changed, 319 insertions(+) create mode 100644 src/react/hooks/__tests__/useSuspenseFragment.test.tsx diff --git a/src/react/hooks/__tests__/useSuspenseFragment.test.tsx b/src/react/hooks/__tests__/useSuspenseFragment.test.tsx new file mode 100644 index 00000000000..8927804424b --- /dev/null +++ b/src/react/hooks/__tests__/useSuspenseFragment.test.tsx @@ -0,0 +1,319 @@ +import { + useSuspenseFragment, + UseSuspenseFragmentResult, +} from "../useSuspenseFragment"; +import { createProfiler, useTrackRenders } from "../../../testing/internal"; +import { act, render } from "@testing-library/react"; +import { + ApolloClient, + gql, + InMemoryCache, + TypedDocumentNode, +} from "../../../core"; +import React, { Suspense } from "react"; +import { ApolloProvider } from "../../context"; + +function createDefaultProfiler() { + return createProfiler({ + initialSnapshot: { + result: null as UseSuspenseFragmentResult | null, + }, + }); +} + +function createDefaultTrackedComponents() { + function SuspenseFallback() { + useTrackRenders(); + return
Loading
; + } + + return { SuspenseFallback }; +} + +test("suspends until cache value is complete", async () => { + interface ItemFragment { + __typename: "Item"; + id: number; + text: string; + } + + const Profiler = createDefaultProfiler(); + const { SuspenseFallback } = createDefaultTrackedComponents(); + + const client = new ApolloClient({ cache: new InMemoryCache() }); + + const fragment: TypedDocumentNode = gql` + fragment ItemFragment on Item { + id + text + } + `; + + function App() { + useTrackRenders(); + + const result = useSuspenseFragment({ + fragment, + from: { __typename: "Item", id: 1 }, + }); + + Profiler.replaceSnapshot({ result }); + + return null; + } + + render(, { + wrapper: ({ children }) => { + return ( + + + }>{children} + + + ); + }, + }); + + { + const { renderedComponents } = await Profiler.takeRender(); + + expect(renderedComponents).toStrictEqual([SuspenseFallback]); + } + + client.writeFragment({ + fragment, + data: { + __typename: "Item", + id: 1, + text: "Item #1", + }, + }); + + { + const { snapshot, renderedComponents } = await Profiler.takeRender(); + + expect(renderedComponents).toStrictEqual([App]); + expect(snapshot.result).toEqual({ + data: { + __typename: "Item", + id: 1, + text: "Item #1", + }, + }); + } + + await expect(Profiler).not.toRerender(); +}); + +test("updates when the cache updates", async () => { + interface ItemFragment { + __typename: "Item"; + id: number; + text: string; + } + + const Profiler = createDefaultProfiler(); + const { SuspenseFallback } = createDefaultTrackedComponents(); + + const client = new ApolloClient({ cache: new InMemoryCache() }); + + const fragment: TypedDocumentNode = gql` + fragment ItemFragment on Item { + id + text + } + `; + + function App() { + useTrackRenders(); + + const result = useSuspenseFragment({ + fragment, + from: { __typename: "Item", id: 1 }, + }); + + Profiler.replaceSnapshot({ result }); + + return null; + } + + render(, { + wrapper: ({ children }) => { + return ( + + + }>{children} + + + ); + }, + }); + + { + const { renderedComponents } = await Profiler.takeRender(); + + expect(renderedComponents).toStrictEqual([SuspenseFallback]); + } + + client.writeFragment({ + fragment, + data: { + __typename: "Item", + id: 1, + text: "Item #1", + }, + }); + + { + const { snapshot, renderedComponents } = await Profiler.takeRender(); + + expect(renderedComponents).toStrictEqual([App]); + expect(snapshot.result).toEqual({ + data: { + __typename: "Item", + id: 1, + text: "Item #1", + }, + }); + } + + act(() => { + client.writeFragment({ + fragment, + data: { + __typename: "Item", + id: 1, + text: "Item #1 (updated)", + }, + }); + }); + + { + const { snapshot, renderedComponents } = await Profiler.takeRender(); + + expect(renderedComponents).toStrictEqual([App]); + expect(snapshot.result).toEqual({ + data: { + __typename: "Item", + id: 1, + text: "Item #1 (updated)", + }, + }); + } + + await expect(Profiler).not.toRerender(); +}); + +test("resuspends when data goes missing until complete again", async () => { + interface ItemFragment { + __typename: "Item"; + id: number; + text: string; + } + + const Profiler = createDefaultProfiler(); + const { SuspenseFallback } = createDefaultTrackedComponents(); + + const client = new ApolloClient({ cache: new InMemoryCache() }); + + const fragment: TypedDocumentNode = gql` + fragment ItemFragment on Item { + id + text + } + `; + + function App() { + useTrackRenders(); + + const result = useSuspenseFragment({ + fragment, + from: { __typename: "Item", id: 1 }, + }); + + Profiler.replaceSnapshot({ result }); + + return null; + } + + render(, { + wrapper: ({ children }) => { + return ( + + + }>{children} + + + ); + }, + }); + + { + const { renderedComponents } = await Profiler.takeRender(); + + expect(renderedComponents).toStrictEqual([SuspenseFallback]); + } + + client.writeFragment({ + fragment, + data: { + __typename: "Item", + id: 1, + text: "Item #1", + }, + }); + + { + const { snapshot, renderedComponents } = await Profiler.takeRender(); + + expect(renderedComponents).toStrictEqual([App]); + expect(snapshot.result).toEqual({ + data: { + __typename: "Item", + id: 1, + text: "Item #1", + }, + }); + } + + act(() => { + client.cache.modify({ + id: "Item:1", + fields: { + text: (_, { DELETE }) => DELETE, + }, + }); + }); + + { + const { renderedComponents } = await Profiler.takeRender(); + + expect(renderedComponents).toStrictEqual([SuspenseFallback]); + } + + act(() => { + client.writeFragment({ + fragment, + data: { + __typename: "Item", + id: 1, + text: "Item #1 (updated)", + }, + }); + }); + + { + const { snapshot, renderedComponents } = await Profiler.takeRender(); + + expect(renderedComponents).toStrictEqual([App]); + expect(snapshot.result).toEqual({ + data: { + __typename: "Item", + id: 1, + text: "Item #1 (updated)", + }, + }); + } + + await expect(Profiler).not.toRerender(); +}); From 901c15ff59d9d3b77b6223bd92a86e08a9d4e677 Mon Sep 17 00:00:00 2001 From: Jerel Miller Date: Tue, 17 Sep 2024 18:47:49 -0600 Subject: [PATCH 05/43] Add exports for useSuspenseFragment --- src/react/hooks/index.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/react/hooks/index.ts b/src/react/hooks/index.ts index 78fc82c61f4..3fe6ccbc27d 100644 --- a/src/react/hooks/index.ts +++ b/src/react/hooks/index.ts @@ -11,6 +11,8 @@ export type { UseSuspenseQueryResult } from "./useSuspenseQuery.js"; export { useSuspenseQuery } from "./useSuspenseQuery.js"; export type { UseBackgroundQueryResult } from "./useBackgroundQuery.js"; export { useBackgroundQuery } from "./useBackgroundQuery.js"; +export type { UseSuspenseFragmentResult } from "./useSuspenseFragment.js"; +export { useSuspenseFragment } from "./useSuspenseFragment.js"; export type { LoadQueryFunction, UseLoadableQueryResult, From f930f45a90034143a6810ecb01c8035341baed13 Mon Sep 17 00:00:00 2001 From: Jerel Miller Date: Tue, 17 Sep 2024 18:48:33 -0600 Subject: [PATCH 06/43] Remove unused code --- src/react/internal/cache/SuspenseCache.ts | 27 ++--------------------- 1 file changed, 2 insertions(+), 25 deletions(-) diff --git a/src/react/internal/cache/SuspenseCache.ts b/src/react/internal/cache/SuspenseCache.ts index 39c6f86fbd7..b96de20431d 100644 --- a/src/react/internal/cache/SuspenseCache.ts +++ b/src/react/internal/cache/SuspenseCache.ts @@ -3,11 +3,8 @@ import type { ObservableQuery, WatchFragmentResult, } from "../../../core/index.js"; -import type { Observable, PromiseWithState } from "../../../utilities/index.js"; -import { - canUseWeakMap, - wrapPromiseWithState, -} from "../../../utilities/index.js"; +import type { Observable } from "../../../utilities/index.js"; +import { canUseWeakMap } from "../../../utilities/index.js"; import { InternalQueryReference } from "./QueryReference.js"; import type { CacheKey } from "./types.js"; import { FragmentReference } from "./FragmentReference.js"; @@ -84,23 +81,3 @@ export class SuspenseCache { ref.current = queryRef; } } - -interface PromiseWithStateAndResolvers { - promise: PromiseWithState; - resolve: (value: T | PromiseLike) => void; - reject: (reason?: unknown) => void; -} - -function withResolvers(): PromiseWithStateAndResolvers { - let resolve!: (value: T | PromiseLike) => void; - let reject!: (reason?: unknown) => void; - - const promise = wrapPromiseWithState( - new Promise((res, rej) => { - resolve = res; - reject = rej; - }) - ); - - return { promise, resolve, reject }; -} From c54d8fb7caf9e59371b9a79e10fad4b3d7336db5 Mon Sep 17 00:00:00 2001 From: Jerel Miller Date: Tue, 17 Sep 2024 18:50:01 -0600 Subject: [PATCH 07/43] Update exports snapshot --- src/__tests__/__snapshots__/exports.ts.snap | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/__tests__/__snapshots__/exports.ts.snap b/src/__tests__/__snapshots__/exports.ts.snap index e21e634c864..379695ffb14 100644 --- a/src/__tests__/__snapshots__/exports.ts.snap +++ b/src/__tests__/__snapshots__/exports.ts.snap @@ -67,6 +67,7 @@ Array [ "useReactiveVar", "useReadQuery", "useSubscription", + "useSuspenseFragment", "useSuspenseQuery", ] `; @@ -293,6 +294,7 @@ Array [ "useReactiveVar", "useReadQuery", "useSubscription", + "useSuspenseFragment", "useSuspenseQuery", ] `; @@ -338,6 +340,7 @@ Array [ "useReactiveVar", "useReadQuery", "useSubscription", + "useSuspenseFragment", "useSuspenseQuery", ] `; From 5fc5aea972dba753410f62e53ee7c593de7c9a6b Mon Sep 17 00:00:00 2001 From: Jerel Miller Date: Wed, 18 Sep 2024 08:38:40 -0600 Subject: [PATCH 08/43] Wrap useSuspenseFragment hook for streaming package --- src/react/hooks/internal/wrapHook.ts | 2 ++ src/react/hooks/useSuspenseFragment.ts | 26 ++++++++++++++++++++++++-- 2 files changed, 26 insertions(+), 2 deletions(-) diff --git a/src/react/hooks/internal/wrapHook.ts b/src/react/hooks/internal/wrapHook.ts index 59b112c3216..175d3e72a5b 100644 --- a/src/react/hooks/internal/wrapHook.ts +++ b/src/react/hooks/internal/wrapHook.ts @@ -5,6 +5,7 @@ import type { useReadQuery, useFragment, useQueryRefHandlers, + useSuspenseFragment, } from "../index.js"; import type { QueryManager } from "../../../core/QueryManager.js"; import type { ApolloClient } from "../../../core/ApolloClient.js"; @@ -17,6 +18,7 @@ interface WrappableHooks { createQueryPreloader: typeof createQueryPreloader; useQuery: typeof useQuery; useSuspenseQuery: typeof useSuspenseQuery; + useSuspenseFragment: typeof useSuspenseFragment; useBackgroundQuery: typeof useBackgroundQuery; useReadQuery: typeof useReadQuery; useFragment: typeof useFragment; diff --git a/src/react/hooks/useSuspenseFragment.ts b/src/react/hooks/useSuspenseFragment.ts index d10272a6a61..47244ff89f4 100644 --- a/src/react/hooks/useSuspenseFragment.ts +++ b/src/react/hooks/useSuspenseFragment.ts @@ -1,4 +1,9 @@ -import type { ApolloClient, Reference, StoreObject } from "../../core/index.js"; +import type { + ApolloClient, + OperationVariables, + Reference, + StoreObject, +} from "../../core/index.js"; import { canonicalStringify } from "../../cache/index.js"; import type { Cache } from "../../cache/index.js"; import { useApolloClient } from "./useApolloClient.js"; @@ -7,6 +12,7 @@ import type { CacheKey } from "../internal/index.js"; import React from "rehackt"; import type { FragmentKey } from "../internal/cache/types.js"; import { __use } from "./internal/__use.js"; +import { wrapHook } from "./internal/index.js"; export interface UseSuspenseFragmentOptions extends Omit< @@ -33,7 +39,23 @@ export interface UseSuspenseFragmentOptions export type UseSuspenseFragmentResult = { data: TData }; -export function useSuspenseFragment( +export function useSuspenseFragment< + TData = unknown, + TVariables extends OperationVariables = OperationVariables, +>( + options: UseSuspenseFragmentOptions +): UseSuspenseFragmentResult { + return wrapHook( + "useSuspenseFragment", + _useSuspenseFragment, + useApolloClient(typeof options === "object" ? options.client : undefined) + )(options); +} + +function _useSuspenseFragment< + TData = unknown, + TVariables extends OperationVariables = OperationVariables, +>( options: UseSuspenseFragmentOptions ): UseSuspenseFragmentResult { const client = useApolloClient(options.client); From b71dffe40ce4703edbba65f7a2f89422caaa36f5 Mon Sep 17 00:00:00 2001 From: Jerel Miller Date: Wed, 18 Sep 2024 13:55:43 -0600 Subject: [PATCH 09/43] Handle different cache ids in useSuspenseFragment --- src/react/hooks/useSuspenseFragment.ts | 15 +++++++++++---- src/react/internal/cache/SuspenseCache.ts | 4 ++-- src/react/internal/cache/types.ts | 6 ++++++ 3 files changed, 19 insertions(+), 6 deletions(-) diff --git a/src/react/hooks/useSuspenseFragment.ts b/src/react/hooks/useSuspenseFragment.ts index 47244ff89f4..45c708fb2dc 100644 --- a/src/react/hooks/useSuspenseFragment.ts +++ b/src/react/hooks/useSuspenseFragment.ts @@ -8,9 +8,8 @@ import { canonicalStringify } from "../../cache/index.js"; import type { Cache } from "../../cache/index.js"; import { useApolloClient } from "./useApolloClient.js"; import { getSuspenseCache } from "../internal/index.js"; -import type { CacheKey } from "../internal/index.js"; -import React from "rehackt"; -import type { FragmentKey } from "../internal/cache/types.js"; +import React, { useMemo } from "rehackt"; +import type { FragmentCacheKey, FragmentKey } from "../internal/cache/types.js"; import { __use } from "./internal/__use.js"; import { wrapHook } from "./internal/index.js"; @@ -59,8 +58,16 @@ function _useSuspenseFragment< options: UseSuspenseFragmentOptions ): UseSuspenseFragmentResult { const client = useApolloClient(options.client); + const { from } = options; + const { cache } = client; - const cacheKey: CacheKey = [ + const id = useMemo( + () => (typeof from === "string" ? from : cache.identify(from)), + [cache, from] + )!; + + const cacheKey: FragmentCacheKey = [ + id, options.fragment, canonicalStringify(options.variables), ]; diff --git a/src/react/internal/cache/SuspenseCache.ts b/src/react/internal/cache/SuspenseCache.ts index b96de20431d..7ae77b49473 100644 --- a/src/react/internal/cache/SuspenseCache.ts +++ b/src/react/internal/cache/SuspenseCache.ts @@ -6,7 +6,7 @@ import type { import type { Observable } from "../../../utilities/index.js"; import { canUseWeakMap } from "../../../utilities/index.js"; import { InternalQueryReference } from "./QueryReference.js"; -import type { CacheKey } from "./types.js"; +import type { CacheKey, FragmentCacheKey } from "./types.js"; import { FragmentReference } from "./FragmentReference.js"; export interface SuspenseCacheOptions { @@ -58,7 +58,7 @@ export class SuspenseCache { } getFragmentRef( - cacheKey: CacheKey, + cacheKey: FragmentCacheKey, createObservable: () => Observable> ) { const ref = this.fragmentRefs.lookupArray(cacheKey) as { diff --git a/src/react/internal/cache/types.ts b/src/react/internal/cache/types.ts index f45a55eda8f..a163431ad9d 100644 --- a/src/react/internal/cache/types.ts +++ b/src/react/internal/cache/types.ts @@ -6,6 +6,12 @@ export type CacheKey = [ ...queryKey: any[], ]; +export type FragmentCacheKey = [ + cacheId: string, + fragment: DocumentNode, + stringifiedVariables: string, +]; + export interface QueryKey { __queryKey?: string; } From 62d375df58549a17987b9105fb6c7cc1a2d10dff Mon Sep 17 00:00:00 2001 From: Jerel Miller Date: Mon, 16 Dec 2024 17:13:38 -0700 Subject: [PATCH 10/43] Update tests to use render stream library --- .../__tests__/useSuspenseFragment.test.tsx | 157 +++++++++--------- 1 file changed, 77 insertions(+), 80 deletions(-) diff --git a/src/react/hooks/__tests__/useSuspenseFragment.test.tsx b/src/react/hooks/__tests__/useSuspenseFragment.test.tsx index 8927804424b..b58b4ce2b0f 100644 --- a/src/react/hooks/__tests__/useSuspenseFragment.test.tsx +++ b/src/react/hooks/__tests__/useSuspenseFragment.test.tsx @@ -2,8 +2,6 @@ import { useSuspenseFragment, UseSuspenseFragmentResult, } from "../useSuspenseFragment"; -import { createProfiler, useTrackRenders } from "../../../testing/internal"; -import { act, render } from "@testing-library/react"; import { ApolloClient, gql, @@ -12,9 +10,14 @@ import { } from "../../../core"; import React, { Suspense } from "react"; import { ApolloProvider } from "../../context"; +import { + createRenderStream, + disableActEnvironment, + useTrackRenders, +} from "@testing-library/react-render-stream"; -function createDefaultProfiler() { - return createProfiler({ +function createDefaultRenderStream() { + return createRenderStream({ initialSnapshot: { result: null as UseSuspenseFragmentResult | null, }, @@ -37,7 +40,7 @@ test("suspends until cache value is complete", async () => { text: string; } - const Profiler = createDefaultProfiler(); + const { render, takeRender, replaceSnapshot } = createDefaultRenderStream(); const { SuspenseFallback } = createDefaultTrackedComponents(); const client = new ApolloClient({ cache: new InMemoryCache() }); @@ -57,25 +60,25 @@ test("suspends until cache value is complete", async () => { from: { __typename: "Item", id: 1 }, }); - Profiler.replaceSnapshot({ result }); + replaceSnapshot({ result }); return null; } - render(, { - wrapper: ({ children }) => { - return ( - - - }>{children} - - - ); - }, - }); + using _disabledAct = disableActEnvironment(); + await render( + }> + + , + { + wrapper: ({ children }) => { + return {children}; + }, + } + ); { - const { renderedComponents } = await Profiler.takeRender(); + const { renderedComponents } = await takeRender(); expect(renderedComponents).toStrictEqual([SuspenseFallback]); } @@ -90,7 +93,7 @@ test("suspends until cache value is complete", async () => { }); { - const { snapshot, renderedComponents } = await Profiler.takeRender(); + const { snapshot, renderedComponents } = await takeRender(); expect(renderedComponents).toStrictEqual([App]); expect(snapshot.result).toEqual({ @@ -102,7 +105,7 @@ test("suspends until cache value is complete", async () => { }); } - await expect(Profiler).not.toRerender(); + await expect(takeRender).not.toRerender(); }); test("updates when the cache updates", async () => { @@ -112,7 +115,7 @@ test("updates when the cache updates", async () => { text: string; } - const Profiler = createDefaultProfiler(); + const { takeRender, render, replaceSnapshot } = createDefaultRenderStream(); const { SuspenseFallback } = createDefaultTrackedComponents(); const client = new ApolloClient({ cache: new InMemoryCache() }); @@ -132,25 +135,25 @@ test("updates when the cache updates", async () => { from: { __typename: "Item", id: 1 }, }); - Profiler.replaceSnapshot({ result }); + replaceSnapshot({ result }); return null; } - render(, { - wrapper: ({ children }) => { - return ( - - - }>{children} - - - ); - }, - }); + using _disabledAct = disableActEnvironment(); + await render( + }> + + , + { + wrapper: ({ children }) => ( + {children} + ), + } + ); { - const { renderedComponents } = await Profiler.takeRender(); + const { renderedComponents } = await takeRender(); expect(renderedComponents).toStrictEqual([SuspenseFallback]); } @@ -165,7 +168,7 @@ test("updates when the cache updates", async () => { }); { - const { snapshot, renderedComponents } = await Profiler.takeRender(); + const { snapshot, renderedComponents } = await takeRender(); expect(renderedComponents).toStrictEqual([App]); expect(snapshot.result).toEqual({ @@ -177,19 +180,17 @@ test("updates when the cache updates", async () => { }); } - act(() => { - client.writeFragment({ - fragment, - data: { - __typename: "Item", - id: 1, - text: "Item #1 (updated)", - }, - }); + client.writeFragment({ + fragment, + data: { + __typename: "Item", + id: 1, + text: "Item #1 (updated)", + }, }); { - const { snapshot, renderedComponents } = await Profiler.takeRender(); + const { snapshot, renderedComponents } = await takeRender(); expect(renderedComponents).toStrictEqual([App]); expect(snapshot.result).toEqual({ @@ -201,7 +202,7 @@ test("updates when the cache updates", async () => { }); } - await expect(Profiler).not.toRerender(); + await expect(takeRender).not.toRerender(); }); test("resuspends when data goes missing until complete again", async () => { @@ -211,7 +212,7 @@ test("resuspends when data goes missing until complete again", async () => { text: string; } - const Profiler = createDefaultProfiler(); + const { takeRender, render, replaceSnapshot } = createDefaultRenderStream(); const { SuspenseFallback } = createDefaultTrackedComponents(); const client = new ApolloClient({ cache: new InMemoryCache() }); @@ -231,25 +232,25 @@ test("resuspends when data goes missing until complete again", async () => { from: { __typename: "Item", id: 1 }, }); - Profiler.replaceSnapshot({ result }); + replaceSnapshot({ result }); return null; } - render(, { - wrapper: ({ children }) => { - return ( - - - }>{children} - - - ); - }, - }); + using _disabledAct = disableActEnvironment(); + await render( + }> + + , + { + wrapper: ({ children }) => ( + {children} + ), + } + ); { - const { renderedComponents } = await Profiler.takeRender(); + const { renderedComponents } = await takeRender(); expect(renderedComponents).toStrictEqual([SuspenseFallback]); } @@ -264,7 +265,7 @@ test("resuspends when data goes missing until complete again", async () => { }); { - const { snapshot, renderedComponents } = await Profiler.takeRender(); + const { snapshot, renderedComponents } = await takeRender(); expect(renderedComponents).toStrictEqual([App]); expect(snapshot.result).toEqual({ @@ -276,34 +277,30 @@ test("resuspends when data goes missing until complete again", async () => { }); } - act(() => { - client.cache.modify({ - id: "Item:1", - fields: { - text: (_, { DELETE }) => DELETE, - }, - }); + client.cache.modify({ + id: "Item:1", + fields: { + text: (_, { DELETE }) => DELETE, + }, }); { - const { renderedComponents } = await Profiler.takeRender(); + const { renderedComponents } = await takeRender(); expect(renderedComponents).toStrictEqual([SuspenseFallback]); } - act(() => { - client.writeFragment({ - fragment, - data: { - __typename: "Item", - id: 1, - text: "Item #1 (updated)", - }, - }); + client.writeFragment({ + fragment, + data: { + __typename: "Item", + id: 1, + text: "Item #1 (updated)", + }, }); { - const { snapshot, renderedComponents } = await Profiler.takeRender(); + const { snapshot, renderedComponents } = await takeRender(); expect(renderedComponents).toStrictEqual([App]); expect(snapshot.result).toEqual({ @@ -315,5 +312,5 @@ test("resuspends when data goes missing until complete again", async () => { }); } - await expect(Profiler).not.toRerender(); + await expect(takeRender).not.toRerender(); }); From 941c050c3b2ff3711bb362e485ef532e21023215 Mon Sep 17 00:00:00 2001 From: Jerel Miller Date: Mon, 16 Dec 2024 17:14:27 -0700 Subject: [PATCH 11/43] Add useSuspenseFragment to list of ignored React 17 tests --- config/jest.config.js | 1 + 1 file changed, 1 insertion(+) diff --git a/config/jest.config.js b/config/jest.config.js index 977c2e8e80a..011836cc41f 100644 --- a/config/jest.config.js +++ b/config/jest.config.js @@ -44,6 +44,7 @@ const react17TestFileIgnoreList = [ // We only support Suspense with React 18, so don't test suspense hooks with // React 17 "src/testing/experimental/__tests__/createTestSchema.test.tsx", + "src/react/hooks/__tests__/useSuspenseFragment.test.tsx", "src/react/hooks/__tests__/useSuspenseQuery.test.tsx", "src/react/hooks/__tests__/useBackgroundQuery.test.tsx", "src/react/hooks/__tests__/useLoadableQuery.test.tsx", From f9b843ce46ddb9d1d1e063dab9f337498e52cc3a Mon Sep 17 00:00:00 2001 From: Jerel Miller Date: Mon, 16 Dec 2024 17:21:49 -0700 Subject: [PATCH 12/43] Fix types to work with data masking --- .../hooks/__tests__/useSuspenseFragment.test.tsx | 12 ++++++++---- src/react/hooks/useSuspenseFragment.ts | 10 +++++----- src/react/internal/cache/FragmentReference.ts | 13 +++++++------ 3 files changed, 20 insertions(+), 15 deletions(-) diff --git a/src/react/hooks/__tests__/useSuspenseFragment.test.tsx b/src/react/hooks/__tests__/useSuspenseFragment.test.tsx index b58b4ce2b0f..dbb823d63dc 100644 --- a/src/react/hooks/__tests__/useSuspenseFragment.test.tsx +++ b/src/react/hooks/__tests__/useSuspenseFragment.test.tsx @@ -6,6 +6,7 @@ import { ApolloClient, gql, InMemoryCache, + MaybeMasked, TypedDocumentNode, } from "../../../core"; import React, { Suspense } from "react"; @@ -19,7 +20,7 @@ import { function createDefaultRenderStream() { return createRenderStream({ initialSnapshot: { - result: null as UseSuspenseFragmentResult | null, + result: null as UseSuspenseFragmentResult> | null, }, }); } @@ -40,7 +41,8 @@ test("suspends until cache value is complete", async () => { text: string; } - const { render, takeRender, replaceSnapshot } = createDefaultRenderStream(); + const { render, takeRender, replaceSnapshot } = + createDefaultRenderStream(); const { SuspenseFallback } = createDefaultTrackedComponents(); const client = new ApolloClient({ cache: new InMemoryCache() }); @@ -115,7 +117,8 @@ test("updates when the cache updates", async () => { text: string; } - const { takeRender, render, replaceSnapshot } = createDefaultRenderStream(); + const { takeRender, render, replaceSnapshot } = + createDefaultRenderStream(); const { SuspenseFallback } = createDefaultTrackedComponents(); const client = new ApolloClient({ cache: new InMemoryCache() }); @@ -212,7 +215,8 @@ test("resuspends when data goes missing until complete again", async () => { text: string; } - const { takeRender, render, replaceSnapshot } = createDefaultRenderStream(); + const { takeRender, render, replaceSnapshot } = + createDefaultRenderStream(); const { SuspenseFallback } = createDefaultTrackedComponents(); const client = new ApolloClient({ cache: new InMemoryCache() }); diff --git a/src/react/hooks/useSuspenseFragment.ts b/src/react/hooks/useSuspenseFragment.ts index 45c708fb2dc..9af2dfc0e45 100644 --- a/src/react/hooks/useSuspenseFragment.ts +++ b/src/react/hooks/useSuspenseFragment.ts @@ -1,5 +1,6 @@ import type { ApolloClient, + MaybeMasked, OperationVariables, Reference, StoreObject, @@ -36,7 +37,7 @@ export interface UseSuspenseFragmentOptions client?: ApolloClient; } -export type UseSuspenseFragmentResult = { data: TData }; +export type UseSuspenseFragmentResult = { data: MaybeMasked }; export function useSuspenseFragment< TData = unknown, @@ -77,10 +78,9 @@ function _useSuspenseFragment< () => client.watchFragment(options) ); - let [current, setPromise] = React.useState<[FragmentKey, Promise]>([ - fragmentRef.key, - fragmentRef.promise, - ]); + let [current, setPromise] = React.useState< + [FragmentKey, Promise>] + >([fragmentRef.key, fragmentRef.promise]); if (current[0] !== fragmentRef.key) { // eslint-disable-next-line react-compiler/react-compiler diff --git a/src/react/internal/cache/FragmentReference.ts b/src/react/internal/cache/FragmentReference.ts index 6e817036cc3..34add02a342 100644 --- a/src/react/internal/cache/FragmentReference.ts +++ b/src/react/internal/cache/FragmentReference.ts @@ -1,4 +1,5 @@ import type { WatchFragmentResult } from "../../../cache/index.js"; +import type { MaybeMasked } from "../../../masking/index.js"; import { createFulfilledPromise, wrapPromiseWithState, @@ -20,13 +21,13 @@ interface FragmentReferenceOptions { export class FragmentReference { public readonly observable: Observable>; public readonly key: FragmentKey = {}; - public promise!: FragmentRefPromise; + public promise!: FragmentRefPromise>; - private resolve: ((result: TData) => void) | undefined; + private resolve: ((result: MaybeMasked) => void) | undefined; private reject: ((error: unknown) => void) | undefined; private subscription!: ObservableSubscription; - private listeners = new Set>(); + private listeners = new Set>>(); private references = 0; @@ -48,7 +49,7 @@ export class FragmentReference { this.subscribeToFragment(); } - listen(listener: Listener) { + listen(listener: Listener>) { this.listeners.add(listener); return () => { @@ -117,13 +118,13 @@ export class FragmentReference { this.reject?.(error); } - private deliver(promise: FragmentRefPromise) { + private deliver(promise: FragmentRefPromise>) { this.listeners.forEach((listener) => listener(promise)); } private createPendingPromise() { return wrapPromiseWithState( - new Promise((resolve, reject) => { + new Promise>((resolve, reject) => { this.resolve = resolve; this.reject = reject; }) From 7c588e1a7ff68eafc6b80d582985326e09a1dea9 Mon Sep 17 00:00:00 2001 From: Jerel Miller Date: Tue, 17 Dec 2024 11:37:22 -0700 Subject: [PATCH 13/43] Add test that ensures error is thrown when passing non-fragment to useSuspenseFragment --- .../__tests__/useSuspenseFragment.test.tsx | 25 +++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/src/react/hooks/__tests__/useSuspenseFragment.test.tsx b/src/react/hooks/__tests__/useSuspenseFragment.test.tsx index dbb823d63dc..c9d1a945f79 100644 --- a/src/react/hooks/__tests__/useSuspenseFragment.test.tsx +++ b/src/react/hooks/__tests__/useSuspenseFragment.test.tsx @@ -16,6 +16,10 @@ import { disableActEnvironment, useTrackRenders, } from "@testing-library/react-render-stream"; +import { spyOnConsole } from "../../../testing/internal"; +import { renderHook } from "@testing-library/react"; +import { InvariantError } from "ts-invariant"; +import { MockedProvider } from "../../../testing"; function createDefaultRenderStream() { return createRenderStream({ @@ -34,6 +38,27 @@ function createDefaultTrackedComponents() { return { SuspenseFallback }; } +test("validates the GraphQL document is a fragment", () => { + using _ = spyOnConsole("error"); + + const fragment = gql` + query ShouldThrow { + createException + } + `; + + expect(() => { + renderHook( + () => useSuspenseFragment({ fragment, from: { __typename: "Nope" } }), + { wrapper: ({ children }) => {children} } + ); + }).toThrow( + new InvariantError( + "Found a query operation named 'ShouldThrow'. No operations are allowed when using a fragment as a query. Only fragments are allowed." + ) + ); +}); + test("suspends until cache value is complete", async () => { interface ItemFragment { __typename: "Item"; From e5b536df89254ca9cc9b2cf7d8164879b6b5c45f Mon Sep 17 00:00:00 2001 From: Jerel Miller Date: Tue, 17 Dec 2024 12:45:11 -0700 Subject: [PATCH 14/43] Inline the cache key --- src/react/hooks/useSuspenseFragment.ts | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/src/react/hooks/useSuspenseFragment.ts b/src/react/hooks/useSuspenseFragment.ts index 9af2dfc0e45..275b5baefad 100644 --- a/src/react/hooks/useSuspenseFragment.ts +++ b/src/react/hooks/useSuspenseFragment.ts @@ -10,7 +10,7 @@ import type { Cache } from "../../cache/index.js"; import { useApolloClient } from "./useApolloClient.js"; import { getSuspenseCache } from "../internal/index.js"; import React, { useMemo } from "rehackt"; -import type { FragmentCacheKey, FragmentKey } from "../internal/cache/types.js"; +import type { FragmentKey } from "../internal/cache/types.js"; import { __use } from "./internal/__use.js"; import { wrapHook } from "./internal/index.js"; @@ -67,14 +67,8 @@ function _useSuspenseFragment< [cache, from] )!; - const cacheKey: FragmentCacheKey = [ - id, - options.fragment, - canonicalStringify(options.variables), - ]; - const fragmentRef = getSuspenseCache(client).getFragmentRef( - cacheKey, + [id, options.fragment, canonicalStringify(options.variables)], () => client.watchFragment(options) ); From 5c3d736c7a7d4179efcd8e424f8ef82b93cc59e2 Mon Sep 17 00:00:00 2001 From: Jerel Miller Date: Tue, 17 Dec 2024 12:55:51 -0700 Subject: [PATCH 15/43] Add test for complete result on first render --- .../__tests__/useSuspenseFragment.test.tsx | 66 +++++++++++++++++++ 1 file changed, 66 insertions(+) diff --git a/src/react/hooks/__tests__/useSuspenseFragment.test.tsx b/src/react/hooks/__tests__/useSuspenseFragment.test.tsx index c9d1a945f79..84bba329849 100644 --- a/src/react/hooks/__tests__/useSuspenseFragment.test.tsx +++ b/src/react/hooks/__tests__/useSuspenseFragment.test.tsx @@ -343,3 +343,69 @@ test("resuspends when data goes missing until complete again", async () => { await expect(takeRender).not.toRerender(); }); + +test("does not suspend and returns cache data when data is alredy in the cache", async () => { + interface ItemFragment { + __typename: "Item"; + id: number; + text: string; + } + + const { takeRender, render, replaceSnapshot } = + createDefaultRenderStream(); + const { SuspenseFallback } = createDefaultTrackedComponents(); + + const client = new ApolloClient({ cache: new InMemoryCache() }); + + const fragment: TypedDocumentNode = gql` + fragment ItemFragment on Item { + id + text + } + `; + + client.writeFragment({ + fragment, + data: { __typename: "Item", id: 1, text: "Cached" }, + }); + + function App() { + useTrackRenders(); + + const result = useSuspenseFragment({ + fragment, + from: { __typename: "Item", id: 1 }, + }); + + replaceSnapshot({ result }); + + return null; + } + + using _disabledAct = disableActEnvironment(); + await render( + }> + + , + { + wrapper: ({ children }) => ( + {children} + ), + } + ); + + { + const { snapshot, renderedComponents } = await takeRender(); + + expect(renderedComponents).toStrictEqual([App]); + expect(snapshot.result).toEqual({ + data: { + __typename: "Item", + id: 1, + text: "Cached", + }, + }); + } + + await expect(takeRender).not.toRerender(); +}); From 7a079365c079546076c2b81d94cc0ffa90ded5d9 Mon Sep 17 00:00:00 2001 From: Jerel Miller Date: Tue, 17 Dec 2024 13:51:36 -0700 Subject: [PATCH 16/43] Create observable in FragmentReference --- src/react/hooks/useSuspenseFragment.ts | 8 ++++++-- src/react/internal/cache/FragmentReference.ts | 16 ++++++++++++---- src/react/internal/cache/SuspenseCache.ts | 11 +++++++---- 3 files changed, 25 insertions(+), 10 deletions(-) diff --git a/src/react/hooks/useSuspenseFragment.ts b/src/react/hooks/useSuspenseFragment.ts index 275b5baefad..051b89154f5 100644 --- a/src/react/hooks/useSuspenseFragment.ts +++ b/src/react/hooks/useSuspenseFragment.ts @@ -67,9 +67,13 @@ function _useSuspenseFragment< [cache, from] )!; - const fragmentRef = getSuspenseCache(client).getFragmentRef( + const fragmentRef = getSuspenseCache(client).getFragmentRef< + TData, + TVariables + >( [id, options.fragment, canonicalStringify(options.variables)], - () => client.watchFragment(options) + client, + options ); let [current, setPromise] = React.useState< diff --git a/src/react/internal/cache/FragmentReference.ts b/src/react/internal/cache/FragmentReference.ts index 34add02a342..0fe2f423221 100644 --- a/src/react/internal/cache/FragmentReference.ts +++ b/src/react/internal/cache/FragmentReference.ts @@ -1,4 +1,8 @@ -import type { WatchFragmentResult } from "../../../cache/index.js"; +import type { + WatchFragmentOptions, + WatchFragmentResult, +} from "../../../cache/index.js"; +import type { ApolloClient } from "../../../core/ApolloClient.js"; import type { MaybeMasked } from "../../../masking/index.js"; import { createFulfilledPromise, @@ -18,7 +22,10 @@ interface FragmentReferenceOptions { onDispose?: () => void; } -export class FragmentReference { +export class FragmentReference< + TData = unknown, + TVariables = Record, +> { public readonly observable: Observable>; public readonly key: FragmentKey = {}; public promise!: FragmentRefPromise>; @@ -32,14 +39,15 @@ export class FragmentReference { private references = 0; constructor( - observable: Observable>, + client: ApolloClient, + watchFragmentOptions: WatchFragmentOptions, options: FragmentReferenceOptions ) { this.dispose = this.dispose.bind(this); this.handleNext = this.handleNext.bind(this); this.handleError = this.handleError.bind(this); - this.observable = observable; + this.observable = client.watchFragment(watchFragmentOptions); if (options.onDispose) { this.onDispose = options.onDispose; diff --git a/src/react/internal/cache/SuspenseCache.ts b/src/react/internal/cache/SuspenseCache.ts index 7ae77b49473..ec69f0acc57 100644 --- a/src/react/internal/cache/SuspenseCache.ts +++ b/src/react/internal/cache/SuspenseCache.ts @@ -1,6 +1,8 @@ import { Trie } from "@wry/trie"; import type { + ApolloClient, ObservableQuery, + WatchFragmentOptions, WatchFragmentResult, } from "../../../core/index.js"; import type { Observable } from "../../../utilities/index.js"; @@ -57,16 +59,17 @@ export class SuspenseCache { return ref.current; } - getFragmentRef( + getFragmentRef( cacheKey: FragmentCacheKey, - createObservable: () => Observable> + client: ApolloClient, + options: WatchFragmentOptions ) { const ref = this.fragmentRefs.lookupArray(cacheKey) as { - current?: FragmentReference; + current?: FragmentReference; }; if (!ref.current) { - ref.current = new FragmentReference(createObservable(), { + ref.current = new FragmentReference(client, options, { onDispose: () => { delete ref.current; }, From f42452ddb76370ceee38297bad015e839e350c8d Mon Sep 17 00:00:00 2001 From: Jerel Miller Date: Tue, 17 Dec 2024 14:04:48 -0700 Subject: [PATCH 17/43] Handle case where cache data is already written --- src/react/hooks/useSuspenseFragment.ts | 9 ++-- src/react/internal/cache/FragmentReference.ts | 44 ++++++++++++++++++- src/react/internal/cache/SuspenseCache.ts | 4 +- 3 files changed, 47 insertions(+), 10 deletions(-) diff --git a/src/react/hooks/useSuspenseFragment.ts b/src/react/hooks/useSuspenseFragment.ts index 051b89154f5..9f281bec1ba 100644 --- a/src/react/hooks/useSuspenseFragment.ts +++ b/src/react/hooks/useSuspenseFragment.ts @@ -70,11 +70,10 @@ function _useSuspenseFragment< const fragmentRef = getSuspenseCache(client).getFragmentRef< TData, TVariables - >( - [id, options.fragment, canonicalStringify(options.variables)], - client, - options - ); + >([id, options.fragment, canonicalStringify(options.variables)], client, { + ...options, + from: id, + }); let [current, setPromise] = React.useState< [FragmentKey, Promise>] diff --git a/src/react/internal/cache/FragmentReference.ts b/src/react/internal/cache/FragmentReference.ts index 0fe2f423221..346b59087b0 100644 --- a/src/react/internal/cache/FragmentReference.ts +++ b/src/react/internal/cache/FragmentReference.ts @@ -40,7 +40,9 @@ export class FragmentReference< constructor( client: ApolloClient, - watchFragmentOptions: WatchFragmentOptions, + watchFragmentOptions: WatchFragmentOptions & { + from: string; + }, options: FragmentReferenceOptions ) { this.dispose = this.dispose.bind(this); @@ -53,7 +55,12 @@ export class FragmentReference< this.onDispose = options.onDispose; } - this.promise = this.createPendingPromise(); + const diff = this.getDiff(client, watchFragmentOptions); + + this.promise = + diff.complete ? + createFulfilledPromise(diff.result) + : this.createPendingPromise(); this.subscribeToFragment(); } @@ -112,6 +119,14 @@ export class FragmentReference< break; } case "fulfilled": { + // This can occur when we already have a result written to the cache and + // we subscribe for the first time. We create a fulfilled promise in the + // constructor with a value that is the same as the first emitted value + // so we want to skip delivering it. + if (this.promise.value === result.data) { + return; + } + this.promise = result.complete ? createFulfilledPromise(result.data) @@ -138,4 +153,29 @@ export class FragmentReference< }) ); } + + private getDiff( + client: ApolloClient, + options: WatchFragmentOptions & { from: string } + ) { + const { cache } = client; + const { from, fragment, fragmentName } = options; + + const diff = cache.diff({ + ...options, + query: cache["getFragmentDoc"](fragment, fragmentName), + returnPartialData: true, + id: from, + optimistic: true, + }); + + return { + ...diff, + result: client["queryManager"].maskFragment({ + fragment, + fragmentName, + data: diff.result, + }) as MaybeMasked, + }; + } } diff --git a/src/react/internal/cache/SuspenseCache.ts b/src/react/internal/cache/SuspenseCache.ts index ec69f0acc57..dcaecc9a902 100644 --- a/src/react/internal/cache/SuspenseCache.ts +++ b/src/react/internal/cache/SuspenseCache.ts @@ -3,9 +3,7 @@ import type { ApolloClient, ObservableQuery, WatchFragmentOptions, - WatchFragmentResult, } from "../../../core/index.js"; -import type { Observable } from "../../../utilities/index.js"; import { canUseWeakMap } from "../../../utilities/index.js"; import { InternalQueryReference } from "./QueryReference.js"; import type { CacheKey, FragmentCacheKey } from "./types.js"; @@ -62,7 +60,7 @@ export class SuspenseCache { getFragmentRef( cacheKey: FragmentCacheKey, client: ApolloClient, - options: WatchFragmentOptions + options: WatchFragmentOptions & { from: string } ) { const ref = this.fragmentRefs.lookupArray(cacheKey) as { current?: FragmentReference; From a18fff102c4c14458457d154058ac6a551816604 Mon Sep 17 00:00:00 2001 From: Jerel Miller Date: Tue, 17 Dec 2024 14:08:11 -0700 Subject: [PATCH 18/43] Fix typo --- src/react/hooks/__tests__/useSuspenseFragment.test.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/react/hooks/__tests__/useSuspenseFragment.test.tsx b/src/react/hooks/__tests__/useSuspenseFragment.test.tsx index 84bba329849..7b4d25ecabe 100644 --- a/src/react/hooks/__tests__/useSuspenseFragment.test.tsx +++ b/src/react/hooks/__tests__/useSuspenseFragment.test.tsx @@ -344,7 +344,7 @@ test("resuspends when data goes missing until complete again", async () => { await expect(takeRender).not.toRerender(); }); -test("does not suspend and returns cache data when data is alredy in the cache", async () => { +test("does not suspend and returns cache data when data is already in the cache", async () => { interface ItemFragment { __typename: "Item"; id: number; From 558474108d8b607db9c72ac2ba38de1be0b493aa Mon Sep 17 00:00:00 2001 From: Jerel Miller Date: Tue, 17 Dec 2024 14:09:12 -0700 Subject: [PATCH 19/43] Add test to ensure cache updates are received after initial result --- .../__tests__/useSuspenseFragment.test.tsx | 84 +++++++++++++++++++ 1 file changed, 84 insertions(+) diff --git a/src/react/hooks/__tests__/useSuspenseFragment.test.tsx b/src/react/hooks/__tests__/useSuspenseFragment.test.tsx index 7b4d25ecabe..acee9da8f96 100644 --- a/src/react/hooks/__tests__/useSuspenseFragment.test.tsx +++ b/src/react/hooks/__tests__/useSuspenseFragment.test.tsx @@ -409,3 +409,87 @@ test("does not suspend and returns cache data when data is already in the cache" await expect(takeRender).not.toRerender(); }); + +test("receives cache updates after initial result when data is written to the cache before mounted", async () => { + interface ItemFragment { + __typename: "Item"; + id: number; + text: string; + } + + const { takeRender, render, replaceSnapshot } = + createDefaultRenderStream(); + const { SuspenseFallback } = createDefaultTrackedComponents(); + + const client = new ApolloClient({ cache: new InMemoryCache() }); + + const fragment: TypedDocumentNode = gql` + fragment ItemFragment on Item { + id + text + } + `; + + client.writeFragment({ + fragment, + data: { __typename: "Item", id: 1, text: "Cached" }, + }); + + function App() { + useTrackRenders(); + + const result = useSuspenseFragment({ + fragment, + from: { __typename: "Item", id: 1 }, + }); + + replaceSnapshot({ result }); + + return null; + } + + using _disabledAct = disableActEnvironment(); + await render( + }> + + , + { + wrapper: ({ children }) => ( + {children} + ), + } + ); + + { + const { snapshot, renderedComponents } = await takeRender(); + + expect(renderedComponents).toStrictEqual([App]); + expect(snapshot.result).toEqual({ + data: { + __typename: "Item", + id: 1, + text: "Cached", + }, + }); + } + + client.writeFragment({ + fragment, + data: { __typename: "Item", id: 1, text: "Updated" }, + }); + + { + const { snapshot, renderedComponents } = await takeRender(); + + expect(renderedComponents).toStrictEqual([App]); + expect(snapshot.result).toEqual({ + data: { + __typename: "Item", + id: 1, + text: "Updated", + }, + }); + } + + await expect(takeRender).not.toRerender(); +}); From 48ff08e30d95a33f04699bbd7a06c24479519034 Mon Sep 17 00:00:00 2001 From: Jerel Miller Date: Tue, 17 Dec 2024 14:17:23 -0700 Subject: [PATCH 20/43] Add test to ensure data can be overwritten --- .../__tests__/useSuspenseFragment.test.tsx | 48 +++++++++++++++++++ 1 file changed, 48 insertions(+) diff --git a/src/react/hooks/__tests__/useSuspenseFragment.test.tsx b/src/react/hooks/__tests__/useSuspenseFragment.test.tsx index acee9da8f96..50e0c4d6c1f 100644 --- a/src/react/hooks/__tests__/useSuspenseFragment.test.tsx +++ b/src/react/hooks/__tests__/useSuspenseFragment.test.tsx @@ -14,6 +14,7 @@ import { ApolloProvider } from "../../context"; import { createRenderStream, disableActEnvironment, + renderHookToSnapshotStream, useTrackRenders, } from "@testing-library/react-render-stream"; import { spyOnConsole } from "../../../testing/internal"; @@ -493,3 +494,50 @@ test("receives cache updates after initial result when data is written to the ca await expect(takeRender).not.toRerender(); }); + +test("allows the client to be overridden", async () => { + interface ItemFragment { + __typename: "Item"; + id: number; + text: string; + } + + const fragment: TypedDocumentNode = gql` + fragment ItemFragment on Item { + id + text + } + `; + + const defaultClient = new ApolloClient({ cache: new InMemoryCache() }); + const client = new ApolloClient({ cache: new InMemoryCache() }); + + defaultClient.writeFragment({ + fragment, + data: { __typename: "Item", id: 1, text: "Should not be used" }, + }); + + client.writeFragment({ + fragment, + data: { __typename: "Item", id: 1, text: "Item #1" }, + }); + + using _disabledAct = disableActEnvironment(); + const { takeSnapshot } = await renderHookToSnapshotStream( + () => + useSuspenseFragment({ + fragment, + client, + from: { __typename: "Item", id: 1 }, + }), + { + wrapper: ({ children }) => ( + {children} + ), + } + ); + + const { data } = await takeSnapshot(); + + expect(data).toEqual({ __typename: "Item", id: 1, text: "Item #1" }); +}); From dd2d5ce12db2b0956481e43019fe848ac44de293 Mon Sep 17 00:00:00 2001 From: Jerel Miller Date: Tue, 17 Dec 2024 14:22:15 -0700 Subject: [PATCH 21/43] Add test to ensure client is provided --- .../hooks/__tests__/useSuspenseFragment.test.tsx | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/src/react/hooks/__tests__/useSuspenseFragment.test.tsx b/src/react/hooks/__tests__/useSuspenseFragment.test.tsx index 50e0c4d6c1f..3e3d107f19c 100644 --- a/src/react/hooks/__tests__/useSuspenseFragment.test.tsx +++ b/src/react/hooks/__tests__/useSuspenseFragment.test.tsx @@ -541,3 +541,19 @@ test("allows the client to be overridden", async () => { expect(data).toEqual({ __typename: "Item", id: 1, text: "Item #1" }); }); + +test("throws if no client is provided", () => { + using _spy = spyOnConsole("error"); + expect(() => + renderHook(() => + useSuspenseFragment({ + fragment: gql` + fragment ShouldThrow on Error { + shouldThrow + } + `, + from: {}, + }) + ) + ).toThrow(/pass an ApolloClient/); +}); From 5d9c2e1951eb737a087a763166f7d9aa7038ec62 Mon Sep 17 00:00:00 2001 From: Jerel Miller Date: Tue, 17 Dec 2024 15:35:33 -0700 Subject: [PATCH 22/43] Add test to ensure changing from values suspends correctly --- .../__tests__/useSuspenseFragment.test.tsx | 124 ++++++++++++++++-- 1 file changed, 110 insertions(+), 14 deletions(-) diff --git a/src/react/hooks/__tests__/useSuspenseFragment.test.tsx b/src/react/hooks/__tests__/useSuspenseFragment.test.tsx index 3e3d107f19c..b6611622e3c 100644 --- a/src/react/hooks/__tests__/useSuspenseFragment.test.tsx +++ b/src/react/hooks/__tests__/useSuspenseFragment.test.tsx @@ -60,6 +60,22 @@ test("validates the GraphQL document is a fragment", () => { ); }); +test("throws if no client is provided", () => { + using _spy = spyOnConsole("error"); + expect(() => + renderHook(() => + useSuspenseFragment({ + fragment: gql` + fragment ShouldThrow on Error { + shouldThrow + } + `, + from: {}, + }) + ) + ).toThrow(/pass an ApolloClient/); +}); + test("suspends until cache value is complete", async () => { interface ItemFragment { __typename: "Item"; @@ -542,18 +558,98 @@ test("allows the client to be overridden", async () => { expect(data).toEqual({ __typename: "Item", id: 1, text: "Item #1" }); }); -test("throws if no client is provided", () => { - using _spy = spyOnConsole("error"); - expect(() => - renderHook(() => - useSuspenseFragment({ - fragment: gql` - fragment ShouldThrow on Error { - shouldThrow - } - `, - from: {}, - }) - ) - ).toThrow(/pass an ApolloClient/); +test("suspends until data is complete when changing `from` with no data written to cache", async () => { + interface ItemFragment { + __typename: "Item"; + id: number; + text: string; + } + + const fragment: TypedDocumentNode = gql` + fragment ItemFragment on Item { + id + text + } + `; + + const { takeRender, replaceSnapshot, render } = + createDefaultRenderStream(); + const { SuspenseFallback } = createDefaultTrackedComponents(); + + const client = new ApolloClient({ cache: new InMemoryCache() }); + + client.writeFragment({ + fragment, + data: { __typename: "Item", id: 1, text: "Item #1" }, + }); + + using _disabledAct = disableActEnvironment(); + function App({ id }: { id: number }) { + useTrackRenders(); + + const result = useSuspenseFragment({ + fragment, + from: { __typename: "Item", id }, + }); + + replaceSnapshot({ result }); + + return null; + } + + const { rerender } = await render( + }> + + , + { + wrapper: ({ children }) => ( + {children} + ), + } + ); + + { + const { snapshot, renderedComponents } = await takeRender(); + + expect(renderedComponents).toStrictEqual([App]); + expect(snapshot.result).toEqual({ + data: { + __typename: "Item", + id: 1, + text: "Item #1", + }, + }); + } + + await rerender( + }> + + + ); + + { + const { renderedComponents } = await takeRender(); + + expect(renderedComponents).toStrictEqual([SuspenseFallback]); + } + + client.writeFragment({ + fragment, + data: { __typename: "Item", id: 2, text: "Item #2" }, + }); + + { + const { snapshot, renderedComponents } = await takeRender(); + + expect(renderedComponents).toStrictEqual([App]); + expect(snapshot.result).toEqual({ + data: { + __typename: "Item", + id: 2, + text: "Item #2", + }, + }); + } + + await expect(takeRender).not.toRerender(); }); From febce9a38c1d29cd7ed06511fffb06225805ee4b Mon Sep 17 00:00:00 2001 From: Jerel Miller Date: Tue, 17 Dec 2024 15:37:13 -0700 Subject: [PATCH 23/43] Add test for changing from with data already written to cache --- .../__tests__/useSuspenseFragment.test.tsx | 90 +++++++++++++++++++ 1 file changed, 90 insertions(+) diff --git a/src/react/hooks/__tests__/useSuspenseFragment.test.tsx b/src/react/hooks/__tests__/useSuspenseFragment.test.tsx index b6611622e3c..1717dba0604 100644 --- a/src/react/hooks/__tests__/useSuspenseFragment.test.tsx +++ b/src/react/hooks/__tests__/useSuspenseFragment.test.tsx @@ -653,3 +653,93 @@ test("suspends until data is complete when changing `from` with no data written await expect(takeRender).not.toRerender(); }); + +test("does not suspend when changing `from` with data already written to cache", async () => { + interface ItemFragment { + __typename: "Item"; + id: number; + text: string; + } + + const fragment: TypedDocumentNode = gql` + fragment ItemFragment on Item { + id + text + } + `; + + const { takeRender, replaceSnapshot, render } = + createDefaultRenderStream(); + const { SuspenseFallback } = createDefaultTrackedComponents(); + + const client = new ApolloClient({ cache: new InMemoryCache() }); + + client.writeFragment({ + fragment, + data: { __typename: "Item", id: 1, text: "Item #1" }, + }); + + client.writeFragment({ + fragment, + data: { __typename: "Item", id: 2, text: "Item #2" }, + }); + + using _disabledAct = disableActEnvironment(); + function App({ id }: { id: number }) { + useTrackRenders(); + + const result = useSuspenseFragment({ + fragment, + from: { __typename: "Item", id }, + }); + + replaceSnapshot({ result }); + + return null; + } + + const { rerender } = await render( + }> + + , + { + wrapper: ({ children }) => ( + {children} + ), + } + ); + + { + const { snapshot, renderedComponents } = await takeRender(); + + expect(renderedComponents).toStrictEqual([App]); + expect(snapshot.result).toEqual({ + data: { + __typename: "Item", + id: 1, + text: "Item #1", + }, + }); + } + + await rerender( + }> + + + ); + + { + const { snapshot, renderedComponents } = await takeRender(); + + expect(renderedComponents).toStrictEqual([App]); + expect(snapshot.result).toEqual({ + data: { + __typename: "Item", + id: 2, + text: "Item #2", + }, + }); + } + + await expect(takeRender).not.toRerender(); +}); From 316d0cb5526fcb3d0d7773c67cd7c656ff3fa91d Mon Sep 17 00:00:00 2001 From: Jerel Miller Date: Tue, 17 Dec 2024 15:41:26 -0700 Subject: [PATCH 24/43] Add test to check @nonreactive --- .../__tests__/useSuspenseFragment.test.tsx | 55 +++++++++++++++++++ 1 file changed, 55 insertions(+) diff --git a/src/react/hooks/__tests__/useSuspenseFragment.test.tsx b/src/react/hooks/__tests__/useSuspenseFragment.test.tsx index 1717dba0604..f3cad92f672 100644 --- a/src/react/hooks/__tests__/useSuspenseFragment.test.tsx +++ b/src/react/hooks/__tests__/useSuspenseFragment.test.tsx @@ -743,3 +743,58 @@ test("does not suspend when changing `from` with data already written to cache", await expect(takeRender).not.toRerender(); }); + +it("does not rerender when fields with @nonreactive change", async () => { + interface ItemFragment { + __typename: "Item"; + id: number; + text: string; + } + + const fragment: TypedDocumentNode = gql` + fragment ItemFragment on Item { + id + text @nonreactive + } + `; + + const client = new ApolloClient({ cache: new InMemoryCache() }); + + client.writeFragment({ + fragment, + data: { __typename: "Item", id: 1, text: "Item #1" }, + }); + + using _disabledAct = disableActEnvironment(); + + const { takeSnapshot } = await renderHookToSnapshotStream( + () => + useSuspenseFragment({ fragment, from: { __typename: "Item", id: 1 } }), + { + wrapper: ({ children }) => ( + {children} + ), + } + ); + + { + const { data } = await takeSnapshot(); + + expect(data).toEqual({ + __typename: "Item", + id: 1, + text: "Item #1", + }); + } + + client.writeFragment({ + fragment, + data: { + __typename: "Item", + id: 1, + text: "Item #1 (updated)", + }, + }); + + await expect(takeSnapshot).not.toRerender(); +}); From 5e189846d9a1892393ed47c11df4c54aba5d217e Mon Sep 17 00:00:00 2001 From: Jerel Miller Date: Tue, 17 Dec 2024 15:58:38 -0700 Subject: [PATCH 25/43] Add test for @nonreactive with nested fragment --- .../__tests__/useSuspenseFragment.test.tsx | 65 +++++++++++++++++++ 1 file changed, 65 insertions(+) diff --git a/src/react/hooks/__tests__/useSuspenseFragment.test.tsx b/src/react/hooks/__tests__/useSuspenseFragment.test.tsx index f3cad92f672..632b7cd8da3 100644 --- a/src/react/hooks/__tests__/useSuspenseFragment.test.tsx +++ b/src/react/hooks/__tests__/useSuspenseFragment.test.tsx @@ -798,3 +798,68 @@ it("does not rerender when fields with @nonreactive change", async () => { await expect(takeSnapshot).not.toRerender(); }); + +it("does not rerender when fields with @nonreactive on nested fragment change", async () => { + interface ItemFragment { + __typename: "Item"; + id: number; + text: string; + } + + const fragment: TypedDocumentNode = gql` + fragment ItemFragment on Item { + id + ...ItemFields @nonreactive + } + + fragment ItemFields on Item { + text + } + `; + + const client = new ApolloClient({ cache: new InMemoryCache() }); + + client.writeFragment({ + fragment, + fragmentName: "ItemFragment", + data: { __typename: "Item", id: 1, text: "Item #1" }, + }); + + using _disabledAct = disableActEnvironment(); + + const { takeSnapshot } = await renderHookToSnapshotStream( + () => + useSuspenseFragment({ + fragment, + fragmentName: "ItemFragment", + from: { __typename: "Item", id: 1 }, + }), + { + wrapper: ({ children }) => ( + {children} + ), + } + ); + + { + const { data } = await takeSnapshot(); + + expect(data).toEqual({ + __typename: "Item", + id: 1, + text: "Item #1", + }); + } + + client.writeFragment({ + fragment, + fragmentName: "ItemFragment", + data: { + __typename: "Item", + id: 1, + text: "Item #1 (updated)", + }, + }); + + await expect(takeSnapshot).not.toRerender(); +}); From 8842eff990869af3fbfed299755e3aa3d8e73a48 Mon Sep 17 00:00:00 2001 From: Jerel Miller Date: Tue, 17 Dec 2024 16:05:53 -0700 Subject: [PATCH 26/43] Add failing test for suspending when not passing key fields to fragment --- .../__tests__/useSuspenseFragment.test.tsx | 59 +++++++++++++++++++ 1 file changed, 59 insertions(+) diff --git a/src/react/hooks/__tests__/useSuspenseFragment.test.tsx b/src/react/hooks/__tests__/useSuspenseFragment.test.tsx index 632b7cd8da3..5f43ab222f2 100644 --- a/src/react/hooks/__tests__/useSuspenseFragment.test.tsx +++ b/src/react/hooks/__tests__/useSuspenseFragment.test.tsx @@ -863,3 +863,62 @@ it("does not rerender when fields with @nonreactive on nested fragment change", await expect(takeSnapshot).not.toRerender(); }); + +// TODO: Update when https://github.com/apollographql/apollo-client/issues/12003 is fixed +it.failing( + "warns and suspends when passing parent object to `from` when key fields are missing", + async () => { + using _ = spyOnConsole("warn"); + + interface Fragment { + age: number; + } + + const fragment: TypedDocumentNode = gql` + fragment UserFields on User { + age + } + `; + + const client = new ApolloClient({ cache: new InMemoryCache() }); + + const { replaceSnapshot, render, takeRender } = + createDefaultRenderStream(); + const { SuspenseFallback } = createDefaultTrackedComponents(); + + function App() { + const result = useSuspenseFragment({ + fragment, + from: { __typename: "User" }, + }); + + replaceSnapshot({ result }); + + return null; + } + + using _disabledAct = disableActEnvironment(); + await render( + }> + + , + { + wrapper: ({ children }) => ( + {children} + ), + } + ); + + expect(console.warn).toHaveBeenCalledTimes(1); + expect(console.warn).toHaveBeenCalledWith( + "Could not identify object passed to `from` for '%s' fragment, either because the object is non-normalized or the key fields are missing. If you are masking this object, please ensure the key fields are requested by the parent object.", + "UserFields" + ); + + { + const { renderedComponents } = await takeRender(); + + expect(renderedComponents).toStrictEqual([SuspenseFallback]); + } + } +); From 23d6b3ec53857277931616051f5257cd5525b49b Mon Sep 17 00:00:00 2001 From: Jerel Miller Date: Tue, 17 Dec 2024 16:12:15 -0700 Subject: [PATCH 27/43] Use deep equality in FragmentReference --- src/react/internal/cache/FragmentReference.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/react/internal/cache/FragmentReference.ts b/src/react/internal/cache/FragmentReference.ts index 346b59087b0..0bb05a9c98d 100644 --- a/src/react/internal/cache/FragmentReference.ts +++ b/src/react/internal/cache/FragmentReference.ts @@ -1,3 +1,4 @@ +import { equal } from "@wry/equality"; import type { WatchFragmentOptions, WatchFragmentResult, @@ -123,7 +124,7 @@ export class FragmentReference< // we subscribe for the first time. We create a fulfilled promise in the // constructor with a value that is the same as the first emitted value // so we want to skip delivering it. - if (this.promise.value === result.data) { + if (equal(this.promise.value, result.data)) { return; } From dccaaacd3a159a150c13bac572d091088448a871 Mon Sep 17 00:00:00 2001 From: Jerel Miller Date: Tue, 17 Dec 2024 16:12:24 -0700 Subject: [PATCH 28/43] Add tests for data masking --- .../__tests__/useSuspenseFragment.test.tsx | 151 ++++++++++++++++++ 1 file changed, 151 insertions(+) diff --git a/src/react/hooks/__tests__/useSuspenseFragment.test.tsx b/src/react/hooks/__tests__/useSuspenseFragment.test.tsx index 5f43ab222f2..13460e53e19 100644 --- a/src/react/hooks/__tests__/useSuspenseFragment.test.tsx +++ b/src/react/hooks/__tests__/useSuspenseFragment.test.tsx @@ -922,3 +922,154 @@ it.failing( } } ); + +test("returns masked fragment when data masking is enabled", async () => { + type Post = { + __typename: "Post"; + id: number; + title: string; + } & { " $fragmentRefs"?: { PostFields: PostFields } }; + + type PostFields = { + __typename: "Post"; + updatedAt: string; + } & { " $fragmentName"?: "PostFields" }; + + const client = new ApolloClient({ + dataMasking: true, + cache: new InMemoryCache(), + }); + + const fragment: TypedDocumentNode = gql` + fragment PostFragment on Post { + id + title + ...PostFields + } + + fragment PostFields on Post { + updatedAt + } + `; + + client.writeFragment({ + fragment, + fragmentName: "PostFragment", + data: { + __typename: "Post", + id: 1, + title: "Blog post", + updatedAt: "2024-01-01", + }, + }); + + using _disabledAct = disableActEnvironment(); + const { takeSnapshot } = await renderHookToSnapshotStream( + () => + useSuspenseFragment({ + fragment, + fragmentName: "PostFragment", + from: { __typename: "Post", id: 1 }, + }), + { + wrapper: ({ children }) => ( + {children} + ), + } + ); + + { + const snapshot = await takeSnapshot(); + + expect(snapshot).toEqual({ + data: { + __typename: "Post", + id: 1, + title: "Blog post", + }, + }); + } + + await expect(takeSnapshot).not.toRerender(); +}); + +test("does not rerender for cache writes to masked fields", async () => { + type Post = { + __typename: "Post"; + id: number; + title: string; + } & { " $fragmentRefs"?: { PostFields: PostFields } }; + + type PostFields = { + __typename: "Post"; + updatedAt: string; + } & { " $fragmentName"?: "PostFields" }; + + const client = new ApolloClient({ + dataMasking: true, + cache: new InMemoryCache(), + }); + + const fragment: TypedDocumentNode = gql` + fragment PostFragment on Post { + id + title + ...PostFields + } + + fragment PostFields on Post { + updatedAt + } + `; + + client.writeFragment({ + fragment, + fragmentName: "PostFragment", + data: { + __typename: "Post", + id: 1, + title: "Blog post", + updatedAt: "2024-01-01", + }, + }); + + using _disabledAct = disableActEnvironment(); + const { takeSnapshot } = await renderHookToSnapshotStream( + () => + useSuspenseFragment({ + fragment, + fragmentName: "PostFragment", + from: { __typename: "Post", id: 1 }, + }), + { + wrapper: ({ children }) => ( + {children} + ), + } + ); + + { + const snapshot = await takeSnapshot(); + + expect(snapshot).toEqual({ + data: { + __typename: "Post", + id: 1, + title: "Blog post", + }, + }); + } + + client.writeFragment({ + fragment, + fragmentName: "PostFragment", + data: { + __typename: "Post", + id: 1, + title: "Blog post", + updatedAt: "2024-02-01", + }, + }); + + await expect(takeSnapshot).not.toRerender(); +}); From 4cbe24de52416c5194834345924d62b37abb9a6e Mon Sep 17 00:00:00 2001 From: Jerel Miller Date: Tue, 17 Dec 2024 16:22:44 -0700 Subject: [PATCH 29/43] Add test to check for cache updates to masked fields --- .../__tests__/useSuspenseFragment.test.tsx | 140 ++++++++++++++++++ 1 file changed, 140 insertions(+) diff --git a/src/react/hooks/__tests__/useSuspenseFragment.test.tsx b/src/react/hooks/__tests__/useSuspenseFragment.test.tsx index 13460e53e19..928e3e53871 100644 --- a/src/react/hooks/__tests__/useSuspenseFragment.test.tsx +++ b/src/react/hooks/__tests__/useSuspenseFragment.test.tsx @@ -4,8 +4,11 @@ import { } from "../useSuspenseFragment"; import { ApolloClient, + FragmentType, gql, InMemoryCache, + Masked, + MaskedDocumentNode, MaybeMasked, TypedDocumentNode, } from "../../../core"; @@ -1073,3 +1076,140 @@ test("does not rerender for cache writes to masked fields", async () => { await expect(takeSnapshot).not.toRerender(); }); + +test("updates child fragments for cache updates to masked fields", async () => { + type Post = { + __typename: "Post"; + id: number; + title: string; + } & { " $fragmentRefs"?: { PostFields: PostFields } }; + + type PostFields = { + __typename: "Post"; + updatedAt: string; + } & { " $fragmentName"?: "PostFields" }; + + const client = new ApolloClient({ + dataMasking: true, + cache: new InMemoryCache(), + }); + + const postFieldsFragment: MaskedDocumentNode = gql` + fragment PostFields on Post { + updatedAt + } + `; + + const postFragment: MaskedDocumentNode = gql` + fragment PostFragment on Post { + id + title + ...PostFields + } + + ${postFieldsFragment} + `; + + client.writeFragment({ + fragment: postFragment, + fragmentName: "PostFragment", + data: { + __typename: "Post", + id: 1, + title: "Blog post", + updatedAt: "2024-01-01", + }, + }); + + const { render, mergeSnapshot, takeRender } = createRenderStream({ + initialSnapshot: { + parent: null as UseSuspenseFragmentResult> | null, + child: null as UseSuspenseFragmentResult> | null, + }, + }); + + function Parent() { + useTrackRenders(); + const parent = useSuspenseFragment({ + fragment: postFragment, + fragmentName: "PostFragment", + from: { __typename: "Post", id: 1 }, + }); + + mergeSnapshot({ parent }); + + return ; + } + + function Child({ post }: { post: FragmentType }) { + useTrackRenders(); + const child = useSuspenseFragment({ + fragment: postFieldsFragment, + from: post, + }); + + mergeSnapshot({ child }); + return null; + } + + using _disabledAct = disableActEnvironment(); + await render(, { + wrapper: ({ children }) => ( + {children} + ), + }); + + { + const { snapshot } = await takeRender(); + + expect(snapshot).toEqual({ + parent: { + data: { + __typename: "Post", + id: 1, + title: "Blog post", + }, + }, + child: { + data: { + __typename: "Post", + updatedAt: "2024-01-01", + }, + }, + }); + } + + client.writeFragment({ + fragment: postFragment, + fragmentName: "PostFragment", + data: { + __typename: "Post", + id: 1, + title: "Blog post", + updatedAt: "2024-02-01", + }, + }); + + { + const { snapshot, renderedComponents } = await takeRender(); + + expect(renderedComponents).toStrictEqual([Child]); + expect(snapshot).toEqual({ + parent: { + data: { + __typename: "Post", + id: 1, + title: "Blog post", + }, + }, + child: { + data: { + __typename: "Post", + updatedAt: "2024-02-01", + }, + }, + }); + } + + await expect(takeRender).not.toRerender(); +}); From 5248b63ec3e63a25cb850bd817fa980670c90347 Mon Sep 17 00:00:00 2001 From: Jerel Miller Date: Wed, 18 Dec 2024 12:48:02 -0700 Subject: [PATCH 30/43] Add support for `null` value as `from` in `useSuspenseFragment` --- .../__tests__/useSuspenseFragment.test.tsx | 66 +++++++++++++ src/react/hooks/useSuspenseFragment.ts | 97 ++++++++++++++----- 2 files changed, 141 insertions(+), 22 deletions(-) diff --git a/src/react/hooks/__tests__/useSuspenseFragment.test.tsx b/src/react/hooks/__tests__/useSuspenseFragment.test.tsx index 928e3e53871..ca98bad87ca 100644 --- a/src/react/hooks/__tests__/useSuspenseFragment.test.tsx +++ b/src/react/hooks/__tests__/useSuspenseFragment.test.tsx @@ -24,6 +24,7 @@ import { spyOnConsole } from "../../../testing/internal"; import { renderHook } from "@testing-library/react"; import { InvariantError } from "ts-invariant"; import { MockedProvider } from "../../../testing"; +import { expectTypeOf } from "expect-type"; function createDefaultRenderStream() { return createRenderStream({ @@ -926,6 +927,37 @@ it.failing( } ); +test("returns null if `from` is `null`", async () => { + interface ItemFragment { + __typename: "Item"; + id: number; + text: string; + } + + const fragment: TypedDocumentNode = gql` + fragment ItemFragment on Item { + id + text + } + `; + + const client = new ApolloClient({ cache: new InMemoryCache() }); + + using _disabledAct = disableActEnvironment(); + const { takeSnapshot } = await renderHookToSnapshotStream( + () => useSuspenseFragment({ fragment, from: null }), + { + wrapper: ({ children }) => ( + {children} + ), + } + ); + + const { data } = await takeSnapshot(); + + expect(data).toBeNull(); +}); + test("returns masked fragment when data masking is enabled", async () => { type Post = { __typename: "Post"; @@ -1213,3 +1245,37 @@ test("updates child fragments for cache updates to masked fields", async () => { await expect(takeRender).not.toRerender(); }); + +describe.skip("type tests", () => { + test("returns TData when from is a non-null value", () => { + const fragment: TypedDocumentNode<{ foo: string }> = gql``; + + const { data } = useSuspenseFragment({ + fragment, + from: { __typename: "Query" }, + }); + + expectTypeOf(data).branded.toEqualTypeOf<{ foo: string }>(); + }); + + test("returns TData | null when from is null", () => { + type Data = { foo: string }; + type Vars = Record; + const fragment: TypedDocumentNode = gql``; + + const { data } = useSuspenseFragment({ fragment, from: null }); + + expectTypeOf(data).branded.toEqualTypeOf(); + }); + + test("returns TData | null when from is nullable", () => { + type Post = { __typename: "Post"; id: number }; + type Vars = Record; + const fragment: TypedDocumentNode = gql``; + const author = {} as { post: Post | null }; + + const { data } = useSuspenseFragment({ fragment, from: author.post }); + + expectTypeOf(data).branded.toEqualTypeOf(); + }); +}); diff --git a/src/react/hooks/useSuspenseFragment.ts b/src/react/hooks/useSuspenseFragment.ts index 9f281bec1ba..f944af06808 100644 --- a/src/react/hooks/useSuspenseFragment.ts +++ b/src/react/hooks/useSuspenseFragment.ts @@ -23,7 +23,7 @@ export interface UseSuspenseFragmentOptions Cache.ReadFragmentOptions, "id" | "variables" | "returnPartialData" > { - from: StoreObject | Reference | string; + from: StoreObject | Reference | string | null; // Override this field to make it optional (default: true). optimistic?: boolean; /** @@ -39,12 +39,51 @@ export interface UseSuspenseFragmentOptions export type UseSuspenseFragmentResult = { data: MaybeMasked }; +const NULL_PLACEHOLDER = [] as unknown as [ + FragmentKey, + Promise | null>, +]; + +export function useSuspenseFragment< + TData, + TVariables extends OperationVariables, +>( + options: UseSuspenseFragmentOptions & { + from: {}; + } +): UseSuspenseFragmentResult; + +export function useSuspenseFragment< + TData, + TVariables extends OperationVariables, +>( + options: UseSuspenseFragmentOptions & { + from: null; + } +): UseSuspenseFragmentResult; + +export function useSuspenseFragment< + TData, + TVariables extends OperationVariables, +>( + options: UseSuspenseFragmentOptions & { + from: {} | null; + } +): UseSuspenseFragmentResult; + +export function useSuspenseFragment< + TData, + TVariables extends OperationVariables, +>( + options: UseSuspenseFragmentOptions +): UseSuspenseFragmentResult; + export function useSuspenseFragment< TData = unknown, TVariables extends OperationVariables = OperationVariables, >( options: UseSuspenseFragmentOptions -): UseSuspenseFragmentResult { +): UseSuspenseFragmentResult { return wrapHook( "useSuspenseFragment", _useSuspenseFragment, @@ -57,35 +96,41 @@ function _useSuspenseFragment< TVariables extends OperationVariables = OperationVariables, >( options: UseSuspenseFragmentOptions -): UseSuspenseFragmentResult { +): UseSuspenseFragmentResult { const client = useApolloClient(options.client); const { from } = options; const { cache } = client; const id = useMemo( - () => (typeof from === "string" ? from : cache.identify(from)), + () => + typeof from === "string" ? from + : from === null ? null + : cache.identify(from), [cache, from] - )!; + ) as string | null; - const fragmentRef = getSuspenseCache(client).getFragmentRef< - TData, - TVariables - >([id, options.fragment, canonicalStringify(options.variables)], client, { - ...options, - from: id, - }); + const fragmentRef = + id === null ? null : ( + getSuspenseCache(client).getFragmentRef( + [id, options.fragment, canonicalStringify(options.variables)], + client, + { ...options, from: id } + ) + ); let [current, setPromise] = React.useState< - [FragmentKey, Promise>] - >([fragmentRef.key, fragmentRef.promise]); - - if (current[0] !== fragmentRef.key) { - // eslint-disable-next-line react-compiler/react-compiler - current[0] = fragmentRef.key; - current[1] = fragmentRef.promise; - } + [FragmentKey, Promise | null>] + >( + fragmentRef === null ? NULL_PLACEHOLDER : ( + [fragmentRef.key, fragmentRef.promise] + ) + ); React.useEffect(() => { + if (fragmentRef === null) { + return; + } + const dispose = fragmentRef.retain(); const removeListener = fragmentRef.listen((promise) => { setPromise([fragmentRef.key, promise]); @@ -97,9 +142,17 @@ function _useSuspenseFragment< }; }, [fragmentRef]); - let promise = current[1]; + if (fragmentRef === null) { + return { data: null }; + } + + if (current[0] !== fragmentRef!.key) { + // eslint-disable-next-line react-compiler/react-compiler + current[0] = fragmentRef!.key; + current[1] = fragmentRef!.promise; + } - const data = __use(promise); + const data = __use(current[1]); return { data }; } From 45d0741ef7edf7351659944e41f57443817f89e5 Mon Sep 17 00:00:00 2001 From: Jerel Miller Date: Wed, 18 Dec 2024 12:55:24 -0700 Subject: [PATCH 31/43] Add test to ensure changing from null works as expected --- .../__tests__/useSuspenseFragment.test.tsx | 61 +++++++++++++++++++ 1 file changed, 61 insertions(+) diff --git a/src/react/hooks/__tests__/useSuspenseFragment.test.tsx b/src/react/hooks/__tests__/useSuspenseFragment.test.tsx index ca98bad87ca..5703bd16826 100644 --- a/src/react/hooks/__tests__/useSuspenseFragment.test.tsx +++ b/src/react/hooks/__tests__/useSuspenseFragment.test.tsx @@ -958,6 +958,67 @@ test("returns null if `from` is `null`", async () => { expect(data).toBeNull(); }); +test("returns cached value when `from` changes from `null` to non-null value", async () => { + interface ItemFragment { + __typename: "Item"; + id: number; + text: string; + } + + const fragment: TypedDocumentNode = gql` + fragment ItemFragment on Item { + id + text + } + `; + + const client = new ApolloClient({ cache: new InMemoryCache() }); + + client.writeFragment({ + fragment, + data: { + __typename: "Item", + id: 1, + text: "Item #1", + }, + }); + + using _disabledAct = disableActEnvironment(); + const { takeSnapshot, rerender } = await renderHookToSnapshotStream( + ({ id }) => + useSuspenseFragment({ + fragment, + from: id === null ? null : { __typename: "Item", id }, + }), + { + initialProps: { id: null as null | number }, + wrapper: ({ children }) => ( + {children} + ), + } + ); + + { + const { data } = await takeSnapshot(); + + expect(data).toBeNull(); + } + + await rerender({ id: 1 }); + + { + const { data } = await takeSnapshot(); + + expect(data).toEqual({ + __typename: "Item", + id: 1, + text: "Item #1", + }); + } + + await expect(takeSnapshot).not.toRerender(); +}); + test("returns masked fragment when data masking is enabled", async () => { type Post = { __typename: "Post"; From 39f871d0139ea6afc14e430ba3f699aa6ce60bf2 Mon Sep 17 00:00:00 2001 From: Jerel Miller Date: Wed, 18 Dec 2024 13:17:21 -0700 Subject: [PATCH 32/43] Remove unneeded type cast --- src/react/hooks/useSuspenseFragment.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/react/hooks/useSuspenseFragment.ts b/src/react/hooks/useSuspenseFragment.ts index f944af06808..2e69d65bb68 100644 --- a/src/react/hooks/useSuspenseFragment.ts +++ b/src/react/hooks/useSuspenseFragment.ts @@ -146,10 +146,10 @@ function _useSuspenseFragment< return { data: null }; } - if (current[0] !== fragmentRef!.key) { + if (current[0] !== fragmentRef.key) { // eslint-disable-next-line react-compiler/react-compiler - current[0] = fragmentRef!.key; - current[1] = fragmentRef!.promise; + current[0] = fragmentRef.key; + current[1] = fragmentRef.promise; } const data = __use(current[1]); From ce33bc38ad31cd2715d1998be0adc8b7176cc64e Mon Sep 17 00:00:00 2001 From: Jerel Miller Date: Wed, 18 Dec 2024 13:17:53 -0700 Subject: [PATCH 33/43] Add test to ensure useSuspenseFragment suspends when switching from null --- .../__tests__/useSuspenseFragment.test.tsx | 88 +++++++++++++++++++ 1 file changed, 88 insertions(+) diff --git a/src/react/hooks/__tests__/useSuspenseFragment.test.tsx b/src/react/hooks/__tests__/useSuspenseFragment.test.tsx index 5703bd16826..6268e6e7a26 100644 --- a/src/react/hooks/__tests__/useSuspenseFragment.test.tsx +++ b/src/react/hooks/__tests__/useSuspenseFragment.test.tsx @@ -1019,6 +1019,94 @@ test("returns cached value when `from` changes from `null` to non-null value", a await expect(takeSnapshot).not.toRerender(); }); +test("suspends until cached value is available when `from` changes from `null` to non-null value", async () => { + interface ItemFragment { + __typename: "Item"; + id: number; + text: string; + } + + const fragment: TypedDocumentNode = gql` + fragment ItemFragment on Item { + id + text + } + `; + + const client = new ApolloClient({ cache: new InMemoryCache() }); + + const { takeRender, render, replaceSnapshot } = + createDefaultRenderStream(); + const { SuspenseFallback } = createDefaultTrackedComponents(); + + function App({ id }: { id: number | null }) { + useTrackRenders(); + const result = useSuspenseFragment({ + fragment, + from: id === null ? null : { __typename: "Item", id }, + }); + + replaceSnapshot({ result }); + + return null; + } + + using _disabledAct = disableActEnvironment(); + const { rerender } = await render( + }> + + , + { + wrapper: ({ children }) => ( + {children} + ), + } + ); + + { + const { snapshot, renderedComponents } = await takeRender(); + + expect(renderedComponents).toStrictEqual([App]); + expect(snapshot.result).toEqual({ data: null }); + } + + await rerender( + }> + + + ); + + { + const { renderedComponents } = await takeRender(); + + expect(renderedComponents).toStrictEqual([SuspenseFallback]); + } + + client.writeFragment({ + fragment, + data: { + __typename: "Item", + id: 1, + text: "Item #1", + }, + }); + + { + const { snapshot, renderedComponents } = await takeRender(); + + expect(renderedComponents).toStrictEqual([App]); + expect(snapshot.result).toEqual({ + data: { + __typename: "Item", + id: 1, + text: "Item #1", + }, + }); + } + + await expect(takeRender).not.toRerender(); +}); + test("returns masked fragment when data masking is enabled", async () => { type Post = { __typename: "Post"; From a672701f6aac260813c23fc7ed51772ee3cca991 Mon Sep 17 00:00:00 2001 From: Jerel Miller Date: Wed, 18 Dec 2024 13:19:14 -0700 Subject: [PATCH 34/43] Add test to ensure switching from non-null to null works as expected --- .../__tests__/useSuspenseFragment.test.tsx | 61 +++++++++++++++++++ 1 file changed, 61 insertions(+) diff --git a/src/react/hooks/__tests__/useSuspenseFragment.test.tsx b/src/react/hooks/__tests__/useSuspenseFragment.test.tsx index 6268e6e7a26..d4ef67097a9 100644 --- a/src/react/hooks/__tests__/useSuspenseFragment.test.tsx +++ b/src/react/hooks/__tests__/useSuspenseFragment.test.tsx @@ -1019,6 +1019,67 @@ test("returns cached value when `from` changes from `null` to non-null value", a await expect(takeSnapshot).not.toRerender(); }); +test("returns null value when `from` changes from non-null value to `null`", async () => { + interface ItemFragment { + __typename: "Item"; + id: number; + text: string; + } + + const fragment: TypedDocumentNode = gql` + fragment ItemFragment on Item { + id + text + } + `; + + const client = new ApolloClient({ cache: new InMemoryCache() }); + + client.writeFragment({ + fragment, + data: { + __typename: "Item", + id: 1, + text: "Item #1", + }, + }); + + using _disabledAct = disableActEnvironment(); + const { takeSnapshot, rerender } = await renderHookToSnapshotStream( + ({ id }) => + useSuspenseFragment({ + fragment, + from: id === null ? null : { __typename: "Item", id }, + }), + { + initialProps: { id: 1 as null | number }, + wrapper: ({ children }) => ( + {children} + ), + } + ); + + { + const { data } = await takeSnapshot(); + + expect(data).toEqual({ + __typename: "Item", + id: 1, + text: "Item #1", + }); + } + + await rerender({ id: null }); + + { + const { data } = await takeSnapshot(); + + expect(data).toBeNull(); + } + + await expect(takeSnapshot).not.toRerender(); +}); + test("suspends until cached value is available when `from` changes from `null` to non-null value", async () => { interface ItemFragment { __typename: "Item"; From 91beda7cca30f99d1bae3c4a9c0880d747b11009 Mon Sep 17 00:00:00 2001 From: Jerel Miller Date: Wed, 18 Dec 2024 13:21:32 -0700 Subject: [PATCH 35/43] Update api report and size limits --- .api-reports/api-report-react.api.md | 34 +++++++ .api-reports/api-report-react_hooks.api.md | 34 +++++++ .api-reports/api-report-react_internal.api.md | 99 ++++++++++++++++++- .api-reports/api-report.api.md | 34 +++++++ .size-limits.json | 4 +- 5 files changed, 201 insertions(+), 4 deletions(-) diff --git a/.api-reports/api-report-react.api.md b/.api-reports/api-report-react.api.md index 16e002d9106..293d3ea4854 100644 --- a/.api-reports/api-report-react.api.md +++ b/.api-reports/api-report-react.api.md @@ -2416,6 +2416,40 @@ export function useSubscription(options: UseSuspenseFragmentOptions & { + from: {}; +}): UseSuspenseFragmentResult; + +// @public (undocumented) +export function useSuspenseFragment(options: UseSuspenseFragmentOptions & { + from: null; +}): UseSuspenseFragmentResult; + +// @public (undocumented) +export function useSuspenseFragment(options: UseSuspenseFragmentOptions & { + from: {} | null; +}): UseSuspenseFragmentResult; + +// @public (undocumented) +export function useSuspenseFragment(options: UseSuspenseFragmentOptions): UseSuspenseFragmentResult; + +// @public (undocumented) +interface UseSuspenseFragmentOptions extends Omit, NoInfer>, "id" | "query" | "optimistic" | "previousResult" | "returnPartialData">, Omit, "id" | "variables" | "returnPartialData"> { + client?: ApolloClient; + // (undocumented) + from: StoreObject | Reference | string | null; + // (undocumented) + optimistic?: boolean; +} + +// @public (undocumented) +export type UseSuspenseFragmentResult = { + data: MaybeMasked; +}; + // @public (undocumented) export function useSuspenseQuery, "variables">>(query: DocumentNode | TypedDocumentNode, options?: SuspenseQueryHookOptions, NoInfer_2> & TOptions): UseSuspenseQueryResult | undefined : TData | undefined : TOptions["returnPartialData"] extends true ? TOptions["skip"] extends boolean ? DeepPartial | undefined : DeepPartial : TOptions["skip"] extends boolean ? TData | undefined : TData, TVariables>; diff --git a/.api-reports/api-report-react_hooks.api.md b/.api-reports/api-report-react_hooks.api.md index b252b2a672f..247bf875c05 100644 --- a/.api-reports/api-report-react_hooks.api.md +++ b/.api-reports/api-report-react_hooks.api.md @@ -2249,6 +2249,40 @@ export function useSubscription(options: UseSuspenseFragmentOptions & { + from: {}; +}): UseSuspenseFragmentResult; + +// @public (undocumented) +export function useSuspenseFragment(options: UseSuspenseFragmentOptions & { + from: null; +}): UseSuspenseFragmentResult; + +// @public (undocumented) +export function useSuspenseFragment(options: UseSuspenseFragmentOptions & { + from: {} | null; +}): UseSuspenseFragmentResult; + +// @public (undocumented) +export function useSuspenseFragment(options: UseSuspenseFragmentOptions): UseSuspenseFragmentResult; + +// @public (undocumented) +interface UseSuspenseFragmentOptions extends Omit, NoInfer>, "id" | "query" | "optimistic" | "previousResult" | "returnPartialData">, Omit, "id" | "variables" | "returnPartialData"> { + client?: ApolloClient; + // (undocumented) + from: StoreObject | Reference | string | null; + // (undocumented) + optimistic?: boolean; +} + +// @public (undocumented) +export type UseSuspenseFragmentResult = { + data: MaybeMasked; +}; + // Warning: (ae-forgotten-export) The symbol "SuspenseQueryHookOptions" needs to be exported by the entry point index.d.ts // // @public (undocumented) diff --git a/.api-reports/api-report-react_internal.api.md b/.api-reports/api-report-react_internal.api.md index 3e43a1dafbb..ef1459bfc52 100644 --- a/.api-reports/api-report-react_internal.api.md +++ b/.api-reports/api-report-react_internal.api.md @@ -813,6 +813,19 @@ interface FieldSpecifier { variables?: Record; } +// @public (undocumented) +type FragmentCacheKey = [ +cacheId: string, +fragment: DocumentNode, +stringifiedVariables: string +]; + +// @public (undocumented) +interface FragmentKey { + // (undocumented) + __fragmentKey?: string; +} + // @public interface FragmentMap { // (undocumented) @@ -822,6 +835,41 @@ interface FragmentMap { // @public (undocumented) type FragmentMatcher = (rootValue: any, typeCondition: string, context: any) => boolean; +// @public (undocumented) +class FragmentReference> { + // Warning: (ae-forgotten-export) The symbol "FragmentReferenceOptions" needs to be exported by the entry point index.d.ts + constructor(client: ApolloClient, watchFragmentOptions: WatchFragmentOptions & { + from: string; + }, options: FragmentReferenceOptions); + // Warning: (ae-forgotten-export) The symbol "FragmentKey" needs to be exported by the entry point index.d.ts + // + // (undocumented) + readonly key: FragmentKey; + // Warning: (ae-forgotten-export) The symbol "Listener_2" needs to be exported by the entry point index.d.ts + // + // (undocumented) + listen(listener: Listener_2>): () => void; + // (undocumented) + readonly observable: Observable>; + // Warning: (ae-forgotten-export) The symbol "FragmentRefPromise" needs to be exported by the entry point index.d.ts + // + // (undocumented) + promise: FragmentRefPromise>; + // (undocumented) + retain(): () => void; +} + +// @public (undocumented) +interface FragmentReferenceOptions { + // (undocumented) + onDispose?: () => void; +} + +// Warning: (ae-forgotten-export) The symbol "PromiseWithState" needs to be exported by the entry point index.d.ts +// +// @public (undocumented) +type FragmentRefPromise = PromiseWithState; + // @public (undocumented) type FragmentType = [ TData @@ -1042,6 +1090,9 @@ type IsStrictlyAny = UnionToIntersection> extends never ? true // @public (undocumented) type Listener = (promise: QueryRefPromise) => void; +// @public (undocumented) +type Listener_2 = (promise: FragmentRefPromise) => void; + // @public (undocumented) class LocalState { // Warning: (ae-forgotten-export) The symbol "LocalStateOptions" needs to be exported by the entry point index.d.ts @@ -1771,8 +1822,6 @@ export interface QueryReference extends Q toPromise?: unknown; } -// Warning: (ae-forgotten-export) The symbol "PromiseWithState" needs to be exported by the entry point index.d.ts -// // @public (undocumented) type QueryRefPromise = PromiseWithState>>; @@ -2004,6 +2053,13 @@ class SuspenseCache { constructor(options?: SuspenseCacheOptions); // (undocumented) add(cacheKey: CacheKey, queryRef: InternalQueryReference): void; + // Warning: (ae-forgotten-export) The symbol "FragmentCacheKey" needs to be exported by the entry point index.d.ts + // Warning: (ae-forgotten-export) The symbol "FragmentReference" needs to be exported by the entry point index.d.ts + // + // (undocumented) + getFragmentRef(cacheKey: FragmentCacheKey, client: ApolloClient, options: WatchFragmentOptions & { + from: string; + }): FragmentReference; // (undocumented) getQueryRef(cacheKey: CacheKey, createObservable: () => ObservableQuery): InternalQueryReference; } @@ -2255,6 +2311,41 @@ interface UseReadQueryResult { networkStatus: NetworkStatus; } +// Warning: (ae-forgotten-export) The symbol "UseSuspenseFragmentOptions" needs to be exported by the entry point index.d.ts +// Warning: (ae-forgotten-export) The symbol "UseSuspenseFragmentResult" needs to be exported by the entry point index.d.ts +// +// @public (undocumented) +function useSuspenseFragment(options: UseSuspenseFragmentOptions & { + from: {}; +}): UseSuspenseFragmentResult; + +// @public (undocumented) +function useSuspenseFragment(options: UseSuspenseFragmentOptions & { + from: null; +}): UseSuspenseFragmentResult; + +// @public (undocumented) +function useSuspenseFragment(options: UseSuspenseFragmentOptions & { + from: {} | null; +}): UseSuspenseFragmentResult; + +// @public (undocumented) +function useSuspenseFragment(options: UseSuspenseFragmentOptions): UseSuspenseFragmentResult; + +// @public (undocumented) +interface UseSuspenseFragmentOptions extends Omit, NoInfer>, "id" | "query" | "optimistic" | "previousResult" | "returnPartialData">, Omit, "id" | "variables" | "returnPartialData"> { + client?: ApolloClient; + // (undocumented) + from: StoreObject | Reference | string | null; + // (undocumented) + optimistic?: boolean; +} + +// @public (undocumented) +type UseSuspenseFragmentResult = { + data: MaybeMasked; +}; + // Warning: (ae-forgotten-export) The symbol "SuspenseQueryHookOptions" needs to be exported by the entry point index.d.ts // Warning: (ae-forgotten-export) The symbol "UseSuspenseQueryResult" needs to be exported by the entry point index.d.ts // @@ -2382,6 +2473,10 @@ interface WrappableHooks { // // (undocumented) useReadQuery: typeof useReadQuery; + // Warning: (ae-forgotten-export) The symbol "useSuspenseFragment" needs to be exported by the entry point index.d.ts + // + // (undocumented) + useSuspenseFragment: typeof useSuspenseFragment; // Warning: (ae-forgotten-export) The symbol "useSuspenseQuery" needs to be exported by the entry point index.d.ts // // (undocumented) diff --git a/.api-reports/api-report.api.md b/.api-reports/api-report.api.md index 850d6e603b9..4fa157e274a 100644 --- a/.api-reports/api-report.api.md +++ b/.api-reports/api-report.api.md @@ -3087,6 +3087,40 @@ export function useSubscription(options: UseSuspenseFragmentOptions & { + from: {}; +}): UseSuspenseFragmentResult; + +// @public (undocumented) +export function useSuspenseFragment(options: UseSuspenseFragmentOptions & { + from: null; +}): UseSuspenseFragmentResult; + +// @public (undocumented) +export function useSuspenseFragment(options: UseSuspenseFragmentOptions & { + from: {} | null; +}): UseSuspenseFragmentResult; + +// @public (undocumented) +export function useSuspenseFragment(options: UseSuspenseFragmentOptions): UseSuspenseFragmentResult; + +// @public (undocumented) +interface UseSuspenseFragmentOptions extends Omit, NoInfer>, "id" | "query" | "optimistic" | "previousResult" | "returnPartialData">, Omit, "id" | "variables" | "returnPartialData"> { + client?: ApolloClient; + // (undocumented) + from: StoreObject | Reference | string | null; + // (undocumented) + optimistic?: boolean; +} + +// @public (undocumented) +export type UseSuspenseFragmentResult = { + data: MaybeMasked; +}; + // @public (undocumented) export function useSuspenseQuery, "variables">>(query: DocumentNode | TypedDocumentNode, options?: SuspenseQueryHookOptions, NoInfer_2> & TOptions): UseSuspenseQueryResult | undefined : TData | undefined : TOptions["returnPartialData"] extends true ? TOptions["skip"] extends boolean ? DeepPartial | undefined : DeepPartial : TOptions["skip"] extends boolean ? TData | undefined : TData, TVariables>; diff --git a/.size-limits.json b/.size-limits.json index 54621796c0c..600748d6030 100644 --- a/.size-limits.json +++ b/.size-limits.json @@ -1,4 +1,4 @@ { - "dist/apollo-client.min.cjs": 41639, - "import { ApolloClient, InMemoryCache, HttpLink } from \"dist/index.js\" (production)": 34381 + "dist/apollo-client.min.cjs": 42108, + "import { ApolloClient, InMemoryCache, HttpLink } from \"dist/index.js\" (production)": 34349 } From 5f09a507692e282c62ef10e386b1f7b2e4094b97 Mon Sep 17 00:00:00 2001 From: Jerel Miller Date: Wed, 18 Dec 2024 13:25:01 -0700 Subject: [PATCH 36/43] Add changeset --- .changeset/blue-comics-train.md | 9 +++++++++ 1 file changed, 9 insertions(+) create mode 100644 .changeset/blue-comics-train.md diff --git a/.changeset/blue-comics-train.md b/.changeset/blue-comics-train.md new file mode 100644 index 00000000000..6001d50d059 --- /dev/null +++ b/.changeset/blue-comics-train.md @@ -0,0 +1,9 @@ +--- +"@apollo/client": minor +--- + +Add a new `useSuspenseFragment` hook. + +`useSuspenseFragment` suspends until `data` is complete. It is a drop-in +replacement for `useFragment` when you prefer to use Suspense to control the +loading state of a fragment. From 6a4132d28b7b1acbb8ab3eb4ad7423f2ad88fff8 Mon Sep 17 00:00:00 2001 From: Jerel Miller Date: Thu, 19 Dec 2024 12:39:35 -0700 Subject: [PATCH 37/43] Add tests to ensure suspense fragment is torn down --- .../__tests__/useSuspenseFragment.test.tsx | 112 +++++++++++++++++- 1 file changed, 111 insertions(+), 1 deletion(-) diff --git a/src/react/hooks/__tests__/useSuspenseFragment.test.tsx b/src/react/hooks/__tests__/useSuspenseFragment.test.tsx index d4ef67097a9..7f9a54545b1 100644 --- a/src/react/hooks/__tests__/useSuspenseFragment.test.tsx +++ b/src/react/hooks/__tests__/useSuspenseFragment.test.tsx @@ -23,7 +23,7 @@ import { import { spyOnConsole } from "../../../testing/internal"; import { renderHook } from "@testing-library/react"; import { InvariantError } from "ts-invariant"; -import { MockedProvider } from "../../../testing"; +import { MockedProvider, wait } from "../../../testing"; import { expectTypeOf } from "expect-type"; function createDefaultRenderStream() { @@ -1456,6 +1456,116 @@ test("updates child fragments for cache updates to masked fields", async () => { await expect(takeRender).not.toRerender(); }); +test("tears down the subscription on unmount", async () => { + interface ItemFragment { + __typename: "Item"; + id: number; + text: string; + } + + const fragment: TypedDocumentNode = gql` + fragment ItemFragment on Item { + id + text + } + `; + + const cache = new InMemoryCache(); + const client = new ApolloClient({ cache }); + + client.writeFragment({ + fragment, + data: { __typename: "Item", id: 1, text: "Item #1" }, + }); + + using _disabledAct = disableActEnvironment(); + const { unmount, takeSnapshot } = await renderHookToSnapshotStream( + () => + useSuspenseFragment({ fragment, from: { __typename: "Item", id: 1 } }), + { + wrapper: ({ children }) => ( + {children} + ), + } + ); + + { + const { data } = await takeSnapshot(); + + expect(data).toEqual({ __typename: "Item", id: 1, text: "Item #1" }); + } + + expect(cache["watches"].size).toBe(1); + + unmount(); + // We need to wait a tick since the cleanup is run in a setTimeout to + // prevent strict mode bugs. + await wait(0); + + expect(cache["watches"].size).toBe(0); +}); + +test("tears down all watches when rendering multiple records", async () => { + interface ItemFragment { + __typename: "Item"; + id: number; + text: string; + } + + const fragment: TypedDocumentNode = gql` + fragment ItemFragment on Item { + id + text + } + `; + + const cache = new InMemoryCache(); + const client = new ApolloClient({ cache }); + + client.writeFragment({ + fragment, + data: { __typename: "Item", id: 1, text: "Item #1" }, + }); + + client.writeFragment({ + fragment, + data: { __typename: "Item", id: 2, text: "Item #2" }, + }); + + using _disabledAct = disableActEnvironment(); + const { unmount, rerender, takeSnapshot } = await renderHookToSnapshotStream( + ({ id }) => + useSuspenseFragment({ fragment, from: { __typename: "Item", id } }), + { + initialProps: { id: 1 }, + wrapper: ({ children }) => ( + {children} + ), + } + ); + + { + const { data } = await takeSnapshot(); + + expect(data).toEqual({ __typename: "Item", id: 1, text: "Item #1" }); + } + + await rerender({ id: 2 }); + + { + const { data } = await takeSnapshot(); + + expect(data).toEqual({ __typename: "Item", id: 2, text: "Item #2" }); + } + + unmount(); + // We need to wait a tick since the cleanup is run in a setTimeout to + // prevent strict mode bugs. + await wait(0); + + expect(cache["watches"].size).toBe(0); +}); + describe.skip("type tests", () => { test("returns TData when from is a non-null value", () => { const fragment: TypedDocumentNode<{ foo: string }> = gql``; From accc6c0c8b89b67a2b2f32fdec99baf9ad0e9b50 Mon Sep 17 00:00:00 2001 From: Jerel Miller Date: Thu, 19 Dec 2024 13:25:42 -0700 Subject: [PATCH 38/43] Add support for autoDisposeTimeoutMs in useSuspenseFragment --- .../__tests__/useSuspenseFragment.test.tsx | 221 +++++++++++++++++- src/react/internal/cache/FragmentReference.ts | 18 ++ src/react/internal/cache/SuspenseCache.ts | 1 + 3 files changed, 237 insertions(+), 3 deletions(-) diff --git a/src/react/hooks/__tests__/useSuspenseFragment.test.tsx b/src/react/hooks/__tests__/useSuspenseFragment.test.tsx index 7f9a54545b1..f4bef592621 100644 --- a/src/react/hooks/__tests__/useSuspenseFragment.test.tsx +++ b/src/react/hooks/__tests__/useSuspenseFragment.test.tsx @@ -20,11 +20,12 @@ import { renderHookToSnapshotStream, useTrackRenders, } from "@testing-library/react-render-stream"; -import { spyOnConsole } from "../../../testing/internal"; -import { renderHook } from "@testing-library/react"; +import { renderAsync, spyOnConsole } from "../../../testing/internal"; +import { act, renderHook, screen, waitFor } from "@testing-library/react"; import { InvariantError } from "ts-invariant"; -import { MockedProvider, wait } from "../../../testing"; +import { MockedProvider, MockSubscriptionLink, wait } from "../../../testing"; import { expectTypeOf } from "expect-type"; +import userEvent from "@testing-library/user-event"; function createDefaultRenderStream() { return createRenderStream({ @@ -1566,6 +1567,220 @@ test("tears down all watches when rendering multiple records", async () => { expect(cache["watches"].size).toBe(0); }); +test("tears down watches after default autoDisposeTimeoutMs if component never renders again after suspending", async () => { + jest.useFakeTimers(); + interface ItemFragment { + __typename: "Item"; + id: number; + text: string; + } + + const fragment: TypedDocumentNode = gql` + fragment ItemFragment on Item { + id + text + } + `; + + const cache = new InMemoryCache(); + const user = userEvent.setup({ advanceTimers: jest.advanceTimersByTime }); + const link = new MockSubscriptionLink(); + const client = new ApolloClient({ link, cache }); + + function App() { + const [showItem, setShowItem] = React.useState(true); + + return ( + + + {showItem && ( + + + + )} + + ); + } + + function Item() { + const { data } = useSuspenseFragment({ + fragment, + from: { __typename: "Item", id: 1 }, + }); + + return {data.text}; + } + + await renderAsync(); + + // Ensure suspends immediately + expect(screen.getByText("Loading item...")).toBeInTheDocument(); + + // Hide the greeting before it finishes loading data + await act(() => user.click(screen.getByText("Hide item"))); + + expect(screen.queryByText("Loading item...")).not.toBeInTheDocument(); + + client.writeFragment({ + fragment, + data: { __typename: "Item", id: 1, text: "Item #1" }, + }); + + // clear the microtask queue + await act(() => Promise.resolve()); + + expect(cache["watches"].size).toBe(1); + + jest.advanceTimersByTime(30_000); + + expect(cache["watches"].size).toBe(0); + + jest.useRealTimers(); +}); + +test("tears down watches after configured autoDisposeTimeoutMs if component never renders again after suspending", async () => { + jest.useFakeTimers(); + interface ItemFragment { + __typename: "Item"; + id: number; + text: string; + } + + const fragment: TypedDocumentNode = gql` + fragment ItemFragment on Item { + id + text + } + `; + + const user = userEvent.setup({ advanceTimers: jest.advanceTimersByTime }); + const link = new MockSubscriptionLink(); + const cache = new InMemoryCache(); + const client = new ApolloClient({ + link, + cache, + defaultOptions: { + react: { + suspense: { + autoDisposeTimeoutMs: 5000, + }, + }, + }, + }); + + function App() { + const [showItem, setShowItem] = React.useState(true); + + return ( + + + {showItem && ( + + + + )} + + ); + } + + function Item() { + const { data } = useSuspenseFragment({ + fragment, + from: { __typename: "Item", id: 1 }, + }); + + return {data.text}; + } + + await renderAsync(); + + // Ensure suspends immediately + expect(screen.getByText("Loading item...")).toBeInTheDocument(); + + // Hide the greeting before it finishes loading data + await act(() => user.click(screen.getByText("Hide item"))); + + expect(screen.queryByText("Loading item...")).not.toBeInTheDocument(); + + client.writeFragment({ + fragment, + data: { __typename: "Item", id: 1, text: "Item #1" }, + }); + + // clear the microtask queue + await act(() => Promise.resolve()); + + expect(cache["watches"].size).toBe(1); + + jest.advanceTimersByTime(5000); + + expect(cache["watches"].size).toBe(0); + + jest.useRealTimers(); +}); + +test("cancels autoDisposeTimeoutMs if the component renders before timer finishes", async () => { + jest.useFakeTimers(); + interface ItemFragment { + __typename: "Item"; + id: number; + text: string; + } + + const fragment: TypedDocumentNode = gql` + fragment ItemFragment on Item { + id + text + } + `; + + const link = new MockSubscriptionLink(); + const cache = new InMemoryCache(); + const client = new ApolloClient({ link, cache }); + + function App() { + return ( + + + + + + ); + } + + function Item() { + const { data } = useSuspenseFragment({ + fragment, + from: { __typename: "Item", id: 1 }, + }); + + return {data.text}; + } + + await renderAsync(); + + // Ensure suspends immediately + expect(screen.getByText("Loading item...")).toBeInTheDocument(); + + client.writeFragment({ + fragment, + data: { __typename: "Item", id: 1, text: "Item #1" }, + }); + + // clear the microtask queue + await act(() => Promise.resolve()); + + await waitFor(() => { + expect(screen.getByText("Item #1")).toBeInTheDocument(); + }); + + jest.advanceTimersByTime(30_000); + + expect(cache["watches"].size).toBe(1); + + jest.useRealTimers(); +}); + describe.skip("type tests", () => { test("returns TData when from is a non-null value", () => { const fragment: TypedDocumentNode<{ foo: string }> = gql``; diff --git a/src/react/internal/cache/FragmentReference.ts b/src/react/internal/cache/FragmentReference.ts index 0bb05a9c98d..85453892bcc 100644 --- a/src/react/internal/cache/FragmentReference.ts +++ b/src/react/internal/cache/FragmentReference.ts @@ -20,6 +20,7 @@ type FragmentRefPromise = PromiseWithState; type Listener = (promise: FragmentRefPromise) => void; interface FragmentReferenceOptions { + autoDisposeTimeoutMs?: number; onDispose?: () => void; } @@ -36,6 +37,7 @@ export class FragmentReference< private subscription!: ObservableSubscription; private listeners = new Set>>(); + private autoDisposeTimeoutId?: NodeJS.Timeout; private references = 0; @@ -58,11 +60,26 @@ export class FragmentReference< const diff = this.getDiff(client, watchFragmentOptions); + // Start a timer that will automatically dispose of the query if the + // suspended resource does not use this fragmentRef in the given time. This + // helps prevent memory leaks when a component has unmounted before the + // query has finished loading. + const startDisposeTimer = () => { + if (!this.references) { + this.autoDisposeTimeoutId = setTimeout( + this.dispose, + options.autoDisposeTimeoutMs ?? 30_000 + ); + } + }; + this.promise = diff.complete ? createFulfilledPromise(diff.result) : this.createPendingPromise(); this.subscribeToFragment(); + + this.promise.then(startDisposeTimer, startDisposeTimer); } listen(listener: Listener>) { @@ -75,6 +92,7 @@ export class FragmentReference< retain() { this.references++; + clearTimeout(this.autoDisposeTimeoutId); let disposed = false; return () => { diff --git a/src/react/internal/cache/SuspenseCache.ts b/src/react/internal/cache/SuspenseCache.ts index dcaecc9a902..03bf0c43049 100644 --- a/src/react/internal/cache/SuspenseCache.ts +++ b/src/react/internal/cache/SuspenseCache.ts @@ -68,6 +68,7 @@ export class SuspenseCache { if (!ref.current) { ref.current = new FragmentReference(client, options, { + autoDisposeTimeoutMs: this.options.autoDisposeTimeoutMs, onDispose: () => { delete ref.current; }, From 1946b9ca1d7e06cb1f58e12d3159d205bc5846d0 Mon Sep 17 00:00:00 2001 From: jerelmiller Date: Thu, 19 Dec 2024 20:44:25 +0000 Subject: [PATCH 39/43] Clean up Prettier, Size-limit, and Api-Extractor --- .api-reports/api-report-react_internal.api.md | 2 ++ .size-limits.json | 4 ++-- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/.api-reports/api-report-react_internal.api.md b/.api-reports/api-report-react_internal.api.md index ef1459bfc52..11743726a4b 100644 --- a/.api-reports/api-report-react_internal.api.md +++ b/.api-reports/api-report-react_internal.api.md @@ -861,6 +861,8 @@ class FragmentReference> { // @public (undocumented) interface FragmentReferenceOptions { + // (undocumented) + autoDisposeTimeoutMs?: number; // (undocumented) onDispose?: () => void; } diff --git a/.size-limits.json b/.size-limits.json index 600748d6030..6087f492816 100644 --- a/.size-limits.json +++ b/.size-limits.json @@ -1,4 +1,4 @@ { - "dist/apollo-client.min.cjs": 42108, - "import { ApolloClient, InMemoryCache, HttpLink } from \"dist/index.js\" (production)": 34349 + "dist/apollo-client.min.cjs": 42174, + "import { ApolloClient, InMemoryCache, HttpLink } from \"dist/index.js\" (production)": 34381 } From 972805d076ce97459064d27000c62b801ab5a9cb Mon Sep 17 00:00:00 2001 From: Jerel Miller Date: Fri, 20 Dec 2024 09:49:59 -0700 Subject: [PATCH 40/43] Default TVariables to OperationVariables --- src/react/hooks/useSuspenseFragment.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/react/hooks/useSuspenseFragment.ts b/src/react/hooks/useSuspenseFragment.ts index 2e69d65bb68..e3bd085ac7c 100644 --- a/src/react/hooks/useSuspenseFragment.ts +++ b/src/react/hooks/useSuspenseFragment.ts @@ -46,7 +46,7 @@ const NULL_PLACEHOLDER = [] as unknown as [ export function useSuspenseFragment< TData, - TVariables extends OperationVariables, + TVariables extends OperationVariables = OperationVariables, >( options: UseSuspenseFragmentOptions & { from: {}; @@ -55,7 +55,7 @@ export function useSuspenseFragment< export function useSuspenseFragment< TData, - TVariables extends OperationVariables, + TVariables extends OperationVariables = OperationVariables, >( options: UseSuspenseFragmentOptions & { from: null; @@ -64,7 +64,7 @@ export function useSuspenseFragment< export function useSuspenseFragment< TData, - TVariables extends OperationVariables, + TVariables extends OperationVariables = OperationVariables, >( options: UseSuspenseFragmentOptions & { from: {} | null; @@ -73,7 +73,7 @@ export function useSuspenseFragment< export function useSuspenseFragment< TData, - TVariables extends OperationVariables, + TVariables extends OperationVariables = OperationVariables, >( options: UseSuspenseFragmentOptions ): UseSuspenseFragmentResult; From b727b1dc4ec94164c4d52b1db678ef3e0aebd3bc Mon Sep 17 00:00:00 2001 From: Jerel Miller Date: Fri, 20 Dec 2024 09:50:11 -0700 Subject: [PATCH 41/43] Add explicit generic arguments to tests --- .../__tests__/useSuspenseFragment.test.tsx | 54 +++++++++++++++---- 1 file changed, 44 insertions(+), 10 deletions(-) diff --git a/src/react/hooks/__tests__/useSuspenseFragment.test.tsx b/src/react/hooks/__tests__/useSuspenseFragment.test.tsx index f4bef592621..a38c3f0fceb 100644 --- a/src/react/hooks/__tests__/useSuspenseFragment.test.tsx +++ b/src/react/hooks/__tests__/useSuspenseFragment.test.tsx @@ -1783,14 +1783,26 @@ test("cancels autoDisposeTimeoutMs if the component renders before timer finishe describe.skip("type tests", () => { test("returns TData when from is a non-null value", () => { - const fragment: TypedDocumentNode<{ foo: string }> = gql``; + type Data = { foo: string }; + const fragment: TypedDocumentNode = gql``; - const { data } = useSuspenseFragment({ - fragment, - from: { __typename: "Query" }, - }); + { + const { data } = useSuspenseFragment({ + fragment, + from: { __typename: "Query" }, + }); - expectTypeOf(data).branded.toEqualTypeOf<{ foo: string }>(); + expectTypeOf(data).branded.toEqualTypeOf(); + } + + { + const { data } = useSuspenseFragment({ + fragment: gql``, + from: { __typename: "Query" }, + }); + + expectTypeOf(data).branded.toEqualTypeOf(); + } }); test("returns TData | null when from is null", () => { @@ -1798,9 +1810,20 @@ describe.skip("type tests", () => { type Vars = Record; const fragment: TypedDocumentNode = gql``; - const { data } = useSuspenseFragment({ fragment, from: null }); + { + const { data } = useSuspenseFragment({ fragment, from: null }); - expectTypeOf(data).branded.toEqualTypeOf(); + expectTypeOf(data).branded.toEqualTypeOf(); + } + + { + const { data } = useSuspenseFragment({ + fragment: gql``, + from: null, + }); + + expectTypeOf(data).branded.toEqualTypeOf(); + } }); test("returns TData | null when from is nullable", () => { @@ -1809,8 +1832,19 @@ describe.skip("type tests", () => { const fragment: TypedDocumentNode = gql``; const author = {} as { post: Post | null }; - const { data } = useSuspenseFragment({ fragment, from: author.post }); + { + const { data } = useSuspenseFragment({ fragment, from: author.post }); - expectTypeOf(data).branded.toEqualTypeOf(); + expectTypeOf(data).branded.toEqualTypeOf(); + } + + { + const { data } = useSuspenseFragment({ + fragment: gql``, + from: author.post, + }); + + expectTypeOf(data).branded.toEqualTypeOf(); + } }); }); From bbeb330c02cb6875ed99b4def966ca22f4f80420 Mon Sep 17 00:00:00 2001 From: Jerel Miller Date: Fri, 20 Dec 2024 09:52:00 -0700 Subject: [PATCH 42/43] Allow for FragmentType --- src/react/hooks/useSuspenseFragment.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/react/hooks/useSuspenseFragment.ts b/src/react/hooks/useSuspenseFragment.ts index e3bd085ac7c..de498328ecd 100644 --- a/src/react/hooks/useSuspenseFragment.ts +++ b/src/react/hooks/useSuspenseFragment.ts @@ -1,6 +1,5 @@ import type { ApolloClient, - MaybeMasked, OperationVariables, Reference, StoreObject, @@ -13,6 +12,7 @@ import React, { useMemo } from "rehackt"; import type { FragmentKey } from "../internal/cache/types.js"; import { __use } from "./internal/__use.js"; import { wrapHook } from "./internal/index.js"; +import type { FragmentType, MaybeMasked } from "../../masking/index.js"; export interface UseSuspenseFragmentOptions extends Omit< @@ -23,7 +23,7 @@ export interface UseSuspenseFragmentOptions Cache.ReadFragmentOptions, "id" | "variables" | "returnPartialData" > { - from: StoreObject | Reference | string | null; + from: StoreObject | Reference | FragmentType> | string | null; // Override this field to make it optional (default: true). optimistic?: boolean; /** From fefec82a9cae5b489811cc57bef9a819c205fe7f Mon Sep 17 00:00:00 2001 From: jerelmiller Date: Fri, 20 Dec 2024 16:55:37 +0000 Subject: [PATCH 43/43] Clean up Prettier, Size-limit, and Api-Extractor --- .api-reports/api-report-react.api.md | 10 +++++----- .api-reports/api-report-react_hooks.api.md | 10 +++++----- .api-reports/api-report-react_internal.api.md | 10 +++++----- .api-reports/api-report.api.md | 10 +++++----- 4 files changed, 20 insertions(+), 20 deletions(-) diff --git a/.api-reports/api-report-react.api.md b/.api-reports/api-report-react.api.md index 293d3ea4854..456b7e0c74b 100644 --- a/.api-reports/api-report-react.api.md +++ b/.api-reports/api-report-react.api.md @@ -2419,28 +2419,28 @@ export function useSubscription(options: UseSuspenseFragmentOptions & { +export function useSuspenseFragment(options: UseSuspenseFragmentOptions & { from: {}; }): UseSuspenseFragmentResult; // @public (undocumented) -export function useSuspenseFragment(options: UseSuspenseFragmentOptions & { +export function useSuspenseFragment(options: UseSuspenseFragmentOptions & { from: null; }): UseSuspenseFragmentResult; // @public (undocumented) -export function useSuspenseFragment(options: UseSuspenseFragmentOptions & { +export function useSuspenseFragment(options: UseSuspenseFragmentOptions & { from: {} | null; }): UseSuspenseFragmentResult; // @public (undocumented) -export function useSuspenseFragment(options: UseSuspenseFragmentOptions): UseSuspenseFragmentResult; +export function useSuspenseFragment(options: UseSuspenseFragmentOptions): UseSuspenseFragmentResult; // @public (undocumented) interface UseSuspenseFragmentOptions extends Omit, NoInfer>, "id" | "query" | "optimistic" | "previousResult" | "returnPartialData">, Omit, "id" | "variables" | "returnPartialData"> { client?: ApolloClient; // (undocumented) - from: StoreObject | Reference | string | null; + from: StoreObject | Reference | FragmentType> | string | null; // (undocumented) optimistic?: boolean; } diff --git a/.api-reports/api-report-react_hooks.api.md b/.api-reports/api-report-react_hooks.api.md index 247bf875c05..38930968676 100644 --- a/.api-reports/api-report-react_hooks.api.md +++ b/.api-reports/api-report-react_hooks.api.md @@ -2252,28 +2252,28 @@ export function useSubscription(options: UseSuspenseFragmentOptions & { +export function useSuspenseFragment(options: UseSuspenseFragmentOptions & { from: {}; }): UseSuspenseFragmentResult; // @public (undocumented) -export function useSuspenseFragment(options: UseSuspenseFragmentOptions & { +export function useSuspenseFragment(options: UseSuspenseFragmentOptions & { from: null; }): UseSuspenseFragmentResult; // @public (undocumented) -export function useSuspenseFragment(options: UseSuspenseFragmentOptions & { +export function useSuspenseFragment(options: UseSuspenseFragmentOptions & { from: {} | null; }): UseSuspenseFragmentResult; // @public (undocumented) -export function useSuspenseFragment(options: UseSuspenseFragmentOptions): UseSuspenseFragmentResult; +export function useSuspenseFragment(options: UseSuspenseFragmentOptions): UseSuspenseFragmentResult; // @public (undocumented) interface UseSuspenseFragmentOptions extends Omit, NoInfer>, "id" | "query" | "optimistic" | "previousResult" | "returnPartialData">, Omit, "id" | "variables" | "returnPartialData"> { client?: ApolloClient; // (undocumented) - from: StoreObject | Reference | string | null; + from: StoreObject | Reference | FragmentType> | string | null; // (undocumented) optimistic?: boolean; } diff --git a/.api-reports/api-report-react_internal.api.md b/.api-reports/api-report-react_internal.api.md index 11743726a4b..1fe912f490e 100644 --- a/.api-reports/api-report-react_internal.api.md +++ b/.api-reports/api-report-react_internal.api.md @@ -2317,28 +2317,28 @@ interface UseReadQueryResult { // Warning: (ae-forgotten-export) The symbol "UseSuspenseFragmentResult" needs to be exported by the entry point index.d.ts // // @public (undocumented) -function useSuspenseFragment(options: UseSuspenseFragmentOptions & { +function useSuspenseFragment(options: UseSuspenseFragmentOptions & { from: {}; }): UseSuspenseFragmentResult; // @public (undocumented) -function useSuspenseFragment(options: UseSuspenseFragmentOptions & { +function useSuspenseFragment(options: UseSuspenseFragmentOptions & { from: null; }): UseSuspenseFragmentResult; // @public (undocumented) -function useSuspenseFragment(options: UseSuspenseFragmentOptions & { +function useSuspenseFragment(options: UseSuspenseFragmentOptions & { from: {} | null; }): UseSuspenseFragmentResult; // @public (undocumented) -function useSuspenseFragment(options: UseSuspenseFragmentOptions): UseSuspenseFragmentResult; +function useSuspenseFragment(options: UseSuspenseFragmentOptions): UseSuspenseFragmentResult; // @public (undocumented) interface UseSuspenseFragmentOptions extends Omit, NoInfer>, "id" | "query" | "optimistic" | "previousResult" | "returnPartialData">, Omit, "id" | "variables" | "returnPartialData"> { client?: ApolloClient; // (undocumented) - from: StoreObject | Reference | string | null; + from: StoreObject | Reference | FragmentType> | string | null; // (undocumented) optimistic?: boolean; } diff --git a/.api-reports/api-report.api.md b/.api-reports/api-report.api.md index 4fa157e274a..46d85258c76 100644 --- a/.api-reports/api-report.api.md +++ b/.api-reports/api-report.api.md @@ -3090,28 +3090,28 @@ export function useSubscription(options: UseSuspenseFragmentOptions & { +export function useSuspenseFragment(options: UseSuspenseFragmentOptions & { from: {}; }): UseSuspenseFragmentResult; // @public (undocumented) -export function useSuspenseFragment(options: UseSuspenseFragmentOptions & { +export function useSuspenseFragment(options: UseSuspenseFragmentOptions & { from: null; }): UseSuspenseFragmentResult; // @public (undocumented) -export function useSuspenseFragment(options: UseSuspenseFragmentOptions & { +export function useSuspenseFragment(options: UseSuspenseFragmentOptions & { from: {} | null; }): UseSuspenseFragmentResult; // @public (undocumented) -export function useSuspenseFragment(options: UseSuspenseFragmentOptions): UseSuspenseFragmentResult; +export function useSuspenseFragment(options: UseSuspenseFragmentOptions): UseSuspenseFragmentResult; // @public (undocumented) interface UseSuspenseFragmentOptions extends Omit, NoInfer>, "id" | "query" | "optimistic" | "previousResult" | "returnPartialData">, Omit, "id" | "variables" | "returnPartialData"> { client?: ApolloClient; // (undocumented) - from: StoreObject | Reference | string | null; + from: StoreObject | Reference | FragmentType> | string | null; // (undocumented) optimistic?: boolean; }