Skip to content

fix(undoRedo): clear redo stack when using limit option#630

Open
sgup wants to merge 1 commit intoLegendApp:mainfrom
sgup:fix/undoredo-limit-clears-redo-stack
Open

fix(undoRedo): clear redo stack when using limit option#630
sgup wants to merge 1 commit intoLegendApp:mainfrom
sgup:fix/undoredo-limit-clears-redo-stack

Conversation

@sgup
Copy link

@sgup sgup commented Jan 22, 2026

Problem

When the limit option is set on undoRedo(), making a change after undoing does not clear the redo stack. This causes history corruption where undone states persist and can be incorrectly restored.

Steps to reproduce:

  1. Create undoRedo with a limit: undoRedo(obs$, { limit: 10 })
  2. Make changes: A → B → C
  3. Undo (now at B, can redo to C)
  4. Make a new change: B → D
  5. Bug: Redo stack still contains C, history is corrupted

Root Cause

The original code had mutually exclusive branches:

if (options?.limit) {
    // Only keeps last N items - does NOT clear redo stack!
    history = history.slice(Math.max(0, history.length - options.limit));
} else {
    // Correctly clears redo stack
    history = history.slice(0, historyPointer + 1);
}

Solution

Always clear redo history first, then apply the limit separately:

// Always clear any redo history first (items after current position)
history = history.slice(0, historyPointer + 1);

// Then apply the limit if specified
if (options?.limit) {
    history = history.slice(Math.max(0, history.length - options.limit));
}

Testing

Added regression test 'Undo/Redo with limit clears redo stack on new change' that verifies the fix.

All existing tests pass.

When the `limit` option was set, making a change after undoing would not
clear the redo stack. This caused history corruption where undone states
would persist and could be incorrectly restored.

The fix ensures redo history is always cleared first (by slicing to
historyPointer + 1), then the limit is applied separately.

Before:
- limit set: history.slice(Math.max(0, history.length - limit))
- no limit: history.slice(0, historyPointer + 1)

After:
- always: history.slice(0, historyPointer + 1)  // clear redos
- if limit: history.slice(Math.max(0, history.length - limit))  // apply limit

Added regression test to verify the fix.
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

Successfully merging this pull request may close these issues.

1 participant