rocicorp / replicache

Realtime Sync for Any Backend Stack
https://doc.replicache.dev
1k stars 37 forks source link

Undo/Redo #1008

Open aboodman opened 1 year ago

aboodman commented 1 year ago

Undo/redo is a very common feature request in collaborative applications and doing it correctly is somewhat subtle (e.g., see posts by Figma and Liveblocks).

Many Replicache users have created undo/redo on their own without too much trouble but it would be good to provide a solid, tested library analogous to https://github.com/rocicorp/fractional-indexing and other helper libraries we've released.

arv commented 1 year ago

What else needs to be done here? Is https://github.com/rocicorp/undo enough to close this?

@cesara

cesara commented 1 year ago

@arv there is a particular scenario that undo is not being handled correctly.

Reference our Redoing undo notion doc for a clearer understanding of the issue.

We can open a new Issue to handle the invariant.

arv commented 1 year ago

I see. This is still future work then.

heymartinadams commented 1 year ago

Can we use this library already in production, or should we hold off until you launch this feature as part of the official Replicache? Undo/redo is a highly requested feature for us.

aboodman commented 1 year ago

Here is the notion doc @cesara mentions: https://replicache.notion.site/Redoing-undo-c0183b91d12c4272a3482e3233cc2890?pvs=4.

It describes the edge case which this library doesn't handle.

isaacHagoel commented 1 month ago

I think the notion doc misinterprets the "Figma principle" by ignoring the "and nothing else has happened in between" bit.

if the user presses undo n times, then redo n times, and nothing else has happened in between, then the document should end up how it was before pressing undo.

I think it would be more correct to think about the problem in the following way (everything I say below is in line with the Figma principle): The purpose of Undo is to allow the user to experiment safely and roll-back mistakes. Destroying work by another user (and potentially corrupting the entire "document") shouldn't be a possible outcome. Therefore Undo should only be allowed if it is "conflicts free" (meaning - the unit on which the undo is about to operate was has the same value that the undoing user put there). Redo can be thought of as Undo of the last Undo. If undo is only allowed when it's conflicts free, a redo just mean re-applying the original mutation. It should also only be allowed as long as it's conflict free (== no one else changed the value after the undo that's about to be reverted).

When these conditions are not met the corresponding undo/redo operations should be skipped (and canUndo/canRedo should reflect it if there are not other actions the user can legally undo/redo).

Otherwise, it leads to nonsensical results (user A clicks redo and that behaves as if user B clicked undo, confusing both) I've been reading some academic paper on the subject (this is an interesting one, is talks about an API that contains transpose and hasConflict functions) and planning to do more research and play with some existing systems to see what they do. Putting this here in case it is helpful and/or in case someone wants to point out some error in my thinking.

aboodman commented 1 month ago

What you describe is very different than what Figma or any other collaborative editor I've ever used does, in particular:

Therefore Undo should only be allowed if it is "conflicts free" (meaning - the unit on which the undo is about to operate was has the same value that the undoing user put there).

and:

When these conditions are not met the corresponding undo/redo operations should be skipped (and canUndo/canRedo should reflect it if there are not other actions the user can legally undo/redo).

You can easily experiment in Figma and see that it doesn't behave this way. The video I pasted above shows one example.

Our goal is to match the Figma behavior. There are some edge cases where we don't currently. The Notion doc describes the problem and a potential solution.

isaacHagoel commented 1 month ago

it's actually very normal to lose the ability to Redo once the state has diverged from the state that the "matching" undo operation created. Every app I can think of does that. Here is a quick example from Canva (notice how typing after undoing disables the redo button): https://github.com/rocicorp/replicache/assets/20507787/b0141313-1397-44aa-bde5-a7a1458efef3 I tried to apply the same principle to conflicting edits done by other users (skipping might be a bad idea 😄). I don't think any multiplayer app 100% figured out the general-case best-practices yet, not even the mighty FIgma.

Sounds like you have a clear goal though, and I respect that. I think you guys are awesome. All the best!

isaacHagoel commented 2 weeks ago

Just for future reference in case anyone finds it useful. I've dug into the subject, created a POC implementation and wrote this: https://dev.to/isaachagoel/you-dont-know-undoredo-4hol

One of the readers pointed me to the undo/redo implementation by ProseMirror that implements similar ideas: https://github.com/ProseMirror/prosemirror-history/tree/master/src