From 8bab37574c9ddc0421c5df7434de91eb3bcdafa2 Mon Sep 17 00:00:00 2001 From: Andy Ingram Date: Wed, 2 Oct 2024 15:40:07 +0100 Subject: [PATCH 1/4] Fix @_required directive when the schema is available If graphcache was configured with the full schema, the `@_required` directive would be ignored on nullable fields. This diff tweaks the condition to fix that behaviour. --- exchanges/graphcache/src/operations/query.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/exchanges/graphcache/src/operations/query.ts b/exchanges/graphcache/src/operations/query.ts index 2a298acedd..54b9c5cd5d 100644 --- a/exchanges/graphcache/src/operations/query.ts +++ b/exchanges/graphcache/src/operations/query.ts @@ -534,7 +534,7 @@ const readSelection = ( (directives.optional || (optionalRef && !directives.required) || !!getFieldError(ctx) || - (store.schema && + (!directives.required && store.schema && isFieldNullable(store.schema, typename, fieldName, ctx.store.logger))) ) { // The field is uncached or has errored, so it'll be set to null and skipped From ed68fe5e395443d98b01e0350c7be06a77ebdf3e Mon Sep 17 00:00:00 2001 From: Andy Ingram Date: Wed, 2 Oct 2024 23:10:25 +0100 Subject: [PATCH 2/4] Add test and changeset --- .changeset/breezy-tomatoes-sing.md | 5 ++ .../graphcache/src/cacheExchange.test.ts | 65 +++++++++++++++++++ exchanges/graphcache/src/operations/query.ts | 3 +- 3 files changed, 72 insertions(+), 1 deletion(-) create mode 100644 .changeset/breezy-tomatoes-sing.md diff --git a/.changeset/breezy-tomatoes-sing.md b/.changeset/breezy-tomatoes-sing.md new file mode 100644 index 0000000000..72994c5a14 --- /dev/null +++ b/.changeset/breezy-tomatoes-sing.md @@ -0,0 +1,5 @@ +--- +'@urql/exchange-graphcache': minor +--- + +Allow @_required directive to be used in combination with configured schemas diff --git a/exchanges/graphcache/src/cacheExchange.test.ts b/exchanges/graphcache/src/cacheExchange.test.ts index 832d32f638..cda29dc2b4 100644 --- a/exchanges/graphcache/src/cacheExchange.test.ts +++ b/exchanges/graphcache/src/cacheExchange.test.ts @@ -1544,6 +1544,71 @@ describe('directives', () => { expect(reexecuteOperation).toHaveBeenCalledTimes(0); expect(result.mock.calls[0][0].data).toEqual(null); }); + + it('does not return missing fields when nullable fields from a defined schema are marked as required in the query', () => { + const client = createClient({ + url: 'http://0.0.0.0', + exchanges: [], + }); + const { source: ops$, next } = makeSubject(); + + const query = gql` + { + latestTodo { + id + text + completed @_required + } + } + `; + + const operation = client.createRequestOperation('query', { + key: 1, + query, + variables: undefined, + }); + + const queryResult: OperationResult = { + ...queryResponse, + operation, + data: { + __typename: 'Query', + latestTodo: [ + { + id: '1', + text: 'learn urql', + completed: null, + __typename: 'Todo', + }, + ], + }, + }; + + const response = vi.fn((forwardOp: Operation): OperationResult => { + if (forwardOp.key === 1) return queryResult; + return undefined as any; + }); + + const result = vi.fn(); + const forward: ExchangeIO = ops$ => pipe(ops$, map(response), share); + + pipe( + cacheExchange({ + schema: minifyIntrospectionQuery( + // eslint-disable-next-line + require('./test-utils/simple_schema.json') + ), + })({ forward, client, dispatchDebug })(ops$), + tap(result), + publish + ); + + next(operation); + + console.log(result.mock.calls[0][0]) + + expect(result.mock.calls[0][0].data).toEqual(null); + }); }); describe('optimistic updates', () => { diff --git a/exchanges/graphcache/src/operations/query.ts b/exchanges/graphcache/src/operations/query.ts index 54b9c5cd5d..3ad90a2ecf 100644 --- a/exchanges/graphcache/src/operations/query.ts +++ b/exchanges/graphcache/src/operations/query.ts @@ -534,7 +534,8 @@ const readSelection = ( (directives.optional || (optionalRef && !directives.required) || !!getFieldError(ctx) || - (!directives.required && store.schema && + (!directives.required && + store.schema && isFieldNullable(store.schema, typename, fieldName, ctx.store.logger))) ) { // The field is uncached or has errored, so it'll be set to null and skipped From e649395809e823705a06fd38dd7cd35473b98a54 Mon Sep 17 00:00:00 2001 From: Andy Ingram Date: Wed, 2 Oct 2024 23:13:18 +0100 Subject: [PATCH 3/4] cleanup --- exchanges/graphcache/src/cacheExchange.test.ts | 2 -- 1 file changed, 2 deletions(-) diff --git a/exchanges/graphcache/src/cacheExchange.test.ts b/exchanges/graphcache/src/cacheExchange.test.ts index cda29dc2b4..8247b13070 100644 --- a/exchanges/graphcache/src/cacheExchange.test.ts +++ b/exchanges/graphcache/src/cacheExchange.test.ts @@ -1605,8 +1605,6 @@ describe('directives', () => { next(operation); - console.log(result.mock.calls[0][0]) - expect(result.mock.calls[0][0].data).toEqual(null); }); }); From a910c50b5bf9874d1a976c11b136721a037ad08b Mon Sep 17 00:00:00 2001 From: Andy Ingram Date: Wed, 2 Oct 2024 23:49:54 +0100 Subject: [PATCH 4/4] Fix the test --- .../graphcache/src/cacheExchange.test.ts | 67 ++++++++++++++----- 1 file changed, 52 insertions(+), 15 deletions(-) diff --git a/exchanges/graphcache/src/cacheExchange.test.ts b/exchanges/graphcache/src/cacheExchange.test.ts index 8247b13070..20e8dbe1e2 100644 --- a/exchanges/graphcache/src/cacheExchange.test.ts +++ b/exchanges/graphcache/src/cacheExchange.test.ts @@ -1552,40 +1552,69 @@ describe('directives', () => { }); const { source: ops$, next } = makeSubject(); + const initialQuery = gql` + query { + latestTodo { + id + } + } + `; + const query = gql` { latestTodo { id - text - completed @_required + author @_required { + id + name + } } } `; - const operation = client.createRequestOperation('query', { + const initialQueryOperation = client.createRequestOperation('query', { key: 1, + query: initialQuery, + variables: undefined, + }); + + const queryOperation = client.createRequestOperation('query', { + key: 2, query, variables: undefined, }); + const initialQueryResult: OperationResult = { + ...queryResponse, + operation: initialQueryOperation, + data: { + __typename: 'Query', + latestTodo: { + __typename: 'Todo', + id: '1', + }, + }, + }; + const queryResult: OperationResult = { ...queryResponse, - operation, + operation: queryOperation, data: { __typename: 'Query', - latestTodo: [ - { - id: '1', - text: 'learn urql', - completed: null, - __typename: 'Todo', - }, - ], + latestTodo: { + __typename: 'Todo', + id: '1', + author: null, + }, }, }; const response = vi.fn((forwardOp: Operation): OperationResult => { - if (forwardOp.key === 1) return queryResult; + if (forwardOp.key === 1) { + return initialQueryResult; + } else if (forwardOp.key === 2) { + return queryResult; + } return undefined as any; }); @@ -1603,9 +1632,17 @@ describe('directives', () => { publish ); - next(operation); + next(initialQueryOperation); + vi.runAllTimers(); + next(queryOperation); + vi.runAllTimers(); - expect(result.mock.calls[0][0].data).toEqual(null); + expect(result.mock.calls[0][0].data).toEqual({ + latestTodo: { + id: '1', + }, + }); + expect(result.mock.calls[1][0].data).toEqual(null); }); });