From bda573572c84f7edf38530cd37aa61416660dd5a Mon Sep 17 00:00:00 2001 From: Phil Pluckthun Date: Wed, 30 Aug 2023 13:23:25 +0100 Subject: [PATCH 1/3] fix(core): Force client.reexecuteOperation to dispatch --- .changeset/wicked-seahorses-smash.md | 5 ++++ packages/core/src/client.test.ts | 40 ++++++++++++++++++++++++++++ packages/core/src/client.ts | 4 +++ 3 files changed, 49 insertions(+) create mode 100644 .changeset/wicked-seahorses-smash.md diff --git a/.changeset/wicked-seahorses-smash.md b/.changeset/wicked-seahorses-smash.md new file mode 100644 index 0000000000..208dd47189 --- /dev/null +++ b/.changeset/wicked-seahorses-smash.md @@ -0,0 +1,5 @@ +--- +'@urql/core': patch +--- + +Explicitly unblock `client.reexecuteOperation` calls to allow stalled operations from continuing and re-executing. Previously, this could cause `@urql/exchange-graphcache` to stall if an optimistic mutation led to a cache miss. diff --git a/packages/core/src/client.test.ts b/packages/core/src/client.test.ts index dd14d6aa3d..67c379e571 100755 --- a/packages/core/src/client.test.ts +++ b/packages/core/src/client.test.ts @@ -700,6 +700,46 @@ describe('deduplication behavior', () => { expect(onOperation).toHaveBeenCalledTimes(2); }); + + it('unblocks operations on call to reexecuteOperation', async () => { + const onOperation = vi.fn(); + const onResult = vi.fn(); + + let hasSent = false; + const exchange: Exchange = () => ops$ => + pipe( + ops$, + onPush(onOperation), + map(op => ({ + hasNext: false, + stale: false, + data: 'test', + operation: op, + })), + filter(() => hasSent || !(hasSent = true)) + ); + + const client = createClient({ + url: 'test', + exchanges: [exchange], + }); + + const operation = makeOperation('query', queryOperation, { + ...queryOperation.context, + requestPolicy: 'cache-first', + }); + + pipe(client.executeRequestOperation(operation), subscribe(onResult)); + + expect(onOperation).toHaveBeenCalledTimes(1); + expect(onResult).toHaveBeenCalledTimes(0); + + client.reexecuteOperation(operation); + await Promise.resolve(); + + expect(onOperation).toHaveBeenCalledTimes(2); + expect(onResult).toHaveBeenCalledTimes(1); + }); }); describe('shared sources behavior', () => { diff --git a/packages/core/src/client.ts b/packages/core/src/client.ts index 0ad6586f6b..fecd3d0c58 100755 --- a/packages/core/src/client.ts +++ b/packages/core/src/client.ts @@ -729,6 +729,10 @@ export const Client: new (opts: ClientOptions) => Client = function Client( if (operation.kind === 'teardown') { dispatchOperation(operation); } else if (operation.kind === 'mutation' || active.has(operation.key)) { + let queued = false; + for (let i = 0; i < queue.length; i++) + queued = queued || queue[i].key === operation.key; + if (!queued) dispatched.delete(operation.key); queue.push(operation); Promise.resolve().then(dispatchOperation); } From f42b1c8a36b467b1725dbba7e1abe378d368e27b Mon Sep 17 00:00:00 2001 From: Phil Pluckthun Date: Wed, 30 Aug 2023 13:37:34 +0100 Subject: [PATCH 2/3] Add test checking that __typename checks update dependencies --- exchanges/graphcache/src/store/data.test.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/exchanges/graphcache/src/store/data.test.ts b/exchanges/graphcache/src/store/data.test.ts index de0046522e..03f5441e65 100644 --- a/exchanges/graphcache/src/store/data.test.ts +++ b/exchanges/graphcache/src/store/data.test.ts @@ -13,6 +13,7 @@ describe('garbage collection', () => { it('erases orphaned entities', () => { InMemoryData.writeRecord('Todo:1', '__typename', 'Todo'); InMemoryData.writeRecord('Todo:1', 'id', '1'); + InMemoryData.writeRecord('Todo:2', '__typename', 'Todo'); InMemoryData.writeRecord('Query', '__typename', 'Query'); InMemoryData.writeLink('Query', 'todo', 'Todo:1'); @@ -27,7 +28,7 @@ describe('garbage collection', () => { expect(InMemoryData.readRecord('Todo:1', 'id')).toBe(undefined); expect(InMemoryData.getCurrentDependencies()).toEqual( - new Set(['Todo:1', 'Query.todo']) + new Set(['Todo:1', 'Todo:2', 'Query.todo']) ); }); From 17f8157884ba518f07c0698c09ab0e4d5fa44179 Mon Sep 17 00:00:00 2001 From: Phil Pluckthun Date: Wed, 30 Aug 2023 14:30:42 +0100 Subject: [PATCH 3/3] Add test for query after mutation invalidation timing --- .../graphcache/src/cacheExchange.test.ts | 139 ++++++++++++++++++ 1 file changed, 139 insertions(+) diff --git a/exchanges/graphcache/src/cacheExchange.test.ts b/exchanges/graphcache/src/cacheExchange.test.ts index 9c2313e67b..c31d2d27a6 100644 --- a/exchanges/graphcache/src/cacheExchange.test.ts +++ b/exchanges/graphcache/src/cacheExchange.test.ts @@ -1257,6 +1257,145 @@ describe('optimistic updates', () => { expect(updates.Mutation.addAuthor).toHaveBeenCalledTimes(2); expect(response).toHaveBeenCalledTimes(2); expect(result).toHaveBeenCalledTimes(4); + expect(reexec).toHaveBeenCalledTimes(2); + + next(opOne); + vi.runAllTimers(); + expect(result).toHaveBeenCalledTimes(5); + }); + + it('does not block subsequent query operations', () => { + vi.useFakeTimers(); + + const authorsQuery = gql` + query { + authors { + id + name + } + } + `; + + const authorsQueryData = { + __typename: 'Query', + authors: [ + { + __typename: 'Author', + id: '123', + name: 'Author', + }, + ], + }; + + const mutation = gql` + mutation { + deleteAuthor { + id + name + } + } + `; + + const optimisticMutationData = { + __typename: 'Mutation', + deleteAuthor: { + __typename: 'Author', + id: '123', + name: '[REDACTED OFFLINE]', + }, + }; + + const client = createClient({ + url: 'http://0.0.0.0', + exchanges: [], + }); + const { source: ops$, next } = makeSubject(); + + const reexec = vi + .spyOn(client, 'reexecuteOperation') + .mockImplementation(next); + + const opOne = client.createRequestOperation('query', { + key: 1, + query: authorsQuery, + variables: undefined, + }); + + const opMutation = client.createRequestOperation('mutation', { + key: 2, + query: mutation, + variables: undefined, + }); + + const response = vi.fn((forwardOp: Operation): OperationResult => { + if (forwardOp.key === 1) { + return { ...queryResponse, operation: opOne, data: authorsQueryData }; + } else if (forwardOp.key === 2) { + return { + ...queryResponse, + operation: opMutation, + data: { + __typename: 'Mutation', + deleteAuthor: optimisticMutationData.deleteAuthor, + }, + }; + } + + return undefined as any; + }); + + const result = vi.fn(); + const forward: ExchangeIO = ops$ => + pipe(ops$, delay(1), map(response), share); + + const optimistic = { + deleteAuthor: vi.fn(() => optimisticMutationData.deleteAuthor) as any, + }; + + const updates = { + Mutation: { + deleteAuthor: vi.fn((_data, _, cache) => { + cache.invalidate({ + __typename: 'Author', + id: optimisticMutationData.deleteAuthor.id, + }); + }), + }, + }; + + pipe( + cacheExchange({ optimistic, updates })({ + forward, + client, + dispatchDebug, + })(ops$), + tap(result), + publish + ); + + next(opOne); + vi.runAllTimers(); + expect(response).toHaveBeenCalledTimes(1); + expect(result).toHaveBeenCalledTimes(1); + + next(opMutation); + expect(response).toHaveBeenCalledTimes(1); + expect(optimistic.deleteAuthor).toHaveBeenCalledTimes(1); + expect(updates.Mutation.deleteAuthor).toHaveBeenCalledTimes(1); + expect(reexec).toHaveBeenCalledTimes(1); + expect(result).toHaveBeenCalledTimes(1); + + vi.runAllTimers(); + + expect(updates.Mutation.deleteAuthor).toHaveBeenCalledTimes(2); + expect(response).toHaveBeenCalledTimes(2); + expect(result).toHaveBeenCalledTimes(2); + expect(reexec).toHaveBeenCalledTimes(2); + expect(reexec.mock.calls[1][0]).toMatchObject(opOne); + + next(opOne); + vi.runAllTimers(); + expect(result).toHaveBeenCalledTimes(3); }); });