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

Automatic undo/redo #3364

Merged
merged 28 commits into from
Apr 24, 2024
Merged

Automatic undo/redo #3364

merged 28 commits into from
Apr 24, 2024

Conversation

SomeHats
Copy link
Contributor

@SomeHats SomeHats commented Apr 4, 2024

Our undo-redo system before this diff is based on commands. A command is:

  • A function that produces some data required to perform and undo a change
  • A function that actually performs the change, based on the data
  • Another function that undoes the change, based on the data
  • Optionally, a function to redo the change, although in practice we never use this

Each command that gets run is added to the undo/redo stack unless it says it shouldn't be.

This diff replaces this system of commands with a new one where all changes to the store are automatically recorded in the undo/redo stack. You can imagine the new history manager like a tape recorder - it automatically records everything that happens to the store in a special diff, unless you "pause" the recording and ask it not to. Undo and redo rewind/fast-forward the tape to certain marks.

As the command concept is gone, the things that were commands are now just functions that manipulate the store.

One other change here is that the store's after-phase callbacks (and the after-phase side-effects as a result) are now batched up and called at the end of certain key operations. For example, applyDiff would previously call all the afterCreate callbacks before making any removals from the diff. Now, it (and anything else that uses store.atomic(fn) will defer firing any after callbacks until the end of an operation. before callbacks are still called part-way through operations.

Design options

Automatic recording is a fairly large big semantic change, particularly to the standalone store.put/store.remove etc. commands. We could instead make not-recording the default, and make recording opt-in instead. However, I think auto-record-by-default is the right choice for a few reasons:

  1. Switching to a recording-based vs command-based undo-redo model is fundamentally a big semantic change. In the past, store.put etc. were always ignored. Now, regardless of whether we choose record-by-default or ignore-by-default, the behaviour of store.put is context dependant.
  2. Switching to ignore-by-default means that either our commands don't record undo/redo history any more (unless wrapped in editor.history.record, a far larger semantic change) or they have to always-record/all accept a history options bag. If we choose always-record, we can't use commands within history.ignore as they'll start recording again. If we choose the history options bag, we have to accept those options in 10s of methods - basically the entire Editor api surface.

Overall, given that some breaking semantic change here is unavoidable, I think that record-by-default hits the right balance of tradeoffs. I think it's a better API going forward, whilst also not being too disruptive as the APIs it affects are very "deep" ones that we don't typically encourage people to use.

Change Type

  • sdk — Changes the tldraw SDK
  • improvement — Improving existing features
  • galaxy brain — Architectural changes

Release Note

Breaking changes

1. History Options

Previously, some (not all!) commands accepted a history options object with squashing, ephemeral, and preserveRedoStack flags. Squashing enabled/disabled a memory optimisation (storing individual commands vs squashing them together). Ephemeral stopped a command from affecting the undo/redo stack at all. Preserve redo stack stopped commands from wiping the redo stack. These flags were never available consistently - some commands had them and others didn't.

In this version, most of these flags have been removed. squashing is gone entirely (everything squashes & does so much faster than before). There were a couple of commands that had a special default - for example, updateInstanceState used to default to being ephemeral. Those maintain the defaults, but the options look a little different now - {ephemeral: true} is now {history: 'ignore'} and {preserveRedoStack: true} is now {history: 'record-preserveRedoStack'}.

If you were previously using these options in places where they've now been removed, you can use wrap them with editor.history.ignore(fn) or editor.history.batch(fn, {history: 'record-preserveRedoStack'}). For example,

editor.nudgeShapes(..., { ephemeral: true })

can now be written as

editor.history.ignore(() => {
    editor.nudgeShapes(...)
})
2. Automatic recording

Previously, only commands (e.g. editor.updateShapes and things that use it) were added to the undo/redo stack. Everything else (e.g. editor.store.put) wasn't. Now, everything that touches the store is recorded in the undo/redo stack (unless it's part of mergeRemoteChanges). You can use editor.history.ignore(fn) as above if you want to make other changes to the store that aren't recorded - this is short for editor.history.batch(fn, {history: 'ignore'})

When upgrading to this version of tldraw, you shouldn't need to change anything unless you're using store.put, store.remove, or store.applyDiff outside of store.mergeRemoteChanges. If you are, you can preserve the functionality of those not being recorded by wrapping them either in mergeRemoteChanges (if they're multiplayer-related) or history.ignore as appropriate.

3. Side effects

Before this diff, any changes in side-effects weren't captured by the undo-redo stack. This was actually the motivation for this change in the first place! But it's a pretty big change, and if you're using side effects we recommend you double-check how they interact with undo/redo before/after this change. To get the old behaviour back, wrap your side effects in editor.history.ignore.

4. Mark options

Previously, editor.mark(id) accepted two additional boolean parameters: onUndo and onRedo. If these were set to false, then when undoing or redoing we'd skip over that mark and keep going until we found one with those values set to true. We've removed those options - if you're using them, let us know and we'll figure out an alternative!

Copy link

vercel bot commented Apr 4, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated (UTC)
examples ✅ Ready (Inspect) Visit Preview Apr 24, 2024 4:11pm
tldraw-docs ✅ Ready (Inspect) Visit Preview Apr 24, 2024 4:11pm

@steveruizok
Copy link
Collaborator

IIRC we tried something in this direction at an earlier point when designing undo / redo. Several commands have different "redos" than "dos", which couldn't be captured by anything automated, though I no longer see any places where the "redo" is used.

Copy link

@hossain666 hossain666 left a comment

Choose a reason for hiding this comment

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

packages/tlschema/src/records/TLPageState.ts

This comment was marked as spam.

@hossain666

This comment was marked as spam.

@mimecuvalo mimecuvalo mentioned this pull request Apr 30, 2024
15 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
galaxy brain Architectural changes improvement Improves existing features sdk Affects the tldraw sdk
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants