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

Rollback support #144

Open
davidbrochart opened this issue Sep 30, 2023 · 2 comments
Open

Rollback support #144

davidbrochart opened this issue Sep 30, 2023 · 2 comments

Comments

@davidbrochart
Copy link
Collaborator

I know that Yrs doesn't support rollback, but allows undoing changes. Still, I'm wondering if it would make sense to be able to cancel a transaction. One use case I'm thinking about is when a change to a document occurs while in a transaction. In this case we might want to react to that change and not commit the current changes, to do something different.
To implement this, I think we could add a wrapper around the current types, that would not call Yrs right away but instead register the changes to actually be done when the transaction context manager exits. These "views" should reflect the changes right away though, so that we can read their content. For instance:

with doc.begin_transaction() as txn:
    text.extend(txn, "foo")  # the change is done on the view, no call to Yrs yet
    if len(text) != 3:  # the view mirrors the underlying YText's state
        txn.cancel()  # rollback
    #  when the context manager exits, the view changes are committed if the transaction was not cancelled

But since this code is blocking, a document change cannot be seen in the middle of a transaction. So this would really be useful if the API is async, as mentioned here (but for a different reason).

@Horusiath
Copy link

This is dangerous for few reasons:

  1. It doesn't really describe the state of the system. You still can call methods on the transactions, docs and other types (including nested collections) that don't know about the changes in your wrapper.
  2. It we were to include rollbacks natively it can lead to dangerous scenarios ie. make change, then produce update and broadcast it, then rollback the changes. This way produced update is not only no longer valid but it will cause the document state corruption in subsequent modifications.
  3. Often transactions are short lived ie. for time needed to enter a key stroke. Rolling them back may turn out to be not very useful. This is exactly why UndoManager is not bound to a transaction and has separate way of tracking changes. Also since undo/redo are compensating actions, they don't suffer from issues produced by pt2.

@davidbrochart
Copy link
Collaborator Author

I mainly agree with 3. When we want to rollback because of an external update, the arrival time of the update is very unpredictable anyway, because it may be sent over a network or some kind of IO. So the update may arrive after the transaction, in which case it's too late to rollback.

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

No branches or pull requests

2 participants