Skip to content

Commit

Permalink
Merge pull request #8718 from apollographql/brian-network-only
Browse files Browse the repository at this point in the history
Fix `ObservableQuery.getCurrentResult()` returning cached data with certain fetch policies.
  • Loading branch information
brainkim committed Aug 27, 2021
2 parents f7bda84 + 4d75007 commit f006376
Show file tree
Hide file tree
Showing 4 changed files with 62 additions and 8 deletions.
6 changes: 6 additions & 0 deletions CHANGELOG.md
Expand Up @@ -5,6 +5,12 @@
- Warn when calling `refetch({ variables })` instead of `refetch(variables)`, except for queries that declare a variable named `$variables` (uncommon). <br/>
[@benjamn](https://github.com/benjamn) in [#8702](https://github.com/apollographql/apollo-client/pull/8702)

### Bug Fixes

- Fix ObservableQuery.getCurrentResult() returning cached data with certain fetch policies. <br/>
[@brainkim](https://github.com/brainkim) in [#8718](https://github.com/apollographql/apollo-client/pull/8718)


## Apollo Client 3.4.9

### Bug Fixes
Expand Down
26 changes: 20 additions & 6 deletions src/core/ObservableQuery.ts
Expand Up @@ -207,21 +207,35 @@ export class ObservableQuery<
networkStatus,
} as ApolloQueryResult<TData>;

// If this.options.query has @client(always: true) fields, we cannot trust
// diff.result, since it was read from the cache without running local
// resolvers (and it's too late to run resolvers now, since we must return a
// result synchronously).
if (!this.queryManager.transform(this.options.query).hasForcedResolvers) {
const { fetchPolicy = "cache-first" } = this.options;
// The presence of lastResult means a result has been received and
// this.options.variables haven't changed since then, so its absence means
// either there hasn't been a result yet (so these policies definitely
// should skip the cache) or there's been a result but it was for different
// variables (again, skipping the cache seems right).
const shouldReturnCachedData = lastResult || (
fetchPolicy !== 'network-only' &&
fetchPolicy !== 'no-cache' &&
fetchPolicy !== 'standby'
);
if (
shouldReturnCachedData &&
// If this.options.query has @client(always: true) fields, we cannot
// trust diff.result, since it was read from the cache without running
// local resolvers (and it's too late to run resolvers now, since we must
// return a result synchronously).
!this.queryManager.transform(this.options.query).hasForcedResolvers
) {
const diff = this.queryInfo.getDiff();

if (diff.complete || this.options.returnPartialData) {
result.data = diff.result;
}

if (equal(result.data, {})) {
result.data = void 0 as any;
}

const { fetchPolicy = "cache-first" } = this.options;
if (diff.complete) {
// If the diff is complete, and we're using a FetchPolicy that
// terminates after a complete cache read, we can assume the next
Expand Down
4 changes: 2 additions & 2 deletions src/core/__tests__/ObservableQuery.ts
Expand Up @@ -2027,7 +2027,7 @@ describe('ObservableQuery', () => {
});

expect(observable.getCurrentResult()).toEqual({
data: dataOne,
data: undefined,
loading: true,
networkStatus: NetworkStatus.loading,
});
Expand All @@ -2036,7 +2036,7 @@ describe('ObservableQuery', () => {
if (handleCount === 1) {
expect(subResult).toEqual({
loading: true,
data: dataOne,
data: undefined,
networkStatus: NetworkStatus.loading,
});
} else if (handleCount === 2) {
Expand Down
34 changes: 34 additions & 0 deletions src/react/hooks/__tests__/useQuery.test.tsx
Expand Up @@ -523,6 +523,40 @@ describe('useQuery Hook', () => {
expect(result.current.loading).toBe(false);
expect(result.current.data).toEqual(mocks[1].result.data);
});

it('should not use the cache when using `network-only`', async () => {
const query = gql`{ hello }`;
const mocks = [
{
request: { query },
result: { data: { hello: 'from link' } },
},
];

const cache = new InMemoryCache();
cache.writeQuery({
query,
data: { hello: 'from cache' },
});

const { result, waitForNextUpdate } = renderHook(
() => useQuery(query, { fetchPolicy: 'network-only' }),
{
wrapper: ({ children }) => (
<MockedProvider mocks={mocks} cache={cache}>
{children}
</MockedProvider>
),
},
);

expect(result.current.loading).toBe(true);
expect(result.current.data).toBe(undefined);

await waitForNextUpdate();
expect(result.current.loading).toBe(false);
expect(result.current.data).toEqual({ hello: 'from link' });
});
});

describe('polling', () => {
Expand Down

0 comments on commit f006376

Please sign in to comment.