Skip to content

Commit

Permalink
fix(graphcache): Mark deferred, uncached results as partial (#3163)
Browse files Browse the repository at this point in the history
  • Loading branch information
kitten authored Apr 20, 2023
1 parent 8e14849 commit b5e50b4
Show file tree
Hide file tree
Showing 6 changed files with 39 additions and 27 deletions.
5 changes: 5 additions & 0 deletions .changeset/rich-taxis-end.md
Original file line number Diff line number Diff line change
@@ -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.
2 changes: 1 addition & 1 deletion exchanges/graphcache/e2e-tests/query.spec.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Expand Down
27 changes: 15 additions & 12 deletions exchanges/graphcache/src/cacheExchange.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down Expand Up @@ -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;
}
}
}),
Expand Down Expand Up @@ -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,
Expand All @@ -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);
});
});
24 changes: 11 additions & 13 deletions exchanges/graphcache/src/cacheExchange.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ interface OperationResultWithMeta extends Partial<OperationResult> {
operation: Operation;
outcome: CacheOutcome;
dependencies: Dependencies;
hasNext: boolean;
}

type Operations = Set<number>;
Expand Down Expand Up @@ -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';
Expand All @@ -211,6 +212,7 @@ export const cacheExchange =
operation,
data: result.data,
dependencies: result.dependencies,
hasNext: result.hasNext,
};
};

Expand Down Expand Up @@ -347,19 +349,15 @@ 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';

// 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, {
Expand All @@ -368,8 +366,8 @@ export const cacheExchange =
data: res.data,
error: res.error,
extensions: res.extensions,
stale: shouldReexecute || isPartial,
hasNext: false,
stale: shouldReexecute && !res.hasNext,
hasNext: shouldReexecute && res.hasNext,
};

if (!shouldReexecute) {
Expand Down
6 changes: 5 additions & 1 deletion exchanges/graphcache/src/operations/query.ts
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@ import {
export interface QueryResult {
dependencies: Dependencies;
partial: boolean;
hasNext: boolean;
data: null | Data;
}

Expand Down Expand Up @@ -121,6 +122,7 @@ export const read = (
return {
dependencies: getCurrentDependencies(),
partial: ctx.partial || !data,
hasNext: ctx.hasNext,
data: data || null,
};
};
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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)) ||
Expand All @@ -481,6 +484,7 @@ const readSelection = (
}

ctx.partial = ctx.partial || hasPartials;
ctx.hasNext = ctx.hasNext || hasNext;
return isQuery && hasPartials && !hasFields
? undefined
: hasChanged
Expand Down
2 changes: 2 additions & 0 deletions exchanges/graphcache/src/operations/shared.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ export interface Context {
fieldName: string;
error: ErrorLike | undefined;
partial: boolean;
hasNext: boolean;
optimistic: boolean;
__internal: {
path: Array<string | number>;
Expand Down Expand Up @@ -79,6 +80,7 @@ export const makeContext = (
fieldName: '',
error: undefined,
partial: false,
hasNext: false,
optimistic: !!optimistic,
__internal: {
path: [],
Expand Down

0 comments on commit b5e50b4

Please sign in to comment.