Skip to content

Commit

Permalink
Avoid costly cloneDeep calls if { assumeImmutableResults: true }.
Browse files Browse the repository at this point in the history
Part of the plan I outlined in this comment:
#4464 (comment)

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.
  • Loading branch information
benjamn committed Mar 6, 2019
1 parent 48ea511 commit 29c0fa1
Show file tree
Hide file tree
Showing 4 changed files with 118 additions and 3 deletions.
9 changes: 9 additions & 0 deletions packages/apollo-client/src/ApolloClient.ts
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ export type ApolloClientOptions<TCacheShape> = {
connectToDevTools?: boolean;
queryDeduplication?: boolean;
defaultOptions?: DefaultOptions;
assumeImmutableResults?: boolean;
resolvers?: Resolvers | Resolvers[];
typeDefs?: string | string[] | DocumentNode | DocumentNode[];
fragmentMatcher?: FragmentMatcher;
Expand Down Expand Up @@ -103,6 +104,12 @@ export default class ApolloClient<TCacheShape> 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".
*
Expand All @@ -120,6 +127,7 @@ export default class ApolloClient<TCacheShape> implements DataProxy {
connectToDevTools,
queryDeduplication = true,
defaultOptions,
assumeImmutableResults = false,
resolvers,
typeDefs,
fragmentMatcher,
Expand Down Expand Up @@ -244,6 +252,7 @@ export default class ApolloClient<TCacheShape> implements DataProxy {
version: clientAwarenessVersion!,
},
localState: this.localState,
assumeImmutableResults,
onBroadcast: () => {
if (this.devToolsHookCb) {
this.devToolsHookCb({
Expand Down
6 changes: 4 additions & 2 deletions packages/apollo-client/src/core/ObservableQuery.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 };
Expand Down Expand Up @@ -611,7 +612,8 @@ export class ObservableQuery<
const observer: Observer<ApolloQueryResult<TData>> = {
next: (result: ApolloQueryResult<TData>) => {
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) => {
Expand Down
4 changes: 4 additions & 0 deletions packages/apollo-client/src/core/QueryManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ export class QueryManager<TStore> {
public mutationStore: MutationStore = new MutationStore();
public queryStore: QueryStore = new QueryStore();
public dataStore: DataStore<TStore>;
public readonly assumeImmutableResults: boolean;

private deduplicator: ApolloLink;
private queryDeduplication: boolean;
Expand Down Expand Up @@ -94,6 +95,7 @@ export class QueryManager<TStore> {
ssrMode = false,
clientAwareness = {},
localState,
assumeImmutableResults,
}: {
link: ApolloLink;
queryDeduplication?: boolean;
Expand All @@ -102,6 +104,7 @@ export class QueryManager<TStore> {
ssrMode?: boolean;
clientAwareness?: Record<string, string>;
localState?: LocalState<TStore>;
assumeImmutableResults?: boolean;
}) {
this.link = link;
this.deduplicator = ApolloLink.from([new Deduplicator(), link]);
Expand All @@ -111,6 +114,7 @@ export class QueryManager<TStore> {
this.clientAwareness = clientAwareness;
this.localState = localState || new LocalState({ cache: store.getCache() });
this.ssrMode = ssrMode;
this.assumeImmutableResults = !!assumeImmutableResults;
}

/**
Expand Down
102 changes: 101 additions & 1 deletion packages/apollo-client/src/core/__tests__/ObservableQuery.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
})),
});
};

Expand Down Expand Up @@ -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<any[]>((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(
Expand Down

0 comments on commit 29c0fa1

Please sign in to comment.