Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Mutate update function DataProxy/ApolloCache changes #5956

Merged
merged 1 commit into from
Feb 18, 2020

Conversation

hwillson
Copy link
Member

This PR gives the mutate update function direct access to the cache, instead of using a scaled down DataProxy. While the DataProxy approach helped in the past as a way to batch cache updates, it is no longer necessary. We are only ever passing a cache instance into update, which means limiting the type to be a DataProxy gives us no additional advantages. The only other DataProxy implementor in the AC codebase is ApolloClient, which is never passed to the update function. Also, if we keep using DataProxy in the mutate update function, then every method we want to expose from the cache through DataProxy has to be also added to ApolloClient. This doesn't always make sense to do, especially when considering cache functions like identify (e.g. ApolloClient.identify() seems off).

This PR also exposes optional identify and modify API elements in ApolloCache. Since update now has access to the full ApolloCache instance passed in, identify and modify can be used in the update function. Note that the ApolloCache implementations of identify and modify do nothing of value; they should be overridden by ApolloCache subclasses that are interested in using them, like InMemoryCache does. By including them in ApolloCache as basically no-op methods, we're avoiding forcing alternative cache implementations that subclass ApolloCache to have to support them.

Copy link
Member

@benjamn benjamn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for going the extra mile with the DataProxy elimination and thoughtful reorganizations of types!

This commit gives the mutate `update` function direct
access to the cache, instead of using a scaled down
`DataProxy`. While the `DataProxy` approach helped in the
past as a way to batch cache updates, it is no longer
necessary. We are only ever passing a cache instance into
`update`, which means limiting the type to be a `DataProxy`
gives us no additional advantages. The only other `DataProxy`
implementor in the AC codebase is `ApolloClient`, which
is never passed to the `update` function.

This commit also exposes optional `identify` and `modify`
API elements in `ApolloCache`. Since `update` now has
access to the full `ApolloCache` instance passed in,
`identify` and `modify` can be used in the update function.
Note that the `ApolloCache` implementations of `identify`
and `modify` do nothing of value; they should be overridden
by `ApolloCache` subclasses that are interested in using them,
like `InMemoryCache` does. By including them in `ApolloCache`
as basically no-op methods, we're avoiding forcing
alternative cache implementations that subclass `ApolloCache`
to have to support them.
@hwillson hwillson merged commit 83c7c30 into master Feb 18, 2020
@hwillson hwillson deleted the mutate-update-no-data-proxy branch February 18, 2020 18:13
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 16, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants