From f328f50665fab24fa71e3568ac74b94134541351 Mon Sep 17 00:00:00 2001 From: Jovi De Croock Date: Sun, 22 Mar 2020 21:29:21 +0100 Subject: [PATCH] (graphcache) - optimistic support for mutations without selection (#657) * support optimistic scalars (optimistic mutations without a selectionset) * add changeset * combine clauses * Refactor away fieldData duplication * Fix data[fieldAlias] not being updated by optimistic updater Co-authored-by: Phil Pluckthun --- .changeset/twelve-trainers-sing.md | 5 + .../graphcache/src/cacheExchange.test.ts | 129 +++++++++++++++++- exchanges/graphcache/src/operations/write.ts | 41 +++--- 3 files changed, 154 insertions(+), 21 deletions(-) create mode 100644 .changeset/twelve-trainers-sing.md diff --git a/.changeset/twelve-trainers-sing.md b/.changeset/twelve-trainers-sing.md new file mode 100644 index 0000000000..71b0761428 --- /dev/null +++ b/.changeset/twelve-trainers-sing.md @@ -0,0 +1,5 @@ +--- +'@urql/exchange-graphcache': minor +--- + +Support optimistic values for mutations without a selectionset diff --git a/exchanges/graphcache/src/cacheExchange.test.ts b/exchanges/graphcache/src/cacheExchange.test.ts index 38cfbc18b5..b3f108497a 100644 --- a/exchanges/graphcache/src/cacheExchange.test.ts +++ b/exchanges/graphcache/src/cacheExchange.test.ts @@ -397,7 +397,7 @@ describe('data dependencies', () => { expect(result).toHaveBeenCalledTimes(2); }); - it('writes optimistic mutations to the cache', () => { + it('does not reach updater when mutation has no selectionset in optimistic phase', () => { jest.useFakeTimers(); const mutation = gql` @@ -452,6 +452,133 @@ describe('data dependencies', () => { jest.runAllTimers(); expect(updates.Mutation.concealAuthor).toHaveBeenCalledTimes(1); }); + + it('does reach updater when mutation has no selectionset in optimistic phase with optimistic update', () => { + jest.useFakeTimers(); + + const mutation = gql` + mutation { + concealAuthor + } + `; + + const mutationData = { + __typename: 'Mutation', + concealAuthor: true, + }; + + const client = createClient({ url: 'http://0.0.0.0' }); + const { source: ops$, next } = makeSubject(); + + jest.spyOn(client, 'reexecuteOperation').mockImplementation(next); + + const opMutation = client.createRequestOperation('mutation', { + key: 1, + query: mutation, + }); + + const response = jest.fn( + (forwardOp: Operation): OperationResult => { + if (forwardOp.key === 1) { + return { operation: opMutation, data: mutationData }; + } + + return undefined as any; + } + ); + + const result = jest.fn(); + const forward: ExchangeIO = ops$ => pipe(ops$, delay(1), map(response)); + + const updates = { + Mutation: { + concealAuthor: jest.fn(), + }, + }; + + const optimistic = { + concealAuthor: jest.fn(() => true) as any, + }; + + pipe( + cacheExchange({ updates, optimistic })({ forward, client })(ops$), + tap(result), + publish + ); + + next(opMutation); + expect(optimistic.concealAuthor).toHaveBeenCalledTimes(1); + expect(updates.Mutation.concealAuthor).toHaveBeenCalledTimes(1); + + jest.runAllTimers(); + expect(updates.Mutation.concealAuthor).toHaveBeenCalledTimes(2); + }); + + it('respects aliases in the optimistic update data that is written', () => { + jest.useFakeTimers(); + + const mutation = gql` + mutation { + concealed: concealAuthor + } + `; + + const mutationData = { + __typename: 'Mutation', + concealed: true, + }; + + const client = createClient({ url: 'http://0.0.0.0' }); + const { source: ops$, next } = makeSubject(); + + jest.spyOn(client, 'reexecuteOperation').mockImplementation(next); + + const opMutation = client.createRequestOperation('mutation', { + key: 1, + query: mutation, + }); + + const response = jest.fn( + (forwardOp: Operation): OperationResult => { + if (forwardOp.key === 1) { + return { operation: opMutation, data: mutationData }; + } + + return undefined as any; + } + ); + + const result = jest.fn(); + const forward: ExchangeIO = ops$ => pipe(ops$, delay(1), map(response)); + + const updates = { + Mutation: { + concealAuthor: jest.fn(), + }, + }; + + const optimistic = { + concealAuthor: jest.fn(() => true) as any, + }; + + pipe( + cacheExchange({ updates, optimistic })({ forward, client })(ops$), + tap(result), + publish + ); + + next(opMutation); + expect(optimistic.concealAuthor).toHaveBeenCalledTimes(1); + expect(updates.Mutation.concealAuthor).toHaveBeenCalledTimes(1); + + const data = updates.Mutation.concealAuthor.mock.calls[0][0]; + // Expect both fields to exist + expect(data.concealed).toBe(true); + expect(data.concealAuthor).toBe(true); + + jest.runAllTimers(); + expect(updates.Mutation.concealAuthor).toHaveBeenCalledTimes(2); + }); }); describe('optimistic updates', () => { diff --git a/exchanges/graphcache/src/operations/write.ts b/exchanges/graphcache/src/operations/write.ts index 047cacc885..cc712e8d25 100644 --- a/exchanges/graphcache/src/operations/write.ts +++ b/exchanges/graphcache/src/operations/write.ts @@ -190,7 +190,8 @@ const writeSelection = ( const fieldName = getName(node); const fieldArgs = getFieldArguments(node, ctx.variables); const fieldKey = keyOfField(fieldName, fieldArgs); - const fieldValue = data[getFieldAlias(node)]; + const fieldAlias = getFieldAlias(node); + let fieldValue = data[fieldAlias]; if (process.env.NODE_ENV !== 'production') { if (!isRoot && fieldValue === undefined) { @@ -219,35 +220,34 @@ const writeSelection = ( } } - if (node.selectionSet) { - let fieldData: Data | NullArray | null; - // Process optimistic updates, if this is a `writeOptimistic` operation - // otherwise read the field value from data and write it - if (ctx.optimistic && isRoot) { - const resolver = ctx.store.optimisticMutations[fieldName]; - if (!resolver) continue; - // We have to update the context to reflect up-to-date ResolveInfo - updateContext(ctx, typename, typename, fieldKey, fieldName); - fieldData = ensureData( - resolver(fieldArgs || makeDict(), ctx.store, ctx) - ); - data[fieldName] = fieldData; - } else { - fieldData = ensureData(fieldValue); - } + if (ctx.optimistic && isRoot) { + const resolver = ctx.store.optimisticMutations[fieldName]; + if (!resolver) continue; + // We have to update the context to reflect up-to-date ResolveInfo + updateContext(ctx, typename, typename, fieldKey, fieldName); + fieldValue = data[fieldAlias] = ensureData( + resolver(fieldArgs || makeDict(), ctx.store, ctx) + ); + } + if (node.selectionSet) { // Process the field and write links for the child entities that have been written if (entityKey && !isRoot) { const key = joinKeys(entityKey, fieldKey); - const link = writeField(ctx, getSelectionSet(node), fieldData, key); + const link = writeField( + ctx, + getSelectionSet(node), + ensureData(fieldValue), + key + ); InMemoryData.writeLink(entityKey || typename, fieldKey, link); } else { - writeField(ctx, getSelectionSet(node), fieldData); + writeField(ctx, getSelectionSet(node), ensureData(fieldValue)); } } else if (entityKey && !isRoot) { // This is a leaf node, so we're setting the field's value directly InMemoryData.writeRecord(entityKey || typename, fieldKey, fieldValue); - } else if (ctx.optimistic && isRoot) continue; + } if (isRoot) { // We have to update the context to reflect up-to-date ResolveInfo @@ -263,6 +263,7 @@ const writeSelection = ( // so that the data is already available in-store if necessary const updater = ctx.store.updates[typename][fieldName]; if (updater) { + data[fieldName] = fieldValue; updater(data, fieldArgs || makeDict(), ctx.store, ctx); } }