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

(graphcache) - Reenable cumulative optimistic updates #1074

Merged
merged 5 commits into from
Oct 20, 2020

Conversation

kitten
Copy link
Member

@kitten kitten commented Oct 20, 2020

This was discovered in this Discussions thread: #1073

Summary

In #866 we previously disallowed optimistic updates to read other optimistically written data, which meant to prevent conflicts between these data layers that will later trickle down into permanent layers. This isn't a concern anymore because optimistic layers will never contribute to concrete, non-optimistic data and are thrown away, if they're for optimistic mutations.

The edge case we aimed to prevent is also not possible anymore because we now never apply concrete updates on top of optimistic ones since #750, namely:

  • Block refetching of operations that have active optimistic updates and keep track of them (see above)
  • Keep track of optimistic mutation that are pending at the same time, and flush their results simultaneously, also erasing the optimistically updated (i.e. blocked) dependencies at the same time

I'm not 100% sure anymore at this time, why we deemed this unsafe before, but it shouldn't be.

Set of changes

  • Allow optimistic updates to see other optimistically written data
  • Rename currentIgnoreOptimistic to just currentOptimistic
  • Fix getNode to allow currentOptimistic to remain true so that updatePersist doesn't queue up noop updates for optimistic mutations

@changeset-bot
Copy link

changeset-bot bot commented Oct 20, 2020

🦋 Changeset detected

Latest commit: db86066

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@urql/exchange-graphcache Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

Copy link
Collaborator

@JoviDeCroock JoviDeCroock left a comment

Choose a reason for hiding this comment

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

Great find!

This is just to prevent us from messing this up in the future.
@kitten kitten merged commit 28afd23 into main Oct 20, 2020
@kitten kitten deleted the fix/cumulative-optimistic-updates branch October 20, 2020 13:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants