Skip to content

Commit

Permalink
Fix an issue where useSuspenseQuery would not respond to cache upda…
Browse files Browse the repository at this point in the history
…tes when using a cache-first fetch policy with data in the cache when the hook is mounted (#10651)

This fixes an issue where `useSuspenseQuery` would not respond to cache updates
when using a cache-first `fetchPolicy` after the hook was mounted with data
already in the cache.

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 in `reobserve` by
marking the queryInfo as ready if we detect that the result is also
ready.

Co-authored-by: Alessia Bellisario <alessia@apollographql.com>
  • Loading branch information
jerelmiller and alessbell committed Mar 15, 2023
1 parent 3a3a8bc commit 8355d0e
Show file tree
Hide file tree
Showing 3 changed files with 84 additions and 0 deletions.
5 changes: 5 additions & 0 deletions .changeset/funny-peas-change.md
@@ -0,0 +1,5 @@
---
'@apollo/client': patch
---

Fixes an issue where `useSuspenseQuery` would not respond to cache updates when using a cache-first `fetchPolicy` after the hook was mounted with data already in the cache.
41 changes: 41 additions & 0 deletions src/react/hooks/__tests__/useSuspenseQuery.test.tsx
Expand Up @@ -1810,6 +1810,47 @@ describe('useSuspenseQuery', () => {
}
);

// https://github.com/apollographql/apollo-client/issues/10478
it('responds to cache updates when data is already in the cache while using a cache-first fetch policy', async () => {
const { query, mocks } = useSimpleQueryCase();

const client = new ApolloClient({
cache: new InMemoryCache(),
link: new MockLink(mocks),
});

client.writeQuery({
query,
data: { greeting: 'Hello from cache' },
});

const { result } = renderSuspenseHook(
() => useSuspenseQuery(query, { fetchPolicy: 'cache-first' }),
{ client }
);

expect(result.current.data).toEqual({ greeting: 'Hello from cache' });

// Allow time for the subscription in the hook to set itself up since it is
// wrapped in a setTimeout (to handle Strict mode bugs). Without this
// `wait`, `subscribe` isn't called until after our test updates the cache
// via `writeQuery`, which then emits the most recent result, which is the
// updated value.
await wait(0);

client.writeQuery({
query,
data: { greeting: 'Updated hello' },
});

await waitFor(() => {
expect(result.current).toMatchObject({
data: { greeting: 'Updated hello' },
error: undefined,
});
});
});

it('uses the default fetch policy from the client when none provided in options', async () => {
const { query, mocks } = useSimpleQueryCase();

Expand Down
38 changes: 38 additions & 0 deletions src/react/hooks/useSuspenseQuery.ts
Expand Up @@ -11,6 +11,7 @@ import {
TypedDocumentNode,
WatchQueryOptions,
WatchQueryFetchPolicy,
NetworkStatus,
} from '../../core';
import {
compact,
Expand Down Expand Up @@ -373,6 +374,43 @@ function useObservableQueryResult<TData>(observable: ObservableQuery<TData>) {
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 () => {
Expand Down

0 comments on commit 8355d0e

Please sign in to comment.