From 53cfa80cb6853f445421d6773a5c24f78966f50d Mon Sep 17 00:00:00 2001 From: Ben Newman Date: Thu, 12 Sep 2019 14:20:53 -0400 Subject: [PATCH] Stop calling addTypenameToDocument before InMemoryCache reads/writes. Adding __typename fields to the query by calling addTypenameToDocument used to be necessary for writing results to the cache, but now __typename fields in result objects are automatically written to the cache, so no transformation is necessary. ApolloClient still calls cache.transformDocument to get a query with __typename fields to send to the server, and the cache still relies on results from the server coming back with __typename fields, but the cache no longer needs the query to be transformed to include __typename. Compared to PR #5311, this commit should not be a breaking change. --- .../__snapshots__/ApolloClient.ts.snap | 1 + .../__tests__/__snapshots__/cache.ts.snap | 20 ++++++++++++ .../inmemory/__tests__/diffAgainstStore.ts | 9 ++++++ src/cache/inmemory/__tests__/readFromStore.ts | 6 ++++ src/cache/inmemory/__tests__/writeToStore.ts | 20 ++++++++++-- src/cache/inmemory/inMemoryCache.ts | 25 ++++++++------- src/cache/inmemory/readFromStore.ts | 31 ++++++++++++++----- src/cache/inmemory/writeToStore.ts | 16 +++++----- src/utilities/storeUtils.ts | 3 ++ 9 files changed, 101 insertions(+), 30 deletions(-) diff --git a/src/__tests__/__snapshots__/ApolloClient.ts.snap b/src/__tests__/__snapshots__/ApolloClient.ts.snap index 7e1f778fd06..08e5848b250 100644 --- a/src/__tests__/__snapshots__/ApolloClient.ts.snap +++ b/src/__tests__/__snapshots__/ApolloClient.ts.snap @@ -37,6 +37,7 @@ Object { "h": 9, }, "bar:foobar": Object { + "__typename": "bar", "e": 5, "f": 6, "id": "foobar", diff --git a/src/cache/inmemory/__tests__/__snapshots__/cache.ts.snap b/src/cache/inmemory/__tests__/__snapshots__/cache.ts.snap index cd85b05f9a4..b03d1ba007a 100644 --- a/src/cache/inmemory/__tests__/__snapshots__/cache.ts.snap +++ b/src/cache/inmemory/__tests__/__snapshots__/cache.ts.snap @@ -6,6 +6,7 @@ Object { "i": 7, }, "foo": Object { + "__typename": "Foo", "e": 4, "h": Object { "__ref": "bar", @@ -22,6 +23,7 @@ Object { "k": 9, }, "foo": Object { + "__typename": "Foo", "e": 4, "f": 5, "g": 6, @@ -35,11 +37,13 @@ Object { exports[`Cache writeFragment will write some deeply nested data into the store at any id (1/2) 3`] = ` Object { "bar": Object { + "__typename": "Bar", "i": 10, "j": 8, "k": 9, }, "foo": Object { + "__typename": "Foo", "e": 4, "f": 5, "g": 6, @@ -53,11 +57,13 @@ Object { exports[`Cache writeFragment will write some deeply nested data into the store at any id (1/2) 4`] = ` Object { "bar": Object { + "__typename": "Bar", "i": 10, "j": 11, "k": 12, }, "foo": Object { + "__typename": "Foo", "e": 4, "f": 5, "g": 6, @@ -71,11 +77,13 @@ Object { exports[`Cache writeFragment will write some deeply nested data into the store at any id (1/2) 5`] = ` Object { "bar": Object { + "__typename": "Bar", "i": 7, "j": 8, "k": 9, }, "foo": Object { + "__typename": "Foo", "e": 4, "f": 5, "g": 6, @@ -89,11 +97,13 @@ Object { exports[`Cache writeFragment will write some deeply nested data into the store at any id (1/2) 6`] = ` Object { "bar": Object { + "__typename": "Bar", "i": 10, "j": 11, "k": 12, }, "foo": Object { + "__typename": "Foo", "e": 4, "f": 5, "g": 6, @@ -110,6 +120,7 @@ Object { "i": 7, }, "foo": Object { + "__typename": "Foo", "e": 4, "h": Object { "__ref": "bar", @@ -126,6 +137,7 @@ Object { "k": 9, }, "foo": Object { + "__typename": "Foo", "e": 4, "f": 5, "g": 6, @@ -139,11 +151,13 @@ Object { exports[`Cache writeFragment will write some deeply nested data into the store at any id (2/2) 3`] = ` Object { "bar": Object { + "__typename": "Bar", "i": 10, "j": 8, "k": 9, }, "foo": Object { + "__typename": "Foo", "e": 4, "f": 5, "g": 6, @@ -157,11 +171,13 @@ Object { exports[`Cache writeFragment will write some deeply nested data into the store at any id (2/2) 4`] = ` Object { "bar": Object { + "__typename": "Bar", "i": 10, "j": 11, "k": 12, }, "foo": Object { + "__typename": "Foo", "e": 4, "f": 5, "g": 6, @@ -175,11 +191,13 @@ Object { exports[`Cache writeFragment will write some deeply nested data into the store at any id (2/2) 5`] = ` Object { "bar": Object { + "__typename": "Bar", "i": 7, "j": 8, "k": 9, }, "foo": Object { + "__typename": "Foo", "e": 4, "f": 5, "g": 6, @@ -193,11 +211,13 @@ Object { exports[`Cache writeFragment will write some deeply nested data into the store at any id (2/2) 6`] = ` Object { "bar": Object { + "__typename": "Bar", "i": 10, "j": 11, "k": 12, }, "foo": Object { + "__typename": "Foo", "e": 4, "f": 5, "g": 6, diff --git a/src/cache/inmemory/__tests__/diffAgainstStore.ts b/src/cache/inmemory/__tests__/diffAgainstStore.ts index 206574248a6..87ba55570dc 100644 --- a/src/cache/inmemory/__tests__/diffAgainstStore.ts +++ b/src/cache/inmemory/__tests__/diffAgainstStore.ts @@ -418,6 +418,7 @@ describe('diffing queries against the store', () => { expect(simpleDiff.result).toEqual({ people_one: { + __typename: 'Person', name: 'Luke Skywalker', }, }); @@ -429,6 +430,7 @@ describe('diffing queries against the store', () => { expect(inlineDiff.result).toEqual({ people_one: { + __typename: 'Person', name: 'Luke Skywalker', }, }); @@ -440,6 +442,7 @@ describe('diffing queries against the store', () => { expect(namedDiff.result).toEqual({ people_one: { + __typename: 'Person', name: 'Luke Skywalker', }, }); @@ -1085,24 +1088,30 @@ describe('diffing queries against the store', () => { expect(result).toEqual({ user: { + __typename: 'User', id: 1, name: 'Ben', company: { + __typename: 'Company', id: 1, name: 'Apollo', users: [ { + __typename: 'User', id: 1, name: 'Ben', company: { + __typename: 'Company', id: 1, name: 'Apollo', }, }, { + __typename: 'User', id: 2, name: 'James', company: { + __typename: 'Company', id: 1, name: 'Apollo', }, diff --git a/src/cache/inmemory/__tests__/readFromStore.ts b/src/cache/inmemory/__tests__/readFromStore.ts index d092a3f828e..641a6ac930b 100644 --- a/src/cache/inmemory/__tests__/readFromStore.ts +++ b/src/cache/inmemory/__tests__/readFromStore.ts @@ -62,6 +62,7 @@ describe('reading from the store', () => { }); expect(stripSymbols(queryResult)).toEqual({ + __typename: 'Query', nestedObj: { innerArray: [{ id: 'abcdef', someField: 3 }], }, @@ -356,6 +357,7 @@ describe('reading from the store', () => { // The result of the query shouldn't contain __data_id fields expect(stripSymbols(queryResult)).toEqual({ + __typename: 'Query', stringField: 'This is a string!', numberField: 5, nullField: null, @@ -837,23 +839,27 @@ describe('reading from the store', () => { }), ).toEqual({ author: { + __typename: 'Author', name: 'Toni Morrison', books: [ { title: 'The Bluest Eye', publisher: { + __typename: 'Publisher', name: 'Holt, Rinehart and Winston', }, }, { title: 'Song of Solomon', publisher: { + __typename: 'Publisher', name: 'Alfred A. Knopf, Inc.', }, }, { title: 'Beloved', publisher: { + __typename: 'Publisher', name: 'Alfred A. Knopf, Inc.', }, }, diff --git a/src/cache/inmemory/__tests__/writeToStore.ts b/src/cache/inmemory/__tests__/writeToStore.ts index 152ffda1138..a2d3220f2a0 100644 --- a/src/cache/inmemory/__tests__/writeToStore.ts +++ b/src/cache/inmemory/__tests__/writeToStore.ts @@ -1746,7 +1746,11 @@ describe('writing to the store', () => { ROOT_QUERY: { animals: [ { - species: { name: 'cat' }, + __typename: 'Animal', + species: { + __typename: 'Cat', + name: 'cat', + }, }, ], }, @@ -1772,7 +1776,11 @@ describe('writing to the store', () => { ROOT_QUERY: { animals: [ { - species: { name: 'dog' }, + __typename: 'Animal', + species: { + __typename: 'Dog', + name: 'dog', + }, }, ], }, @@ -1821,7 +1829,11 @@ describe('writing to the store', () => { ROOT_QUERY: { animals: [ { - species: { name: 'cat' }, + __typename: 'Animal', + species: { + __typename: 'Cat', + name: 'cat', + }, }, ], }, @@ -1848,11 +1860,13 @@ describe('writing to the store', () => { expect(store.toObject()).toEqual({ 'Dog__dog-species': { id: 'dog-species', + __typename: 'Dog', name: 'dog', }, ROOT_QUERY: { animals: [ { + __typename: 'Animal', species: makeReference('Dog__dog-species'), }, ], diff --git a/src/cache/inmemory/inMemoryCache.ts b/src/cache/inmemory/inMemoryCache.ts index 83dd15885cb..6db3cacfc6b 100644 --- a/src/cache/inmemory/inMemoryCache.ts +++ b/src/cache/inmemory/inMemoryCache.ts @@ -87,6 +87,7 @@ export class InMemoryCache extends ApolloCache { }); this.storeReader = new StoreReader({ + addTypename: this.addTypename, cacheKeyRoot: this.cacheKeyRoot, possibleTypes: this.possibleTypes, }); @@ -134,7 +135,7 @@ export class InMemoryCache extends ApolloCache { return this.storeReader.readQueryFromStore({ store: options.optimistic ? this.optimisticData : this.data, - query: this.transformDocument(options.query), + query: options.query, variables: options.variables, rootId: options.rootId, previousResult: options.previousResult, @@ -142,26 +143,26 @@ export class InMemoryCache extends ApolloCache { }) || null; } - public write(write: Cache.WriteOptions): void { + public write(options: Cache.WriteOptions): void { this.storeWriter.writeQueryToStore({ store: this.data, - query: this.transformDocument(write.query), - result: write.result, - dataId: write.dataId, - variables: write.variables, + query: options.query, + result: options.result, + dataId: options.dataId, + variables: options.variables, dataIdFromObject: this.config.dataIdFromObject, }); this.broadcastWatches(); } - public diff(query: Cache.DiffOptions): Cache.DiffResult { + public diff(options: Cache.DiffOptions): Cache.DiffResult { return this.storeReader.diffQueryAgainstStore({ - store: query.optimistic ? this.optimisticData : this.data, - query: this.transformDocument(query.query), - variables: query.variables, - returnPartialData: query.returnPartialData, - previousResult: query.previousResult, + store: options.optimistic ? this.optimisticData : this.data, + query: options.query, + variables: options.variables, + returnPartialData: options.returnPartialData, + previousResult: options.previousResult, config: this.config, }); } diff --git a/src/cache/inmemory/readFromStore.ts b/src/cache/inmemory/readFromStore.ts index 8f4ee183140..258c861b8df 100644 --- a/src/cache/inmemory/readFromStore.ts +++ b/src/cache/inmemory/readFromStore.ts @@ -93,25 +93,30 @@ type ExecSubSelectedArrayOptions = { type PossibleTypes = import('./inMemoryCache').InMemoryCache['possibleTypes']; export interface StoreReaderConfig { + addTypename?: boolean; cacheKeyRoot?: KeyTrie; possibleTypes?: PossibleTypes; } export class StoreReader { - private possibleTypes?: PossibleTypes; + private config: StoreReaderConfig; + + constructor(config?: StoreReaderConfig) { + const cacheKeyRoot = + config && config.cacheKeyRoot || new KeyTrie(canUseWeakMap); + + this.config = { + addTypename: true, + cacheKeyRoot, + ...config, + }; - constructor({ - cacheKeyRoot = new KeyTrie(canUseWeakMap), - possibleTypes, - }: StoreReaderConfig = {}) { const { executeStoreQuery, executeSelectionSet, executeSubSelectedArray, } = this; - this.possibleTypes = possibleTypes; - this.executeStoreQuery = wrap((options: ExecStoreQueryOptions) => { return executeStoreQuery.call(this, options); }, { @@ -316,6 +321,16 @@ export class StoreReader { typename = object && object.__typename; } + if (this.config.addTypename) { + const typenameFromStore = object && object.__typename; + if (typeof typenameFromStore === 'string') { + // Ensure we always include a default value for the __typename field, + // if we have one, and this.config.addTypename is true. Note that this + // field can be overridden by other merged objects. + objectsToMerge.push({ __typename: typenameFromStore }); + } + } + function handleMissing(result: ExecResult): T { if (result.missing) { finalResult.missing = finalResult.missing || []; @@ -358,7 +373,7 @@ export class StoreReader { const match = fragmentMatches( fragment, typename, - this.possibleTypes, + this.config.possibleTypes, ); if (match && (object || typename === 'Query')) { diff --git a/src/cache/inmemory/writeToStore.ts b/src/cache/inmemory/writeToStore.ts index 68c03119a8e..f26c90b476b 100644 --- a/src/cache/inmemory/writeToStore.ts +++ b/src/cache/inmemory/writeToStore.ts @@ -123,8 +123,9 @@ export class StoreWriter { private processSelectionSet({ result, selectionSet, - typename, context, + typename = getTypenameFromResult( + result, selectionSet, context.fragmentMap), }: { result: any; selectionSet: SelectionSetNode; @@ -176,12 +177,6 @@ export class StoreWriter { ); } } else { - // If the typename of the object we're processing was not provided, - // compute it lazily. - typename = - typename || - getTypenameFromResult(result, selectionSet, context.fragmentMap); - // This is not a field, so it must be a fragment, either inline or named const fragment = getFragmentFromSelection( selection, @@ -208,6 +203,13 @@ export class StoreWriter { } }); + if ( + typeof typename === 'string' && + newFields.__typename === void 0 + ) { + newFields.__typename = typename; + } + return newFields; } diff --git a/src/utilities/storeUtils.ts b/src/utilities/storeUtils.ts index c484e0de8ec..9bf27b5017b 100644 --- a/src/utilities/storeUtils.ts +++ b/src/utilities/storeUtils.ts @@ -284,6 +284,9 @@ export function getTypenameFromResult( } } } + if (typeof result.__typename === 'string') { + return result.__typename; + } } export function isField(selection: SelectionNode): selection is FieldNode {