From 932252b0c54792ec8c5095de1b42c005a91ffe6d Mon Sep 17 00:00:00 2001 From: Jerel Miller Date: Tue, 28 Mar 2023 13:25:07 -0600 Subject: [PATCH] Fix the compatibility between `useSuspenseQuery` and React's `useDeferredValue` and `startTransition` APIs (#10672) Co-authored-by: Ben Newman --- .changeset/neat-rockets-sleep.md | 9 + .prettierignore | 5 + config/bundlesize.ts | 2 +- src/core/ObservableQuery.ts | 25 +- src/core/networkStatus.ts | 10 + src/core/watchQueryOptions.ts | 6 - src/react/cache/QuerySubscription.ts | 170 +++ src/react/cache/SuspenseCache.ts | 113 +- .../hooks/__tests__/useSuspenseQuery.test.tsx | 1283 ++++++++++++++--- src/react/hooks/internal/__use.ts | 19 + src/react/hooks/internal/index.ts | 2 + .../internal/useIsomorphicLayoutEffect.ts | 9 +- .../useStrictModeSafeCleanupEffect.ts | 13 + src/react/hooks/useSuspenseQuery.ts | 451 ++---- src/react/types/types.ts | 7 - src/utilities/common/mergeOptions.ts | 4 +- src/utilities/promises/decoration.ts | 58 + 17 files changed, 1572 insertions(+), 614 deletions(-) create mode 100644 .changeset/neat-rockets-sleep.md create mode 100644 src/react/cache/QuerySubscription.ts create mode 100644 src/react/hooks/internal/__use.ts create mode 100644 src/react/hooks/internal/useStrictModeSafeCleanupEffect.ts create mode 100644 src/utilities/promises/decoration.ts diff --git a/.changeset/neat-rockets-sleep.md b/.changeset/neat-rockets-sleep.md new file mode 100644 index 00000000000..0ba581b2d3f --- /dev/null +++ b/.changeset/neat-rockets-sleep.md @@ -0,0 +1,9 @@ +--- +'@apollo/client': patch +--- + +Fix the compatibility between `useSuspenseQuery` and React's `useDeferredValue` and `startTransition` APIs to allow React to show stale UI while the changes to the variable cause the component to suspend. + +# Breaking change + +`nextFetchPolicy` support has been removed from `useSuspenseQuery`. If you are using this option, remove it, otherwise it will be ignored. diff --git a/.prettierignore b/.prettierignore index 9f4a4cfdcca..3e284ae579e 100644 --- a/.prettierignore +++ b/.prettierignore @@ -28,6 +28,11 @@ src/react/* # Allow src/react/cache !src/react/cache/ +# Allowed utilities +!src/utilities/ +src/utilities/* +!src/utilities/promises/ + ## Allowed React Hooks !src/react/hooks/ src/react/hooks/* diff --git a/config/bundlesize.ts b/config/bundlesize.ts index c65cce731d9..2d6b1674585 100644 --- a/config/bundlesize.ts +++ b/config/bundlesize.ts @@ -3,7 +3,7 @@ import { join } from "path"; import { gzipSync } from "zlib"; import bytes from "bytes"; -const gzipBundleByteLengthLimit = bytes("34.1KB"); +const gzipBundleByteLengthLimit = bytes("34.14KB"); const minFile = join("dist", "apollo-client.min.cjs"); const minPath = join(__dirname, "..", minFile); const gzipByteLen = gzipSync(readFileSync(minPath)).byteLength; diff --git a/src/core/ObservableQuery.ts b/src/core/ObservableQuery.ts index a72b5669f62..3042641f0fe 100644 --- a/src/core/ObservableQuery.ts +++ b/src/core/ObservableQuery.ts @@ -110,8 +110,6 @@ export class ObservableQuery< options: WatchQueryOptions; }) { super((observer: Observer>) => { - const { fetchOnFirstSubscribe = true } = options - // Zen Observable has its own error function, so in order to log correctly // we need to provide a custom error callback. try { @@ -134,7 +132,7 @@ export class ObservableQuery< // Initiate observation of this query if it hasn't been reported to // the QueryManager yet. - if (first && fetchOnFirstSubscribe) { + if (first) { // Blindly catching here prevents unhandled promise rejections, // and is safe because the ObservableQuery handles this error with // this.observer.error, so we're not just swallowing the error by @@ -784,15 +782,10 @@ Did you mean to call refetch(variables) instead of refetch({ variables })?`); return this.last; } - // For cases like suspense with a deferred query where we need a custom - // promise wrapped around the concast, we need access to the raw concast - // created from `reobserve`. This function provides the original `reobserve` - // functionality, but returns a concast instead of a promise. Most consumers - // should prefer `reobserve` instead of this function. - public reobserveAsConcast( + public reobserve( newOptions?: Partial>, newNetworkStatus?: NetworkStatus - ): Concast> { + ): Promise> { this.isTornDown = false; const useDisposableConcast = @@ -865,17 +858,7 @@ Did you mean to call refetch(variables) instead of refetch({ variables })?`); concast.addObserver(observer); - return concast; - } - - public reobserve( - newOptions?: Partial>, - newNetworkStatus?: NetworkStatus, - ): Promise> { - return this.reobserveAsConcast( - newOptions, - newNetworkStatus - ).promise + return concast.promise; } // (Re)deliver the current result to this.observers without applying fetch diff --git a/src/core/networkStatus.ts b/src/core/networkStatus.ts index 08915e3a702..292bafad4ba 100644 --- a/src/core/networkStatus.ts +++ b/src/core/networkStatus.ts @@ -54,3 +54,13 @@ export function isNetworkRequestInFlight( ): boolean { return networkStatus ? networkStatus < 7 : false; } + +/** + * Returns true if the network request is in ready or error state according to a given network + * status. + */ +export function isNetworkRequestSettled( + networkStatus?: NetworkStatus, +): boolean { + return networkStatus === 7 || networkStatus === 8; +} diff --git a/src/core/watchQueryOptions.ts b/src/core/watchQueryOptions.ts index 4e073251c29..fc51af907bf 100644 --- a/src/core/watchQueryOptions.ts +++ b/src/core/watchQueryOptions.ts @@ -145,12 +145,6 @@ export interface WatchQueryOptions { diff --git a/src/react/cache/QuerySubscription.ts b/src/react/cache/QuerySubscription.ts new file mode 100644 index 00000000000..07c7b22e44b --- /dev/null +++ b/src/react/cache/QuerySubscription.ts @@ -0,0 +1,170 @@ +import { + ApolloError, + ApolloQueryResult, + DocumentNode, + NetworkStatus, + ObservableQuery, + OperationVariables, +} from '../../core'; +import { isNetworkRequestSettled } from '../../core/networkStatus'; +import { + Concast, + ObservableSubscription, + hasAnyDirectives, +} from '../../utilities'; +import { invariant } from '../../utilities/globals'; +import { wrap } from 'optimism'; + +type Listener = (result: ApolloQueryResult) => void; + +type FetchMoreOptions = Parameters< + ObservableQuery['fetchMore'] +>[0]; + +function wrapWithCustomPromise( + concast: Concast> +) { + return new Promise>((resolve, reject) => { + // Unlike `concast.promise`, we want to resolve the promise on the initial + // chunk of the deferred query. This allows the component to unsuspend + // when we get the initial set of data, rather than waiting until all + // chunks have been loaded. + const subscription = concast.subscribe({ + next: (value) => { + resolve(value); + subscription.unsubscribe(); + }, + error: reject, + }); + }); +} + +const isMultipartQuery = wrap((query: DocumentNode) => { + return hasAnyDirectives(['defer', 'stream'], query); +}); + +interface QuerySubscriptionOptions { + onDispose?: () => void; + autoDisposeTimeoutMs?: number; +} + +export class QuerySubscription { + public result: ApolloQueryResult; + public promise: Promise>; + public readonly observable: ObservableQuery; + + private subscription: ObservableSubscription; + private listeners = new Set>(); + private autoDisposeTimeoutId: NodeJS.Timeout; + + constructor( + observable: ObservableQuery, + options: QuerySubscriptionOptions = Object.create(null) + ) { + this.listen = this.listen.bind(this); + this.handleNext = this.handleNext.bind(this); + this.handleError = this.handleError.bind(this); + this.dispose = this.dispose.bind(this); + this.observable = observable; + this.result = observable.getCurrentResult(); + + if (options.onDispose) { + this.onDispose = options.onDispose; + } + + this.subscription = observable.subscribe({ + next: this.handleNext, + error: this.handleError, + }); + + // This error should never happen since the `.subscribe` call above + // will ensure a concast is set on the observable via the `reobserve` + // call. Unless something is going horribly wrong and completely messing + // around with the internals of the observable, there should always be a + // concast after subscribing. + invariant( + observable['concast'], + 'Unexpected error: A concast was not found on the observable.' + ); + + const concast = observable['concast']; + + this.promise = isMultipartQuery(observable.query) + ? wrapWithCustomPromise(concast) + : concast.promise; + + // Start a timer that will automatically dispose of the query if the + // suspended resource does not use this subscription in the given time. This + // helps prevent memory leaks when a component has unmounted before the + // query has finished loading. + this.autoDisposeTimeoutId = setTimeout( + this.dispose, + options.autoDisposeTimeoutMs ?? 30_000 + ); + } + + listen(listener: Listener) { + // As soon as the component listens for updates, we know it has finished + // suspending and is ready to receive updates, so we can remove the auto + // dispose timer. + clearTimeout(this.autoDisposeTimeoutId); + + this.listeners.add(listener); + + return () => { + this.listeners.delete(listener); + }; + } + + refetch(variables: OperationVariables | undefined) { + this.promise = this.observable.refetch(variables); + + return this.promise; + } + + fetchMore(options: FetchMoreOptions) { + this.promise = this.observable.fetchMore(options); + + return this.promise; + } + + dispose() { + this.subscription.unsubscribe(); + this.onDispose(); + } + + private onDispose() { + // noop. overridable by options + } + + private handleNext(result: ApolloQueryResult) { + // If we encounter an error with the new result after we have successfully + // fetched a previous result, we should set the new result data to the last + // successful result. + if ( + isNetworkRequestSettled(result.networkStatus) && + this.result.data && + result.data === void 0 + ) { + result.data = this.result.data; + } + + this.result = result; + this.deliver(result); + } + + private handleError(error: ApolloError) { + const result = { + ...this.result, + error, + networkStatus: NetworkStatus.error, + }; + + this.result = result; + this.deliver(result); + } + + private deliver(result: ApolloQueryResult) { + this.listeners.forEach((listener) => listener(result)); + } +} diff --git a/src/react/cache/SuspenseCache.ts b/src/react/cache/SuspenseCache.ts index 8532b2da68d..22112b9661f 100644 --- a/src/react/cache/SuspenseCache.ts +++ b/src/react/cache/SuspenseCache.ts @@ -1,87 +1,66 @@ +import { Trie } from '@wry/trie'; import { - ApolloQueryResult, + ApolloClient, DocumentNode, ObservableQuery, OperationVariables, TypedDocumentNode, } from '../../core'; import { canonicalStringify } from '../../cache'; +import { canUseWeakMap } from '../../utilities'; +import { QuerySubscription } from './QuerySubscription'; -interface CacheEntry { - observable: ObservableQuery; - fulfilled: boolean; - promise: Promise>; +type CacheKey = [ApolloClient, DocumentNode, string]; + +interface SuspenseCacheOptions { + /** + * Specifies the amount of time, in milliseconds, the suspense cache will wait + * for a suspended component to read from the suspense cache before it + * automatically disposes of the query. This prevents memory leaks when a + * component unmounts before a suspended resource finishes loading. Increase + * the timeout if your queries take longer than than the specified time to + * prevent your queries from suspending over and over. + * + * Defaults to 30 seconds. + */ + autoDisposeTimeoutMs?: number; } export class SuspenseCache { - private queries = new Map< - DocumentNode, - Map> - >(); - - add( - query: DocumentNode | TypedDocumentNode, - variables: TVariables | undefined, - { - promise, - observable, - }: { promise: Promise; observable: ObservableQuery } - ) { - const variablesKey = this.getVariablesKey(variables); - const map = this.queries.get(query) || new Map(); - - const entry: CacheEntry = { - observable, - fulfilled: false, - promise: promise - .catch(() => { - // Throw away the error as we only care to track when the promise has - // been fulfilled - }) - .finally(() => { - entry.fulfilled = true; - }), - }; - - map.set(variablesKey, entry); + private cacheKeys = new Trie( + canUseWeakMap, + (cacheKey: CacheKey) => cacheKey + ); - this.queries.set(query, map); - - return entry; - } + private subscriptions = new Map(); + private options: SuspenseCacheOptions; - lookup< - TData = any, - TVariables extends OperationVariables = OperationVariables - >( - query: DocumentNode | TypedDocumentNode, - variables: TVariables | undefined - ): CacheEntry | undefined { - return this.queries - .get(query) - ?.get(this.getVariablesKey(variables)) as CacheEntry; + constructor(options: SuspenseCacheOptions = Object.create(null)) { + this.options = options; } - remove(query: DocumentNode, variables: OperationVariables | undefined) { - const map = this.queries.get(query); - - if (!map) { - return; - } - - const key = this.getVariablesKey(variables); - const entry = map.get(key); - - if (entry && !entry.observable.hasObservers()) { - map.delete(key); - } + getSubscription( + client: ApolloClient, + query: DocumentNode | TypedDocumentNode, + variables: OperationVariables | undefined, + createObservable: () => ObservableQuery + ) { + const cacheKey = this.cacheKeys.lookup( + client, + query, + canonicalStringify(variables) + ); - if (map.size === 0) { - this.queries.delete(query); + if (!this.subscriptions.has(cacheKey)) { + this.subscriptions.set( + cacheKey, + new QuerySubscription(createObservable(), { + autoDisposeTimeoutMs: this.options.autoDisposeTimeoutMs, + onDispose: () => this.subscriptions.delete(cacheKey), + }) + ); } - } - private getVariablesKey(variables: OperationVariables | undefined) { - return canonicalStringify(variables || Object.create(null)); + return this.subscriptions.get(cacheKey)! as QuerySubscription; } } diff --git a/src/react/hooks/__tests__/useSuspenseQuery.test.tsx b/src/react/hooks/__tests__/useSuspenseQuery.test.tsx index 50eb069f02c..8cd4c220fee 100644 --- a/src/react/hooks/__tests__/useSuspenseQuery.test.tsx +++ b/src/react/hooks/__tests__/useSuspenseQuery.test.tsx @@ -2,11 +2,13 @@ import React, { Fragment, StrictMode, Suspense } from 'react'; import { act, screen, + render, renderHook, waitFor, RenderHookOptions, RenderHookResult, } from '@testing-library/react'; +import userEvent from '@testing-library/user-event'; import { ErrorBoundary } from 'react-error-boundary'; import { GraphQLError } from 'graphql'; import { InvariantError } from 'ts-invariant'; @@ -63,6 +65,10 @@ interface Renders { frames: Result[]; } +interface SimpleQueryData { + greeting: string; +} + function renderSuspenseHook( render: (initialProps: Props) => Result, options: RenderSuspenseHookOptions = Object.create(null) @@ -82,15 +88,19 @@ function renderSuspenseHook( }; const { - cache, - client, - link, mocks = [], suspenseCache = new SuspenseCache(), strictMode, ...renderHookOptions } = options; + const client = + options.client || + new ApolloClient({ + cache: options.cache || new InMemoryCache(), + link: options.link || new MockLink(mocks), + }); + const view = renderHook( (props) => { renders.count++; @@ -116,20 +126,9 @@ function renderSuspenseHook( renders.errors.push(error); }} > - {client ? ( - - {children} - - ) : ( - - {children} - - )} + + {children} + @@ -142,11 +141,7 @@ function renderSuspenseHook( } function useSimpleQueryCase() { - interface QueryData { - greeting: string; - } - - const query: TypedDocumentNode = gql` + const query: TypedDocumentNode = gql` query UserQuery { greeting } @@ -192,7 +187,12 @@ function usePaginatedCase() { const { offset = 0, limit = 2 } = operation.variables; const letters = data.slice(offset, offset + limit); - return Observable.of({ data: { letters } }); + return new Observable((observer) => { + setTimeout(() => { + observer.next({ data: { letters } }); + observer.complete(); + }, 10); + }); }); return { query, link, data }; @@ -345,27 +345,29 @@ describe('useSuspenseQuery', () => { }); it('prioritizes the `suspenseCache` option over the context value', () => { - const { query } = useSimpleQueryCase(); + const { query, mocks } = useSimpleQueryCase(); const directSuspenseCache = new SuspenseCache(); const contextSuspenseCache = new SuspenseCache(); + const client = new ApolloClient({ + link: new MockLink(mocks), + cache: new InMemoryCache(), + }); + renderHook( () => useSuspenseQuery(query, { suspenseCache: directSuspenseCache }), { wrapper: ({ children }) => ( - + {children} - + ), } ); - expect(directSuspenseCache.lookup(query, {})).toBeTruthy(); - expect(contextSuspenseCache.lookup(query, {})).not.toBeTruthy(); + expect(directSuspenseCache['subscriptions'].size).toBe(1); + expect(contextSuspenseCache['subscriptions'].size).toBe(0); }); it('ensures a valid fetch policy is used', () => { @@ -659,64 +661,293 @@ describe('useSuspenseQuery', () => { { client, suspenseCache } ); - // We don't subscribe to the observable until after the component has been - // unsuspended, so we need to wait for the result await waitFor(() => expect(result.current.data).toEqual(mocks[0].result.data) ); expect(client.getObservableQueries().size).toBe(1); - expect(suspenseCache.lookup(query, undefined)).toBeDefined(); + expect(suspenseCache['subscriptions'].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(client.getObservableQueries().size).toBe(0); - expect(suspenseCache.lookup(query, undefined)).toBeUndefined(); + expect(suspenseCache['subscriptions'].size).toBe(0); }); - it('does not remove query from suspense cache if other queries are using it', async () => { - const { query, mocks } = useSimpleQueryCase(); + it('tears down all queries when rendering with multiple variable sets', async () => { + const { query, mocks } = useVariablesQueryCase(); const client = new ApolloClient({ - link: new ApolloLink(() => Observable.of(mocks[0].result)), + link: new MockLink(mocks), cache: new InMemoryCache(), }); const suspenseCache = new SuspenseCache(); - const wrapper = ({ children }: { children: React.ReactNode }) => ( - - {children} - + const { rerender, result, unmount } = renderSuspenseHook( + ({ id }) => useSuspenseQuery(query, { variables: { id } }), + { client, suspenseCache, initialProps: { id: '1' } } ); - const { result: result1, unmount } = renderHook( - () => useSuspenseQuery(query), - { wrapper } + await waitFor(() => + expect(result.current.data).toEqual(mocks[0].result.data) ); - const { result: result2 } = renderHook(() => useSuspenseQuery(query), { - wrapper, - }); + rerender({ id: '2' }); - // We don't subscribe to the observable until after the component has been - // unsuspended, so we need to wait for the results of all queries await waitFor(() => { - expect(result1.current.data).toEqual(mocks[0].result.data); + expect(result.current.data).toEqual(mocks[1].result.data); }); + + expect(client.getObservableQueries().size).toBe(2); + expect(suspenseCache['subscriptions'].size).toBe(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(client.getObservableQueries().size).toBe(0); + expect(suspenseCache['subscriptions'].size).toBe(0); + }); + + it('tears down all queries when multiple clients are used', async () => { + const { query } = useVariablesQueryCase(); + + const client1 = new ApolloClient({ + link: new MockLink([ + { + request: { query, variables: { id: '1' } }, + result: { data: { character: { id: '1', name: 'Client 1' } } }, + }, + ]), + cache: new InMemoryCache(), + }); + + const client2 = new ApolloClient({ + link: new MockLink([ + { + request: { query, variables: { id: '1' } }, + result: { data: { character: { id: '1', name: 'Client 2' } } }, + }, + ]), + cache: new InMemoryCache(), + }); + + const suspenseCache = new SuspenseCache(); + + const { rerender, result, unmount } = renderSuspenseHook( + ({ client }) => + useSuspenseQuery(query, { client, variables: { id: '1' } }), + { suspenseCache, initialProps: { client: client1 } } + ); + + await waitFor(() => + expect(result.current.data).toEqual({ + character: { id: '1', name: 'Client 1' }, + }) + ); + + rerender({ client: client2 }); + await waitFor(() => { - expect(result2.current.data).toEqual(mocks[0].result.data); + expect(result.current.data).toEqual({ + character: { id: '1', name: 'Client 2' }, + }); }); - // Because they are the same query, the 2 components use the same observable - // in the suspense cache - expect(client.getObservableQueries().size).toBe(1); - expect(suspenseCache.lookup(query, undefined)).toBeDefined(); + expect(client1.getObservableQueries().size).toBe(1); + expect(client2.getObservableQueries().size).toBe(1); + expect(suspenseCache['subscriptions'].size).toBe(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(client1.getObservableQueries().size).toBe(0); + expect(client2.getObservableQueries().size).toBe(0); + expect(suspenseCache['subscriptions'].size).toBe(0); + }); + + it('tears down the query if the component never renders again after suspending', async () => { + jest.useFakeTimers(); + const { query } = useSimpleQueryCase(); + const user = userEvent.setup({ advanceTimers: jest.advanceTimersByTime }); + const link = new MockSubscriptionLink(); + const client = new ApolloClient({ + link, + cache: new InMemoryCache(), + }); + const suspenseCache = new SuspenseCache(); + + function App() { + const [showGreeting, setShowGreeting] = React.useState(true); + + return ( + + + {showGreeting && ( + + + + )} + + ); + } + + function Greeting() { + const { data } = useSuspenseQuery(query); + + return {data.greeting}; + } + + render(); + + // Ensure suspends immediately + expect(screen.getByText('Loading greeting...')).toBeInTheDocument(); + + // Hide the greeting before it finishes loading data + await act(() => user.click(screen.getByText('Hide greeting'))); + + expect(screen.queryByText('Loading greeting...')).not.toBeInTheDocument(); + + link.simulateResult({ result: { data: { greeting: 'Hello' } } }); + link.simulateComplete(); + + expect(client.getObservableQueries().size).toBe(1); + expect(suspenseCache['subscriptions'].size).toBe(1); + + jest.advanceTimersByTime(30_000); + + expect(client.getObservableQueries().size).toBe(0); + expect(suspenseCache['subscriptions'].size).toBe(0); + + jest.useRealTimers(); + + // Avoid act warnings for a suspended resource + // eslint-disable-next-line testing-library/no-unnecessary-act + await act(() => wait(0)); + }); + + it('has configurable auto dispose timer if the component never renders again after suspending', async () => { + jest.useFakeTimers(); + const { query } = useSimpleQueryCase(); + const user = userEvent.setup({ advanceTimers: jest.advanceTimersByTime }); + const link = new MockSubscriptionLink(); + const client = new ApolloClient({ + link, + cache: new InMemoryCache(), + }); + + const suspenseCache = new SuspenseCache({ autoDisposeTimeoutMs: 5000 }); + + function App() { + const [showGreeting, setShowGreeting] = React.useState(true); + + return ( + + + {showGreeting && ( + + + + )} + + ); + } + + function Greeting() { + const { data } = useSuspenseQuery(query); + + return {data.greeting}; + } + + render(); + + // Ensure suspends immediately + expect(screen.getByText('Loading greeting...')).toBeInTheDocument(); + + // Hide the greeting before it finishes loading data + await act(() => user.click(screen.getByText('Hide greeting'))); + + expect(screen.queryByText('Loading greeting...')).not.toBeInTheDocument(); + + link.simulateResult({ result: { data: { greeting: 'Hello' } } }); + link.simulateComplete(); + expect(client.getObservableQueries().size).toBe(1); - expect(suspenseCache.lookup(query, undefined)).toBeDefined(); + expect(suspenseCache['subscriptions'].size).toBe(1); + + jest.advanceTimersByTime(5_000); + + expect(client.getObservableQueries().size).toBe(0); + expect(suspenseCache['subscriptions'].size).toBe(0); + + jest.useRealTimers(); + + // Avoid act warnings for a suspended resource + // eslint-disable-next-line testing-library/no-unnecessary-act + await act(() => wait(0)); + }); + + it('cancels auto dispose if the component renders before timer finishes', async () => { + jest.useFakeTimers(); + const { query } = useSimpleQueryCase(); + const link = new ApolloLink(() => { + return new Observable((observer) => { + setTimeout(() => { + observer.next({ data: { greeting: 'Hello' } }); + observer.complete(); + }, 10); + }); + }); + const client = new ApolloClient({ + link, + cache: new InMemoryCache(), + }); + + const suspenseCache = new SuspenseCache(); + + function App() { + return ( + + + + + + ); + } + + function Greeting() { + const { data } = useSuspenseQuery(query); + + return {data.greeting}; + } + + render(); + + // Ensure suspends immediately + expect(screen.getByText('Loading greeting...')).toBeInTheDocument(); + + jest.advanceTimersByTime(10); + + await waitFor(() => { + expect(screen.getByText('Hello')).toBeInTheDocument(); + }); + + jest.advanceTimersByTime(30_000); + + expect(client.getObservableQueries().size).toBe(1); + expect(suspenseCache['subscriptions'].size).toBe(1); + + jest.useRealTimers(); }); it('allows the client to be overridden', async () => { @@ -784,38 +1015,344 @@ describe('useSuspenseQuery', () => { // to render 2 frames after the initial suspense. expect(renders.frames).toMatchObject([ { - data: { greeting: 'local hello' }, + data: { greeting: 'local hello' }, + networkStatus: NetworkStatus.ready, + error: undefined, + }, + { + data: { greeting: 'local hello' }, + networkStatus: NetworkStatus.ready, + error: undefined, + }, + ]); + }); + + it('returns the client used in the result', async () => { + const { query } = useSimpleQueryCase(); + + const client = new ApolloClient({ + link: new ApolloLink(() => + Observable.of({ data: { greeting: 'hello' } }) + ), + cache: new InMemoryCache(), + }); + + const { result } = renderSuspenseHook(() => useSuspenseQuery(query), { + client, + }); + + // wait for query to finish suspending to avoid warnings + await waitFor(() => { + expect(result.current.data).toEqual({ greeting: 'hello' }); + }); + + expect(result.current.client).toBe(client); + }); + + it('suspends when changing variables', async () => { + const { query, mocks } = useVariablesQueryCase(); + + const { result, rerender, renders } = renderSuspenseHook( + ({ id }) => useSuspenseQuery(query, { variables: { id } }), + { mocks, initialProps: { id: '1' } } + ); + + await waitFor(() => { + expect(result.current).toMatchObject({ + ...mocks[0].result, + networkStatus: NetworkStatus.ready, + error: undefined, + }); + }); + + rerender({ id: '2' }); + + await waitFor(() => { + expect(result.current).toMatchObject({ + ...mocks[1].result, + networkStatus: NetworkStatus.ready, + error: undefined, + }); + }); + + expect(renders.count).toBe(4); + expect(renders.suspenseCount).toBe(2); + expect(renders.frames).toMatchObject([ + { + ...mocks[0].result, + networkStatus: NetworkStatus.ready, + error: undefined, + }, + { + ...mocks[1].result, + networkStatus: NetworkStatus.ready, + error: undefined, + }, + ]); + }); + + it('suspends and fetches data from new client when changing clients', async () => { + const { query } = useSimpleQueryCase(); + + const client1 = new ApolloClient({ + cache: new InMemoryCache(), + link: new MockLink([ + { + request: { query }, + result: { data: { greeting: 'Hello client 1' } }, + }, + ]), + }); + + const client2 = new ApolloClient({ + cache: new InMemoryCache(), + link: new MockLink([ + { + request: { query }, + result: { data: { greeting: 'Hello client 2' } }, + }, + ]), + }); + + const { result, rerender, renders } = renderSuspenseHook( + ({ client }) => useSuspenseQuery(query, { client }), + { initialProps: { client: client1 } } + ); + + await waitFor(() => { + expect(result.current).toMatchObject({ + data: { greeting: 'Hello client 1' }, + networkStatus: NetworkStatus.ready, + error: undefined, + }); + }); + + rerender({ client: client2 }); + + await waitFor(() => { + expect(result.current).toMatchObject({ + data: { greeting: 'Hello client 2' }, + networkStatus: NetworkStatus.ready, + error: undefined, + }); + }); + + expect(renders.count).toBe(4); + expect(renders.suspenseCount).toBe(2); + expect(renders.frames).toMatchObject([ + { + data: { greeting: 'Hello client 1' }, + networkStatus: NetworkStatus.ready, + error: undefined, + }, + { + data: { greeting: 'Hello client 2' }, + networkStatus: NetworkStatus.ready, + error: undefined, + }, + ]); + }); + + it('responds to cache updates after changing variables', async () => { + const { query, mocks } = useVariablesQueryCase(); + + const client = new ApolloClient({ + cache: new InMemoryCache(), + link: new MockLink(mocks), + }); + + const { result, rerender, renders } = renderSuspenseHook( + ({ id }) => useSuspenseQuery(query, { variables: { id } }), + { client, initialProps: { id: '1' } } + ); + + await waitFor(() => { + expect(result.current).toMatchObject({ + ...mocks[0].result, + networkStatus: NetworkStatus.ready, + error: undefined, + }); + }); + + rerender({ id: '2' }); + + await waitFor(() => { + expect(result.current).toMatchObject({ + ...mocks[1].result, + networkStatus: NetworkStatus.ready, + error: undefined, + }); + }); + + client.writeQuery({ + query, + variables: { id: '2' }, + data: { character: { id: '2', name: 'Cached hero' } }, + }); + + await waitFor(() => { + expect(result.current).toMatchObject({ + data: { character: { id: '2', name: 'Cached hero' } }, + networkStatus: NetworkStatus.ready, + error: undefined, + }); + }); + + expect(renders.suspenseCount).toBe(2); + expect(renders.count).toBe(5); + expect(renders.frames).toMatchObject([ + { + ...mocks[0].result, + networkStatus: NetworkStatus.ready, + error: undefined, + }, + { + ...mocks[1].result, + networkStatus: NetworkStatus.ready, + error: undefined, + }, + { + data: { character: { id: '2', name: 'Cached hero' } }, + networkStatus: NetworkStatus.ready, + error: undefined, + }, + ]); + }); + + it('uses previously fetched result and does not suspend when switching back to already fetched variables', async () => { + const { query, mocks } = useVariablesQueryCase(); + + const { result, rerender, renders } = renderSuspenseHook( + ({ id }) => useSuspenseQuery(query, { variables: { id } }), + { mocks, initialProps: { id: '1' } } + ); + + await waitFor(() => { + expect(result.current).toMatchObject({ + ...mocks[0].result, + networkStatus: NetworkStatus.ready, + error: undefined, + }); + }); + + rerender({ id: '2' }); + + await waitFor(() => { + expect(result.current).toMatchObject({ + ...mocks[1].result, + networkStatus: NetworkStatus.ready, + error: undefined, + }); + }); + + rerender({ id: '1' }); + + expect(result.current).toMatchObject({ + ...mocks[0].result, + networkStatus: NetworkStatus.ready, + error: undefined, + }); + + expect(renders.count).toBe(5); + expect(renders.suspenseCount).toBe(2); + expect(renders.frames).toMatchObject([ + { + ...mocks[0].result, + networkStatus: NetworkStatus.ready, + error: undefined, + }, + { + ...mocks[1].result, networkStatus: NetworkStatus.ready, error: undefined, }, { - data: { greeting: 'local hello' }, + ...mocks[0].result, networkStatus: NetworkStatus.ready, error: undefined, }, ]); }); - it('returns the client used in the result', async () => { - const { query } = useSimpleQueryCase(); + it('responds to cache updates after changing back to already fetched variables', async () => { + const { query, mocks } = useVariablesQueryCase(); const client = new ApolloClient({ - link: new ApolloLink(() => - Observable.of({ data: { greeting: 'hello' } }) - ), cache: new InMemoryCache(), + link: new MockLink(mocks), }); - const { result } = renderSuspenseHook(() => useSuspenseQuery(query), { - client, + const { result, rerender, renders } = renderSuspenseHook( + ({ id }) => useSuspenseQuery(query, { variables: { id } }), + { client, initialProps: { id: '1' } } + ); + + await waitFor(() => { + expect(result.current).toMatchObject({ + ...mocks[0].result, + networkStatus: NetworkStatus.ready, + error: undefined, + }); }); - // wait for query to finish suspending to avoid warnings + rerender({ id: '2' }); + await waitFor(() => { - expect(result.current.data).toEqual({ greeting: 'hello' }); + expect(result.current).toMatchObject({ + ...mocks[1].result, + networkStatus: NetworkStatus.ready, + error: undefined, + }); }); - expect(result.current.client).toBe(client); + rerender({ id: '1' }); + + await waitFor(() => { + expect(result.current).toMatchObject({ + ...mocks[0].result, + networkStatus: NetworkStatus.ready, + error: undefined, + }); + }); + + client.writeQuery({ + query, + variables: { id: '1' }, + data: { character: { id: '1', name: 'Cached hero' } }, + }); + + await waitFor(() => { + expect(result.current).toMatchObject({ + data: { character: { id: '1', name: 'Cached hero' } }, + networkStatus: NetworkStatus.ready, + error: undefined, + }); + }); + + expect(renders.suspenseCount).toBe(2); + expect(renders.count).toBe(6); + expect(renders.frames).toMatchObject([ + { + ...mocks[0].result, + networkStatus: NetworkStatus.ready, + error: undefined, + }, + { + ...mocks[1].result, + networkStatus: NetworkStatus.ready, + error: undefined, + }, + { + ...mocks[0].result, + networkStatus: NetworkStatus.ready, + error: undefined, + }, + { + data: { character: { id: '1', name: 'Cached hero' } }, + networkStatus: NetworkStatus.ready, + error: undefined, + }, + ]); }); it('does not suspend when data is in the cache and using a "cache-first" fetch policy', async () => { @@ -1062,7 +1599,7 @@ describe('useSuspenseQuery', () => { }); }); - expect(renders.count).toBe(5); + expect(renders.count).toBe(4); expect(renders.suspenseCount).toBe(1); expect(renders.frames).toMatchObject([ { @@ -1075,11 +1612,6 @@ describe('useSuspenseQuery', () => { networkStatus: NetworkStatus.ready, error: undefined, }, - { - ...mocks[0].result, - networkStatus: NetworkStatus.ready, - error: undefined, - }, { ...mocks[1].result, networkStatus: NetworkStatus.ready, @@ -1531,7 +2063,7 @@ describe('useSuspenseQuery', () => { }); }); - expect(renders.count).toBe(5); + expect(renders.count).toBe(4); expect(renders.suspenseCount).toBe(1); expect(renders.frames).toMatchObject([ { @@ -1544,11 +2076,6 @@ describe('useSuspenseQuery', () => { networkStatus: NetworkStatus.ready, error: undefined, }, - { - ...mocks[0].result, - networkStatus: NetworkStatus.ready, - error: undefined, - }, { ...mocks[1].result, networkStatus: NetworkStatus.ready, @@ -1563,7 +2090,7 @@ describe('useSuspenseQuery', () => { 'no-cache', 'cache-and-network', ])( - 'returns previous data on refetch when changing variables and using a "%s" with an "initial" suspense policy', + 'returns previous data when changing variables and using a "%s" with an "initial" suspense policy', async (fetchPolicy) => { const { query, mocks } = useVariablesQueryCase(); @@ -1611,7 +2138,7 @@ describe('useSuspenseQuery', () => { }, { ...mocks[0].result, - networkStatus: NetworkStatus.ready, + networkStatus: NetworkStatus.setVariables, error: undefined, }, { @@ -1798,17 +2325,11 @@ describe('useSuspenseQuery', () => { // Renders: // 1. Initiate fetch and suspend // 2. Unsuspend and return results from initial fetch - // 3. Change variables - // 4. Initiate refetch and suspend + // 3. Change variables and suspend // 5. Unsuspend and return results from refetch - expect(renders.count).toBe(5); + expect(renders.count).toBe(4); expect(renders.suspenseCount).toBe(2); expect(renders.frames).toMatchObject([ - { - ...mocks[0].result, - networkStatus: NetworkStatus.ready, - error: undefined, - }, { ...mocks[0].result, networkStatus: NetworkStatus.ready, @@ -1881,17 +2402,11 @@ describe('useSuspenseQuery', () => { // Renders: // 1. Initiate fetch and suspend // 2. Unsuspend and return results from initial fetch - // 3. Change queries - // 4. Initiate refetch and suspend + // 3. Change queries and suspend // 5. Unsuspend and return results from refetch - expect(renders.count).toBe(5); + expect(renders.count).toBe(4); expect(renders.suspenseCount).toBe(2); expect(renders.frames).toMatchObject([ - { - ...mocks[0].result, - networkStatus: NetworkStatus.ready, - error: undefined, - }, { ...mocks[0].result, networkStatus: NetworkStatus.ready, @@ -2257,13 +2772,6 @@ describe('useSuspenseQuery', () => { networkStatus: NetworkStatus.ready, error: undefined, }, - { - data: { - vars: { source: 'local', globalOnlyVar: true, localOnlyVar: true }, - }, - networkStatus: NetworkStatus.ready, - error: undefined, - }, { data: { vars: { source: 'rerender', globalOnlyVar: true, localOnlyVar: true }, @@ -2417,6 +2925,33 @@ describe('useSuspenseQuery', () => { consoleSpy.mockRestore(); }); + it('throws errors when suspensePolicy is set to initial', async () => { + const consoleSpy = jest.spyOn(console, 'error').mockImplementation(); + + const { query, mocks } = useErrorCase({ + networkError: new Error('Could not fetch'), + }); + + const { renders } = renderSuspenseHook( + () => useSuspenseQuery(query, { suspensePolicy: 'initial' }), + { mocks } + ); + + await waitFor(() => expect(renders.errorCount).toBe(1)); + + expect(renders.errors.length).toBe(1); + expect(renders.suspenseCount).toBe(1); + expect(renders.frames).toEqual([]); + + const [error] = renders.errors as ApolloError[]; + + expect(error).toBeInstanceOf(ApolloError); + expect(error.networkError).toEqual(new Error('Could not fetch')); + expect(error.graphQLErrors).toEqual([]); + + consoleSpy.mockRestore(); + }); + it('tears down subscription when throwing an error', async () => { const consoleSpy = jest.spyOn(console, 'error').mockImplementation(); @@ -2751,6 +3286,67 @@ describe('useSuspenseQuery', () => { ]); }); + it('responds to cache updates and clears errors after an error returns when errorPolicy is set to "ignore"', async () => { + const graphQLError = new GraphQLError('`id` should not be null'); + + const { query, mocks } = useErrorCase({ graphQLErrors: [graphQLError] }); + + const client = new ApolloClient({ + link: new MockLink(mocks), + cache: new InMemoryCache(), + }); + + const { result, renders } = renderSuspenseHook( + () => useSuspenseQuery(query, { errorPolicy: 'ignore' }), + { client } + ); + + await waitFor(() => { + expect(result.current).toMatchObject({ + data: undefined, + networkStatus: NetworkStatus.ready, + error: undefined, + }); + }); + + client.writeQuery({ + query, + data: { + currentUser: { + id: '1', + name: 'Cache User', + }, + }, + }); + + await waitFor(() => { + expect(result.current).toMatchObject({ + data: { + currentUser: { + id: '1', + name: 'Cache User', + }, + }, + networkStatus: NetworkStatus.ready, + error: undefined, + }); + }); + + expect(renders.count).toBe(3); + expect(renders.frames).toMatchObject([ + { + data: undefined, + networkStatus: NetworkStatus.ready, + error: undefined, + }, + { + data: { currentUser: { id: '1', name: 'Cache User' } }, + networkStatus: NetworkStatus.ready, + error: undefined, + }, + ]); + }); + it('throws network errors when errorPolicy is set to "all"', async () => { const consoleSpy = jest.spyOn(console, 'error').mockImplementation(); @@ -2804,7 +3400,7 @@ describe('useSuspenseQuery', () => { expect(renders.frames).toMatchObject([ { data: undefined, - networkStatus: NetworkStatus.ready, + networkStatus: NetworkStatus.error, error: new ApolloError({ graphQLErrors: [graphQLError] }), }, ]); @@ -2816,6 +3412,67 @@ describe('useSuspenseQuery', () => { expect(error!.graphQLErrors).toEqual([graphQLError]); }); + it('responds to cache updates and clears errors after an error returns when errorPolicy is set to "all"', async () => { + const graphQLError = new GraphQLError('`id` should not be null'); + + const { query, mocks } = useErrorCase({ graphQLErrors: [graphQLError] }); + + const client = new ApolloClient({ + link: new MockLink(mocks), + cache: new InMemoryCache(), + }); + + const { result, renders } = renderSuspenseHook( + () => useSuspenseQuery(query, { errorPolicy: 'all' }), + { client } + ); + + await waitFor(() => { + expect(result.current).toMatchObject({ + data: undefined, + networkStatus: NetworkStatus.error, + error: new ApolloError({ graphQLErrors: [graphQLError] }), + }); + }); + + client.writeQuery({ + query, + data: { + currentUser: { + id: '1', + name: 'Cache User', + }, + }, + }); + + await waitFor(() => { + expect(result.current).toMatchObject({ + data: { + currentUser: { + id: '1', + name: 'Cache User', + }, + }, + networkStatus: NetworkStatus.ready, + error: undefined, + }); + }); + + expect(renders.count).toBe(3); + expect(renders.frames).toMatchObject([ + { + data: undefined, + networkStatus: NetworkStatus.error, + error: new ApolloError({ graphQLErrors: [graphQLError] }), + }, + { + data: { currentUser: { id: '1', name: 'Cache User' } }, + networkStatus: NetworkStatus.ready, + error: undefined, + }, + ]); + }); + it('handles multiple graphql errors when errorPolicy is set to "all"', async () => { const graphQLErrors = [ new GraphQLError('Fool me once'), @@ -2845,7 +3502,7 @@ describe('useSuspenseQuery', () => { expect(renders.frames).toMatchObject([ { data: undefined, - networkStatus: NetworkStatus.ready, + networkStatus: NetworkStatus.error, error: expectedError, }, ]); @@ -2875,7 +3532,7 @@ describe('useSuspenseQuery', () => { await waitFor(() => { expect(result.current).toMatchObject({ data: { currentUser: { id: '1', name: null } }, - networkStatus: NetworkStatus.ready, + networkStatus: NetworkStatus.error, error: expectedError, }); }); @@ -2883,7 +3540,7 @@ describe('useSuspenseQuery', () => { expect(renders.frames).toMatchObject([ { data: { currentUser: { id: '1', name: null } }, - networkStatus: NetworkStatus.ready, + networkStatus: NetworkStatus.error, error: expectedError, }, ]); @@ -2950,7 +3607,7 @@ describe('useSuspenseQuery', () => { await waitFor(() => { expect(result.current).toMatchObject({ data: undefined, - networkStatus: NetworkStatus.ready, + networkStatus: NetworkStatus.error, error: expectedError, }); }); @@ -2965,19 +3622,14 @@ describe('useSuspenseQuery', () => { }); }); - expect(renders.count).toBe(5); + expect(renders.count).toBe(4); expect(renders.errorCount).toBe(0); expect(renders.errors).toEqual([]); expect(renders.suspenseCount).toBe(2); expect(renders.frames).toMatchObject([ { data: undefined, - networkStatus: NetworkStatus.ready, - error: expectedError, - }, - { - data: undefined, - networkStatus: NetworkStatus.ready, + networkStatus: NetworkStatus.error, error: expectedError, }, { @@ -3030,7 +3682,7 @@ describe('useSuspenseQuery', () => { await waitFor(() => { expect(result.current).toMatchObject({ data: undefined, - networkStatus: NetworkStatus.ready, + networkStatus: NetworkStatus.error, error: expectedError, }); }); @@ -3052,13 +3704,13 @@ describe('useSuspenseQuery', () => { expect(renders.frames).toMatchObject([ { data: undefined, - networkStatus: NetworkStatus.ready, + networkStatus: NetworkStatus.error, error: expectedError, }, { data: undefined, - networkStatus: NetworkStatus.ready, - error: expectedError, + networkStatus: NetworkStatus.setVariables, + error: undefined, }, { ...mocks[1].result, @@ -3341,7 +3993,7 @@ describe('useSuspenseQuery', () => { }); }); - expect(renders.count).toBe(3); + expect(renders.count).toBe(4); expect(renders.suspenseCount).toBe(1); expect(renders.frames).toMatchObject([ { @@ -3349,6 +4001,11 @@ describe('useSuspenseQuery', () => { networkStatus: NetworkStatus.ready, error: undefined, }, + { + ...mocks[0].result, + networkStatus: NetworkStatus.refetch, + error: undefined, + }, { ...mocks[1].result, networkStatus: NetworkStatus.ready, @@ -3421,6 +4078,80 @@ describe('useSuspenseQuery', () => { consoleSpy.mockRestore(); }); + it('throws errors when errors are returned after calling `refetch` with suspensePolicy set to "initial"', async () => { + const consoleSpy = jest.spyOn(console, 'error').mockImplementation(); + + const query = gql` + query UserQuery($id: String!) { + user(id: $id) { + id + name + } + } + `; + + const mocks = [ + { + request: { query, variables: { id: '1' } }, + result: { + data: { user: { id: '1', name: 'Captain Marvel' } }, + }, + }, + { + request: { query, variables: { id: '1' } }, + result: { + errors: [new GraphQLError('Something went wrong')], + }, + }, + ]; + + const { result, renders } = renderSuspenseHook( + () => + useSuspenseQuery(query, { + suspensePolicy: 'initial', + variables: { id: '1' }, + }), + { mocks } + ); + + await waitFor(() => { + expect(result.current).toMatchObject({ + ...mocks[0].result, + networkStatus: NetworkStatus.ready, + error: undefined, + }); + }); + + act(() => { + result.current.refetch(); + }); + + await waitFor(() => { + expect(renders.errorCount).toBe(1); + }); + + expect(renders.errors).toEqual([ + new ApolloError({ + graphQLErrors: [new GraphQLError('Something went wrong')], + }), + ]); + + expect(renders.frames).toMatchObject([ + { + ...mocks[0].result, + networkStatus: NetworkStatus.ready, + error: undefined, + }, + { + ...mocks[0].result, + networkStatus: NetworkStatus.refetch, + error: undefined, + }, + ]); + + consoleSpy.mockRestore(); + }); + it('ignores errors returned after calling `refetch` when errorPolicy is set to "ignore"', async () => { const query = gql` query UserQuery($id: String!) { @@ -3536,7 +4267,7 @@ describe('useSuspenseQuery', () => { await waitFor(() => { expect(result.current).toMatchObject({ ...mocks[0].result, - networkStatus: NetworkStatus.ready, + networkStatus: NetworkStatus.error, error: expectedError, }); }); @@ -3551,7 +4282,7 @@ describe('useSuspenseQuery', () => { }, { ...mocks[0].result, - networkStatus: NetworkStatus.ready, + networkStatus: NetworkStatus.error, error: expectedError, }, ]); @@ -3610,7 +4341,7 @@ describe('useSuspenseQuery', () => { await waitFor(() => { expect(result.current).toMatchObject({ data: mocks[1].result.data, - networkStatus: NetworkStatus.ready, + networkStatus: NetworkStatus.error, error: expectedError, }); }); @@ -3625,7 +4356,7 @@ describe('useSuspenseQuery', () => { }, { data: mocks[1].result.data, - networkStatus: NetworkStatus.ready, + networkStatus: NetworkStatus.error, error: expectedError, }, ]); @@ -3707,7 +4438,7 @@ describe('useSuspenseQuery', () => { }); }); - expect(renders.count).toBe(3); + expect(renders.count).toBe(4); expect(renders.suspenseCount).toBe(1); expect(renders.frames).toMatchObject([ { @@ -3715,6 +4446,11 @@ describe('useSuspenseQuery', () => { networkStatus: NetworkStatus.ready, error: undefined, }, + { + data: { letters: data.slice(0, 2) }, + networkStatus: NetworkStatus.fetchMore, + error: undefined, + }, { data: { letters: data.slice(2, 4) }, networkStatus: NetworkStatus.ready, @@ -3820,64 +4556,6 @@ describe('useSuspenseQuery', () => { ]); }); - it('applies nextFetchPolicy after initial suspense', async () => { - const { query, mocks } = useVariablesQueryCase(); - - const cache = new InMemoryCache(); - - // network-only should bypass this cached result and suspend the component - cache.writeQuery({ - query, - data: { character: { id: '1', name: 'Cached Hulk' } }, - variables: { id: '1' }, - }); - - // cache-first should read from this result on the rerender - cache.writeQuery({ - query, - data: { character: { id: '2', name: 'Cached Black Widow' } }, - variables: { id: '2' }, - }); - - const { result, renders, rerender } = renderSuspenseHook( - ({ id }) => - useSuspenseQuery(query, { - fetchPolicy: 'network-only', - // There is no way to trigger a followup query using nextFetchPolicy - // when this is a string vs a function. When changing variables, - // the `fetchPolicy` is reset back to `initialFetchPolicy` before the - // request is sent, negating the `nextFetchPolicy`. Using `refetch` or - // `fetchMore` sets the `fetchPolicy` to `network-only`, which negates - // the value. Using a function seems to be the only way to force a - // `nextFetchPolicy` without resorting to lower-level methods - // (i.e. `observable.reobserve`) - nextFetchPolicy: () => 'cache-first', - variables: { id }, - }), - { cache, mocks, initialProps: { id: '1' } } - ); - - await waitFor(() => { - expect(result.current).toMatchObject({ - ...mocks[0].result, - networkStatus: NetworkStatus.ready, - error: undefined, - }); - }); - - rerender({ id: '2' }); - - await waitFor(() => { - expect(result.current).toMatchObject({ - data: { character: { id: '2', name: 'Cached Black Widow' } }, - networkStatus: NetworkStatus.ready, - error: undefined, - }); - }); - - expect(renders.suspenseCount).toBe(1); - }); - it('honors refetchWritePolicy set to "overwrite"', async () => { const query: TypedDocumentNode< { primes: number[] }, @@ -5434,7 +6112,7 @@ describe('useSuspenseQuery', () => { name: 'R2-D2', }, }, - networkStatus: NetworkStatus.ready, + networkStatus: NetworkStatus.error, error: new ApolloError({ graphQLErrors: [ new GraphQLError( @@ -5486,7 +6164,7 @@ describe('useSuspenseQuery', () => { name: 'R2-D2', }, }, - networkStatus: NetworkStatus.ready, + networkStatus: NetworkStatus.error, error: new ApolloError({ graphQLErrors: [ new GraphQLError( @@ -5755,4 +6433,233 @@ describe('useSuspenseQuery', () => { }, ]); }); + + it('works with useDeferredValue', async () => { + const user = userEvent.setup(); + + interface Variables { + query: string; + } + + interface Data { + search: { query: string }; + } + + const QUERY: TypedDocumentNode = gql` + query SearchQuery($query: String!) { + search(query: $query) { + query + } + } + `; + + const link = new ApolloLink(({ variables }) => { + return new Observable((observer) => { + setTimeout(() => { + observer.next({ + data: { search: { query: variables.query } }, + }); + observer.complete(); + }, 10); + }); + }); + + const client = new ApolloClient({ + link, + cache: new InMemoryCache(), + }); + + const suspenseCache = new SuspenseCache(); + + function App() { + const [query, setValue] = React.useState(''); + const deferredQuery = React.useDeferredValue(query); + + return ( + + + setValue(e.target.value)} + /> + }> + + + + ); + } + + function SuspenseFallback() { + return

Loading

; + } + + function Results({ query }: { query: string }) { + const { data } = useSuspenseQuery(QUERY, { variables: { query } }); + + return
{data.search.query}
; + } + + render(); + + const input = screen.getByLabelText('Search'); + + expect(screen.getByText('Loading')).toBeInTheDocument(); + + expect(await screen.findByTestId('result')).toBeInTheDocument(); + + await act(() => user.type(input, 'ab')); + + await waitFor(() => { + expect(screen.getByTestId('result')).toHaveTextContent('ab'); + }); + + await act(() => user.type(input, 'c')); + + // useDeferredValue will try rerendering the component with the new value + // in the background. If it suspends with the new value, React will show the + // stale UI until the component is done suspending. + // + // Here we should not see the suspense fallback while the component suspends + // until the search finishes loading. Seeing the suspense fallback is an + // indication that we are suspending the component too late in the process. + expect(screen.queryByText('Loading')).not.toBeInTheDocument(); + expect(screen.getByTestId('result')).toHaveTextContent('ab'); + + // Eventually we should see the updated text content once its done + // suspending. + await waitFor(() => { + expect(screen.getByTestId('result')).toHaveTextContent('abc'); + }); + }); + + it('works with startTransition to change variables', async () => { + type Variables = { + id: string; + }; + + interface Data { + todo: { + id: string; + name: string; + completed: boolean; + }; + } + const user = userEvent.setup(); + + const query: TypedDocumentNode = gql` + query TodoItemQuery($id: ID!) { + todo(id: $id) { + id + name + completed + } + } + `; + + const mocks: MockedResponse[] = [ + { + request: { query, variables: { id: '1' } }, + result: { + data: { todo: { id: '1', name: 'Clean room', completed: false } }, + }, + delay: 10, + }, + { + request: { query, variables: { id: '2' } }, + result: { + data: { todo: { id: '2', name: 'Take out trash', completed: true } }, + }, + delay: 10, + }, + ]; + + const client = new ApolloClient({ + link: new MockLink(mocks), + cache: new InMemoryCache(), + }); + + const suspenseCache = new SuspenseCache(); + + function App() { + const [id, setId] = React.useState('1'); + + return ( + + }> + + + + ); + } + + function SuspenseFallback() { + return

Loading

; + } + + function Todo({ + id, + onChange, + }: { + id: string; + onChange: (id: string) => void; + }) { + const { data } = useSuspenseQuery(query, { variables: { id } }); + const [isPending, startTransition] = React.useTransition(); + const { todo } = data; + + return ( + <> + +
+ {todo.name} + {todo.completed && ' (completed)'} +
+ + ); + } + + render(); + + expect(screen.getByText('Loading')).toBeInTheDocument(); + + expect(await screen.findByTestId('todo')).toBeInTheDocument(); + + const todo = screen.getByTestId('todo'); + const button = screen.getByText('Refresh'); + + expect(todo).toHaveTextContent('Clean room'); + + await act(() => user.click(button)); + + // startTransition will avoid rendering the suspense fallback for already + // revealed content if the state update inside the transition causes the + // component to suspend. + // + // Here we should not see the suspense fallback while the component suspends + // until the todo is finished loading. Seeing the suspense fallback is an + // indication that we are suspending the component too late in the process. + expect(screen.queryByText('Loading')).not.toBeInTheDocument(); + + // We can ensure this works with isPending from useTransition in the process + expect(todo).toHaveAttribute('aria-busy', 'true'); + + // Ensure we are showing the stale UI until the new todo has loaded + expect(todo).toHaveTextContent('Clean room'); + + // Eventually we should see the updated todo content once its done + // suspending. + await waitFor(() => { + expect(todo).toHaveTextContent('Take out trash (completed)'); + }); + }); }); diff --git a/src/react/hooks/internal/__use.ts b/src/react/hooks/internal/__use.ts new file mode 100644 index 00000000000..9d57f0600fa --- /dev/null +++ b/src/react/hooks/internal/__use.ts @@ -0,0 +1,19 @@ +import { wrapPromiseWithState } from '../../../utilities/promises/decoration'; + +// TODO: Replace the use of this with React's use once its available. For now, +// this mimics its functionality for promises by adding +// properties to the promise instance and reading them synchronously. This is +// named with two underscores to allow this hook to evade typical rules of +// hooks (i.e. it can be used conditionally) +export function __use(promise: Promise) { + const statefulPromise = wrapPromiseWithState(promise); + + switch (statefulPromise.status) { + case 'pending': + throw statefulPromise; + case 'rejected': + throw statefulPromise.reason; + case 'fulfilled': + return statefulPromise.value; + } +} diff --git a/src/react/hooks/internal/index.ts b/src/react/hooks/internal/index.ts index 3db9a013096..a41c0902245 100644 --- a/src/react/hooks/internal/index.ts +++ b/src/react/hooks/internal/index.ts @@ -1,3 +1,5 @@ // These hooks are used internally and are not exported publicly by the library export { useDeepMemo } from './useDeepMemo'; export { useIsomorphicLayoutEffect } from './useIsomorphicLayoutEffect'; +export { useStrictModeSafeCleanupEffect } from './useStrictModeSafeCleanupEffect'; +export { __use } from './__use'; diff --git a/src/react/hooks/internal/useIsomorphicLayoutEffect.ts b/src/react/hooks/internal/useIsomorphicLayoutEffect.ts index 203cfdcc42b..b8bd88b645d 100644 --- a/src/react/hooks/internal/useIsomorphicLayoutEffect.ts +++ b/src/react/hooks/internal/useIsomorphicLayoutEffect.ts @@ -1,6 +1,11 @@ import { useLayoutEffect, useEffect } from 'react'; -import { canUseLayoutEffect } from '../../../utilities'; +import { canUseDOM } from '../../../utilities'; -export const useIsomorphicLayoutEffect = canUseLayoutEffect +// use canUseDOM here instead of canUseLayoutEffect because we want to be able +// to use useLayoutEffect in our jest tests. useLayoutEffect seems to work fine +// in useSuspenseQuery tests, but to honor the original comment about the +// warnings for useSyncExternalStore implementation, canUseLayoutEffect is left +// alone. +export const useIsomorphicLayoutEffect = canUseDOM ? useLayoutEffect : useEffect; diff --git a/src/react/hooks/internal/useStrictModeSafeCleanupEffect.ts b/src/react/hooks/internal/useStrictModeSafeCleanupEffect.ts new file mode 100644 index 00000000000..6c4528fd20b --- /dev/null +++ b/src/react/hooks/internal/useStrictModeSafeCleanupEffect.ts @@ -0,0 +1,13 @@ +import { useEffect } from 'react'; + +export function useStrictModeSafeCleanupEffect(cleanup: () => void) { + let timeout: NodeJS.Timeout; + + useEffect(() => { + clearTimeout(timeout); + + return () => { + timeout = setTimeout(cleanup); + }; + }, []); +} diff --git a/src/react/hooks/useSuspenseQuery.ts b/src/react/hooks/useSuspenseQuery.ts index a07620ec6d5..eb2aa1ac637 100644 --- a/src/react/hooks/useSuspenseQuery.ts +++ b/src/react/hooks/useSuspenseQuery.ts @@ -1,34 +1,30 @@ import { invariant, __DEV__ } from '../../utilities/globals'; -import { useRef, useEffect, useCallback, useMemo, useState } from 'react'; import { equal } from '@wry/equality'; +import { useRef, useCallback, useMemo } from 'react'; import { ApolloClient, ApolloError, ApolloQueryResult, DocumentNode, - ObservableQuery, OperationVariables, TypedDocumentNode, WatchQueryOptions, WatchQueryFetchPolicy, NetworkStatus, + FetchMoreQueryOptions, } from '../../core'; -import { - compact, - Concast, - isNonEmptyArray, - hasDirectives, -} from '../../utilities'; +import { isNonEmptyArray } from '../../utilities'; import { useApolloClient } from './useApolloClient'; import { DocumentType, verifyDocumentType } from '../parser'; import { + SuspenseQueryHookFetchPolicy, SuspenseQueryHookOptions, ObservableQueryFields, } from '../types/types'; -import { useDeepMemo, useIsomorphicLayoutEffect } from './internal'; +import { useDeepMemo, useStrictModeSafeCleanupEffect, __use } from './internal'; import { useSuspenseCache } from './useSuspenseCache'; import { useSyncExternalStore } from './useSyncExternalStore'; -import { Subscription } from 'zen-observable-ts'; +import { QuerySubscription } from '../cache/QuerySubscription'; export interface UseSuspenseQueryResult< TData = any, @@ -43,10 +39,17 @@ export interface UseSuspenseQueryResult< subscribeToMore: SubscribeToMoreFunction; } -type FetchMoreFunction< - TData, - TVariables extends OperationVariables -> = ObservableQueryFields['fetchMore']; +type FetchMoreFunction = ( + fetchMoreOptions: FetchMoreQueryOptions & { + updateQuery?: ( + previousQueryResult: TData, + options: { + fetchMoreResult: TData; + variables: TVariables; + } + ) => TData; + } +) => Promise>; type RefetchFunction< TData, @@ -58,17 +61,6 @@ type SubscribeToMoreFunction< TVariables extends OperationVariables > = ObservableQueryFields['subscribeToMore']; -const SUPPORTED_FETCH_POLICIES: WatchQueryFetchPolicy[] = [ - 'cache-first', - 'network-only', - 'no-cache', - 'cache-and-network', -]; - -const DEFAULT_FETCH_POLICY = 'cache-first'; -const DEFAULT_SUSPENSE_POLICY = 'always'; -const DEFAULT_ERROR_POLICY = 'none'; - export function useSuspenseQuery_experimental< TData = any, TVariables extends OperationVariables = OperationVariables @@ -76,166 +68,123 @@ export function useSuspenseQuery_experimental< query: DocumentNode | TypedDocumentNode, options: SuspenseQueryHookOptions = Object.create(null) ): UseSuspenseQueryResult { - const suspenseCache = useSuspenseCache(options.suspenseCache); + const didPreviouslySuspend = useRef(false); const client = useApolloClient(options.client); - const watchQueryOptions = useWatchQueryOptions({ query, options, client }); - const previousWatchQueryOptionsRef = useRef(watchQueryOptions); - const deferred = useIsDeferred(query); + const suspenseCache = useSuspenseCache(options.suspenseCache); + const watchQueryOptions = useWatchQueryOptions({ query, options }); + const { returnPartialData = false, variables } = watchQueryOptions; + const { suspensePolicy = 'always' } = options; + const shouldSuspend = + suspensePolicy === 'always' || !didPreviouslySuspend.current; - const { fetchPolicy, errorPolicy, returnPartialData, variables } = - watchQueryOptions; + const subscription = suspenseCache.getSubscription( + client, + query, + variables, + () => client.watchQuery(watchQueryOptions) + ); - let cacheEntry = suspenseCache.lookup(query, variables); + const dispose = useTrackedSubscriptions(subscription); - const [observable] = useState(() => { - return cacheEntry?.observable || client.watchQuery(watchQueryOptions); - }); + useStrictModeSafeCleanupEffect(dispose); - const result = useObservableQueryResult(observable); + let result = useSyncExternalStore( + subscription.listen, + () => subscription.result, + () => subscription.result + ); - const hasFullResult = result.data && !result.partial; - const hasPartialResult = result.data && result.partial; - const usePartialResult = returnPartialData && hasPartialResult; + const previousVariables = useRef(variables); + const previousData = useRef(result.data); + + if (!equal(variables, previousVariables.current)) { + if (result.networkStatus !== NetworkStatus.ready) { + // Since we now create separate ObservableQuery instances per unique + // query + variables combination, we need to manually insert the previous + // data into the returned result to mimic the behavior when changing + // variables from a single ObservableQuery, where the previous result was + // held onto until the request was finished. + result = { + ...result, + data: previousData.current, + networkStatus: NetworkStatus.setVariables, + }; + } - const allowsThrownErrors = - // If we've got a deferred query that errors on an incremental chunk, we - // will have a partial result before the error is collected. We do not want - // to throw errors that have been returned from incremental chunks. Instead - // we offload those errors to the `error` property. - errorPolicy === 'none' && (!deferred || !hasPartialResult); + previousVariables.current = variables; + previousData.current = result.data; + } if ( - result.error && - // Always throw network errors regardless of the error policy - (result.error.networkError || allowsThrownErrors) + result.networkStatus === NetworkStatus.error || + (shouldSuspend && + !shouldUseCachedResult(subscription.result, { + returnPartialData, + fetchPolicy: options.fetchPolicy, + })) ) { - throw result.error; + // Intentionally ignore the result returned from __use since we want to + // observe results from the observable instead of the the promise. + __use(subscription.promise); } - if (result.loading) { - // If we don't have a cache entry, but we are in a loading state, we are on - // the first run of the hook. Kick off a network request so we can suspend - // immediately - if (!cacheEntry) { - cacheEntry = suspenseCache.add(query, variables, { - promise: maybeWrapConcastWithCustomPromise( - observable.reobserveAsConcast(watchQueryOptions), - { deferred } - ), - observable, - }); - } - - const hasUsableResult = - // When we have partial data in the cache, a network request will be kicked - // off to load the full set of data. Avoid suspending when the request is - // in flight to return the partial data immediately. - usePartialResult || - // `cache-and-network` kicks off a network request even with a full set of - // data in the cache, which means the loading state will be set to `true`. - // Avoid suspending in this case. - (fetchPolicy === 'cache-and-network' && hasFullResult); - - if (!hasUsableResult && !cacheEntry.fulfilled) { - throw cacheEntry.promise; - } - } - - useEffect(() => { - const { variables, query } = watchQueryOptions; - const previousOpts = previousWatchQueryOptionsRef.current; - - if (variables !== previousOpts.variables || query !== previousOpts.query) { - suspenseCache.remove(previousOpts.query, previousOpts.variables); - - suspenseCache.add(query, variables, { - promise: observable.reobserve({ query, variables }), - observable, - }); - - previousWatchQueryOptionsRef.current = watchQueryOptions; - } - }, [watchQueryOptions]); - - useEffect(() => { - return () => { - suspenseCache.remove(query, variables); - }; - }, []); + didPreviouslySuspend.current = true; const fetchMore: FetchMoreFunction = useCallback( - (options) => { - const promise = observable.fetchMore(options); - - suspenseCache.add(query, watchQueryOptions.variables, { - promise, - observable, - }); - - return promise; - }, - [observable] + (options) => subscription.fetchMore(options), + [subscription] ); const refetch: RefetchFunction = useCallback( - (variables) => { - const promise = observable.refetch(variables); - - suspenseCache.add(query, watchQueryOptions.variables, { - promise, - observable, - }); - - return promise; - }, - [observable] + (variables) => subscription.refetch(variables), + [subscription] ); const subscribeToMore: SubscribeToMoreFunction = - useCallback((options) => observable.subscribeToMore(options), [observable]); + useCallback( + (options) => subscription.observable.subscribeToMore(options), + [subscription] + ); return useMemo(() => { return { client, data: result.data, - error: errorPolicy === 'ignore' ? void 0 : toApolloError(result), + error: toApolloError(result), networkStatus: result.networkStatus, fetchMore, refetch, subscribeToMore, }; - }, [ - client, - fetchMore, - refetch, - result, - observable, - errorPolicy, - subscribeToMore, - ]); + }, [client, fetchMore, refetch, result, subscribeToMore]); } function validateOptions(options: WatchQueryOptions) { - const { - query, - fetchPolicy = DEFAULT_FETCH_POLICY, - returnPartialData, - } = options; + const { query, fetchPolicy, returnPartialData } = options; verifyDocumentType(query, DocumentType.Query); validateFetchPolicy(fetchPolicy); validatePartialDataReturn(fetchPolicy, returnPartialData); } -function validateFetchPolicy(fetchPolicy: WatchQueryFetchPolicy) { +function validateFetchPolicy( + fetchPolicy: WatchQueryFetchPolicy = 'cache-first' +) { + const supportedFetchPolicies: WatchQueryFetchPolicy[] = [ + 'cache-first', + 'network-only', + 'no-cache', + 'cache-and-network', + ]; + invariant( - SUPPORTED_FETCH_POLICIES.includes(fetchPolicy), + supportedFetchPolicies.includes(fetchPolicy), `The fetch policy \`${fetchPolicy}\` is not supported with suspense.` ); } function validatePartialDataReturn( - fetchPolicy: WatchQueryFetchPolicy, + fetchPolicy: WatchQueryFetchPolicy | undefined, returnPartialData: boolean | undefined ) { if (fetchPolicy === 'no-cache' && returnPartialData) { @@ -251,27 +200,14 @@ function toApolloError(result: ApolloQueryResult) { : result.error; } -function maybeWrapConcastWithCustomPromise( - concast: Concast>, - { deferred }: { deferred: boolean } -): Promise> { - if (deferred) { - return new Promise((resolve, reject) => { - // Unlike `concast.promise`, we want to resolve the promise on the initial - // chunk of the deferred query. This allows the component to unsuspend - // when we get the initial set of data, rather than waiting until all - // chunks have been loaded. - const subscription = concast.subscribe({ - next: (value) => { - resolve(value); - subscription.unsubscribe(); - }, - error: reject, - }); - }); - } +function useTrackedSubscriptions(subscription: QuerySubscription) { + const trackedSubscriptions = useRef(new Set()); + + trackedSubscriptions.current.add(subscription); - return concast.promise; + return function dispose() { + trackedSubscriptions.current.forEach((sub) => sub.dispose()); + }; } interface UseWatchQueryOptionsHookOptions< @@ -280,50 +216,24 @@ interface UseWatchQueryOptionsHookOptions< > { query: DocumentNode | TypedDocumentNode; options: SuspenseQueryHookOptions; - client: ApolloClient; } function useWatchQueryOptions({ query, options, - client, }: UseWatchQueryOptionsHookOptions): WatchQueryOptions< TVariables, TData > { - const { watchQuery: defaultOptions } = client.defaultOptions; - - const watchQueryOptions = useDeepMemo< - WatchQueryOptions - >(() => { - const { - errorPolicy, - fetchPolicy, - suspensePolicy = DEFAULT_SUSPENSE_POLICY, - variables, - ...watchQueryOptions - } = options; - - return { - ...watchQueryOptions, + const watchQueryOptions = useDeepMemo>( + () => ({ + ...options, query, - errorPolicy: - errorPolicy || defaultOptions?.errorPolicy || DEFAULT_ERROR_POLICY, - fetchPolicy: - fetchPolicy || defaultOptions?.fetchPolicy || DEFAULT_FETCH_POLICY, - notifyOnNetworkStatusChange: suspensePolicy === 'always', - // By default, `ObservableQuery` will run `reobserve` the first time - // something `subscribe`s to the observable, which kicks off a network - // request. This creates a problem for suspense because we need to begin - // fetching the data immediately so we can throw the promise on the first - // render. Since we don't subscribe until after we've unsuspended, we need - // to avoid kicking off another network request for the same data we just - // fetched. This option toggles that behavior off to avoid the `reobserve` - // when the observable is first subscribed to. - fetchOnFirstSubscribe: false, - variables: compact({ ...defaultOptions?.variables, ...variables }), - }; - }, [options, query, defaultOptions]); + notifyOnNetworkStatusChange: true, + nextFetchPolicy: void 0, + }), + [options, query] + ); if (__DEV__) { validateOptions(watchQueryOptions); @@ -332,132 +242,33 @@ function useWatchQueryOptions({ return watchQueryOptions; } -function useIsDeferred(query: DocumentNode) { - return useMemo(() => hasDirectives(['defer'], query), [query]); -} - -function useObservableQueryResult(observable: ObservableQuery) { - const resultRef = useRef>(); - const isMountedRef = useRef(false); - const subscribeTimeoutRef = useRef(); - - if (!resultRef.current) { - resultRef.current = observable.getCurrentResult(); +function shouldUseCachedResult( + result: ApolloQueryResult, + { + returnPartialData, + fetchPolicy, + }: { + returnPartialData: boolean | undefined; + fetchPolicy: SuspenseQueryHookFetchPolicy | undefined; + } +) { + if ( + result.networkStatus === NetworkStatus.refetch || + result.networkStatus === NetworkStatus.fetchMore || + result.networkStatus === NetworkStatus.error + ) { + return false; } - // React keeps refs and effects from useSyncExternalStore around after the - // component initially mounts even if the component re-suspends. We need to - // track when the component suspends/unsuspends to ensure we don't try and - // update the component while its suspended since the observable's - // `next` function is called before the promise resolved. - // - // Unlike useEffect, useLayoutEffect will run its cleanup and initialization - // functions each time a component is suspended. - useIsomorphicLayoutEffect(() => { - isMountedRef.current = true; - - return () => { - isMountedRef.current = false; - }; - }, []); - - return useSyncExternalStore( - useCallback( - (forceUpdate) => { - clearTimeout(subscribeTimeoutRef.current); - - function handleUpdate() { - const previousResult = resultRef.current!; - const result = observable.getCurrentResult(); - - if ( - previousResult.loading === result.loading && - previousResult.networkStatus === result.networkStatus && - equal(previousResult.data, result.data) - ) { - return; - } - - resultRef.current = result; - - if (isMountedRef.current) { - forceUpdate(); - } - } - - let subscription: Subscription; - - // We use a `setTimeout` here to avoid issues in React's strict mode - // where the subscription would be torn down, but resubscribing would - // be stuck in the torn down state, therefore updates to the cache would - // not trigger the observable's subscription. This occurs due to the new - // `fetchOnFirstSubscribe` option introduced with `useSuspenseQuery`, - // which avoids issuing a network request (via `reobserve`) when calling - // `subscribe` on the observable. Unfortunately `reobserve` is required - // to put the observable back into a non-torn-down state, which is not - // called due to this option. Instead we try delaying calling subscribe - // for the first time by allowing React to run this effect, tear it down, - // then set it back up again before we resubscribe. - // - // Authors note (Jerel): This feels super hacky and I hate it, but this - // seems like the best approach to avoid completely changing around the - // internals of ObservableQuery, which could introduce new bugs if I'm - // not careful. Ideally we could call `subscribe()`, `unsubscribe()`, - // then `subscribe()` again and be back into a normal state, but this - // might require a sizable refactor to accomplish. - // - // Related to https://github.com/apollographql/apollo-client/issues/10428 - subscribeTimeoutRef.current = setTimeout(() => { - subscription = observable.subscribe({ - next: handleUpdate, - error: handleUpdate, - }); - - // Fixes an issue where mounting this hook with data already in the - // cache while using a cache-first fetch policy did not respond to - // cache updates. - // - // This is due to the fact that this hook manages the - // fetching lifecycle (via `reobserve`) rather than the subscription. - // We disable fetching when subscribing to the observable since we - // kick off the fetch in the first render. This however has some - // downstream issues, since `reobserve` is necessary to set some - // internal state updates on `ObservableQuery` and `QueryInfo`. In - // cases where we can fulfill the result via the cache immediately, we - // avoid calling `reobserve` by subscribing (via the - // `fetchOnFirstSubscribe` option) to avoid the network request, but - // this means the interal state updates do not happen. - // - // In this case, `QueryInfo`, is initialized with a `networkStatus` of - // 1, but because we don't call `reobserve`, this value never gets - // updated to 7 even though `observableQuery.getCurrentResult()` is - // able to correctly set this value to 7. This caused issues where - // `broadcastQueries` was not sending notifications since `queryInfo` - // avoids broadcasting to in-flight queries for fetch policies other - // than cache-only and cache-and-network. - // - // This attempts to patch the behavior expected from `reobserve` by - // marking the queryInfo as ready if we detect that the result is also - // ready. - // - // Related to https://github.com/apollographql/apollo-client/issues/10478 - const result = resultRef.current!; - - if ( - result.networkStatus !== observable['queryInfo'].networkStatus && - result.networkStatus === NetworkStatus.ready - ) { - observable['queryInfo'].markReady(); - } - }); - - return () => { - subscription?.unsubscribe(); - }; - }, - [observable] - ), - () => resultRef.current!, - () => resultRef.current! - ); + switch (fetchPolicy) { + // The default fetch policy is cache-first, so we can treat undefined as + // such. + case void 0: + case 'cache-first': + case 'cache-and-network': { + return Boolean(result.data && (!result.partial || returnPartialData)); + } + default: + return false; + } } diff --git a/src/react/types/types.ts b/src/react/types/types.ts index bfca4225e4e..2f98d814092 100644 --- a/src/react/types/types.ts +++ b/src/react/types/types.ts @@ -18,7 +18,6 @@ import { WatchQueryOptions, WatchQueryFetchPolicy, } from '../../core'; -import { NextFetchPolicyContext } from '../../core/watchQueryOptions'; import { SuspenseCache } from '../cache'; /* Common types */ @@ -132,12 +131,6 @@ export interface SuspenseQueryHookOptions< | 'refetchWritePolicy' > { fetchPolicy?: SuspenseQueryHookFetchPolicy; - nextFetchPolicy?: - | SuspenseQueryHookFetchPolicy - | (( - currentFetchPolicy: SuspenseQueryHookFetchPolicy, - context: NextFetchPolicyContext - ) => SuspenseQueryHookFetchPolicy); suspensePolicy?: SuspensePolicy; suspenseCache?: SuspenseCache; } diff --git a/src/utilities/common/mergeOptions.ts b/src/utilities/common/mergeOptions.ts index 1633a86df29..ec4983dcb04 100644 --- a/src/utilities/common/mergeOptions.ts +++ b/src/utilities/common/mergeOptions.ts @@ -19,9 +19,9 @@ export function mergeOptions< options: TOptions | Partial, ): TOptions { return compact(defaults, options, options.variables && { - variables: { + variables: compact({ ...(defaults && defaults.variables), ...options.variables, - }, + }), }); } diff --git a/src/utilities/promises/decoration.ts b/src/utilities/promises/decoration.ts new file mode 100644 index 00000000000..1935626f579 --- /dev/null +++ b/src/utilities/promises/decoration.ts @@ -0,0 +1,58 @@ +export interface PendingPromise extends Promise { + status: 'pending'; +} + +export interface FulfilledPromise extends Promise { + status: 'fulfilled'; + value: TValue; +} + +export interface RejectedPromise extends Promise { + status: 'rejected'; + reason: unknown; +} + +export type PromiseWithState = + | PendingPromise + | FulfilledPromise + | RejectedPromise; + +export function isStatefulPromise( + promise: Promise +): promise is PromiseWithState { + return 'status' in promise; +} + +export function wrapPromiseWithState( + promise: Promise +): PromiseWithState { + if (isStatefulPromise(promise)) { + return promise; + } + + const pendingPromise = promise as PendingPromise; + pendingPromise.status = 'pending'; + + pendingPromise.then( + (value) => { + if (pendingPromise.status === 'pending') { + const fulfilledPromise = + pendingPromise as unknown as FulfilledPromise; + + fulfilledPromise.status = 'fulfilled'; + fulfilledPromise.value = value; + } + }, + (reason: unknown) => { + if (pendingPromise.status === 'pending') { + const rejectedPromise = + pendingPromise as unknown as RejectedPromise; + + rejectedPromise.status = 'rejected'; + rejectedPromise.reason = reason; + } + } + ); + + return promise as PromiseWithState; +}