diff --git a/CHANGELOG.md b/CHANGELOG.md index b75a80a79ca..015ee3981ca 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -61,6 +61,9 @@ - A `resultCacheMaxSize` option may be passed to the `InMemoryCache` constructor to limit the number of result objects that will be retained in memory (to speed up repeated reads), and calling `cache.reset()` now releases all such memory.
[@SofianHn](https://github.com/SofianHn) in [#8701](https://github.com/apollographql/apollo-client/pull/8701) +- Fully remove result cache entries from LRU dependency system when the corresponding entities are removed from `InMemoryCache` by eviction, or by any other means.
+ [@sofianhn](https://github.com/sofianhn) and [@benjamn](https://github.com/benjamn) in [#8147](https://github.com/apollographql/apollo-client/pull/8147) + ### Documentation TBD diff --git a/package-lock.json b/package-lock.json index be4ed34167a..c19cfc2a754 100644 --- a/package-lock.json +++ b/package-lock.json @@ -10914,9 +10914,9 @@ } }, "optimism": { - "version": "0.16.0", - "resolved": "https://registry.npmjs.org/optimism/-/optimism-0.16.0.tgz", - "integrity": "sha512-p+JNvSj7tsCCCiwb5jFvfNZZL8YMy1G7S8hymB5dwupV7rpu7ftRkMw2yvNY9zfMk0oH/kIGmkPSXaeBNjtWYQ==", + "version": "0.16.1", + "resolved": "https://registry.npmjs.org/optimism/-/optimism-0.16.1.tgz", + "integrity": "sha512-64i+Uw3otrndfq5kaoGNoY7pvOhSsjFEN4bdEFh80MWVk/dbgJfMv7VFDeCT8LxNAlEVhQmdVEbfE7X2nWNIIg==", "requires": { "@wry/context": "^0.6.0", "@wry/trie": "^0.3.0" diff --git a/package.json b/package.json index 9c3c875cf1b..b8190890fb5 100644 --- a/package.json +++ b/package.json @@ -81,7 +81,7 @@ "fast-json-stable-stringify": "^2.0.0", "graphql-tag": "^2.12.3", "hoist-non-react-statics": "^3.3.2", - "optimism": "^0.16.0", + "optimism": "^0.16.1", "prop-types": "^15.7.2", "symbol-observable": "^2.0.0", "ts-invariant": "^0.7.3", diff --git a/src/__tests__/resultCacheCleaning.ts b/src/__tests__/resultCacheCleaning.ts new file mode 100644 index 00000000000..ee60b976e77 --- /dev/null +++ b/src/__tests__/resultCacheCleaning.ts @@ -0,0 +1,186 @@ +import { makeExecutableSchema } from "graphql-tools"; + +import { ApolloClient, Resolvers, gql } from "../core"; +import { InMemoryCache, NormalizedCacheObject } from "../cache"; +import { SchemaLink } from "../link/schema"; + +describe("resultCache cleaning", () => { + const fragments = gql` + fragment user on User { + id + name + } + + fragment reaction on Reaction { + id + type + author { + ...user + } + } + + fragment message on Message { + id + author { + ...user + } + reactions { + ...reaction + } + viewedBy { + ...user + } + } + `; + + const query = gql` + query getChat($id: ID!) { + chat(id: $id) { + id + name + members { + ...user + } + messages { + ...message + } + } + } + ${{ ...fragments }} + `; + + function uuid(label: string) { + return () => + `${label}-${Math.random() + .toString(16) + .substr(2)}`; + } + + function emptyList(len: number) { + return new Array(len).fill(true); + } + + const typeDefs = gql` + type Query { + chat(id: ID!): Chat! + } + + type Chat { + id: ID! + name: String! + messages: [Message!]! + members: [User!]! + } + + type Message { + id: ID! + author: User! + reactions: [Reaction!]! + viewedBy: [User!]! + content: String! + } + + type User { + id: ID! + name: String! + } + + type Reaction { + id: ID! + type: String! + author: User! + } + `; + + const resolvers: Resolvers = { + Query: { + chat(_, { id }) { + return id; + }, + }, + Chat: { + id(id) { + return id; + }, + name(id) { + return id; + }, + messages() { + return emptyList(10); + }, + members() { + return emptyList(10); + }, + }, + Message: { + id: uuid("Message"), + author() { + return { foo: true }; + }, + reactions() { + return emptyList(10); + }, + viewedBy() { + return emptyList(10); + }, + content: uuid("Message-Content"), + }, + User: { + id: uuid("User"), + name: uuid("User.name"), + }, + Reaction: { + id: uuid("Reaction"), + type: uuid("Reaction.type"), + author() { + return { foo: true }; + }, + }, + }; + + let client: ApolloClient; + + beforeEach(() => { + client = new ApolloClient({ + cache: new InMemoryCache, + link: new SchemaLink({ + schema: makeExecutableSchema({ + typeDefs, + resolvers, + }), + }), + }); + }); + + afterEach(() => { + const storeReader = (client.cache as InMemoryCache)["storeReader"]; + expect(storeReader["executeSubSelectedArray"].size).toBeGreaterThan(0); + expect(storeReader["executeSelectionSet"].size).toBeGreaterThan(0); + client.cache.evict({ + id: "ROOT_QUERY", + }); + client.cache.gc(); + expect(storeReader["executeSubSelectedArray"].size).toEqual(0); + expect(storeReader["executeSelectionSet"].size).toEqual(0); + }); + + it(`empties all result caches after eviction - query`, async () => { + await client.query({ + query, + variables: { id: 1 }, + }); + }); + + it(`empties all result caches after eviction - watchQuery`, async () => { + return new Promise((r) => { + const observable = client.watchQuery({ + query, + variables: { id: 1 }, + }); + const unsubscribe = observable.subscribe(() => { + unsubscribe.unsubscribe(); + r(); + }); + }); + }); +}); diff --git a/src/cache/inmemory/entityStore.ts b/src/cache/inmemory/entityStore.ts index 6dc3124df19..5bf4792e026 100644 --- a/src/cache/inmemory/entityStore.ts +++ b/src/cache/inmemory/entityStore.ts @@ -534,7 +534,17 @@ class CacheGroup { public dirty(dataId: string, storeFieldName: string) { if (this.d) { - this.d.dirty(makeDepKey(dataId, storeFieldName)); + this.d.dirty( + makeDepKey(dataId, storeFieldName), + // When storeFieldName === "__exists", that means the entity identified + // by dataId has either disappeared from the cache or was newly added, + // so the result caching system would do well to "forget everything it + // knows" about that object. To achieve that kind of invalidation, we + // not only dirty the associated result cache entry, but also remove it + // completely from the dependency graph. For the optimism implmentation + // details, see https://github.com/benjamn/optimism/pull/195. + storeFieldName === "__exists" ? "forget" : "setDirty", + ); } } @@ -550,6 +560,23 @@ function makeDepKey(dataId: string, storeFieldName: string) { return storeFieldName + '#' + dataId; } +export function maybeDependOnExistenceOfEntity( + store: NormalizedCache, + entityId: string, +) { + if (supportsResultCaching(store)) { + // We use this pseudo-field __exists elsewhere in the EntityStore code to + // represent changes in the existence of the entity object identified by + // entityId. This dependency gets reliably dirtied whenever an object with + // this ID is deleted (or newly created) within this group, so any result + // cache entries (for example, StoreReader#executeSelectionSet results) that + // depend on __exists for this entityId will get dirtied as well, leading to + // the eventual recomputation (instead of reuse) of those result objects the + // next time someone reads them from the cache. + store.group.depend(entityId, "__exists"); + } +} + export namespace EntityStore { // Refer to this class as EntityStore.Root outside this namespace. export class Root extends EntityStore { diff --git a/src/cache/inmemory/readFromStore.ts b/src/cache/inmemory/readFromStore.ts index 0ed395dddcb..2572c6c0607 100644 --- a/src/cache/inmemory/readFromStore.ts +++ b/src/cache/inmemory/readFromStore.ts @@ -31,7 +31,7 @@ import { NormalizedCache, ReadMergeModifyContext, } from './types'; -import { supportsResultCaching } from './entityStore'; +import { maybeDependOnExistenceOfEntity, supportsResultCaching } from './entityStore'; import { getTypenameFromStoreObject } from './helpers'; import { Policies } from './policies'; import { InMemoryCache } from './inMemoryCache'; @@ -70,12 +70,14 @@ function missingFromInvariant( type ExecSelectionSetOptions = { selectionSet: SelectionSetNode; objectOrReference: StoreObject | Reference; + enclosingRef: Reference; context: ReadContext; }; type ExecSubSelectedArrayOptions = { field: FieldNode; array: any[]; + enclosingRef: Reference; context: ReadContext; }; @@ -157,6 +159,11 @@ export class StoreReader { return other; } + maybeDependOnExistenceOfEntity( + options.context.store, + options.enclosingRef.__ref, + ); + // Finally, if we didn't find any useful previous results, run the real // execSelectionSetImpl method with the given options. return this.execSelectionSetImpl(options); @@ -179,6 +186,10 @@ export class StoreReader { }); this.executeSubSelectedArray = wrap((options: ExecSubSelectedArrayOptions) => { + maybeDependOnExistenceOfEntity( + options.context.store, + options.enclosingRef.__ref, + ); return this.execSubSelectedArrayImpl(options); }, { max: this.config.resultCacheMaxSize, @@ -216,9 +227,11 @@ export class StoreReader { ...variables!, }; + const rootRef = makeReference(rootId); const execResult = this.executeSelectionSet({ selectionSet: getMainDefinition(query).selectionSet, - objectOrReference: makeReference(rootId), + objectOrReference: rootRef, + enclosingRef: rootRef, context: { store, query, @@ -273,6 +286,7 @@ export class StoreReader { private execSelectionSetImpl({ selectionSet, objectOrReference, + enclosingRef, context, }: ExecSelectionSetOptions): ExecResult { if (isReference(objectOrReference) && @@ -364,6 +378,7 @@ export class StoreReader { fieldValue = handleMissing(this.executeSubSelectedArray({ field: selection, array: fieldValue, + enclosingRef, context, })); @@ -383,6 +398,7 @@ export class StoreReader { fieldValue = handleMissing(this.executeSelectionSet({ selectionSet: selection.selectionSet, objectOrReference: fieldValue as StoreObject | Reference, + enclosingRef: isReference(fieldValue) ? fieldValue : enclosingRef, context, })); } @@ -431,6 +447,7 @@ export class StoreReader { private execSubSelectedArrayImpl({ field, array, + enclosingRef, context, }: ExecSubSelectedArrayOptions): ExecResult { let missing: MissingFieldError[] | undefined; @@ -463,6 +480,7 @@ export class StoreReader { return handleMissing(this.executeSubSelectedArray({ field, array: item, + enclosingRef, context, }), i); } @@ -472,6 +490,7 @@ export class StoreReader { return handleMissing(this.executeSelectionSet({ selectionSet: field.selectionSet, objectOrReference: item, + enclosingRef: isReference(item) ? item : enclosingRef, context, }), i); }