Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(graphcache): Allow partial optimistic results #3264

Merged
merged 4 commits into from
Jun 12, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/five-icons-agree.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@urql/exchange-graphcache': patch
---

Make "Invalid undefined" warning heuristic smarter and allow for partial optimistic results. Previously, when a partial optimistic result would be passed, a warning would be issued, and in production, fields would be deleted from the cache. Instead, we now only issue a warning if these fields aren't cached already.
2 changes: 0 additions & 2 deletions exchanges/graphcache/src/operations/query.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -164,8 +164,6 @@ describe('Query', () => {
todos: [{ __typename: 'Todo', id: '0', text: 'Solve bug' }],
});

// The warning should be called for `__typename`
expect(console.warn).toHaveBeenCalledTimes(1);
expect(console.error).not.toHaveBeenCalled();
});

Expand Down
8 changes: 5 additions & 3 deletions exchanges/graphcache/src/operations/write.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -162,15 +162,17 @@ describe('Query', () => {
}
`;

write(store, { query }, { field: 'test' } as any);
// This should not overwrite the field
write(store, { query }, { field: undefined } as any);
// Because of us writing an undefined field
expect(console.warn).toHaveBeenCalledTimes(2);
expect((console.warn as any).mock.calls[0][0]).toMatch(
/The field `field` does not exist on `Query`/

expect((console.warn as any).mock.calls[1][0]).toMatch(
/Invalid undefined: The field at `field`/
);

write(store, { query }, { field: 'test' } as any);
write(store, { query }, { field: undefined } as any);
InMemoryData.initDataState('read', store.data, null);
// The field must still be `'test'`
expect(InMemoryData.readRecord('Query', 'field')).toBe('test');
Expand Down
83 changes: 50 additions & 33 deletions exchanges/graphcache/src/operations/write.ts
Original file line number Diff line number Diff line change
Expand Up @@ -210,7 +210,13 @@ const writeSelection = (
const rootField = ctx.store.rootNames[entityKey!] || 'query';
const isRoot = !!ctx.store.rootNames[entityKey!];

const typename = isRoot ? entityKey : data.__typename;
let typename = isRoot ? entityKey : data.__typename;
if (!typename && entityKey && ctx.optimistic) {
typename = InMemoryData.readRecord(entityKey, '__typename') as
| string
| undefined;
}

if (!typename) {
warn(
"Couldn't find __typename when writing.\n" +
Expand Down Expand Up @@ -239,34 +245,6 @@ const writeSelection = (
const fieldAlias = getFieldAlias(node);
let fieldValue = data[ctx.optimistic ? fieldName : fieldAlias];

// Development check of undefined fields
if (process.env.NODE_ENV !== 'production') {
if (
rootField === 'query' &&
fieldValue === undefined &&
!deferRef &&
!ctx.optimistic
) {
const expected =
node.selectionSet === undefined
? 'scalar (number, boolean, etc)'
: 'selection set';

warn(
'Invalid undefined: The field at `' +
fieldKey +
'` is `undefined`, but the GraphQL query expects a ' +
expected +
' for this field.',
13
);

continue; // Skip this field
} else if (ctx.store.schema && typename && fieldName !== '__typename') {
isFieldAvailableOnType(ctx.store.schema, typename, fieldName);
}
}

if (
// Skip typename fields and assume they've already been written above
fieldName === '__typename' ||
Expand All @@ -278,6 +256,12 @@ const writeSelection = (
continue;
}

if (process.env.NODE_ENV !== 'production') {
if (ctx.store.schema && typename && fieldName !== '__typename') {
isFieldAvailableOnType(ctx.store.schema, typename, fieldName);
}
}

// Add the current alias to the walked path before processing the field's value
ctx.__internal.path.push(fieldAlias);

Expand All @@ -298,6 +282,32 @@ const writeSelection = (
fieldValue = ensureData(resolver(fieldArgs || {}, ctx.store, ctx));
}

if (fieldValue === undefined) {
if (process.env.NODE_ENV !== 'production') {
if (
!entityKey ||
!InMemoryData.hasField(entityKey, fieldKey) ||
(ctx.optimistic && !InMemoryData.readRecord(entityKey, '__typename'))
) {
const expected =
node.selectionSet === undefined
? 'scalar (number, boolean, etc)'
: 'selection set';

warn(
'Invalid undefined: The field at `' +
fieldKey +
'` is `undefined`, but the GraphQL query expects a ' +
expected +
' for this field.',
13
);
}
}

continue; // Skip this field
}

if (node.selectionSet) {
// Process the field and write links for the child entities that have been written
if (entityKey && rootField === 'query') {
Expand All @@ -306,7 +316,10 @@ const writeSelection = (
ctx,
getSelectionSet(node),
ensureData(fieldValue),
key
key,
ctx.optimistic
? InMemoryData.readLink(entityKey || typename, fieldKey)
: undefined
);
InMemoryData.writeLink(entityKey || typename, fieldKey, link);
} else {
Expand Down Expand Up @@ -353,7 +366,8 @@ const writeField = (
ctx: Context,
select: SelectionSet,
data: null | Data | NullArray<Data>,
parentFieldKey?: string
parentFieldKey?: string,
prevLink?: Link
): Link | undefined => {
if (Array.isArray(data)) {
const newData = new Array(data.length);
Expand All @@ -365,7 +379,8 @@ const writeField = (
? joinKeys(parentFieldKey, `${i}`)
: undefined;
// Recursively write array data
const links = writeField(ctx, select, data[i], indexKey);
const prevIndex = prevLink != null ? prevLink[i] : undefined;
const links = writeField(ctx, select, data[i], indexKey, prevIndex);
// Link cannot be expressed as a recursive type
newData[i] = links as string | null;
// After processing the field, remove the current index from the path
Expand All @@ -377,7 +392,9 @@ const writeField = (
return getFieldError(ctx) ? undefined : null;
}

const entityKey = ctx.store.keyOfEntity(data);
const entityKey =
ctx.store.keyOfEntity(data) ||
(typeof prevLink === 'string' ? prevLink : null);
const typename = data.__typename;

if (
Expand Down