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

Default to hard rollback #451

Open
alecgibson opened this issue Mar 22, 2021 · 3 comments
Open

Default to hard rollback #451

alecgibson opened this issue Mar 22, 2021 · 3 comments
Labels
breaking-change Breaking change that we may want to pull into a future major version

Comments

@alecgibson
Copy link
Collaborator

In general, the invert() method is not very reliable as an API. Neither json0 and json1 guarantee that any given op is invertible. This can lead to surprising data states when using a "soft" rollback. We should probably default to hard rollback in all cases, and force consumers to opt-in if they want this behaviour (which means they should also hopefully know what they're doing).

Further discussion:

@alecgibson alecgibson added the breaking-change Breaking change that we may want to pull into a future major version label Mar 22, 2021
@alecgibson
Copy link
Collaborator Author

Discussed with @ericyhwang — sounds like a sensible thing to do.

Flag-wise, we can add one at the global level to help consumers ease transition, and also set per-doc for more granular control. We'd need to have "ternary" logic at the doc level: null means inherit from the global flag, or if we've actively set it to true or false, then we should use the doc flag.

@curran
Copy link
Contributor

curran commented Jul 30, 2021

This seems like a good direction to me.

@hsource
Copy link
Contributor

hsource commented Oct 5, 2021

I did a more targeted and backwards-compatible fix in #520 - Fall back to doing a _hardRollback when op.type.invert fails. Let me know if you have any feedback on that! We're running that in a patched production build already.

This can be helpful even if you do change the defaults, as it makes those cases not totally ignore the rollback.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking-change Breaking change that we may want to pull into a future major version
Projects
None yet
Development

No branches or pull requests

3 participants