patch: fix createUndoHistory for 1.0

.changeset/history-undo-multi-source-fix.md

@@ -0,0 +1,9 @@
+---
+"@solid-primitives/history": patch
+---
+
+Fix `createUndoHistory` misfiring restores when tracking multiple sources and one of them pauses (returns a falsy value) while another stays active.
+
+Previously each history entry stored a compacted array of setters, dropping any paused source — so entries on either side of a pause/resume boundary could end up with different lengths, causing `undo`/`redo` to compare setters by the wrong array index and either restore the wrong source or spuriously re-fire one that hadn't actually changed. Entries now keep a fixed-length slot per source (`undefined` when paused), so index alignment is stable across every recorded entry.
+
+Credit to @mesram, whose `createStore`/`createOptimistic`-based rewrite (for the Solid 2.0 line) is the basis for this fix.

packages/history/src/index.ts

@@ -85,19 +85,21 @@ export function createUndoHistory(
 
   let ignoreNext = false;
 
+  // One slot per source, always the same length as `sources` — even when a
+  // source is paused (returns falsy) it keeps its `undefined` slot instead of
+  // being dropped, so entries never end up with mismatched lengths and index
+  // i always refers to the same source across every recorded entry.
+  type Setters = (VoidFunction | undefined)[];
+
   const limit = options?.limit ?? 100,
     sources = Array.isArray(source) ? source.map(s => createMemo(s)) : [source],
     clearIgnore = createMicrotask(() => (ignoreNext = false)),
-    history = createMemo<{ count: Signal<number>; list: VoidFunction[][] }>(
+    history = createMemo<{ count: Signal<number>; list: Setters[] }>(
       p => {
         // always track the sources
-        const setters: VoidFunction[] = [];
-        for (const s of sources) {
-          const setter = s();
-          if (setter) setters.push(setter);
-        }
+        const setters: Setters = sources.map(s => s() || undefined);
 
-        if (ignoreNext || !setters.length) {
+        if (ignoreNext || setters.every(s => s === undefined)) {
           ignoreNext = false;
           return p;
         }
@@ -125,8 +127,13 @@ export function createUndoHistory(
         prevSetters = h.list[newIndex + n]!,
         setters = h.list[newIndex]!;
       for (let i = 0; i < setters.length; i++) {
-        // only call the setter if the current value is different
-        if (setters[i] !== prevSetters[i]) setters[i]!();
+        // only call the setter if it was active on both sides of the move
+        // and the value actually differs — if a source was paused on either
+        // side we have no tracked value to compare against, so skip it
+        // rather than firing a spurious restore
+        const setter = setters[i],
+          prevSetter = prevSetters[i];
+        if (setter !== undefined && prevSetter !== undefined && setter !== prevSetter) setter();
       }
     };
 

packages/history/test/index.test.ts

@@ -287,4 +287,55 @@ describe("createUndoHistory", () => {
       dispose();
     });
   });
+
+  test("multiple sources, one intermittently paused - no spurious/misaligned restores", () => {
+    createRoot(dispose => {
+      const [a, setA] = createSignal(0);
+      const [b, setB] = createSignal(0);
+      const [trackB, setTrackB] = createSignal(true);
+      let bCalls = 0;
+
+      const history = createUndoHistory([
+        () => {
+          const v = a();
+          return () => setA(v);
+        },
+        () => {
+          if (!trackB()) return undefined;
+          const v = b();
+          return () => {
+            bCalls++;
+            setB(v);
+          };
+        },
+      ]);
+
+      // recorded entries: E0(a0,b0) E1(a1,b0) E2(a1,-b paused-) E3(a2,-b paused-)
+      setA(1);
+      setTrackB(false);
+      setA(2);
+
+      history.undo(); // E3 -> E2
+      expect(a()).toBe(1);
+      expect(bCalls).toBe(0); // b never changed, must not fire
+
+      history.undo(); // E2 -> E1 (crosses the pause boundary)
+      expect(a()).toBe(1); // a is legitimately unchanged between E1/E2 — not a bug
+      expect(bCalls).toBe(0); // must not spuriously re-fire b just because array shapes differ across the boundary
+
+      history.undo(); // E1 -> E0
+      expect(a()).toBe(0);
+      expect(bCalls).toBe(0);
+      expect(history.canUndo()).toBe(false);
+
+      history.redo(); // E0 -> E1
+      history.redo(); // E1 -> E2 (crosses the pause boundary going forward)
+      expect(bCalls).toBe(0);
+      history.redo(); // E2 -> E3
+      expect(a()).toBe(2);
+      expect(bCalls).toBe(0);
+
+      dispose();
+    });
+  });
 });