From 40b79d720a6b1da6c407c2f83aea2d3f20b61af3 Mon Sep 17 00:00:00 2001 From: Phil Pluckthun Date: Wed, 19 Apr 2023 11:44:08 +0100 Subject: [PATCH 1/5] fix(graphcache): Mark deferred, uncached results as partial --- .changeset/rich-taxis-end.md | 5 ++++ exchanges/graphcache/src/cacheExchange.ts | 25 +++++++++---------- exchanges/graphcache/src/operations/query.ts | 6 ++++- exchanges/graphcache/src/operations/shared.ts | 2 ++ 4 files changed, 24 insertions(+), 14 deletions(-) create mode 100644 .changeset/rich-taxis-end.md diff --git a/.changeset/rich-taxis-end.md b/.changeset/rich-taxis-end.md new file mode 100644 index 0000000000..c070c731c8 --- /dev/null +++ b/.changeset/rich-taxis-end.md @@ -0,0 +1,5 @@ +--- +'@urql/exchange-graphcache': patch +--- + +Apply `hasNext: true` and fallthrough logic to cached queries that contain deferred, uncached fields. Deferred query results will now be fetched against the API correctly, even if prior requests have been incomplete. diff --git a/exchanges/graphcache/src/cacheExchange.ts b/exchanges/graphcache/src/cacheExchange.ts index 73d99cdb69..17e74a6cc9 100644 --- a/exchanges/graphcache/src/cacheExchange.ts +++ b/exchanges/graphcache/src/cacheExchange.ts @@ -30,6 +30,7 @@ interface OperationResultWithMeta extends Partial { operation: Operation; outcome: CacheOutcome; dependencies: Dependencies; + hasNext: boolean; } type Operations = Set; @@ -197,7 +198,7 @@ export const cacheExchange = ): OperationResultWithMeta => { const result = query(store, operation, results.get(operation.key)); const cacheOutcome: CacheOutcome = result.data - ? !result.partial + ? !result.partial && !result.hasNext ? 'hit' : 'partial' : 'miss'; @@ -211,6 +212,7 @@ export const cacheExchange = operation, data: result.data, dependencies: result.dependencies, + hasNext: result.hasNext, }; }; @@ -346,20 +348,17 @@ export const cacheExchange = ), map((res: OperationResultWithMeta): OperationResult => { const { requestPolicy } = res.operation.context; - - // We don't mark cache-only responses as partial, as this would indicate - // that we expect a new result to come from the network, which cannot - // happen - const isPartial = - res.outcome === 'partial' && requestPolicy !== 'cache-only'; + const isPartial = res.outcome === 'partial'; // We reexecute requests marked as `cache-and-network`, and partial responses, // if we wouldn't cause a request loop const shouldReexecute = - requestPolicy === 'cache-and-network' || - (requestPolicy === 'cache-first' && - isPartial && - !reexecutingOperations.has(res.operation.key)); + requestPolicy !== 'cache-only' && + (res.hasNext || + requestPolicy === 'cache-and-network' || + (requestPolicy === 'cache-first' && + res.outcome === 'partial' && + !reexecutingOperations.has(res.operation.key))); const result: OperationResult = { operation: addMetadata(res.operation, { @@ -368,8 +367,8 @@ export const cacheExchange = data: res.data, error: res.error, extensions: res.extensions, - stale: shouldReexecute || isPartial, - hasNext: false, + stale: shouldReexecute && isPartial, + hasNext: shouldReexecute && res.hasNext, }; if (!shouldReexecute) { diff --git a/exchanges/graphcache/src/operations/query.ts b/exchanges/graphcache/src/operations/query.ts index 4914c907a4..255f614bc8 100644 --- a/exchanges/graphcache/src/operations/query.ts +++ b/exchanges/graphcache/src/operations/query.ts @@ -61,6 +61,7 @@ import { export interface QueryResult { dependencies: Dependencies; partial: boolean; + hasNext: boolean; data: null | Data; } @@ -121,6 +122,7 @@ export const read = ( return { dependencies: getCurrentDependencies(), partial: ctx.partial || !data, + hasNext: ctx.hasNext, data: data || null, }; }; @@ -334,6 +336,7 @@ const readSelection = ( let hasFields = false; let hasPartials = false; + let hasNext = false; let hasChanged = typename !== input.__typename; let node: FieldNode | void; const output = makeData(input); @@ -455,7 +458,7 @@ const readSelection = ( // a partial query result if (dataFieldValue === undefined && deferRef.current) { // The field is undelivered and uncached, but is included in a deferred fragment - hasFields = true; + hasNext = true; } else if ( dataFieldValue === undefined && ((store.schema && isFieldNullable(store.schema, typename, fieldName)) || @@ -481,6 +484,7 @@ const readSelection = ( } ctx.partial = ctx.partial || hasPartials; + ctx.hasNext = ctx.hasNext || hasNext; return isQuery && hasPartials && !hasFields ? undefined : hasChanged diff --git a/exchanges/graphcache/src/operations/shared.ts b/exchanges/graphcache/src/operations/shared.ts index aabb6f28f3..cd743d3774 100644 --- a/exchanges/graphcache/src/operations/shared.ts +++ b/exchanges/graphcache/src/operations/shared.ts @@ -43,6 +43,7 @@ export interface Context { fieldName: string; error: ErrorLike | undefined; partial: boolean; + hasNext: boolean; optimistic: boolean; __internal: { path: Array; @@ -79,6 +80,7 @@ export const makeContext = ( fieldName: '', error: undefined, partial: false, + hasNext: false, optimistic: !!optimistic, __internal: { path: [], From 101f319cc5d834551a5e7c597e60c385d1275153 Mon Sep 17 00:00:00 2001 From: Phil Pluckthun Date: Wed, 19 Apr 2023 11:49:24 +0100 Subject: [PATCH 2/5] Update deferred tests to reflect defer hasNext flag --- .../graphcache/src/cacheExchange.test.ts | 27 ++++++++++--------- 1 file changed, 15 insertions(+), 12 deletions(-) diff --git a/exchanges/graphcache/src/cacheExchange.test.ts b/exchanges/graphcache/src/cacheExchange.test.ts index 6f52a784b2..63e9320e71 100644 --- a/exchanges/graphcache/src/cacheExchange.test.ts +++ b/exchanges/graphcache/src/cacheExchange.test.ts @@ -2411,9 +2411,9 @@ describe('commutativity', () => { }); it('applies deferred results to previous layers', () => { - let normalData: any; - let deferredData: any; - let combinedData: any; + let normalData: OperationResult | undefined; + let deferredData: OperationResult | undefined; + let combinedData: OperationResult | undefined; const client = createClient({ url: 'http://0.0.0.0', @@ -2475,11 +2475,11 @@ describe('commutativity', () => { tap(result => { if (result.operation.kind === 'query') { if (result.operation.key === 1) { - deferredData = result.data; + deferredData = result; } else if (result.operation.key === 42) { - combinedData = result.data; + combinedData = result; } else { - normalData = result.data; + normalData = result; } } }), @@ -2530,9 +2530,9 @@ describe('commutativity', () => { }, }); - expect(normalData).toHaveProperty('node.id', 2); - expect(combinedData).not.toHaveProperty('deferred'); - expect(combinedData).toHaveProperty('node.id', 2); + expect(normalData).toHaveProperty('data.node.id', 2); + expect(combinedData).not.toHaveProperty('data.deferred'); + expect(combinedData).toHaveProperty('data.node.id', 2); nextRes({ ...queryResponse, @@ -2548,8 +2548,11 @@ describe('commutativity', () => { hasNext: true, }); - expect(deferredData).toHaveProperty('deferred.id', 1); - expect(combinedData).toHaveProperty('deferred.id', 1); - expect(combinedData).toHaveProperty('node.id', 2); + expect(deferredData).toHaveProperty('hasNext', true); + expect(deferredData).toHaveProperty('data.deferred.id', 1); + + expect(combinedData).toHaveProperty('hasNext', false); + expect(combinedData).toHaveProperty('data.deferred.id', 1); + expect(combinedData).toHaveProperty('data.node.id', 2); }); }); From 398b4553307284533915773572faa146d079c685 Mon Sep 17 00:00:00 2001 From: Phil Pluckthun Date: Wed, 19 Apr 2023 11:50:52 +0100 Subject: [PATCH 3/5] Clean up redundant variable --- exchanges/graphcache/src/cacheExchange.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/exchanges/graphcache/src/cacheExchange.ts b/exchanges/graphcache/src/cacheExchange.ts index 17e74a6cc9..6879c7ec7d 100644 --- a/exchanges/graphcache/src/cacheExchange.ts +++ b/exchanges/graphcache/src/cacheExchange.ts @@ -348,7 +348,6 @@ export const cacheExchange = ), map((res: OperationResultWithMeta): OperationResult => { const { requestPolicy } = res.operation.context; - const isPartial = res.outcome === 'partial'; // We reexecute requests marked as `cache-and-network`, and partial responses, // if we wouldn't cause a request loop @@ -367,7 +366,7 @@ export const cacheExchange = data: res.data, error: res.error, extensions: res.extensions, - stale: shouldReexecute && isPartial, + stale: shouldReexecute && res.outcome === 'partial', hasNext: shouldReexecute && res.hasNext, }; From 757c1e656da5f9c0c1c69b2cd6ddba0931e03450 Mon Sep 17 00:00:00 2001 From: Phil Pluckthun Date: Wed, 19 Apr 2023 12:23:39 +0100 Subject: [PATCH 4/5] Fix conditional typo in stale flag --- exchanges/graphcache/src/cacheExchange.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/exchanges/graphcache/src/cacheExchange.ts b/exchanges/graphcache/src/cacheExchange.ts index 6879c7ec7d..2d26d7a6c2 100644 --- a/exchanges/graphcache/src/cacheExchange.ts +++ b/exchanges/graphcache/src/cacheExchange.ts @@ -366,7 +366,7 @@ export const cacheExchange = data: res.data, error: res.error, extensions: res.extensions, - stale: shouldReexecute && res.outcome === 'partial', + stale: shouldReexecute && !res.hasNext, hasNext: shouldReexecute && res.hasNext, }; From 432709c4d0f0bd42dfcd5bbe22b17a7c3b8882c2 Mon Sep 17 00:00:00 2001 From: Phil Pluckthun Date: Wed, 19 Apr 2023 15:06:16 +0100 Subject: [PATCH 5/5] Update E2E test to reflect that looping requests now aren't marked as stale --- exchanges/graphcache/e2e-tests/query.spec.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/exchanges/graphcache/e2e-tests/query.spec.tsx b/exchanges/graphcache/e2e-tests/query.spec.tsx index 8609a44285..747b98cf9c 100644 --- a/exchanges/graphcache/e2e-tests/query.spec.tsx +++ b/exchanges/graphcache/e2e-tests/query.spec.tsx @@ -191,7 +191,7 @@ describe('Graphcache Queries', () => { cy.get('#first-data').should('have.text', 'Data: title'); cy.get('#second-data').should('have.text', 'Data: foo'); - cy.get('#second-stale').should('have.text', 'Stale: true'); + cy.get('#second-stale').should('have.text', 'Stale: false'); // TODO: ideally we would be able to keep the error here but... // cy.get('#first-error').should('have.text', 'Error: [GraphQL] Test'); // cy.get('#second-error').should('have.text', 'Error: [GraphQL] Test');