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

Ergonomic improvements for field policy merge and keyArgs functions. #6714

Merged
merged 5 commits into from
Jul 27, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,9 @@
- Allow passing an asynchronous `options.renderFunction` to `getMarkupFromTree`. <br/>
[@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. <br/>
[@benjamn](https://github.com/benjamn) in [#6714](https://github.com/apollographql/apollo-client/pull/6714)

## Apollo Client 3.0.2

## Bug Fixes
Expand Down
38 changes: 37 additions & 1 deletion docs/source/caching/cache-field-behavior.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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
Expand Down Expand Up @@ -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

Expand All @@ -490,7 +525,7 @@ type FieldPolicy<
> = {
keyArgs?: KeySpecifier | KeyArgsFunction | false;
read?: FieldReadFunction<TExisting, TReadResult>;
merge?: FieldMergeFunction<TExisting, TIncoming>;
merge?: FieldMergeFunction<TExisting, TIncoming> | boolean;
};

type KeySpecifier = (string | KeySpecifier)[];
Expand All @@ -501,6 +536,7 @@ type KeyArgsFunction = (
typename: string;
fieldName: string;
field: FieldNode | null;
variables?: Record<string, any>;
},
) => string | KeySpecifier | null | void;

Expand Down
8 changes: 2 additions & 6 deletions src/__tests__/ApolloClient.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
},
},
},
Expand Down Expand Up @@ -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,
},
},
},
Expand Down
13 changes: 5 additions & 8 deletions src/__tests__/fetchMore.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand Down Expand Up @@ -215,9 +215,7 @@ describe('fetchMore on an observable query', () => {
Query: {
fields: {
entry: {
merge(_, incoming) {
return incoming;
},
merge: false,
},
},
},
Expand Down Expand Up @@ -414,7 +412,8 @@ 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 FieldMergeFunction<any>).call(
this, existing, incoming, options);
};

const cache = new InMemoryCache({
Expand Down Expand Up @@ -831,9 +830,7 @@ describe('fetchMore on an observable query with connection', () => {
Query: {
fields: {
entry: {
merge(_, incoming) {
return incoming;
},
merge: false,
},
},
},
Expand Down
4 changes: 1 addition & 3 deletions src/__tests__/optimistic.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
},
},
},
Expand Down
4 changes: 1 addition & 3 deletions src/cache/inmemory/__tests__/cache.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
},
},
},
Expand Down
60 changes: 60 additions & 0 deletions src/cache/inmemory/__tests__/policies.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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") {
Expand Down Expand Up @@ -682,6 +685,9 @@ describe("type policies", function () {
}],
},
},
variables: {
unused: "check me",
},
});

expect(cache.extract()).toEqual({
Expand Down Expand Up @@ -3267,6 +3273,10 @@ describe("type policies", function () {
},
});

testForceMerges(cache);
});

function testForceMerges(cache: InMemoryCache) {
const queryWithAuthorName = gql`
query {
currentlyReading {
Expand Down Expand Up @@ -3443,6 +3453,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) : [];
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can't usefully review this part of the test without a lot more learning.

const seen = new Set<string>();
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);
});
});

Expand Down
21 changes: 19 additions & 2 deletions src/cache/inmemory/policies.ts
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,7 @@ export type KeyArgsFunction = (
typename: string;
fieldName: string;
field: FieldNode | null;
variables?: Record<string, any>;
},
) => KeySpecifier | ReturnType<IdGetter>;

Expand All @@ -98,7 +99,7 @@ export type FieldPolicy<
> = {
keyArgs?: KeySpecifier | KeyArgsFunction | false;
read?: FieldReadFunction<TExisting, TReadResult>;
merge?: FieldMergeFunction<TExisting, TIncoming>;
merge?: FieldMergeFunction<TExisting, TIncoming> | boolean;
};

type StorageType = Record<string, any>;
Expand Down Expand Up @@ -212,6 +213,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<any> =
(existing, incoming, { mergeObjects }) => mergeObjects(existing, incoming);
const mergeFalseFn: FieldMergeFunction<any> = (_, incoming) => incoming;

export type PossibleTypesMap = {
[supertype: string]: string[];
};
Expand Down Expand Up @@ -349,7 +356,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) {
Expand Down Expand Up @@ -477,6 +493,7 @@ export class Policies {
typename,
fieldName,
field: fieldSpec.field || null,
variables: fieldSpec.variables,
};
const args = argsFromFieldSpecifier(fieldSpec);
while (keyFn) {
Expand Down
4 changes: 1 addition & 3 deletions src/core/__tests__/QueryManager/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2167,9 +2167,7 @@ describe('QueryManager', () => {
Query: {
fields: {
info: {
merge(_, incoming) {
return incoming;
},
merge: false,
},
},
},
Expand Down