Avoid costly cloneDeep snapshots when cache results are known to be immutable.

[benjamn]

The cloneDeep helper from apollo-utilities has begun showing up prominently in some performance profiles (#4464), because we were forced to start taking snapshots of previous results to guard against destructive modifications (#4069).

While we cannot mandate immutable data handling for all application code that interacts with Apollo Client, we can give developers the tools to eliminate the runtime cost of using cloneDeep, which is incurred to defend against destructive modifications of cache results. If we don't have to worry about destructive updates, we can skip calling cloneDeep. Some cooperation from the application developer is required to achieve this goal, and that's what this PR enables.

This new (highly recommended) strategy has two parts:

const client = new ApolloClient({
  link: ...,
  cache: new InMemoryCache({
    freezeResults: true, // new
  }),
  assumeImmutableResults: true, // new
});

1. Thanks to Support InMemoryCache({ freezeResults: true }) to help enforce immutability. #4514, when you create an instance of InMemoryCache, you can instruct it to call Object.freeze (in development only) against all the result objects that it creates, which effectively prevents any destructive mutations of those objects. If you have been relying on mutability of cache results, you may need to update some of your code, but the freezeResults option should help you find the problematic spots, because modifications of frozen objects throw an exception (in strict mode). Since the freezing happens only in development, this new functionality has zero runtime cost in production.

2. Once you know that your application code is no longer modifying cache results destructively, you can unlock significant performance improvements by passing assumeImmutableResults: true to the ApolloClient constructor. Once you have agreed not to modify cache results, the client can avoid using cloneDeep to take snapshots of previous results, and identical result (sub)trees will often be === to each other, so your UI rendering code can immediately detect when no changes have happened.

What do we mean by destructive modifications? For example, the following code destructively modifies a result object read from the cache:

const data = client.readQuery({ query });

const myNewTodo = {
  id: '6',
  text: 'Start using Apollo Client.',
  completed: false,
};

data.todos.push(myNewTodo);

client.writeQuery({ query, data });

While this works, Apollo Client has to do a lot of extra work to compensate for the possibility that nested result data might change at any time.

To rewrite this code in a non-destructive style, you should create a new data.todos list that includes the old todos plus myNewTodo:

const data = client.readQuery({ query });

const myNewTodo = {
  id: '6',
  text: 'Start using Apollo Client.',
  completed: false,
};

client.writeQuery({
  query,
  data: {
    todos: [...data.todos, myNewTodo],
  },
});

Both styles will work, but only the second style is compatible with the { assumeImmutableResults: true } option. When you're using new InMemoryCache({ freezeResults }), the data.todos.push(myNewTodo) expression will throw an exception in development (in strict mode), because the data.todos object will be frozen.

[hwillson]

Looks awesome @benjamn!

[hwillson]

I'm really happy to see this, but do you think we need to worry about this potentially impacting backcompat? People really shouldn't be setting their own queryManager, but we've unfortunately allowed it for a while.

[benjamn]

@danilobuerger Any thoughts on how risky adding readonly might be?

[danilobuerger]

What @hwillson said, they shouldn't be setting it, but they might have. But since its just a typing change, its still settable if someone really wanted to. So I don't see an issue with that.

[benjamn]

After sleeping on it (and doing a code search for initQueryManager and queryManager), here's my assessment of the use cases:

• By far the most common use case for calling client.initQueryManager() is simply to ensure client.queryManager exists so it can be examined before the first query happens (e.g. to save a reference to client.queryManager.queryStore). These changes render that effort unnecessary.
• Mistakenly calling initQueryManager in an attempt to reset client state. This still works as well as it ever did, because initQueryManager was previously idempotent, and now does nothing.
• Hypothetical use cases that I haven't actually found in the wild:
  • Setting client.queryManager = null and later calling client.initQueryManager() to recreate it. This no longer works because initQueryManager won't recreate the QueryManager now, so adding readonly to client.queryManager seems like a helpful signal to reconsider this pattern.
  • Saving client.queryManager somewhere, setting it to null temporarily, and then restoring it to the old value later. This still works, as long as you circumvent the readonly restriction with client as any, which seems like a reasonable "I know what I'm doing" requirement.

[danilobuerger]

Good assessment, the only other thing I could come up with is mocking it away for some reason in tests. (However, I don't know why one would want to do that)

[danilobuerger]

This PR sounds fantastic @benjamn ! Could we get a snapshot release of this to get more eyes on testing the performance impact?