apollographql / apollo-client

:rocket:  A fully-featured, production ready caching GraphQL client for every UI framework and GraphQL server.
https://apollographql.com/client
MIT License
19.34k stars 2.66k forks source link

Enable data masking in cache instead of client #12019

Closed jerelmiller closed 1 month ago

jerelmiller commented 1 month ago

Currently you enable dataMasking by setting the option in the ApolloClient constructor. While this has worked well throughout the development of this feature, this approach does not work very well when using the watchFragment API to mask fragment data returned from that method. That method needs to check whether to mask the raw cache data or leave as-is and it has no way of doing that with the current setup.

We approached this earlier in https://github.com/apollographql/apollo-client/pull/11891 which used the ApolloClient constructor to inform the cache when the option was enabled. This however was clunky and thus moved in https://github.com/apollographql/apollo-client/pull/11913 which was a more natural API. Unfortunately https://github.com/apollographql/apollo-client/pull/11913 now again makes it difficult to determine in the cache whether masking is enabled in the cache. For the cache to determine whether masking is enabled, we'd need to revert back to a solution like https://github.com/apollographql/apollo-client/pull/11891. At this point though this seems like a really roundabout way to avoid configuring this option in the cache itself. Instead, I'm proposing we move this configuration to the cache.

This approach will still work for any other cache implementation. Those cache implementations can implement the necessary API to use this feature and provide its own interface for enabling masking for its end users.

changeset-bot[bot] commented 1 month ago

⚠️ No Changeset found

Latest commit: 7c2b71cc944e74f3c1631ea94d29b0ab311d611b

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

jerelmiller commented 1 month ago

Discussed with @phryneas and we think that moving masking into the client.* APIs (i.e. client.readFragment(...), client.watchFragment(...), etc). should be the way to go for now. This means that if you use the same underlying cache APIs directly (cache.readFragment(...), etc), the data returned here will not be masked.

This is a point we will hope to gather feedback on during an experimental release to see if this ends up too confusing or not. As such, we'll keep the config value where it is.