Support eviction of specific entity fields. #5643
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
benjamn
commented
Dec 2, 2019
| @@ -206,12 +206,12 @@ export class InMemoryCache extends ApolloCache<NormalizedCacheObject> { | |||
| return this.policies.identify(object, selectionSet, fragmentMap); | |||
| } | |||
|
|
|||
| public evict(dataId: string): boolean { | |||
| public evict(dataId: string, fieldName?: string): boolean { | |||
There was a problem hiding this comment.
It was tempting to add cache.evictRootQueryField(fieldName) as a shorthand for cache.evict("ROOT_QUERY", fieldName), but then I realized it was the same number of characters, which is a bad news for a shorthand.
hwillson
approved these changes
Dec 3, 2019
When a field has multiple cached values due to being written into the cache at different times with different arguments, it's important that we do not throw away the association between the short field.name.value (an identifier, as defined in the schema) and the longer storeFieldName strings computed from the field and its arguments. This commit ensures that field.name.value is always recoverable from the storeFieldName, by prepending field.name.value + ":" to any storeFieldName that does not already start with field.name.value.
In PR #5617, we shifted from using just IDs for caching results read from the EntityStore to using IDs plus field names, which made the result caching system significantly more precise and less likely to trigger unnecessary rerenders. However, the field names we used in that PR were storeFieldName strings, instead of the actual field names you'd find in your GraphQL schema. Within the InMemoryCache implementation, a storeFieldName is the full string produced by Policies#getStoreFieldName, which must begin with the original field.name.value, but may include an additional suffix that captures field identity based on arguments (and also, but less frequently, directives). For the purposes of result caching, it's important to depend on the entity ID plus the actual field name (field.name.value), rather than the ID plus the storeFieldName, because there can be many different storeFieldName strings for a single field.name.value (and we don't want to waste memory on dependency tracking), and because it's safer (more conservative) to dirty all fields with a particular field.name.value when any field with a matching storeFieldName is updated or evicted, since field values with the same field.name.value often overlap in subtle ways (for example, multiple pages of a paginated list), so it seems dangerous to encourage deleting some but not all of them. Perhaps more importantly, application developers should never have to worry about the concept of a storeFieldName, since it is very much an implementation detail of the InMemoryCache, and the precise format of storeFieldName strings is subject to change.
Evicting an entire entity object is often overkill, when all you really want is to invalidate and refetch specific fields within the entity. This observation is especially true for the ROOT_QUERY object, which should never be evicted in its entirety, but whose individual fields often become stale or need to be recomputed. A critical nuance here is that fields are evicted according to their field.name.value, rather than a specific storeFieldName, since it doesn't make a whole lot of sense to evict the field value associated with a specific set of arguments. Instead, calling cache.evict(id, fieldName) will evict *all* values for that field, regardless of the arguments.
benjamn
force-pushed
the
allow-evicting-specific-fields
branch
from
9c33678
to
5f153e8
Compare
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR adds an additional (optional) parameter to the
cache.evictmethod, to delete a specific field from a normalized entity by callingcache.evict(id, fieldName). The shortercache.evict(id)version works as before.Please read the commit messages to understand what was tricky/subtle about implementing this interface. In particular,
fieldNameis the actual name of the field, as defined in your schema, and thus it may refer to multiple field values (if the field takes arguments), all of which should be invalidated when the field is deleted.