InMemoryCache is deep freezing results

[narciero]

Intended outcome:

Data can be mutated (not ideal, but some 3rd party libs rely on being able to mutate objects).

It would nice if there was a flag to disable object freezing.

Actual outcome:

TypeError: Cannot add property X, object is not extensible

How to reproduce the issue:

new ApolloClient({
  cache: new InMemoryCache(),
  link: new HttpLink({ uri: process.env.REACT_APP_GRAPHQL_URL }),
})

const { loading, data, error } = useQuery<Query>(QUERY);
data.result.X = '';

Versions

  System:
    OS: macOS 10.15.3
  Binaries:
    Node: 12.14.0
    Yarn: 1.22.0
    npm: 6.13.4
  Browsers:
    Chrome: 80.0.3987.116
    Safari: 13.0.5
  npmPackages:
    @apollo/client: ^3.0.0-beta.37 => 3.0.0-beta.37 
    @apollo/link-context: ^2.0.0-beta.3 => 2.0.0-beta.3 
    apollo: ^2.23.0 => 2.23.0

[dylanwulf]

This change was announced back in June: https://blog.apollographql.com/whats-new-in-apollo-client-2-6-b3acf28ecad1

[narciero]

@dylanwulf Yea but it seems the freezeResults flag was removed from the InMemoryCache ctor?

From the article:

In Apollo Client 3.0, we intend to make both freezeResults and assumeImmutableResults true by default

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

[benjamn]

See https://github.com/apollographql/apollo-client/pull/5153.

If you have to provide mutable data to external libs, you should deep-clone it first. Even just JSON.parse(JSON.stringify(data)) will work here, since GraphQL response data is inherently JSON-serializable.

The freezing is necessary because cache results can be shared between multiple consumers, so letting one of them modify the result is unacceptable.

Also, the freezing happens only in development, so it has no production performance penalty, in case you were concerned about that.

[dhazeley]

See #5153.

If you have to provide mutable data to external libs, you should deep-clone it first. Even just JSON.parse(JSON.stringify(data)) will work here, since GraphQL response data is inherently JSON-serializable.

The freezing is necessary because cache results can be shared between multiple consumers, so letting one of them modify the result is unacceptable.

Also, the freezing happens only in development, so it has no production performance penalty, in case you were concerned about that.

Is it possible to disable this feature in development? I am integrating Snipcart into my website and for an unknown reason maybeDeepFreeze will cause errors in development, but not production. If this is not a pattern that is forced in production, is there a reason that it's being forced in development?

[dylanwulf]

@dhazeley I don't think it's possible to disable it. It's true that the deep freeze is disabled in production, but that's only because of performance concerns. If deep freeze didn't negatively impact performance, then it would be enforced in both production and development

[Fi1osof]

Why this required in development mode, if this not using in production mode? I got this error on momentjsField.format('lll') (((

[diesieben07]

I do understand the necessity for immutability in results. However if you have a custom field policy which produces objects with an already immutable API (such as a Luxon DateTime) this freeze is unnecessary and causes problems. While a Luxon DateTime has an immutable API, it has mutable internal state for the purposes of caching. This mutability is not a bad thing and does not need to be prevented for the purposes of Apollo. I propose adding an option to disable freezing for certain types of objects only (instead of flat-out disabling it). This could be accomplished with the help of a "freezing strategy", so you can provide a different implementation for freezing instead of just Object.freeze.

[diesieben07]

In my opinion this issue should be reopened. There needs to be a way to opt-out of this behavior for types with an immutable API (as mentioned above). I propose adding a Symbol, which can be used to exclude objects or classes from maybeDeepFreeze. One could then say (for example): DateTime[isImmutable] = true, which would prevent it from being frozen by Apollo. I think this strikes a good balance between not allowing people to simply bypass this (necessary) mechanism entirely but also not hampering the usability for custom types inside the cache.

It would be nice to get an official stance on this problem before I open a pull request.