Skip to content

Commit

Permalink
Remove experimental cache.modify method.
Browse files Browse the repository at this point in the history
The `cache.modify` API was first introduced in #5909 as a quick way to
transform the values of specific existing fields in the cache.

At the time, `cache.modify` seemed promising as an alternative to the
`readQuery`-transform-`writeQuery` pattern, but feedback has been mixed,
most importantly from our developer experience team, who helped me
understand why `cache.modify` would be difficult to learn and to teach.

While my refactoring in #6221 addressed concerns about broadcasting
`cache.modify` updates automatically, the bigger problem with
`cache.modify` is simply that it requires knowledge of the internal
workings of the cache, making it nearly impossible to explain to
developers who are not already Apollo Client 3 experts.

As much as I wanted `cache.modify` to be a selling point for Apollo Client
3, it simply wasn't safe to use without a firm understanding of concepts
like cache normalization, references, field identity, read/merge functions
and their options API, and the `options.toReference(object, true)` helper
function (#5970).

By contrast, the `readQuery`-transform-`writeQuery` pattern may be a bit
more verbose, but it has none of these problems, because these older
methods work in terms of GraphQL result objects, rather than exposing the
internal data format of the `EntityStore`.

Since `cache.modify` was motivated to some extent by vague power-user
performance concerns, it's worth noting that we recently made `writeQuery`
and `writeFragment` even more efficient when rewriting unchanged results
(#6274), so whatever performance gap there might have been between
`cache.modify` and `readQuery`/`writeQuery` should now be even less
noticeable.

One final reason that `cache.modify` seemed like a good idea was that
custom `merge` functions can interfere with certain `writeQuery` patterns,
such as deleting an item from a paginated list. If you run into trouble
with `merge` functions running when you don't want them to, we recommend
calling `cache.evict` before `cache.writeQuery` to ensure your `merge`
function won't be confused by existing field data when you write your
modified data back into the cache.
  • Loading branch information
benjamn committed May 15, 2020
1 parent 9c69f97 commit d16c605
Show file tree
Hide file tree
Showing 7 changed files with 119 additions and 474 deletions.
14 changes: 0 additions & 14 deletions src/__tests__/client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2942,19 +2942,6 @@ describe('@connection', () => {
checkLastResult(abResults, a456bOyez);
checkLastResult(cResults, { c: "see" });

cache.modify({
c(value) {
expect(value).toBe("see");
return "saw";
},
});
await wait();

checkLastResult(aResults, a456);
checkLastResult(bResults, bOyez);
checkLastResult(abResults, a456bOyez);
checkLastResult(cResults, { c: "saw" });

client.cache.evict("ROOT_QUERY", "c");
await wait();

Expand Down Expand Up @@ -2991,7 +2978,6 @@ describe('@connection', () => {
expect(cResults).toEqual([
{},
{ c: "see" },
{ c: "saw" },
{},
]);

Expand Down
59 changes: 37 additions & 22 deletions src/__tests__/local-state/resolvers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -546,19 +546,22 @@ describe('Writing cache data from resolvers', () => {
resolvers: {
Mutation: {
start() {
const obj = {
__typename: 'Object',
id: 'uniqueId',
field: 1,
};

cache.writeQuery({
query,
data: {
obj: { field: 1, id: 'uniqueId', __typename: 'Object' },
},
data: { obj },
});

cache.modify({
field(value) {
expect(value).toBe(1);
return 2;
},
}, 'Object:uniqueId');
cache.writeFragment({
id: cache.identify(obj)!,
fragment: gql`fragment Field on Object { field }`,
data: { field: 2 },
});

return { start: true };
},
Expand All @@ -574,7 +577,7 @@ describe('Writing cache data from resolvers', () => {
});
});

it('should not overwrite __typename when writing to the cache with an id', () => {
itAsync('should not overwrite __typename when writing to the cache with an id', (resolve, reject) => {
const query = gql`
{
obj @client {
Expand All @@ -600,22 +603,35 @@ describe('Writing cache data from resolvers', () => {
resolvers: {
Mutation: {
start() {
const obj = {
__typename: 'Object',
id: 'uniqueId',
field: {
__typename: 'Field',
field2: 1,
},
};

cache.writeQuery({
query,
data: { obj },
});

cache.writeFragment({
id: cache.identify(obj)!,
fragment: gql`fragment FieldField2 on Object {
field {
field2
}
}`,
data: {
obj: {
field: { field2: 1, __typename: 'Field' },
id: 'uniqueId',
__typename: 'Object',
field: {
__typename: 'Field',
field2: 2,
},
},
});
cache.modify({
field(value: { field2: number }) {
expect(value.field2).toBe(1);
return { ...value, field2: 2 };
},
}, 'Object:uniqueId');

return { start: true };
},
},
Expand All @@ -628,8 +644,7 @@ describe('Writing cache data from resolvers', () => {
.then(({ data }: any) => {
expect(data.obj.__typename).toEqual('Object');
expect(data.obj.field.__typename).toEqual('Field');
})
.catch(e => console.log(e));
}).then(resolve, reject);
});
});

Expand Down
9 changes: 0 additions & 9 deletions src/cache/core/cache.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ import { getFragmentQueryDocument } from '../../utilities/graphql/fragments';
import { StoreObject } from '../../utilities/graphql/storeUtils';
import { DataProxy } from './types/DataProxy';
import { Cache } from './types/Cache';
import { Modifier, Modifiers } from './types/common';

export type Transaction<T> = (c: ApolloCache<T>) => void;

Expand Down Expand Up @@ -78,14 +77,6 @@ export abstract class ApolloCache<TSerialized> implements DataProxy {
public identify(object: StoreObject): string | undefined {
return;
}

public modify(
modifiers: Modifier<any> | Modifiers,
dataId?: string,
optimistic?: boolean,
): boolean {
return false;
}

public gc(): string[] {
return [];
Expand Down
Loading

0 comments on commit d16c605

Please sign in to comment.