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

Simplify EntityStore#merge. #5904

Merged
merged 2 commits into from
Feb 4, 2020
Merged

Simplify EntityStore#merge. #5904

merged 2 commits into from
Feb 4, 2020

Conversation

benjamn
Copy link
Member

@benjamn benjamn commented Feb 3, 2020

More meaningful warnings are coming (#5833) when merging non-normalized data in the cache, and we can save some code by not worrying about edge cases that don't matter.

The change to EntityStore#merge by itself caused an esoteric test failure
in src/cache/inmemory/__tests__/optimistic.ts, but I was able to fix that
by addressing an old TODO in Layer#removeLayer.
Replacing a Reference in the cache is always safe, even with
non-normalized data, since the Reference refers to data safely stored
elsewhere in the cache.

By removing this invariant, we remove the need for most of the surrounding
logic in the storeObjectReconciler function, too, which is why it can be
so much shorter now.
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 0e2a345 into master Feb 4, 2020
@benjamn benjamn deleted the simplify-EntityStore-merge branch February 4, 2020 14:15
@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.

2 participants