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

(graphache) - Fix query traversal for previous null fields #772

Merged
merged 6 commits into from Apr 29, 2020
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/slimy-chairs-sniff.md
@@ -0,0 +1,5 @@
---
'@urql/exchange-graphcache': patch
---

Fix traversal issue, where when a prior selection set has set a nested result field to `null`, a subsequent traversal of this field attempts to access `prevData` on `null`.
70 changes: 69 additions & 1 deletion exchanges/graphcache/src/operations/query.test.ts
Expand Up @@ -42,7 +42,7 @@ describe('Query', () => {
}
);

jest.resetAllMocks();
// jest.resetAllMocks();
});

it('test partial results', () => {
Expand Down Expand Up @@ -187,4 +187,72 @@ describe('Query', () => {
todos: [{ __typename: 'Todo', id: '0', text: 'Solve bug' }],
});
});

it.only('should allow subsequent read when first result was null', () => {
JoviDeCroock marked this conversation as resolved.
Show resolved Hide resolved
const QUERY_WRITE = gql`
query writeTodos {
todos {
...ValidRead
}
}

fragment ValidRead on Todo {
id
}
`;

const QUERY_READ = gql`
query getTodos {
todos {
...MissingRead
}
todos {
id
}
}

fragment ValidRead on Todo {
id
}

fragment MissingRead on Todo {
id
text
}
`;

const store = new Store({
schema: alteredRoot,
});

let { data } = query(store, { query: QUERY_READ });
expect(data).toEqual(null);

write(
store,
{ query: QUERY_WRITE },
{
todos: [
{
__typename: 'Todo',
id: '0',
},
],
__typename: 'Query',
}
);

({ data } = query(store, { query: QUERY_READ }));
expect(data).toEqual({
__typename: 'query_root',
todos: [
{
__typename: 'Todo',
id: '0',
// TODO: By the spec this should actually be `null`
// text: null,
},
],
});
});
});
17 changes: 6 additions & 11 deletions exchanges/graphcache/src/operations/query.ts
Expand Up @@ -391,7 +391,7 @@ const resolveResolverResult = (
fieldName: string,
key: string,
select: SelectionSet,
prevData: void | Data | Data[],
prevData: void | null | Data | Data[],
result: void | DataField
): DataField | void => {
if (Array.isArray(result)) {
Expand All @@ -410,7 +410,7 @@ const resolveResolverResult = (
joinKeys(key, `${i}`),
select,
// Get the inner previous data from prevData
prevData !== undefined ? prevData[i] : undefined,
prevData ? prevData[i] : undefined,
kitten marked this conversation as resolved.
Show resolved Hide resolved
result[i]
);

Expand All @@ -425,7 +425,7 @@ const resolveResolverResult = (
} else if (result === null || result === undefined) {
return result;
} else if (isDataOrKey(result)) {
const data = (prevData === undefined ? {} : prevData) as Data;
const data = (prevData || {}) as Data;
return typeof result === 'string'
? readSelection(ctx, result, select, data)
: readSelection(ctx, key, select, data, result);
Expand All @@ -448,7 +448,7 @@ const resolveLink = (
typename: string,
fieldName: string,
select: SelectionSet,
prevData: void | Data | Data[]
prevData: void | null | Data | Data[]
): DataField | undefined => {
if (Array.isArray(link)) {
const { store } = ctx;
Expand All @@ -462,7 +462,7 @@ const resolveLink = (
typename,
fieldName,
select,
prevData !== undefined ? prevData[i] : undefined
prevData ? prevData[i] : undefined
kitten marked this conversation as resolved.
Show resolved Hide resolved
);
if (childLink === undefined && !_isListNullable) {
return undefined;
Expand All @@ -475,12 +475,7 @@ const resolveLink = (
} else if (link === null) {
return null;
} else {
return readSelection(
ctx,
link,
select,
prevData === undefined ? ({} as any) : prevData
);
return readSelection(ctx, link, select, (prevData || {}) as Data);
}
};

Expand Down