Automatically filter out dangling references from arrays.#6454
Merged
Automatically filter out dangling references from arrays.#6454
Conversation
This commit implements the proposal for automatic filtering of dangling references that I described in #6425 (comment). The filtering functionality demonstrated by #6412 (and updated by #6425) seems useful enough that we might as well make it the default behavior for any array-valued field consumed by a selection set. Note: the presence of field.selectionSet implies the author of the query expects the elements to be objects (or references) rather than scalar values. By making .filter(canRead) automatic, we free developers from having to worry about manually removing any references after evicting entities from the cache. Instead, those dangling references will simply (appear to) disappear from cache results, which is almost always the desired behavior. In case this automatic filtering is not desired, a custom read function can be used to override the filtering, since read functions run before this filtering happens. This commit includes tests demonstrating several options for replacing/filtering dangling references in non-default ways.
benjamn
commented
Jun 17, 2020
Comment on lines
+389
to
+390
| if (field.selectionSet) { | ||
| array = array.filter(context.store.canRead); |
Member
Author
There was a problem hiding this comment.
Note that executeSubSelectedArray is result-cached (like executeSelectionSet), so this filtering only happens when we're recomputing the current result object.
Also note that the original array remains unfiltered in the cache. I described a possible strategy for permanently pruning the list in #6412 (comment), though I don't think that should be necessary in most cases.
This was referenced Jul 10, 2020
This file contains hidden or 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
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
3 participants
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 commit implements the proposal for automatic filtering of dangling references that I described in #6425 (comment).
The filtering functionality demonstrated by #6412 (and updated by #6425) seems useful enough that we might as well make it the default behavior for any array-valued field consumed by a selection set. Note: the presence of
field.selectionSetimplies the author of the query expects the elements to be objects (or references) rather than scalar values. A list of scalar values should not be filtered, since it cannot contain dangling references.By making
.filter(canRead)automatic, we free developers from having to worry about manually removing references from lists after evicting entities from the cache. Instead, those dangling references will simply (appear to) disappear from cache results, which is almost always the desired behavior. Fields whose values hold single (non-list) dangling references cannot be easily filtered in the same way, but you can always write a customreadfunction for the field, and it's somewhat more likely that a refetch will fix those fields correctly.In case the automatic list filtering is not desired, a custom
readfunction can be used to override the filtering, sincereadfunctions run before this filtering happens. This commit includes tests demonstrating several options for replacing/filtering dangling references in non-default ways.