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

Make InMemoryCache#evict remove data from all EntityStore layers. #5773

Merged
merged 3 commits into from
Jan 10, 2020

Conversation

benjamn
Copy link
Member

@benjamn benjamn commented Jan 10, 2020

Folks testing AC3 (👋 @ahrnee) have already reported some confusion about evicted data seeming to reappear in the context of optimistic mutation updates: #5746 (comment)

To prevent this confusion, I think we need to honor the force implied by the word evict and remove all traces of the given dataId/fieldName from all layers the cache, rather than only evicting from the top layer of the cache (namely, this.optimisticData).

Folks testing AC3 have already reported some confusion about evicted data
seeming to reappear in the context of optimistic mutation updates:
#5746 (comment)

To prevent this confusion, I think we need to honor the force implied by
the word "evict" and remove all traces of the given dataId/fieldName from
all layers the cache, rather than only evicting from the top layer of the
cache (that is, this.optimisticData).
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.

Great idea - looks 👍 @benjamn!

@jamshally
Copy link

I think we need to honor the force implied by the word evict and remove all traces of the given dataId/fieldName from all layers the cache

Sounds good! I wasn't aware of different layers to the cache - I'd be interested to understand that better.

Related but separate: It doesn't look like evict is in the DataProxy Type Definition. Is this intentional, or to come?

@hwillson
Copy link
Member

@ahrnee If you're interested, see #4319 for an explanation of the optimistic cache layering.

@benjamn benjamn merged commit 660b0de into master Jan 10, 2020
@benjamn benjamn deleted the evict-from-all-EntityStore-layers branch January 10, 2020 15:58
benjamn added a commit that referenced this pull request Jan 11, 2020
@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