diff --git a/.changeset/calm-bears-buy.md b/.changeset/calm-bears-buy.md new file mode 100644 index 00000000000..0db701bbc5b --- /dev/null +++ b/.changeset/calm-bears-buy.md @@ -0,0 +1,5 @@ +--- +'@apollo/client': patch +--- + +Fix an issue where a call to `refetch`, `fetchMore`, or changing `skip` to `false` that returned a result deeply equal to data in the cache would get stuck in a pending state and never resolve. diff --git a/.size-limit.cjs b/.size-limit.cjs index 9d4f872eeaa..c23c92cdc7c 100644 --- a/.size-limit.cjs +++ b/.size-limit.cjs @@ -1,7 +1,7 @@ const checks = [ { path: "dist/apollo-client.min.cjs", - limit: "37965" + limit: "37990" }, { path: "dist/main.cjs", diff --git a/src/react/cache/QueryReference.ts b/src/react/cache/QueryReference.ts index 09c2c252371..36b9a94a67a 100644 --- a/src/react/cache/QueryReference.ts +++ b/src/react/cache/QueryReference.ts @@ -157,8 +157,7 @@ export class InternalQueryReference { currentFetchPolicy === 'standby' && currentFetchPolicy !== watchQueryOptions.fetchPolicy ) { - this.observable.reobserve(watchQueryOptions); - this.initiateFetch(); + this.initiateFetch(this.observable.reobserve(watchQueryOptions)); } else { this.observable.silentSetOptions(watchQueryOptions); @@ -180,19 +179,11 @@ export class InternalQueryReference { } refetch(variables: OperationVariables | undefined) { - const promise = this.observable.refetch(variables); - - this.initiateFetch(); - - return promise; + return this.initiateFetch(this.observable.refetch(variables)); } fetchMore(options: FetchMoreOptions) { - const promise = this.observable.fetchMore(options); - - this.initiateFetch(); - - return promise; + return this.initiateFetch(this.observable.fetchMore(options)); } private dispose() { @@ -254,7 +245,7 @@ export class InternalQueryReference { this.listeners.forEach((listener) => listener(promise)); } - private initiateFetch() { + private initiateFetch(returnedPromise: Promise>) { this.status = 'loading'; this.promise = new Promise((resolve, reject) => { @@ -263,5 +254,22 @@ export class InternalQueryReference { }); this.promise.catch(() => {}); + + // If the data returned from the fetch is deeply equal to the data already + // in the cache, `handleNext` will not be triggered leaving the promise we + // created in a pending state forever. To avoid this situtation, we attempt + // to resolve the promise if `handleNext` hasn't been run to ensure the + // promise is resolved correctly. + returnedPromise + .then((result) => { + if (this.status === 'loading') { + this.status = 'idle'; + this.result = result; + this.resolve?.(result); + } + }) + .catch(() => {}); + + return returnedPromise; } } diff --git a/src/react/hooks/__tests__/useSuspenseQuery.test.tsx b/src/react/hooks/__tests__/useSuspenseQuery.test.tsx index a1d2c5d8b28..d06eda39599 100644 --- a/src/react/hooks/__tests__/useSuspenseQuery.test.tsx +++ b/src/react/hooks/__tests__/useSuspenseQuery.test.tsx @@ -4161,6 +4161,103 @@ describe('useSuspenseQuery', () => { ]); }); + it('properly resolves `refetch` when returning a result that is deeply equal to data in the cache', 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: false } }, + }, + delay: 10, + }, + ]; + + const client = new ApolloClient({ + link: new MockLink(mocks), + cache: new InMemoryCache(), + }); + + function App() { + return ( + + }> + + + + ); + } + + function SuspenseFallback() { + return

Loading

; + } + + function Todo({ id }: { id: string }) { + const { data, refetch } = useSuspenseQuery(query, { + variables: { id }, + }); + + const { todo } = data; + + return ( +
+ +
+ {todo.name} + {todo.completed && ' (completed)'} +
+
+ ); + } + + render(); + + expect(await screen.findByText('Loading')).toBeInTheDocument(); + + const todo = await screen.findByTestId('todo'); + + expect(todo).toHaveTextContent('Clean room'); + + await act(() => user.click(screen.getByText('Refetch'))); + + expect(screen.getByText('Loading')).toBeInTheDocument(); + + await waitFor(() => { + // Suspense will hide the component until the suspense boundary has + // finished loading so it is still in the DOM. + expect(todo).toBeVisible(); + }); + + expect(todo).toHaveTextContent('Clean room'); + }); + it('re-suspends when calling `refetch` with new variables', async () => { const query = gql` query UserQuery($id: String!) { @@ -4632,6 +4729,68 @@ describe('useSuspenseQuery', () => { ]); }); + it('properly resolves `fetchMore` when returning a result that is deeply equal to data in the cache', async () => { + const { query, link } = usePaginatedCase(); + + const user = userEvent.setup(); + + const client = new ApolloClient({ + link, + cache: new InMemoryCache(), + }); + + function App() { + return ( + + }> + + + + ); + } + + function SuspenseFallback() { + return

Loading

; + } + + function Letters({ offset }: { offset: number }) { + const { data, fetchMore } = useSuspenseQuery(query, { + variables: { offset }, + }); + + return ( +
+ +
+ {data.letters.map(({ letter }) => letter).join('')} +
+
+ ); + } + + render(); + + expect(await screen.findByText('Loading')).toBeInTheDocument(); + + const letters = await screen.findByTestId('letters'); + + expect(letters).toHaveTextContent('AB'); + + await act(() => user.click(screen.getByText('Fetch more'))); + + expect(screen.getByText('Loading')).toBeInTheDocument(); + + await waitFor(() => { + // Suspense will hide the component until the suspense boundary has + // finished loading so it is still in the DOM. + expect(letters).toBeVisible(); + }); + + expect(letters).toHaveTextContent('AB'); + }); + it('suspends when refetching after returning cached data for the initial fetch', async () => { const { query, mocks } = useSimpleQueryCase(); @@ -5183,6 +5342,111 @@ describe('useSuspenseQuery', () => { expect(fetchedSkipResult).toBe(fetchedSkipResult); }); + it('properly resolves when `skip` becomes false when returning a result that is deeply equal to data in the cache', 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: false } }, + }, + delay: 10, + }, + ]; + + const client = new ApolloClient({ + link: new MockLink(mocks), + cache: new InMemoryCache(), + }); + + function App() { + return ( + + }> + + + + ); + } + + function SuspenseFallback() { + return

Loading

; + } + + function Todo({ id }: { id: string }) { + const [skip, setSkip] = React.useState(false); + const { data } = useSuspenseQuery(query, { + // Force a network request that returns the same data from the cache + fetchPolicy: 'network-only', + skip, + variables: { id }, + }); + + const todo = data?.todo; + + return ( + <> + + {todo && ( +
+ {todo.name} + {todo.completed && ' (completed)'} +
+ )} + + ); + } + + render(); + + expect(screen.getByText('Loading')).toBeInTheDocument(); + + const todo = await screen.findByTestId('todo'); + expect(todo).toHaveTextContent('Clean room'); + + // skip false -> true + await act(() => user.click(screen.getByText('Toggle skip'))); + expect(todo).toHaveTextContent('Clean room'); + + // skip true -> false + await act(() => user.click(screen.getByText('Toggle skip'))); + + expect(screen.getByText('Loading')).toBeInTheDocument(); + + await waitFor(() => { + expect(todo).toBeVisible(); + }); + + expect(todo).toHaveTextContent('Clean room'); + }); + it('`skip` option works with `startTransition`', async () => { type Variables = { id: string;