y-crdt / ypy

Python bindings to y-crdt
https://ypy.readthedocs.io/en/latest/
Other
184 stars 22 forks source link

Rollback support #144

Open davidbrochart opened 1 year ago

davidbrochart commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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.