From 49d28f764980d132089ef8f6beca6e766b6120c0 Mon Sep 17 00:00:00 2001 From: Jerel Miller Date: Wed, 3 May 2023 10:33:20 -0600 Subject: [PATCH] Support `startTransition` with `refetch` and `fetchMore` + deprecate `suspensePolicy` (#10809) Co-authored-by: Alessia Bellisario --- .changeset/strange-drinks-report.md | 5 + config/bundlesize.ts | 2 +- src/__tests__/__snapshots__/exports.ts.snap | 2 + src/react/cache/QuerySubscription.ts | 139 ++-- .../hooks/__tests__/useSuspenseQuery.test.tsx | 743 +++++++----------- src/react/hooks/useSuspenseQuery.ts | 131 +-- src/react/types/types.ts | 10 - src/utilities/index.ts | 2 + src/utilities/promises/decoration.ts | 18 + 9 files changed, 442 insertions(+), 610 deletions(-) create mode 100644 .changeset/strange-drinks-report.md diff --git a/.changeset/strange-drinks-report.md b/.changeset/strange-drinks-report.md new file mode 100644 index 00000000000..932975725e4 --- /dev/null +++ b/.changeset/strange-drinks-report.md @@ -0,0 +1,5 @@ +--- +'@apollo/client': patch +--- + +Fixed the ability to use `refetch` and `fetchMore` with React's `startTransition`. The hook will now behave correctly by allowing React to avoid showing the Suspense fallback when these functions are wrapped by `startTransition`. diff --git a/config/bundlesize.ts b/config/bundlesize.ts index e9ed3fd7b58..ef71989ad4b 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("35.00KB"); +const gzipBundleByteLengthLimit = bytes("35.04KB"); const minFile = join("dist", "apollo-client.min.cjs"); const minPath = join(__dirname, "..", minFile); const gzipByteLen = gzipSync(readFileSync(minPath)).byteLength; diff --git a/src/__tests__/__snapshots__/exports.ts.snap b/src/__tests__/__snapshots__/exports.ts.snap index 8cda62e8aac..c1981c18fc9 100644 --- a/src/__tests__/__snapshots__/exports.ts.snap +++ b/src/__tests__/__snapshots__/exports.ts.snap @@ -376,6 +376,8 @@ Array [ "compact", "concatPagination", "createFragmentMap", + "createFulfilledPromise", + "createRejectedPromise", "fixObservableSubclass", "getDefaultValues", "getDirectiveNames", diff --git a/src/react/cache/QuerySubscription.ts b/src/react/cache/QuerySubscription.ts index 966604cdb33..4c2503d6d01 100644 --- a/src/react/cache/QuerySubscription.ts +++ b/src/react/cache/QuerySubscription.ts @@ -1,61 +1,45 @@ import { ApolloError, ApolloQueryResult, - DocumentNode, NetworkStatus, ObservableQuery, OperationVariables, } from '../../core'; import { isNetworkRequestSettled } from '../../core'; import { - Concast, ObservableSubscription, - hasAnyDirectives, + createFulfilledPromise, + createRejectedPromise, } from '../../utilities'; -import { invariant } from '../../utilities/globals'; -import { wrap } from 'optimism'; -type Listener = (result: ApolloQueryResult) => void; +type Listener = () => 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 { +export class QuerySubscription { public result: ApolloQueryResult; - public promise: Promise>; public readonly observable: ObservableQuery; + public promises: { + main: Promise>; + network?: Promise>; + }; + private subscription: ObservableSubscription; - private listeners = new Set>(); + private listeners = new Set(); private autoDisposeTimeoutId: NodeJS.Timeout; + private initialized = false; + private refetching = false; + + private resolve: (result: ApolloQueryResult) => void; + private reject: (error: unknown) => void; constructor( observable: ObservableQuery, @@ -66,32 +50,35 @@ export class QuerySubscription { this.handleError = this.handleError.bind(this); this.dispose = this.dispose.bind(this); this.observable = observable; - this.result = observable.getCurrentResult(); + this.result = observable.getCurrentResult(false); if (options.onDispose) { this.onDispose = options.onDispose; } + if ( + isNetworkRequestSettled(this.result.networkStatus) || + (this.result.data && + (!this.result.partial || this.observable.options.returnPartialData)) + ) { + this.promises = { main: createFulfilledPromise(this.result) }; + this.initialized = true; + this.refetching = false; + } + 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; + if (!this.promises) { + this.promises = { + main: new Promise((resolve, reject) => { + this.resolve = resolve; + this.reject = reject; + }), + }; + } // Start a timer that will automatically dispose of the query if the // suspended resource does not use this subscription in the given time. This @@ -103,7 +90,7 @@ export class QuerySubscription { ); } - listen(listener: Listener) { + 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. @@ -117,15 +104,21 @@ export class QuerySubscription { } refetch(variables: OperationVariables | undefined) { - this.promise = this.observable.refetch(variables); + this.refetching = true; - return this.promise; + const promise = this.observable.refetch(variables); + + this.promises.network = promise; + + return promise; } fetchMore(options: FetchMoreOptions) { - this.promise = this.observable.fetchMore(options); + const promise = this.observable.fetchMore(options); - return this.promise; + this.promises.network = promise; + + return promise; } dispose() { @@ -138,19 +131,27 @@ export class QuerySubscription { } private handleNext(result: ApolloQueryResult) { + if (!this.initialized) { + this.initialized = true; + this.result = result; + this.resolve(result); + return; + } + + if (result.data === this.result.data) { + return; + } + // 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 - ) { + // fetched a previous result, set the new result data to the last successful + // result. + if (this.result.data && result.data === void 0) { result.data = this.result.data; } this.result = result; - this.deliver(result); + this.promises.main = createFulfilledPromise(result); + this.deliver(); } private handleError(error: ApolloError) { @@ -161,10 +162,22 @@ export class QuerySubscription { }; this.result = result; - this.deliver(result); + + if (!this.initialized || this.refetching) { + this.initialized = true; + this.refetching = false; + this.reject(error); + return; + } + + this.result = result; + this.promises.main = result.data + ? createFulfilledPromise(result) + : createRejectedPromise(result); + this.deliver(); } - private deliver(result: ApolloQueryResult) { - this.listeners.forEach((listener) => listener(result)); + private deliver() { + this.listeners.forEach((listener) => listener()); } } diff --git a/src/react/hooks/__tests__/useSuspenseQuery.test.tsx b/src/react/hooks/__tests__/useSuspenseQuery.test.tsx index fa098d1623f..ec1b10f6326 100644 --- a/src/react/hooks/__tests__/useSuspenseQuery.test.tsx +++ b/src/react/hooks/__tests__/useSuspenseQuery.test.tsx @@ -35,6 +35,7 @@ import { compact, concatPagination, getMainDefinition, + offsetLimitPagination, } from '../../../utilities'; import { MockedProvider, @@ -2435,72 +2436,6 @@ describe('useSuspenseQuery', () => { ]); }); - it.each([ - 'cache-first', - 'network-only', - 'no-cache', - 'cache-and-network', - ])( - 'returns previous data when changing variables and using a "%s" with an "initial" suspense policy', - async (fetchPolicy) => { - const { query, mocks } = useVariablesQueryCase(); - - const { result, rerender, renders } = renderSuspenseHook( - ({ id }) => - useSuspenseQuery(query, { - fetchPolicy, - suspensePolicy: 'initial', - variables: { id }, - }), - { mocks, initialProps: { id: '1' } } - ); - - expect(renders.suspenseCount).toBe(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, - }); - }); - - // Renders: - // 1. Initiate fetch and suspend - // 2. Unsuspend and return results from initial fetch - // 3. Change variables - // 4. Unsuspend and return results from refetch - expect(renders.count).toBe(4); - expect(renders.suspenseCount).toBe(1); - expect(renders.frames).toMatchObject([ - { - ...mocks[0].result, - networkStatus: NetworkStatus.ready, - error: undefined, - }, - { - ...mocks[0].result, - networkStatus: NetworkStatus.setVariables, - error: undefined, - }, - { - ...mocks[1].result, - networkStatus: NetworkStatus.ready, - error: undefined, - }, - ]); - } - ); - it.each([ 'cache-first', 'network-only', @@ -3276,33 +3211,6 @@ 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(); @@ -3382,66 +3290,6 @@ describe('useSuspenseQuery', () => { consoleSpy.mockRestore(); }); - it('tears down subscription when throwing an error on refetch when suspensePolicy is "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 client = new ApolloClient({ - cache: new InMemoryCache(), - link: new MockLink(mocks), - }); - - const { result, renders } = renderSuspenseHook( - () => - useSuspenseQuery(query, { - suspensePolicy: 'initial', - variables: { id: '1' }, - }), - { client } - ); - - 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(client.getObservableQueries().size).toBe(0); - - consoleSpy.mockRestore(); - }); - it('throws network errors when errorPolicy is set to "none"', async () => { const consoleSpy = jest.spyOn(console, 'error').mockImplementation(); @@ -3991,86 +3839,6 @@ describe('useSuspenseQuery', () => { ]); }); - it('clears errors when changing variables and errorPolicy is set to "all" with an "initial" suspensePolicy', async () => { - const query = gql` - query UserQuery($id: String!) { - user(id: $id) { - id - name - } - } - `; - - const graphQLErrors = [new GraphQLError('Could not fetch user 1')]; - - const mocks = [ - { - request: { query, variables: { id: '1' } }, - result: { - errors: graphQLErrors, - }, - }, - { - request: { query, variables: { id: '2' } }, - result: { - data: { user: { id: '2', name: 'Captain Marvel' } }, - }, - }, - ]; - - const { result, renders, rerender } = renderSuspenseHook( - ({ id }) => - useSuspenseQuery(query, { - errorPolicy: 'all', - suspensePolicy: 'initial', - variables: { id }, - }), - { mocks, initialProps: { id: '1' } } - ); - - const expectedError = new ApolloError({ graphQLErrors }); - - await waitFor(() => { - expect(result.current).toMatchObject({ - data: undefined, - networkStatus: NetworkStatus.error, - error: expectedError, - }); - }); - - rerender({ id: '2' }); - - await waitFor(() => { - expect(result.current).toMatchObject({ - ...mocks[1].result, - networkStatus: NetworkStatus.ready, - error: undefined, - }); - }); - - expect(renders.count).toBe(4); - expect(renders.errorCount).toBe(0); - expect(renders.errors).toEqual([]); - expect(renders.suspenseCount).toBe(1); - expect(renders.frames).toMatchObject([ - { - data: undefined, - networkStatus: NetworkStatus.error, - error: expectedError, - }, - { - data: undefined, - networkStatus: NetworkStatus.setVariables, - error: undefined, - }, - { - ...mocks[1].result, - networkStatus: NetworkStatus.ready, - error: undefined, - }, - ]); - }); - it('re-suspends when calling `refetch`', async () => { const query = gql` query UserQuery($id: String!) { @@ -4290,81 +4058,6 @@ describe('useSuspenseQuery', () => { ]); }); - it('does not suspend and returns previous data when calling `refetch` and using an "initial" suspensePolicy', async () => { - 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: { - data: { user: { id: '1', name: 'Captain Marvel (updated)' } }, - }, - }, - ]; - - 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(result.current).toMatchObject({ - ...mocks[1].result, - networkStatus: NetworkStatus.ready, - error: undefined, - }); - }); - - expect(renders.count).toBe(4); - expect(renders.suspenseCount).toBe(1); - expect(renders.frames).toMatchObject([ - { - ...mocks[0].result, - networkStatus: NetworkStatus.ready, - error: undefined, - }, - { - ...mocks[0].result, - networkStatus: NetworkStatus.refetch, - error: undefined, - }, - { - ...mocks[1].result, - networkStatus: NetworkStatus.ready, - error: undefined, - }, - ]); - }); - it('throws errors when errors are returned after calling `refetch`', async () => { const consoleSpy = jest.spyOn(console, 'error').mockImplementation(); @@ -4429,80 +4122,6 @@ 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!) { @@ -4686,86 +4305,38 @@ describe('useSuspenseQuery', () => { }); act(() => { - result.current.refetch(); - }); - - await waitFor(() => { - expect(result.current).toMatchObject({ - data: mocks[1].result.data, - networkStatus: NetworkStatus.error, - error: expectedError, - }); - }); - - expect(renders.errorCount).toBe(0); - expect(renders.errors).toEqual([]); - expect(renders.frames).toMatchObject([ - { - ...mocks[0].result, - networkStatus: NetworkStatus.ready, - error: undefined, - }, - { - data: mocks[1].result.data, - networkStatus: NetworkStatus.error, - error: expectedError, - }, - ]); - }); - - it('re-suspends when calling `fetchMore` with different variables', async () => { - const { data, query, link } = usePaginatedCase(); - - const { result, renders } = renderSuspenseHook( - () => useSuspenseQuery(query, { variables: { limit: 2 } }), - { link } - ); - - await waitFor(() => { - expect(result.current).toMatchObject({ - data: { letters: data.slice(0, 2) }, - networkStatus: NetworkStatus.ready, - error: undefined, - }); - }); - - act(() => { - result.current.fetchMore({ variables: { offset: 2 } }); + result.current.refetch(); }); await waitFor(() => { expect(result.current).toMatchObject({ - data: { letters: data.slice(2, 4) }, - networkStatus: NetworkStatus.ready, - error: undefined, + data: mocks[1].result.data, + networkStatus: NetworkStatus.error, + error: expectedError, }); }); - expect(renders.count).toBe(4); - expect(renders.suspenseCount).toBe(2); + expect(renders.errorCount).toBe(0); + expect(renders.errors).toEqual([]); expect(renders.frames).toMatchObject([ { - data: { letters: data.slice(0, 2) }, + ...mocks[0].result, networkStatus: NetworkStatus.ready, error: undefined, }, { - data: { letters: data.slice(2, 4) }, - networkStatus: NetworkStatus.ready, - error: undefined, + data: mocks[1].result.data, + networkStatus: NetworkStatus.error, + error: expectedError, }, ]); }); - it('does not re-suspend when calling `fetchMore` with different variables while using an "initial" suspense policy', async () => { + it('re-suspends when calling `fetchMore` with different variables', async () => { const { data, query, link } = usePaginatedCase(); const { result, renders } = renderSuspenseHook( - () => - useSuspenseQuery(query, { - suspensePolicy: 'initial', - variables: { limit: 2 }, - }), + () => useSuspenseQuery(query, { variables: { limit: 2 } }), { link } ); @@ -4790,18 +4361,13 @@ describe('useSuspenseQuery', () => { }); expect(renders.count).toBe(4); - expect(renders.suspenseCount).toBe(1); + expect(renders.suspenseCount).toBe(2); expect(renders.frames).toMatchObject([ { data: { letters: data.slice(0, 2) }, 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, @@ -7014,6 +6580,287 @@ describe('useSuspenseQuery', () => { }); }); + it('`refetch` works with startTransition to allow React to show stale UI until finished suspending', 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: '1' } }, + result: { + data: { todo: { id: '1', name: 'Clean room', completed: true } }, + }, + delay: 10, + }, + ]; + + const client = new ApolloClient({ + link: new MockLink(mocks), + cache: new InMemoryCache(), + }); + + const suspenseCache = new SuspenseCache(); + + function App() { + return ( + + }> + + + + ); + } + + function SuspenseFallback() { + return

Loading

; + } + + function Todo({ id }: { id: string }) { + const { data, refetch } = 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('Clean room (completed)'); + }); + }); + + it('`fetchMore` works with startTransition to allow React to show stale UI until finished suspending', async () => { + type Variables = { + offset: number; + }; + + interface Todo { + __typename: 'Todo'; + id: string; + name: string; + completed: boolean; + } + + interface Data { + todos: Todo[]; + } + const user = userEvent.setup(); + + const query: TypedDocumentNode = gql` + query TodosQuery($offset: Int!) { + todos(offset: $offset) { + id + name + completed + } + } + `; + + const mocks: MockedResponse[] = [ + { + request: { query, variables: { offset: 0 } }, + result: { + data: { + todos: [ + { + __typename: 'Todo', + id: '1', + name: 'Clean room', + completed: false, + }, + ], + }, + }, + delay: 10, + }, + { + request: { query, variables: { offset: 1 } }, + result: { + data: { + todos: [ + { + __typename: 'Todo', + id: '2', + name: 'Take out trash', + completed: true, + }, + ], + }, + }, + delay: 10, + }, + ]; + + const client = new ApolloClient({ + link: new MockLink(mocks), + cache: new InMemoryCache({ + typePolicies: { + Query: { + fields: { + todos: offsetLimitPagination(), + }, + }, + }, + }), + }); + + const suspenseCache = new SuspenseCache(); + + function App() { + return ( + + }> + + + + ); + } + + function SuspenseFallback() { + return

Loading

; + } + + function Todos() { + const { data, fetchMore } = useSuspenseQuery(query, { + variables: { offset: 0 }, + }); + const [isPending, startTransition] = React.useTransition(); + const { todos } = data; + + return ( + <> + +
+ {todos.map((todo) => ( +
+ {todo.name} + {todo.completed && ' (completed)'} +
+ ))} +
+ + ); + } + + render(); + + expect(screen.getByText('Loading')).toBeInTheDocument(); + + expect(await screen.findByTestId('todos')).toBeInTheDocument(); + + const todos = screen.getByTestId('todos'); + const todo1 = screen.getByTestId('todo:1'); + const button = screen.getByText('Load more'); + + expect(todo1).toBeInTheDocument(); + + 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(todos).toHaveAttribute('aria-busy', 'true'); + + // Ensure we are showing the stale UI until the new todo has loaded + expect(todo1).toHaveTextContent('Clean room'); + + // Eventually we should see the updated todos content once its done + // suspending. + await waitFor(() => { + expect(screen.getByTestId('todo:2')).toHaveTextContent( + 'Take out trash (completed)' + ); + expect(todo1).toHaveTextContent('Clean room'); + }); + }); + describe.skip('type tests', () => { it('returns unknown when TData cannot be inferred', () => { const query = gql` diff --git a/src/react/hooks/useSuspenseQuery.ts b/src/react/hooks/useSuspenseQuery.ts index 99c1e4f85a6..0e19a6f6565 100644 --- a/src/react/hooks/useSuspenseQuery.ts +++ b/src/react/hooks/useSuspenseQuery.ts @@ -1,6 +1,5 @@ import { invariant, __DEV__ } from '../../utilities/globals'; -import { equal } from '@wry/equality'; -import { useRef, useCallback, useMemo } from 'react'; +import { useRef, useCallback, useMemo, useEffect, useState } from 'react'; import { ApolloClient, ApolloError, @@ -17,14 +16,12 @@ import { DeepPartial, isNonEmptyArray } from '../../utilities'; import { useApolloClient } from './useApolloClient'; import { DocumentType, verifyDocumentType } from '../parser'; import { - SuspenseQueryHookFetchPolicy, SuspenseQueryHookOptions, ObservableQueryFields, NoInfer, } from '../types/types'; import { useDeepMemo, useStrictModeSafeCleanupEffect, __use } from './internal'; import { useSuspenseCache } from './useSuspenseCache'; -import { useSyncExternalStore } from './useSyncExternalStore'; import { QuerySubscription } from '../cache/QuerySubscription'; import { canonicalStringify } from '../../cache'; @@ -63,6 +60,8 @@ type SubscribeToMoreFunction< TVariables extends OperationVariables > = ObservableQueryFields['subscribeToMore']; +type Version = 'main' | 'network'; + export function useSuspenseQuery_experimental< TData, TVariables extends OperationVariables, @@ -131,14 +130,13 @@ export function useSuspenseQuery_experimental< NoInfer > = Object.create(null) ): UseSuspenseQueryResult { - const didPreviouslySuspend = useRef(false); const client = useApolloClient(options.client); const suspenseCache = useSuspenseCache(options.suspenseCache); const watchQueryOptions = useWatchQueryOptions({ query, options }); - const { returnPartialData = false, variables } = watchQueryOptions; - const { suspensePolicy = 'always', queryKey = [] } = options; - const shouldSuspend = - suspensePolicy === 'always' || !didPreviouslySuspend.current; + const { variables } = watchQueryOptions; + const { queryKey = [] } = options; + + const [version, setVersion] = usePromiseVersion(); const cacheKey = ( [client, query, canonicalStringify(variables)] as any[] @@ -148,59 +146,32 @@ export function useSuspenseQuery_experimental< client.watchQuery(watchQueryOptions) ); - const dispose = useTrackedSubscriptions(subscription); - - useStrictModeSafeCleanupEffect(dispose); - - let result = useSyncExternalStore( - subscription.listen, - () => subscription.result, - () => subscription.result - ); - - 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, - }; - } - - previousVariables.current = variables; - previousData.current = result.data; - } + useTrackedSubscriptions(subscription); - if ( - result.networkStatus === NetworkStatus.error || - (shouldSuspend && - !shouldUseCachedResult(subscription.result, { - returnPartialData, - fetchPolicy: options.fetchPolicy, - })) - ) { - // Intentionally ignore the result returned from __use since we want to - // observe results from the observable instead of the the promise. - __use(subscription.promise); - } + useEffect(() => { + return subscription.listen(() => { + setVersion('main'); + }); + }, [subscription]); - didPreviouslySuspend.current = true; + const promise = subscription.promises[version] || subscription.promises.main; + const result = __use(promise); const fetchMore: FetchMoreFunction = useCallback( - (options) => subscription.fetchMore(options), + (options) => { + const promise = subscription.fetchMore(options); + setVersion('network'); + return promise; + }, [subscription] ); const refetch: RefetchFunction = useCallback( - (variables) => subscription.refetch(variables), + (variables) => { + const promise = subscription.refetch(variables); + setVersion('network'); + return promise; + }, [subscription] ); @@ -269,9 +240,24 @@ function useTrackedSubscriptions(subscription: QuerySubscription) { trackedSubscriptions.current.add(subscription); - return function dispose() { + useStrictModeSafeCleanupEffect(() => { trackedSubscriptions.current.forEach((sub) => sub.dispose()); - }; + }); +} + +function usePromiseVersion() { + // Use an object as state to force React to re-render when we publish an + // update to the same version (such as sequential cache updates). + const [{ version }, setState] = useState<{ version: Version }>({ + version: 'main', + }); + + const setVersion = useCallback( + (version: Version) => setState({ version }), + [] + ); + + return [version, setVersion] as const; } interface UseWatchQueryOptionsHookOptions< @@ -293,7 +279,7 @@ function useWatchQueryOptions({ () => ({ ...options, query, - notifyOnNetworkStatusChange: true, + notifyOnNetworkStatusChange: false, nextFetchPolicy: void 0, }), [options, query] @@ -305,34 +291,3 @@ function useWatchQueryOptions({ return watchQueryOptions; } - -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; - } - - 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 f083cf726f8..f7b5a4ce240 100644 --- a/src/react/types/types.ts +++ b/src/react/types/types.ts @@ -100,15 +100,6 @@ export interface LazyQueryHookExecOptions< query?: DocumentNode | TypedDocumentNode; } -/** - * suspensePolicy determines how suspense behaves for a refetch. The options are: - * - always (default): Re-suspend a component when a refetch occurs - * - initial: Only suspend on the first fetch - */ -export type SuspensePolicy = - | 'always' - | 'initial' - export type SuspenseQueryHookFetchPolicy = Extract< WatchQueryFetchPolicy, | 'cache-first' @@ -131,7 +122,6 @@ export interface SuspenseQueryHookOptions< | 'refetchWritePolicy' > { fetchPolicy?: SuspenseQueryHookFetchPolicy; - suspensePolicy?: SuspensePolicy; suspenseCache?: SuspenseCache; queryKey?: string | number | any[]; } diff --git a/src/utilities/index.ts b/src/utilities/index.ts index 0a2ae2e2a34..4d4bdfaaf30 100644 --- a/src/utilities/index.ts +++ b/src/utilities/index.ts @@ -83,6 +83,8 @@ export { export { isStatefulPromise, + createFulfilledPromise, + createRejectedPromise, wrapPromiseWithState, } from './promises/decoration'; diff --git a/src/utilities/promises/decoration.ts b/src/utilities/promises/decoration.ts index 1935626f579..1ace8efe2ee 100644 --- a/src/utilities/promises/decoration.ts +++ b/src/utilities/promises/decoration.ts @@ -17,6 +17,24 @@ export type PromiseWithState = | FulfilledPromise | RejectedPromise; +export function createFulfilledPromise(value: TValue) { + const promise = Promise.resolve(value) as FulfilledPromise; + + promise.status = 'fulfilled'; + promise.value = value; + + return promise; +} + +export function createRejectedPromise(reason: unknown) { + const promise = Promise.reject(reason) as RejectedPromise; + + promise.status = 'rejected'; + promise.reason = reason; + + return promise; +} + export function isStatefulPromise( promise: Promise ): promise is PromiseWithState {