Behavior Change of `clearStack`

[rainerhahnekamp]

Context

In withUndoRedo, the clearStack() method currently behaves as a hard reset: After calling it, the current state is removed from history, and the first change afterwards is treated as a brand-new initial state. This means canUndo() will be false after that first change, and only the second change becomes undoable.

This behavior came up in PR #207 and in the related discussion with @GregOnNet, @richardliebmann, and @jdegand.

Problem

During initialization, the stack always stores an initial state, so previous is never null.

When I want to clear the stack because of context switching - for example, moving from one file to another in an editor - the new context also has an initial state. In the current implementation, if I clear the stack after switching, the first change is seen as the initial one. Before the first change happens, the initiale state is null, which is not true.

Example of current behavior:

store.selectNewFile('graz.jpg'); 
// Store now has initial Graz state

store.clearStack(); 
// Graz state removed from history

store.setTitle('Wien'); 
// First change after clear: cannot undo back to Graz

store.undo(); 
// No effect, because Graz state wasn’t kept as baseline

Proposed Change

Change clearStack():

• Clear the history.
• Keep the current state as the new baseline (initial state).
• The next change is undoable back to that baseline.

Proposed soft reset behavior:

store.selectNewFile('graz.jpg'); 
// Graz state is initial

store.clearStack(); 
// Graz state becomes new baseline

store.setTitle('Wien'); 
// First change is undoable back to Graz

store.undo(); 
// Restores Graz baseline

[rainerhahnekamp]

/cc @manfredsteyer

[jdegand]

@rainerhahnekamp Might want to change the name to reflect new functionality. Something like resetHistory or clearHistory.

Do you think skipOnce flag might miss updates? patchState indirectly causes other state updates and the flag switches or doesn't. Alternative would be a pause mechanism:

let recordingPaused = false;

function pauseRecording(fn: () => void) {
  recordingPaused = true;
  try {
    fn();
  } finally {
    recordingPaused = false;
  }
}

Holding down a key could cause multiple calls or flags to overlap. So add a throttle or debounce to this?

[rainerhahnekamp]

@jdegand Thanks for the thoughts. I think we might be talking past each other a bit:

• This isn’t about the old “glitch-free effect” pattern — we don’t use that anymore; withUndoRedo now relies on watchState.
• My concern is the semantics of clearStack(): right now it seems to drop the current/initial state, so the first change after clearing isn’t undoable. As a user, I’d expect the current state to become the new baseline (i.e., the next change is undoable back to it).

I’m not the original author, so maybe the current behavior is intentional. I’d like to understand why and discuss here if we want to change it.

[jdegand]

Reasons to Keep It As-Is (First Change Not Undoable):

1. Intentional Reset

   • If a user clears the stack, they’re intentionally discarding prior state and starting fresh.

2. Avoid Accidental Undo of “Fresh Start”

   • Could be confusing or unwanted hassle.

3. Consistency with Some UX Patterns

   • Many apps do the same thing.

I think use-cases might dictate preference. If the use case leans more toward something incremental, it might be worth capturing the current state as the new baseline.

[GregOnNet]

Hey there,

this week, I am pretty busy.

@rainerhahnekamp's example made me thinking whether the reset-concern is really tight to undo/redo, especually if we allow to undo/redo different slices of the state.

As shown above, .selectNewFile and setTitle may manage different parts of the state
I agree that setTitle if set before, should be undoable.

I need to think a bit more about it, but wanted to leave my thoughts...

[rainerhahnekamp]

@GregOnNet no hurry — it’s not like we broke the library. 🙂

@jdegand could you share a concrete use case where you want an initial state that you can never return to (only to the first change)? That’s effectively what clearStack does today: it discards the true initial state, so undo can’t reach it anymore.

My concern: this goes against how state (and Signals) are modeled. State always exists and has a current value; wiping the baseline feels odd.

[manfredsteyer]

@GregOnNet: I see your point. I can imagine two scenarios.

a) The behavior is acceptable as is: In many applications (perhaps even in most) , there is just one undo/redo stack to avoid confusion
b) If we need more than one, we can currently create another store. When using lightweight stores such as the Signal Stor,e it's anyway quite usual to have several stores per feature.

If we find good use cases where we need several stacks within one store, we can, of course, respect this. However, I'd see this not as part of this discussion but of a separate one.

@jdegand, I also see your point. On the other side, I know what Rainer describes from several apps. To avoid breaking the behavior, we should add a flag to clearStack (or whatever we prefer to call it; I don't have strong feelings about this).

[jdegand]

@rainerhahnekamp A canvas element makes sense with the existing usage.

You might not want to undo back to a state pre-populated from external sources like local storage or stale API data. The skip option helps here by ignoring initial hydration, but edge cases could arise — for example, if the initial state is partially user-driven or hydration occurs after interaction. However, these edge cases are likely rare in practice.

[rainerhahnekamp]

@jdegand, do I understand you correctly that setting the value to null is an edge case for you?

If yes, then I'd propose to keep the current behavior optionally available via flag: store.clearStack({hardReset: true});

This would allow us to:

1. Support the new use case - context switching where you want to clear history but keep current state as baseline
2. Make the API explicit - developers can choose between hard reset ({hardReset: true}) and soft reset (default)

Important: Since this changes the default behavior from hard reset to soft reset, we'd need to provide schematics to automatically update existing code to use store.clearStack({hardReset: true}) to maintain the current behavior. Without schematics, this would be a breaking change.

The implementation would change from:

clearStack(): void {
  undoStack.splice(0);
  redoStack.splice(0);
  previous = null;  // Hard reset
  updateInternal();
}

To:

clearStack(options?: { hardReset?: boolean }): void {
  undoStack.splice(0);
  redoStack.splice(0);
  if (options?.hardReset) {
    previous = null;  // Hard reset (current behavior)
  }
  // else: keep current state as baseline (new behavior)
  updateInternal();
}

This way, the "edge case" of setting to null becomes an explicit, intentional choice rather than the default behavior.

What do you think about this approach? And do you think providing schematics for the migration would be feasible?

/cc @michael-small

[jdegand]

Alternative would be to split the functionality into separate methods:

softReset(): void {
  undoStack.splice(0);
  redoStack.splice(0);
  // Soft reset: keep current state as baseline
  updateInternal();
}

hardReset(): void {
  undoStack.splice(0);
  redoStack.splice(0);
  previous = null; // Hard reset
  updateInternal();
}

Then you wouldn't have to worry about schematics or migration guide. Flags can be a hassle to remember.

If clearStack is the hard reset, the name for the soft reset could be ... checkpoint() or stash().

I don't really have a specific scenario in mind, but I do anticipate there will be an incidence where a full reset would be preferred. I also intend to play around with this and try to pinpoint a good use case.

[GregOnNet]

Hey there,

this is already a great exchange of thoughts.

A few things I noticed so far:

• We discuss a lot of use cases but miss to get more concrete (e.g. Examples, StackBlitz-Demos)
• We tend to say reset instead of clearStack.

The behavior change has been introduced in v19.2.0.
Just to note, the actual behavior is that previousItem is set to null.

@rainerhahnekamp You mentioned resetting the state feels odd. Maybe we can get more into detail here. From my perspective we do not reset the state, but the internal state managed by withUndoRedo. Are we speaking about the same state (store vs. withUNdoRedo?)?

Use Cases

From the current discussion it seems both use cases are present.
What about having a look at other implementations of other libraries how they solve this issue:

• Angular ngxtensions signalHistory | reset intentionally keeps the "last record"
• Vue useManualRefHistory | also keeps the "last record"

So it seems setting previousItem = null causes a different behavior than in other libraries.
(I do not judge whether it is correct or not. It is as is).

Open questions

• If we would revert the current behavior... How can we solve the use-case of context-switches? Which other mechanisms could be used... 🤔
  • Maybe setting the desired state to null before loading the new source. This would cause previousItem to be null. Yeah, it is a manual step not handled by undo/redo but could be documented how this case can be handled.

Ideas

deprecation of clearStack?

Maybe deprecating clearStack would open the door to introduce a more meaningful, stronger naming...

// resetting undo/redo and omitting previousItem - as is now
patchState(store, resetUndoRedo()) // default: { keepLastRecord: true }

//  resetting undo/redo keeping previousItem - as is was before
patchState(store, resetUndoRedo({ keepCurrentRecord: true }))


// ==-- or other way round, depending what is more common/intuitive --==


// resetting undo/redo and omitting previousItem - as is now
patchState(store, resetUndoRedo({ keepLastRecord: false }))

//  resetting undo/redo keeping previousItem - as is was before
patchState(store, resetUndoRedo()) // default: { keepLastRecord: false }

// ==-- or it is not about resetting, but in specifying what the last record is --==
patchState(store, clearUndoRedo({ lastRecord: null })) // default: keep lastRecord

// 🤯 just another API idea to make resetUndoRedo more discoverable
import { withUndoRedo } from '...';

patchState(store, withUndoRedo.reset()) // cons: reset is not tree-shakable, but maybe tree-shaking is not really a thing here
// or 
patchState(store, withUndoRedo.clear())

Context Switch Use case

What we need in our project

• Editor-View opened: Load document 1 from an API
• Editor-View: apply a change to the document
• Editor-View: undo the change
• Editor-View: redo the change
• Editor-View navigation: Load document 2 ➕ reset Undo/Redo
• Editor-View: apply a change to the document
• Editor-View: undo the change
  • ❌ ngrx-toolkit < 19.2: the undo would restore the state of document 1, since it has ben preserved in previousItem
  • ✅ ngrx-toolkit >= 19.2 undo restores document 2' original state

Proposal

1. deprecate clearStack
   • Deprecation Note: Please use clearUndoRedo({ lastRecord: null }) instead
2. add clearUndoRedo-updater function

Case - Keep Last record

The way other libs behave

// Usage Example - Keeping last Record
import { withUndoRedo, clearUndoRedo } from '...'';

withMethods(store => ({
  reload: async () => ({
    patchState(store, clearUndoRedo())

    const someData = await store.someClientService.reload()

    patchState(store, { someData })
  })
}))

Case - Set last record

The case for "context switches", where we need a fresh start

// Usage Example - Keeping last Record
import { withUndoRedo, clearUndoRedo } from '...'';

withMethods(store => ({
  reload: async () => ({
    patchState(store, clearUndoRedo({ lastRecord: null }))

    const someData = await store.someClientService.reload()

    patchState(store, { someData })
  })
}))

Why setting { lastRecord: ...} vs. setting it to null?

We discussed resetting previousItem… In other libraries, it is called lastRecord.
By allowing the developer to set the lastRecord instead of just removing it behind the scence we enable more fine-grained resets if multiple parts of the state can be undone or redone.

[GregOnNet]

By the way I am open to create a PR for this, after we agreed on a solution.

[rainerhahnekamp]

Great detailed response @GregOnNet.

I used the term hardReset because my AI suggested it as the common term, but I’m totally open to better naming.

Let me replay the scenario to check my understanding:

• Current behavior: When the user starts loading document 2 and then clears the stack, the first update after document 2 finishes loading becomes the new initial state. From that point on, undo goes back to that state. Until document 2 is fully loaded, there’s no undo available. Correct?

• Proposed behavior: We start loading document 2, but we only clear the stack after it has finished loading. The difference is that during the loading period, the user could still undo and return to the last state of document 1.

So the two questions for me are:

1. What should be the default?
2. What should we call it?

[GregOnNet]

Hi @rainerhahnekamp,

Current vs. Proposed

• According to the discussion both behaviors serve valid use cases

What should be the default

• I vote for: keeping the lastRecord like the other observed libraries do

What should we call it

• My favorite is patchState(store, clearUndoRedo({ lastRecord: null }))
  • What we call peviousItem would be renamed to `lastRecors``
• store.clearStack remains unchanged but gets deprecated

[rainerhahnekamp]

@GregOnNet just to double‑check my understanding:

• When users call patchState(store, clearUndoRedo()), the current state is kept as the new baseline/last record, and the very next change is undoable back to that baseline. Is that correct?

On the API shape:

• If null in lastRecord is only a sentinel, I would really suggest an explicit boolean flag over overloading null. For example:
  • patchState(store, clearUndoRedo({ lastRecordToNull: true })). Otherwise it might lead to a confusion that a value can be used.
• If setting a value was the actual intention, we need to clearify the type. I’d avoid Partial<State> | null because merging raises ambiguity (what happens to omitted fields). I’d suggest:
  • Require full State: clearUndoRedo({ lastRecord: State | null })

Sorry for making it that complicated. I initially thought it was a walk in the park.