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

Automatically filter out dangling references from arrays. #6454

Merged
merged 3 commits into from
Jun 17, 2020

Conversation

benjamn
Copy link
Member

@benjamn benjamn commented Jun 17, 2020

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. 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 custom read function 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 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.

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.
Copy link
Contributor

@jcreighton jcreighton left a comment

Choose a reason for hiding this comment

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

🦆 🐂 🐔

Comment on lines +389 to +390
if (field.selectionSet) {
array = array.filter(context.store.canRead);
Copy link
Member Author

Choose a reason for hiding this comment

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

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.

Copy link
Member

@hwillson hwillson left a comment

Choose a reason for hiding this comment

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

Looks great @benjamn - thanks!

@benjamn benjamn merged commit 13310e1 into master Jun 17, 2020
@benjamn benjamn deleted the automatic-canRead-filtering branch June 17, 2020 20:26
@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.

3 participants