From 48ea511d2485f6597dfa3e2f3dffc362373d2d2e Mon Sep 17 00:00:00 2001 From: Ben Newman Date: Fri, 1 Mar 2019 17:25:35 -0500 Subject: [PATCH 1/4] Create QueryManager in ApolloClient constructor. Creating the QueryManager lazily has limited value, since almost nothing ApolloClient needs to do can happen without the QueryManager. This change cuts 183 bytes from the minified size of the package (60 bytes after gzip). --- packages/apollo-client/src/ApolloClient.ts | 105 +++++++----------- .../apollo-client/src/__tests__/client.ts | 18 --- 2 files changed, 39 insertions(+), 84 deletions(-) diff --git a/packages/apollo-client/src/ApolloClient.ts b/packages/apollo-client/src/ApolloClient.ts index 3aa67c5ebee..c7541138a86 100644 --- a/packages/apollo-client/src/ApolloClient.ts +++ b/packages/apollo-client/src/ApolloClient.ts @@ -72,7 +72,7 @@ export default class ApolloClient implements DataProxy { public link: ApolloLink; public store: DataStore; public cache: ApolloCache; - public queryManager: QueryManager | undefined; + public readonly queryManager: QueryManager; public disableNetworkFetches: boolean; public version: string; public queryDeduplication: boolean; @@ -80,11 +80,8 @@ export default class ApolloClient implements DataProxy { public readonly typeDefs: ApolloClientOptions['typeDefs']; private devToolsHookCb: Function; - private proxy: ApolloCache | undefined; - private ssrMode: boolean; private resetStoreCallbacks: Array<() => Promise> = []; private clearStoreCallbacks: Array<() => Promise> = []; - private clientAwareness: Record = {}; private localState: LocalState; /** @@ -166,7 +163,6 @@ export default class ApolloClient implements DataProxy { this.store = new DataStore(cache); this.disableNetworkFetches = ssrMode || ssrForceFetchDelay > 0; this.queryDeduplication = queryDeduplication; - this.ssrMode = ssrMode; this.defaultOptions = defaultOptions || {}; this.typeDefs = typeDefs; @@ -231,20 +227,36 @@ export default class ApolloClient implements DataProxy { this.version = version; - if (clientAwarenessName) { - this.clientAwareness.name = clientAwarenessName; - } - - if (clientAwarenessVersion) { - this.clientAwareness.version = clientAwarenessVersion; - } - this.localState = new LocalState({ cache, client: this, resolvers, fragmentMatcher, }); + + this.queryManager = new QueryManager({ + link: this.link, + store: this.store, + queryDeduplication, + ssrMode, + clientAwareness: { + name: clientAwarenessName!, + version: clientAwarenessVersion!, + }, + localState: this.localState, + onBroadcast: () => { + if (this.devToolsHookCb) { + this.devToolsHookCb({ + action: {}, + state: { + queries: this.queryManager.queryStore.getStore(), + mutations: this.queryManager.mutationStore.getStore(), + }, + dataWithOptimisticResults: this.cache.extract(true), + }); + } + }, + }); } /** @@ -295,7 +307,7 @@ export default class ApolloClient implements DataProxy { options = { ...options, fetchPolicy: 'cache-first' }; } - return this.initQueryManager().watchQuery(options); + return this.queryManager.watchQuery(options); } /** @@ -327,7 +339,7 @@ export default class ApolloClient implements DataProxy { options = { ...options, fetchPolicy: 'cache-first' }; } - return this.initQueryManager().query(options); + return this.queryManager.query(options); } /** @@ -347,7 +359,7 @@ export default class ApolloClient implements DataProxy { } as MutationOptions; } - return this.initQueryManager().mutate(options); + return this.queryManager.mutate(options); } /** @@ -357,7 +369,7 @@ export default class ApolloClient implements DataProxy { public subscribe( options: SubscriptionOptions, ): Observable { - return this.initQueryManager().startGraphQLSubscription(options); + return this.queryManager.startGraphQLSubscription(options); } /** @@ -373,7 +385,7 @@ export default class ApolloClient implements DataProxy { options: DataProxy.Query, optimistic: boolean = false, ): T | null { - return this.initProxy().readQuery(options, optimistic); + return this.cache.readQuery(options, optimistic); } /** @@ -394,7 +406,7 @@ export default class ApolloClient implements DataProxy { options: DataProxy.Fragment, optimistic: boolean = false, ): T | null { - return this.initProxy().readFragment(options, optimistic); + return this.cache.readFragment(options, optimistic); } /** @@ -405,8 +417,8 @@ export default class ApolloClient implements DataProxy { public writeQuery( options: DataProxy.WriteQueryOptions, ): void { - const result = this.initProxy().writeQuery(options); - this.initQueryManager().broadcastQueries(); + const result = this.cache.writeQuery(options); + this.queryManager.broadcastQueries(); return result; } @@ -424,8 +436,8 @@ export default class ApolloClient implements DataProxy { public writeFragment( options: DataProxy.WriteFragmentOptions, ): void { - const result = this.initProxy().writeFragment(options); - this.initQueryManager().broadcastQueries(); + const result = this.cache.writeFragment(options); + this.queryManager.broadcastQueries(); return result; } @@ -442,8 +454,8 @@ export default class ApolloClient implements DataProxy { public writeData( options: DataProxy.WriteDataOptions, ): void { - const result = this.initProxy().writeData(options); - this.initQueryManager().broadcastQueries(); + const result = this.cache.writeData(options); + this.queryManager.broadcastQueries(); return result; } @@ -459,32 +471,6 @@ export default class ApolloClient implements DataProxy { * This initializes the query manager that tracks queries and the cache */ public initQueryManager(): QueryManager { - if (!this.queryManager) { - this.queryManager = new QueryManager({ - link: this.link, - store: this.store, - queryDeduplication: this.queryDeduplication, - ssrMode: this.ssrMode, - clientAwareness: this.clientAwareness, - localState: this.localState, - onBroadcast: () => { - if (this.devToolsHookCb) { - this.devToolsHookCb({ - action: {}, - state: { - queries: this.queryManager - ? this.queryManager.queryStore.getStore() - : {}, - mutations: this.queryManager - ? this.queryManager.mutationStore.getStore() - : {}, - }, - dataWithOptimisticResults: this.cache.extract(true), - }); - } - }, - }); - } return this.queryManager; } @@ -581,7 +567,7 @@ export default class ApolloClient implements DataProxy { * Exposes the cache's complete state, in a serializable format for later restoration. */ public extract(optimistic?: boolean): TCacheShape { - return this.initProxy().extract(optimistic); + return this.cache.extract(optimistic); } /** @@ -592,7 +578,7 @@ export default class ApolloClient implements DataProxy { * and also (potentially) during hot reloads. */ public restore(serializedState: TCacheShape): ApolloCache { - return this.initProxy().restore(serializedState); + return this.cache.restore(serializedState); } /** @@ -622,17 +608,4 @@ export default class ApolloClient implements DataProxy { public setLocalStateFragmentMatcher(fragmentMatcher: FragmentMatcher) { this.localState.setFragmentMatcher(fragmentMatcher); } - - /** - * Initializes a data proxy for this client instance if one does not already - * exist and returns either a previously initialized proxy instance or the - * newly initialized instance. - */ - private initProxy(): ApolloCache { - if (!this.proxy) { - this.initQueryManager(); - this.proxy = this.cache; - } - return this.proxy; - } } diff --git a/packages/apollo-client/src/__tests__/client.ts b/packages/apollo-client/src/__tests__/client.ts index 2d3b477e264..57e1df88e3a 100644 --- a/packages/apollo-client/src/__tests__/client.ts +++ b/packages/apollo-client/src/__tests__/client.ts @@ -22,20 +22,6 @@ import { withWarning } from '../util/wrap'; import { mockSingleLink } from '../__mocks__/mockLinks'; describe('client', () => { - it('creates query manager lazily', () => { - const client = new ApolloClient({ - link: ApolloLink.empty(), - cache: new InMemoryCache(), - }); - - expect(client.queryManager).toBeUndefined(); - - // We only create the query manager on the first query - client.initQueryManager(); - expect(client.queryManager).toBeDefined(); - expect(client.cache).toBeDefined(); - }); - it('can be loaded via require', () => { /* tslint:disable */ const ApolloClientRequire = require('../').default; @@ -46,10 +32,6 @@ describe('client', () => { cache: new InMemoryCache(), }); - expect(client.queryManager).toBeUndefined(); - - // We only create the query manager on the first query - client.initQueryManager(); expect(client.queryManager).toBeDefined(); expect(client.cache).toBeDefined(); }); From 29c0fa18b923153d45945d22b3011f179ce4d502 Mon Sep 17 00:00:00 2001 From: Ben Newman Date: Fri, 1 Mar 2019 17:27:38 -0500 Subject: [PATCH 2/4] Avoid costly cloneDeep calls if { assumeImmutableResults: true }. Part of the plan I outlined in this comment: https://github.com/apollographql/apollo-client/issues/4464#issuecomment-467548798 Passing { assumeImmutableResults: true } to the ApolloClient constructor should probably always be accompanied by passing { freezeResults: true } to the InMemoryCache constructor (see #4514), though of course the use of InMemoryCache is optional, and other cache implementations may not support that option. --- packages/apollo-client/src/ApolloClient.ts | 9 ++ .../apollo-client/src/core/ObservableQuery.ts | 6 +- .../apollo-client/src/core/QueryManager.ts | 4 + .../src/core/__tests__/ObservableQuery.ts | 102 +++++++++++++++++- 4 files changed, 118 insertions(+), 3 deletions(-) diff --git a/packages/apollo-client/src/ApolloClient.ts b/packages/apollo-client/src/ApolloClient.ts index c7541138a86..25d34cabcb8 100644 --- a/packages/apollo-client/src/ApolloClient.ts +++ b/packages/apollo-client/src/ApolloClient.ts @@ -55,6 +55,7 @@ export type ApolloClientOptions = { connectToDevTools?: boolean; queryDeduplication?: boolean; defaultOptions?: DefaultOptions; + assumeImmutableResults?: boolean; resolvers?: Resolvers | Resolvers[]; typeDefs?: string | string[] | DocumentNode | DocumentNode[]; fragmentMatcher?: FragmentMatcher; @@ -103,6 +104,12 @@ export default class ApolloClient implements DataProxy { * options supplied to `watchQuery`, `query`, or * `mutate`. * + * @param assumeImmutableResults When this option is true, the client will assume results + * read from the cache are never mutated by application code, + * which enables substantial performance optimizations. Passing + * `{ freezeResults: true }` to the `InMemoryCache` constructor + * can help enforce this immutability. + * * @param name A custom name that can be used to identify this client, when * using Apollo client awareness features. E.g. "iOS". * @@ -120,6 +127,7 @@ export default class ApolloClient implements DataProxy { connectToDevTools, queryDeduplication = true, defaultOptions, + assumeImmutableResults = false, resolvers, typeDefs, fragmentMatcher, @@ -244,6 +252,7 @@ export default class ApolloClient implements DataProxy { version: clientAwarenessVersion!, }, localState: this.localState, + assumeImmutableResults, onBroadcast: () => { if (this.devToolsHookCb) { this.devToolsHookCb({ diff --git a/packages/apollo-client/src/core/ObservableQuery.ts b/packages/apollo-client/src/core/ObservableQuery.ts index 34ad1a644e4..92d421a4556 100644 --- a/packages/apollo-client/src/core/ObservableQuery.ts +++ b/packages/apollo-client/src/core/ObservableQuery.ts @@ -250,7 +250,8 @@ export class ObservableQuery< if (!partial) { this.lastResult = { ...result, stale: false }; - this.lastResultSnapshot = cloneDeep(this.lastResult); + this.lastResultSnapshot = this.queryManager.assumeImmutableResults + ? this.lastResult : cloneDeep(this.lastResult); } return { ...result, partial }; @@ -611,7 +612,8 @@ export class ObservableQuery< const observer: Observer> = { next: (result: ApolloQueryResult) => { this.lastResult = result; - this.lastResultSnapshot = cloneDeep(result); + this.lastResultSnapshot = this.queryManager.assumeImmutableResults + ? result : cloneDeep(result); this.observers.forEach(obs => obs.next && obs.next(result)); }, error: (error: ApolloError) => { diff --git a/packages/apollo-client/src/core/QueryManager.ts b/packages/apollo-client/src/core/QueryManager.ts index e3f8765db18..3e89c572f42 100644 --- a/packages/apollo-client/src/core/QueryManager.ts +++ b/packages/apollo-client/src/core/QueryManager.ts @@ -57,6 +57,7 @@ export class QueryManager { public mutationStore: MutationStore = new MutationStore(); public queryStore: QueryStore = new QueryStore(); public dataStore: DataStore; + public readonly assumeImmutableResults: boolean; private deduplicator: ApolloLink; private queryDeduplication: boolean; @@ -94,6 +95,7 @@ export class QueryManager { ssrMode = false, clientAwareness = {}, localState, + assumeImmutableResults, }: { link: ApolloLink; queryDeduplication?: boolean; @@ -102,6 +104,7 @@ export class QueryManager { ssrMode?: boolean; clientAwareness?: Record; localState?: LocalState; + assumeImmutableResults?: boolean; }) { this.link = link; this.deduplicator = ApolloLink.from([new Deduplicator(), link]); @@ -111,6 +114,7 @@ export class QueryManager { this.clientAwareness = clientAwareness; this.localState = localState || new LocalState({ cache: store.getCache() }); this.ssrMode = ssrMode; + this.assumeImmutableResults = !!assumeImmutableResults; } /** diff --git a/packages/apollo-client/src/core/__tests__/ObservableQuery.ts b/packages/apollo-client/src/core/__tests__/ObservableQuery.ts index f21d93fcb8b..9e1b23495ff 100644 --- a/packages/apollo-client/src/core/__tests__/ObservableQuery.ts +++ b/packages/apollo-client/src/core/__tests__/ObservableQuery.ts @@ -49,7 +49,11 @@ describe('ObservableQuery', () => { const createQueryManager = ({ link }: { link?: ApolloLink }) => { return new QueryManager({ link: link || mockSingleLink(), - store: new DataStore(new InMemoryCache({ addTypename: false })), + assumeImmutableResults: true, + store: new DataStore(new InMemoryCache({ + addTypename: false, + freezeResults: true, + })), }); }; @@ -1690,6 +1694,102 @@ describe('ObservableQuery', () => { }); }); + describe('assumeImmutableResults', () => { + it('should prevent costly (but safe) cloneDeep calls', async () => { + const queryOptions = { + query: gql` + query { + value + } + `, + pollInterval: 20, + }; + + function check({ assumeImmutableResults, freezeResults }) { + const client = new ApolloClient({ + link: mockSingleLink( + { request: queryOptions, result: { data: { value: 1 } } }, + { request: queryOptions, result: { data: { value: 2 } } }, + { request: queryOptions, result: { data: { value: 3 } } }, + ), + assumeImmutableResults, + cache: new InMemoryCache({ freezeResults }), + }); + + const observable = client.watchQuery(queryOptions); + const values = []; + + return new Promise((resolve, reject) => { + observable.subscribe({ + next(result) { + values.push(result.data.value); + try { + result.data.value = 'oyez'; + } catch (error) { + reject(error); + } + client.writeData(result); + }, + error(err) { + expect(err.message).toMatch(/No more mocked responses/); + resolve(values); + }, + }); + }); + } + + // When we assume immutable results, the next method will not fire as a + // result of destructively modifying result.data.value, because the data + // object is still === to the previous object. This behavior might seem + // like a bug, if you are relying on the mutability of results, but the + // cloneDeep calls required to prevent that bug are expensive. Assuming + // immutability is safe only when you write your code in an immutable + // style, but the benefits are well worth the extra effort. + expect( + await check({ + assumeImmutableResults: true, + freezeResults: false, + }), + ).toEqual([1, 2, 3]); + + // When we do not assume immutable results, the observable must do + // extra work to take snapshots of past results, just in case those + // results are destructively modified. The benefit of that work is + // that such mutations can be detected, which is why "oyez" appears + // in the list of values here. This is a somewhat indirect way of + // detecting that cloneDeep must have been called, but at least it + // doesn't violate any abstractions. + expect( + await check({ + assumeImmutableResults: false, + freezeResults: false, + }), + ).toEqual([1, 'oyez', 2, 'oyez', 3, 'oyez']); + + async function checkThrows(assumeImmutableResults) { + try { + await check({ + assumeImmutableResults, + // No matter what value we provide for assumeImmutableResults, if we + // tell the InMemoryCache to deep-freeze its results, destructive + // modifications of the result objects will become fatal. Once you + // start enforcing immutability in this way, you might as well pass + // assumeImmutableResults: true, to prevent calling cloneDeep. + freezeResults: true, + }); + throw new Error('not reached'); + } catch (error) { + expect(error).toBeInstanceOf(TypeError); + expect(error.message).toMatch( + /Cannot assign to read only property 'value'/, + ); + } + } + await checkThrows(true); + await checkThrows(false); + }); + }); + describe('stopPolling', () => { it('does not restart polling after stopping and resubscribing', done => { const observable = mockWatchQuery( From b0a9e4c5e011326d587a4893f347816954c70c1d Mon Sep 17 00:00:00 2001 From: Ben Newman Date: Wed, 6 Mar 2019 13:23:41 -0500 Subject: [PATCH 3/4] Update apollo-boost test that relied on private clientAwareness property. --- packages/apollo-boost/src/__tests__/config.ts | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/packages/apollo-boost/src/__tests__/config.ts b/packages/apollo-boost/src/__tests__/config.ts index 5b0352ac4bc..74ac8dee566 100644 --- a/packages/apollo-boost/src/__tests__/config.ts +++ b/packages/apollo-boost/src/__tests__/config.ts @@ -145,8 +145,9 @@ describe('config', () => { version, }); - expect(client.clientAwareness.name).toEqual(name); - expect(client.clientAwareness.version).toEqual(version); + const { clientAwareness } = client.queryManager as any; + expect(clientAwareness.name).toEqual(name); + expect(clientAwareness.version).toEqual(version); }); const makePromise = res => From 1e740befdc4af214a007eb4b36022467bacf8fb0 Mon Sep 17 00:00:00 2001 From: Ben Newman Date: Wed, 6 Mar 2019 15:48:20 -0500 Subject: [PATCH 4/4] Use invariant.warn to forecast initQueryManager deprecation. --- packages/apollo-client/src/ApolloClient.ts | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/packages/apollo-client/src/ApolloClient.ts b/packages/apollo-client/src/ApolloClient.ts index 25d34cabcb8..d90d305c011 100644 --- a/packages/apollo-client/src/ApolloClient.ts +++ b/packages/apollo-client/src/ApolloClient.ts @@ -480,6 +480,10 @@ export default class ApolloClient implements DataProxy { * This initializes the query manager that tracks queries and the cache */ public initQueryManager(): QueryManager { + invariant.warn( + 'Calling the initQueryManager method is no longer necessary, ' + + 'and it will be removed from ApolloClient in version 3.0.', + ); return this.queryManager; }