From cb383a54bc8d1e4b8df6d0c9794dab88e83b7c32 Mon Sep 17 00:00:00 2001 From: Ben Newman Date: Mon, 27 Jul 2020 16:58:52 -0400 Subject: [PATCH 1/5] Support merge:true and merge:false shorthands in field policies. --- src/__tests__/ApolloClient.ts | 8 +--- src/__tests__/fetchMore.ts | 10 ++--- src/__tests__/optimistic.ts | 4 +- src/cache/inmemory/__tests__/cache.ts | 4 +- src/cache/inmemory/__tests__/policies.ts | 54 ++++++++++++++++++++++++ src/cache/inmemory/policies.ts | 19 ++++++++- src/core/__tests__/QueryManager/index.ts | 4 +- 7 files changed, 79 insertions(+), 24 deletions(-) diff --git a/src/__tests__/ApolloClient.ts b/src/__tests__/ApolloClient.ts index b6341a63b5e..d73d0a31c0b 100644 --- a/src/__tests__/ApolloClient.ts +++ b/src/__tests__/ApolloClient.ts @@ -679,9 +679,7 @@ describe('ApolloClient', () => { d: { // Silence "Cache data may be lost..." warnings by // unconditionally favoring the incoming data. - merge(_, incoming) { - return incoming; - }, + merge: false, }, }, }, @@ -1196,9 +1194,7 @@ describe('ApolloClient', () => { // Deliberately silence "Cache data may be lost..." // warnings by preferring the incoming data, rather // than (say) concatenating the arrays together. - merge(_, incoming) { - return incoming; - }, + merge: false, }, }, }, diff --git a/src/__tests__/fetchMore.ts b/src/__tests__/fetchMore.ts index c1b8bd2956b..30172db2423 100644 --- a/src/__tests__/fetchMore.ts +++ b/src/__tests__/fetchMore.ts @@ -215,9 +215,7 @@ describe('fetchMore on an observable query', () => { Query: { fields: { entry: { - merge(_, incoming) { - return incoming; - }, + merge: false, }, }, }, @@ -414,7 +412,7 @@ describe('fetchMore on an observable query', () => { const { merge } = groceriesFieldPolicy; groceriesFieldPolicy.merge = function (existing, incoming, options) { mergeArgsHistory.push(options.args); - return merge!.call(this, existing, incoming, options); + return (merge as any).call(this, existing, incoming, options); }; const cache = new InMemoryCache({ @@ -831,9 +829,7 @@ describe('fetchMore on an observable query with connection', () => { Query: { fields: { entry: { - merge(_, incoming) { - return incoming; - }, + merge: false, }, }, }, diff --git a/src/__tests__/optimistic.ts b/src/__tests__/optimistic.ts index 126f81e3a69..956b14d8da5 100644 --- a/src/__tests__/optimistic.ts +++ b/src/__tests__/optimistic.ts @@ -135,9 +135,7 @@ describe('optimistic mutation results', () => { // Deliberately silence "Cache data may be lost..." // warnings by favoring the incoming data, rather than // (say) concatenating the arrays together. - merge(_, incoming) { - return incoming; - }, + merge: false, }, }, }, diff --git a/src/cache/inmemory/__tests__/cache.ts b/src/cache/inmemory/__tests__/cache.ts index 9ae975a96d7..f877e68ba66 100644 --- a/src/cache/inmemory/__tests__/cache.ts +++ b/src/cache/inmemory/__tests__/cache.ts @@ -876,9 +876,7 @@ describe('Cache', () => { d: { // Deliberately silence "Cache data may be lost..." // warnings by unconditionally favoring the incoming data. - merge(_, incoming) { - return incoming; - }, + merge: false, }, }, }, diff --git a/src/cache/inmemory/__tests__/policies.ts b/src/cache/inmemory/__tests__/policies.ts index 1d1aa36c515..0586fe9d4d4 100644 --- a/src/cache/inmemory/__tests__/policies.ts +++ b/src/cache/inmemory/__tests__/policies.ts @@ -3267,6 +3267,10 @@ describe("type policies", function () { }, }); + testForceMerges(cache); + }); + + function testForceMerges(cache: InMemoryCache) { const queryWithAuthorName = gql` query { currentlyReading { @@ -3443,6 +3447,56 @@ describe("type policies", function () { }, }, }); + } + + // Same as previous test, except with merge:true for Book.author. + it("can force merging with merge: true", function () { + const cache = new InMemoryCache({ + typePolicies: { + Book: { + keyFields: ["isbn"], + fields: { + author: { + merge: true, + }, + }, + }, + + Author: { + keyFields: false, + fields: { + books: { + merge(existing: any[], incoming: any[], { + isReference, + }) { + const merged = existing ? existing.slice(0) : []; + const seen = new Set(); + if (existing) { + existing.forEach(book => { + if (isReference(book)) { + seen.add(book.__ref); + } + }); + } + incoming.forEach(book => { + if (isReference(book)) { + if (!seen.has(book.__ref)) { + merged.push(book); + seen.add(book.__ref); + } + } else { + merged.push(book); + } + }); + return merged; + }, + }, + }, + }, + }, + }); + + testForceMerges(cache); }); }); diff --git a/src/cache/inmemory/policies.ts b/src/cache/inmemory/policies.ts index 898d659a9bd..6ee198a559c 100644 --- a/src/cache/inmemory/policies.ts +++ b/src/cache/inmemory/policies.ts @@ -98,7 +98,7 @@ export type FieldPolicy< > = { keyArgs?: KeySpecifier | KeyArgsFunction | false; read?: FieldReadFunction; - merge?: FieldMergeFunction; + merge?: FieldMergeFunction | boolean; }; type StorageType = Record; @@ -212,6 +212,12 @@ export const defaultDataIdFromObject = ( const nullKeyFieldsFn: KeyFieldsFunction = () => void 0; const simpleKeyArgsFn: KeyArgsFunction = (_args, context) => context.fieldName; +// These merge functions can be selected by specifying merge:true or +// merge:false in a field policy. +const mergeTrueFn: FieldMergeFunction = + (existing, incoming, { mergeObjects }) => mergeObjects(existing, incoming); +const mergeFalseFn: FieldMergeFunction = (_, incoming) => incoming; + export type PossibleTypesMap = { [supertype: string]: string[]; }; @@ -349,7 +355,16 @@ export class Policies { existing.keyFn; if (typeof read === "function") existing.read = read; - if (typeof merge === "function") existing.merge = merge; + + existing.merge = + typeof merge === "function" ? merge : + // Pass merge:true as a shorthand for a merge implementation + // that returns options.mergeObjects(existing, incoming). + merge === true ? mergeTrueFn : + // Pass merge:false to make incoming always replace existing + // without any warnings about data clobbering. + merge === false ? mergeFalseFn : + existing.merge; } if (existing.read && existing.merge) { diff --git a/src/core/__tests__/QueryManager/index.ts b/src/core/__tests__/QueryManager/index.ts index 6f927080509..02ef884f56e 100644 --- a/src/core/__tests__/QueryManager/index.ts +++ b/src/core/__tests__/QueryManager/index.ts @@ -2167,9 +2167,7 @@ describe('QueryManager', () => { Query: { fields: { info: { - merge(_, incoming) { - return incoming; - }, + merge: false, }, }, }, From 55254966771d65daa11f39379d121321d7ce61fd Mon Sep 17 00:00:00 2001 From: Ben Newman Date: Mon, 27 Jul 2020 17:06:22 -0400 Subject: [PATCH 2/5] Pass context.variables to field policy keyArgs functions. --- src/cache/inmemory/__tests__/policies.ts | 6 ++++++ src/cache/inmemory/policies.ts | 2 ++ 2 files changed, 8 insertions(+) diff --git a/src/cache/inmemory/__tests__/policies.ts b/src/cache/inmemory/__tests__/policies.ts index 0586fe9d4d4..4ced435b09f 100644 --- a/src/cache/inmemory/__tests__/policies.ts +++ b/src/cache/inmemory/__tests__/policies.ts @@ -624,6 +624,9 @@ describe("type policies", function () { expect(context.typename).toBe("Thread"); expect(context.fieldName).toBe("comments"); expect(context.field!.name.value).toBe("comments"); + expect(context.variables).toEqual({ + unused: "check me", + }); if (typeof args!.limit === "number") { if (typeof args!.offset === "number") { @@ -682,6 +685,9 @@ describe("type policies", function () { }], }, }, + variables: { + unused: "check me", + }, }); expect(cache.extract()).toEqual({ diff --git a/src/cache/inmemory/policies.ts b/src/cache/inmemory/policies.ts index 6ee198a559c..90677179c98 100644 --- a/src/cache/inmemory/policies.ts +++ b/src/cache/inmemory/policies.ts @@ -88,6 +88,7 @@ export type KeyArgsFunction = ( typename: string; fieldName: string; field: FieldNode | null; + variables?: Record; }, ) => KeySpecifier | ReturnType; @@ -492,6 +493,7 @@ export class Policies { typename, fieldName, field: fieldSpec.field || null, + variables: fieldSpec.variables, }; const args = argsFromFieldSpecifier(fieldSpec); while (keyFn) { From a388c3e24619078effeea151dafe7e76a4da5853 Mon Sep 17 00:00:00 2001 From: Ben Newman Date: Mon, 27 Jul 2020 17:59:22 -0400 Subject: [PATCH 3/5] Update documentation for PR #6714. --- docs/source/caching/cache-field-behavior.md | 38 ++++++++++++++++++++- 1 file changed, 37 insertions(+), 1 deletion(-) diff --git a/docs/source/caching/cache-field-behavior.md b/docs/source/caching/cache-field-behavior.md index c18b3e2a30a..966822f23db 100644 --- a/docs/source/caching/cache-field-behavior.md +++ b/docs/source/caching/cache-field-behavior.md @@ -208,6 +208,23 @@ const cache = new InMemoryCache({ }); ``` +Since writing this kind of `merge` function can become repetitive, the following shorthand will provide the same behavior: + +```ts +const cache = new InMemoryCache({ + typePolicies: { + Book: { + fields: { + author: { + // Short for always preferring incoming over existing data. + merge: false, + }, + }, + }, + }, +}); +``` + When you use `{ ...existing, ...incoming }`, `Author` objects with differing fields (`name`, `dateOfBirth`) can be combined without losing fields, which is definitely an improvement over blind replacement. But what if the `Author` type defines its own custom `merge` functions for fields of the `incoming` object? Since we're using [object spread syntax](https://2ality.com/2016/10/rest-spread-properties.html), such fields will immediately overwrite fields in `existing`, without triggering any nested `merge` functions. The `{ ...existing, ...incoming }` syntax may be an improvement, but it is not fully correct. @@ -233,6 +250,23 @@ const cache = new InMemoryCache({ Because this `Book.author` field policy has no `Book`- or `Author`-specific logic in it, you can reuse this `merge` function for any field that needs this kind of handling. +Since writing this kind of `merge` function can become repetitive, the following shorthand will provide the same behavior: + +```ts +const cache = new InMemoryCache({ + typePolicies: { + Book: { + fields: { + author: { + // Short for options.mergeObjects(existing, incoming). + merge: true, + }, + }, + }, + }, +}); +``` + In summary, the `Book.author` policy above allows the cache to safely merge the `author` objects of any two `Book` objects that have the same identity. ### Merging arrays of non-normalized objects @@ -475,6 +509,7 @@ An example of a _non-key_ argument is an access token, which is used to authoriz By default, the cache stores a separate value for _every unique combination of argument values you provide when querying a particular field_. When you specify a field's key arguments, the cache understands that any _non_-key arguments don't affect that field's value. Consequently, if you execute two different queries with the `monthForNumber` field, passing the _same_ `number` argument but _different_ `accessToken` arguments, the second query response will overwrite the first, because both invocations use the exact same value for all key arguments. +If you need more control over the behavior of `keyArgs`, you can pass a function instead of an array. This `keyArgs` function will receive the arguments object as its first parameter, and a `context` object providing other relevant details as its second parameter. See `KeyArgsFunction` in the types below for further information. ## `FieldPolicy` API reference @@ -490,7 +525,7 @@ type FieldPolicy< > = { keyArgs?: KeySpecifier | KeyArgsFunction | false; read?: FieldReadFunction; - merge?: FieldMergeFunction; + merge?: FieldMergeFunction | boolean; }; type KeySpecifier = (string | KeySpecifier)[]; @@ -501,6 +536,7 @@ type KeyArgsFunction = ( typename: string; fieldName: string; field: FieldNode | null; + variables?: Record; }, ) => string | KeySpecifier | null | void; From b726d3667de236a20d3c5217dea34b8bbf840aa4 Mon Sep 17 00:00:00 2001 From: Ben Newman Date: Mon, 27 Jul 2020 18:07:16 -0400 Subject: [PATCH 4/5] Mention PR #6714 in CHANGELOG.md. --- CHANGELOG.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index f83e9f55977..79ba3daf955 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -27,6 +27,9 @@ - Allow passing an asynchronous `options.renderFunction` to `getMarkupFromTree`.
[@richardscarrott](https://github.com/richardscarrott) in [#6576](https://github.com/apollographql/apollo-client/pull/6576) +- Ergonomic improvements for `merge` and `keyArgs` functions in cache field policies.
+ [@benjamn](https://github.com/benjamn) in [#6714](https://github.com/apollographql/apollo-client/pull/6714) + ## Apollo Client 3.0.2 ## Bug Fixes From 716fa9de963d0fbe9e3859edf6e7e78e0cefbb49 Mon Sep 17 00:00:00 2001 From: Ben Newman Date: Mon, 27 Jul 2020 18:29:28 -0400 Subject: [PATCH 5/5] Slight tweak to merge function typing in fetchMore tests. --- src/__tests__/fetchMore.ts | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/__tests__/fetchMore.ts b/src/__tests__/fetchMore.ts index 30172db2423..9e983f7d935 100644 --- a/src/__tests__/fetchMore.ts +++ b/src/__tests__/fetchMore.ts @@ -2,7 +2,7 @@ import { assign, cloneDeep } from 'lodash'; import gql from 'graphql-tag'; import { itAsync, mockSingleLink, subscribeAndCount } from '../testing'; -import { InMemoryCache, InMemoryCacheConfig } from '../cache'; +import { InMemoryCache, InMemoryCacheConfig, FieldMergeFunction } from '../cache'; import { ApolloClient, NetworkStatus, ObservableQuery } from '../core'; import { offsetLimitPagination, concatPagination } from '../utilities'; @@ -412,7 +412,8 @@ describe('fetchMore on an observable query', () => { const { merge } = groceriesFieldPolicy; groceriesFieldPolicy.merge = function (existing, incoming, options) { mergeArgsHistory.push(options.args); - return (merge as any).call(this, existing, incoming, options); + return (merge as FieldMergeFunction).call( + this, existing, incoming, options); }; const cache = new InMemoryCache({