-
Notifications
You must be signed in to change notification settings - Fork 227
Feat/store history undo redo #3038
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
base: main
Are you sure you want to change the base?
Changes from all commits
3fa8d31
cba3740
9ffb4d9
7c05976
13800c0
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,80 @@ | ||
| import { describe, expect, it, vi } from 'vitest'; | ||
| import { createStore, batch } from './store'; | ||
|
|
||
| describe('createStore history option', () => { | ||
| it('undo()/redo() round-trip a plain setState', () => { | ||
| const store = createStore(() => ({ count: 0 }), { history: { limit: 50 } }); | ||
| store.setState({ count: 1 }); | ||
| store.setState({ count: 2 }); | ||
|
|
||
| store.undo(); | ||
| expect(store.getState().count).toBe(1); | ||
| store.undo(); | ||
| expect(store.getState().count).toBe(0); | ||
| store.redo(); | ||
| expect(store.getState().count).toBe(1); | ||
| }); | ||
|
|
||
| it('batch() collapses to exactly one history entry', () => { | ||
| const store = createStore((set) => ({ | ||
| count: 0, | ||
| increment: () => set((s) => ({ count: s.count + 1 })), | ||
| }), { history: { limit: 50 } }); | ||
|
|
||
| batch(() => { | ||
| store.getState().increment(); | ||
| store.getState().increment(); | ||
| store.getState().increment(); | ||
| }); | ||
|
|
||
| expect(store.getHistory().past.length).toBe(1); | ||
|
Check failure on line 30 in packages/store/src/history-option.test.ts
|
||
| store.undo(); | ||
| expect(store.getState().count).toBe(0); | ||
| }); | ||
|
|
||
| it('undo() throws when history option is not set', () => { | ||
| const store = createStore(() => ({ count: 0 })); | ||
| expect(() => store.undo()).toThrow(/requires the "history" option/); | ||
| }); | ||
|
|
||
| it('undo() throws when called inside batch()', () => { | ||
| const store = createStore(() => ({ count: 0 }), { history: {} }); | ||
| expect(() => { | ||
| batch(() => { | ||
| store.undo(); | ||
| }); | ||
| }).toThrow(/cannot be called inside batch/); | ||
| }); | ||
|
|
||
| it('coalesceMs merges updates that occur within the window (timestamp-based, not timer-based)', () => { | ||
| vi.useFakeTimers(); | ||
| vi.setSystemTime(0); | ||
| const store = createStore(() => ({ n: 0 }), { history: { coalesceMs: 300 } }); | ||
|
|
||
| store.setState({ n: 1 }); | ||
| vi.setSystemTime(100); // still inside the 300ms window | ||
| store.setState({ n: 2 }); | ||
|
|
||
| expect(store.getHistory().past.length).toBe(1); | ||
|
|
||
| vi.setSystemTime(500); // window has elapsed | ||
| store.setState({ n: 3 }); | ||
| expect(store.getHistory().past.length).toBe(2); | ||
|
|
||
| vi.useRealTimers(); | ||
| }); | ||
|
|
||
| it('limit caps the past stack', () => { | ||
| const store = createStore(() => ({ n: 0 }), { history: { limit: 3 } }); | ||
| for (let i = 1; i <= 5; i++) store.setState({ n: i }); | ||
| expect(store.getHistory().past.length).toBe(3); | ||
| }); | ||
|
|
||
| it('resetHistory() clears both stacks without touching state', () => { | ||
| const store = createStore(() => ({ n: 0 }), { history: {} }); | ||
| store.setState({ n: 1 }); | ||
| store.resetHistory(); | ||
| expect(store.getHistory()).toEqual({ past: [], future: [] }); | ||
| expect(store.getState().n).toBe(1); | ||
| }); | ||
| }); | ||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -41,4 +41,77 @@ describe('createSnapshotHistory', () => { | |||||
|
|
||||||
| expect(history.list().map(snapshot => snapshot.label)).toEqual(['b', 'c']); | ||||||
| }); | ||||||
| it('captures state that includes action functions without throwing', () => { | ||||||
| // Regression test: state created via the (set) => ({...}) pattern | ||||||
| // always includes action methods. structuredClone() throws on | ||||||
| // functions, so capture() must strip them before cloning. | ||||||
| const store = createStore((set) => ({ | ||||||
| count: 0, | ||||||
| increment: () => set((s) => ({ count: s.count + 1 })), | ||||||
| })); | ||||||
| const history = createSnapshotHistory(store); | ||||||
|
|
||||||
| expect(() => history.capture('initial')).not.toThrow(); | ||||||
|
|
||||||
| store.getState().increment(); | ||||||
| store.getState().increment(); | ||||||
| history.capture('after increments'); | ||||||
|
|
||||||
| history.undo(); | ||||||
| expect(store.getState().count).toBe(0); | ||||||
| // action functions must survive a restore, not just data fields | ||||||
| expect(typeof store.getState().increment).toBe('function'); | ||||||
| }); | ||||||
|
|
||||||
| it('restore() with an unknown id throws without moving the cursor', () => { | ||||||
| const store = createStore(() => ({ count: 0 })); | ||||||
| const history = createSnapshotHistory(store); | ||||||
|
|
||||||
| history.capture('a'); | ||||||
| store.setState({ count: 1 }); | ||||||
| history.capture('b'); | ||||||
|
|
||||||
| expect(() => history.restore(9999)).toThrow(/Unknown store snapshot/); | ||||||
| // cursor should be exactly where it was before the failed restore | ||||||
| expect(history.undo()?.state.count).toBe(0); | ||||||
|
Comment on lines
+74
to
+76
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. π Maintainability & Code Quality | π‘ Minor | β‘ Quick win Exercise the state-application failure path. An unknown ID throws before π€ Prompt for AI Agents |
||||||
| }); | ||||||
|
|
||||||
| it('capture() after undo() discards the redo branch', () => { | ||||||
| const store = createStore(() => ({ count: 0 })); | ||||||
| const history = createSnapshotHistory(store); | ||||||
|
|
||||||
| history.capture('a'); | ||||||
| store.setState({ count: 1 }); | ||||||
| history.capture('b'); | ||||||
| history.undo(); | ||||||
|
|
||||||
| store.setState({ count: 99 }); | ||||||
| history.capture('c'); | ||||||
|
|
||||||
| expect(history.redo()).toBeNull(); | ||||||
| }); | ||||||
|
|
||||||
| it('clear() empties history without touching current state', () => { | ||||||
| const store = createStore(() => ({ count: 0 })); | ||||||
| const history = createSnapshotHistory(store); | ||||||
| history.capture('a'); | ||||||
| store.setState({ count: 5 }); | ||||||
| history.capture('b'); | ||||||
|
|
||||||
| history.clear(); | ||||||
|
|
||||||
| expect(history.list()).toEqual([]); | ||||||
| expect(history.undo()).toBeNull(); | ||||||
| expect(store.getState().count).toBe(5); | ||||||
| }); | ||||||
|
|
||||||
| it('returns defensive copies β mutating a returned snapshot does not affect stored history', () => { | ||||||
| const store = createStore(() => ({ nested: { count: 0 } })); | ||||||
| const history = createSnapshotHistory(store); | ||||||
| const snap = history.capture('a'); | ||||||
|
|
||||||
| (snap.state.nested as any).count = 999; | ||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. π Maintainability & Code Quality | π‘ Minor | β‘ Quick win π§© Analysis chainπ Script executed: #!/bin/bash
ast-grep run --lang ts --pattern '$EXPR as any' packages/store/src/snapshot-history.test.tsRepository: Karanjot786/TermUI Length of output: 252 π Script executed: #!/bin/bash
set -euo pipefail
echo "== file excerpt =="
sed -n '90,125p' packages/store/src/snapshot-history.test.ts
echo
echo "== AST outline around test file =="
ast-grep outline packages/store/src/snapshot-history.test.ts || true
echo
echo "== package/type settings snippets =="
for f in tsconfig.json tsconfig.base.json packages/store/tsconfig.json packages/store/src/vitest.config.ts vitest.config.ts pnpm-lock.yaml package.json; do
if [ -f "$f" ]; then
echo "--- $f ---"
sed -n '1,140p' "$f"
fi
done
echo
echo "== snapshot-history source candidates =="
fd -a 'snapshot-history' packages/store/src | while read -r f; do
echo "--- $f ---"
wc -l "$f"
ast-grep outline "$f" || true
sed -n '1,220p' "$f"
doneRepository: Karanjot786/TermUI Length of output: 13162 π Script executed: #!/bin/bash
set -euo pipefail
sed -n '1,85p' packages/store/src/snapshot-history.tsRepository: Karanjot786/TermUI Length of output: 3026 Remove the unannotated
Proposed fix- (snap.state.nested as any).count = 999;
+ snap.state.nested.count = 999;π Committable suggestion
Suggested change
π€ Prompt for AI AgentsSource: Coding guidelines |
||||||
|
|
||||||
| expect(history.list()[0].state.nested.count).toBe(0); | ||||||
| }); | ||||||
| }); | ||||||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -1,15 +1,15 @@ | ||||||||||||||||||||
| import type { Store } from './store.js'; | ||||||||||||||||||||
|
|
||||||||||||||||||||
| export interface StoreSnapshot<T> { | ||||||||||||||||||||
| id: number; | ||||||||||||||||||||
| label?: string; | ||||||||||||||||||||
| state: T; | ||||||||||||||||||||
| } | ||||||||||||||||||||
|
|
||||||||||||||||||||
| export interface SnapshotHistoryOptions { | ||||||||||||||||||||
| limit?: number; | ||||||||||||||||||||
| } | ||||||||||||||||||||
|
|
||||||||||||||||||||
| export interface StoreSnapshotHistory<T extends object> { | ||||||||||||||||||||
| capture(label?: string): StoreSnapshot<T>; | ||||||||||||||||||||
| restore(id: number): StoreSnapshot<T>; | ||||||||||||||||||||
|
|
@@ -18,7 +18,40 @@ export interface StoreSnapshotHistory<T extends object> { | |||||||||||||||||||
| list(): StoreSnapshot<T>[]; | ||||||||||||||||||||
| clear(): void; | ||||||||||||||||||||
| } | ||||||||||||||||||||
|
|
||||||||||||||||||||
|
|
||||||||||||||||||||
| // Deep-clone while stripping function values at every level β action | ||||||||||||||||||||
| // methods (set()-bound closures) live directly on store state and are not | ||||||||||||||||||||
| // structured-cloneable. This mirrors store.ts's own safeDeepClone, which | ||||||||||||||||||||
| // exists for the same reason. | ||||||||||||||||||||
| function cloneState<T>(state: T): T { | ||||||||||||||||||||
| const strip = (value: unknown): unknown => { | ||||||||||||||||||||
| if (value === null || typeof value !== 'object') return value; | ||||||||||||||||||||
| if (value instanceof Date || value instanceof RegExp) return value; | ||||||||||||||||||||
| if (Array.isArray(value)) return value.map(strip); | ||||||||||||||||||||
| if (value instanceof Map) { | ||||||||||||||||||||
| const m = new Map(); | ||||||||||||||||||||
| for (const [k, v] of value) m.set(k, strip(v)); | ||||||||||||||||||||
| return m; | ||||||||||||||||||||
| } | ||||||||||||||||||||
| if (value instanceof Set) { | ||||||||||||||||||||
| const s = new Set(); | ||||||||||||||||||||
| for (const v of value) s.add(strip(v)); | ||||||||||||||||||||
| return s; | ||||||||||||||||||||
| } | ||||||||||||||||||||
| const out: Record<string, unknown> = {}; | ||||||||||||||||||||
| for (const [k, v] of Object.entries(value as Record<string, unknown>)) { | ||||||||||||||||||||
| if (typeof v === 'function') continue; | ||||||||||||||||||||
| out[k] = strip(v); | ||||||||||||||||||||
| } | ||||||||||||||||||||
|
Comment on lines
+27
to
+45
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. π― Functional Correctness | π Major | ποΈ Heavy lift Strip functions before handling container types. Functions in arrays, Sets, Map values, or Map keys currently reach π€ Prompt for AI Agents |
||||||||||||||||||||
| return out; | ||||||||||||||||||||
| }; | ||||||||||||||||||||
|
|
||||||||||||||||||||
| const stripped = strip(state); | ||||||||||||||||||||
| return typeof structuredClone === 'function' | ||||||||||||||||||||
| ? structuredClone(stripped as T) | ||||||||||||||||||||
| : (JSON.parse(JSON.stringify(stripped)) as T); | ||||||||||||||||||||
|
Comment on lines
+26
to
+52
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. π― Functional Correctness | π Major | β‘ Quick win π§© Analysis chainπ Script executed: #!/bin/bash
ast-grep run --lang ts --pattern '$EXPR as $TYPE' packages/store/src/snapshot-history.tsRepository: Karanjot786/TermUI Length of output: 663 π Script executed: #!/bin/bash
set -euo pipefail
echo "File size and relevant contents:"
wc -l packages/store/src/snapshot-history.ts
cat -n packages/store/src/snapshot-history.ts
echo
echo "Type assertion occurrences with inline comments:"
python3 - <<'PY'
from pathlib import Path
p=Path("packages/store/src/snapshot-history.ts")
text=p.read_text()
for i,line in enumerate(text.splitlines(),1):
if " as " in line:
print(f"{i}: {line}")
PYRepository: Karanjot786/TermUI Length of output: 5473 π Script executed: #!/bin/bash
set -euo pipefail
echo "Store.ts outline:"
ast-grep outline packages/store/src/store.ts --view expanded || true
echo
echo "Relevant Store<T> contents:"
wc -l packages/store/src/store.ts
sed -n '1,180p' packages/store/src/store.ts | cat -n
echo
echo "Behavioral probe for cloneState stripping and clone behavior:"
node - <<'JS'
function probeClone(fn) {
const original = {
label: 'x',
value: 1,
action: () => 'action',
nested: { fnField: () => 'nested', list: [{ fnField: () => 'nested item' }, 1], map: new Map([['fnField', () => 'map fn']]), set: new Set([() => 'set fn', 1]) },
};
const stripped = fn(original.state || original ?? original);
return {
originalKeys: Object.keys(original),
returnedKeys: Object.keys(stripped),
topActionAbsent: typeof stripped.action === 'function' ? 'present' : 'absent',
nestedFnAbsent: typeof stripped.nested?.fnField === 'function' ? 'present' : 'absent',
listFnCount: (stripped.nested?.list ?? []).filter(i => typeof i === 'function').length,
mapEntries: [...(stripped.nested?.map ?? [])].filter(([k, v]) => typeof k === 'string' && k.endsWith('Field')).length,
};
}
const hasStructuredClone = typeof globalThis.structuredClone === 'function';
console.log(JSON.stringify({
hasStructuredClone,
stripJson: probeClone(s => (s && typeof s === 'object' && !Array.isArray(s) && !(s instanceof Date || s instanceof RegExp || s instanceof Map || s instanceof Set))
? JSON.parse(JSON.stringify(s)) : s),
stripObjectEntries: probeClone(s => (() => {
const out = {};
for (const [k, v] of Object.entries(s)) {
if (typeof v === 'function') continue;
out[k] = v;
}
return out;
})()),
cloneResult: hasStructuredClone ? probeClone((s) => globalThis.structuredClone(s)) : null,
}));
JS
echo
echo "Check Store<T> action typing/usages:"
rg -n "Store<[^>]+>.*:|Store\\(" packages --glob '*.ts' --glob '*.tsx' | head -120Repository: Karanjot786/TermUI Length of output: 9639 π Script executed: #!/bin/bash
set -euo pipefail
echo "Relevant Store<T> contents:"
sed -n '188,820p' packages/store/src/store.ts | cat -n
echo
echo "Behavioral probe for cloneState stripping and clone behavior:"
node - <<'JS'
function probeClone(fn) {
const original = {
label: 'x',
value: 1,
action: () => 'action',
nested: { fnField: () => 'nested', list: [{ fnField: () => 'nested item' }, 1], map: new Map([['fnField', () => 'map fn']]), set: new Set([() => 'set fn', 1]) },
};
const stripped = fn(original);
return {
originalKeys: Object.keys(original),
returnedKeys: Object.keys(stripped),
topActionAbsent: typeof stripped.action === 'function' ? 'present' : 'absent',
nestedFnAbsent: typeof stripped.nested?.fnField === 'function' ? 'present' : 'absent',
listFnCount: (stripped.nested?.list ?? []).filter(i => typeof i === 'function').length,
mapEntries: [...(stripped.nested?.map ?? [])].filter(([k, v]) => typeof k === 'string' && k.endsWith('Field')).length,
};
}
const hasStructuredClone = typeof globalThis.structuredClone === 'function';
console.log(JSON.stringify({
hasStructuredClone,
stripJson: probeClone(s => ((s && typeof s === 'object' && !Array.isArray(s) && !(s instanceof Date || s instanceof RegExp || s instanceof Map || s instanceof Set))
? JSON.parse(JSON.stringify(s)) : s)),
stripObjectEntries: probeClone(s => (() => {
const out = {};
for (const [k, v] of Object.entries(s)) {
if (typeof v === 'function') continue;
out[k] = v;
}
return out;
})()),
cloneResult: hasStructuredClone ? probeClone((s) => {
const out = {};
for (const [k, v] of Object.entries(s)) {
if (typeof v === 'function') continue;
out[k] = v;
}
return globalThis.structuredClone(out);
}) : null
}, null, 2));
JS
echo
echo "Check Store<T> action factory examples/usages:"
rg -n "createStore\\([^\\n]+=>|set\\([^\\n]+=>\\s*\\({|increment:|reset:|setState\\(" packages --glob '*.ts' --glob '*.tsx' | head -200Repository: Karanjot786/TermUI Length of output: 27949 π Script executed: #!/bin/bash
set -euo pipefail
echo "Behavioral probe for cloneState stripping and clone behavior:"
node - <<'JS'
function probeClone(fn, input) {
const stripped = fn(input);
const arr = stripped?.nested?.list ?? [];
const map = stripped?.nested?.map ?? new Map();
const mapEntries = [...map.entries()].filter(([k, v]) => typeof k === 'string' && k.endsWith('Field'));
return {
originalKeys: Object.keys(input),
returnedKeys: Object.keys(stripped),
topActionAbsent: typeof stripped.action === 'function' ? 'present' : 'absent',
nestedFnAbsent: typeof stripped.nested?.fnField === 'function' ? 'present' : 'absent',
listFnCount: arr.reduce((n, i) => n + (typeof i === 'function' ? 1 : 0), 0),
mapValuesWithFnKey: mapEntries.filter(([, v]) => typeof v === 'function').length,
};
}
const hasStructuredClone = typeof globalThis.structuredClone === 'function';
const input = {
label: 'x',
value: 1,
action: () => 'action',
nested: {
fnField: () => 'nested',
list: [{ fnField: () => 'nested item' }, 1],
map: new Map([['fnField', () => 'map fn']]),
set: new Set([() => 'set fn', 1]),
},
};
console.log(JSON.stringify({
hasStructuredClone,
stripJson: probeClone(s => ((s && typeof s === 'object' && !Array.isArray(s) && !(s instanceof Date || s instanceof RegExp || s instanceof Map || s instanceof Set))
? JSON.parse(JSON.stringify(s)) : s), input),
stripObjectEntries: probeClone(s => {
const out = {};
for (const [k, v] of Object.entries(s)) {
if (typeof v === 'function') continue;
out[k] = v;
}
return out;
}, input),
cloneResult: hasStructuredClone ? probeClone((s) => {
const out = {};
for (const [k, v] of Object.entries(s)) {
if (typeof v === 'function') continue;
out[k] = v;
}
return globalThis.structuredClone(out);
}, input) : null
}, null, 2));
console.log("JSON.stringify preserves custom Date RegExp:");
const obj = { date: new Date("2000-01-01T12:34:56Z"), re: /abc/gi, func: () => {} };
console.log(JSON.stringify(JSON.parse(JSON.stringify(obj))));
JSRepository: Karanjot786/TermUI Length of output: 943 π Script executed: #!/bin/bash
set -euo pipefail
echo "Behavioral probe for cloneState stripping and clone behavior:"
node - <<'JS'
function probeClone(fn, input) {
const stripped = fn(input);
const arr = stripped && stripped.nested && stripped.nested.list ? stripped.nested.list : [];
const map = stripped && stripped.nested && stripped.nested.map ? stripped.nested.map : new Map();
const mapEntries = Array.from(map.entries()).filter(([k, v]) => typeof k === 'string' && k.endsWith('Field'));
return {
originalKeys: Object.keys(input),
returnedKeys: stripped ? Object.keys(stripped) : [],
topActionAbsent: typeof (stripped && stripped.action) === 'function' ? 'present' : 'absent',
nestedFnAbsent: typeof (stripped && stripped.nested && stripped.nested.fnField) === 'function' ? 'present' : 'absent',
listFnCount: arr.reduce((n, i) => n + (typeof i === 'function' ? 1 : 0), 0),
mapValuesWithFnKey: mapEntries.filter(([, v]) => typeof v === 'function').length,
};
}
const hasStructuredClone = typeof globalThis.structuredClone === 'function';
const input = {
label: 'x',
value: 1,
action: () => 'action',
nested: {
fnField: () => 'nested',
list: [{ fnField: () => 'nested item' }, 1],
map: new Map([['fnField', () => 'map fn']]),
set: new Set([() => 'set fn', 1]),
},
};
console.log(JSON.stringify({
hasStructuredClone,
stripJson: probeClone(s => ((s && typeof s === 'object' && !Array.isArray(s) && !(s instanceof Date || s instanceof RegExp || s instanceof Map || s instanceof Set))
? JSON.parse(JSON.stringify(s)) : s), input),
strictObjectEntries: probeClone(s => {
const out = {};
for (const [k, v] of Object.entries(s)) {
if (typeof v === 'function') continue;
out[k] = v;
}
return out;
}, input),
cloneResult: hasStructuredClone ? probeClone((s) => {
const out = {};
for (const [k, v] of Object.entries(s)) {
if (typeof v === 'function') continue;
out[k] = v;
}
return globalThis.structuredClone(out);
}, input) : null
}, null, 2));
console.log("JSON.stringify preserves custom Date RegExp:");
const obj = { date: new Date("2000-01-01T12:34:56Z"), re: /abc/gi, func: () => {} };
console.log(JSON.stringify(JSON.parse(JSON.stringify(obj))));
JSRepository: Karanjot786/TermUI Length of output: 921 π Script executed: #!/bin/bash
set -euo pipefail
node - <<'JS'
console.log(process.version, typeof globalThis.structuredClone);
const input = {
action: function action() { return 'action'; },
nested: {
fnField: function fnField() { return 'nested'; },
list: [function nestedFn() { return 'nested item'; }, 1],
set: new Set([function setFn() {}, 1]),
},
};
console.log("Object.keys(input):", Object.keys(input));
console.log("JSON.stringify(JSON.parse(input)):");
console.log(JSON.stringify(JSON.parse(JSON.stringify(input))));
const stripped = {};
for (const [k, v] of Object.entries(input)) {
if (typeof v === 'function') continue;
stripped[k] = v;
}
console.log("Object.keys(stripped):", Object.keys(stripped));
const clonedMap = new Map();
clonedMap.set('fnField', function mapFn() {});
console.log("cloneWithMapObjectKeys:", Object.keys(clonedMap));
console.log("cloneWithMapMapEntries:", [...clonedMap.entries()]);
console.log("delete function cloneWithMap:");
const clonedDeleteMap = new Map();
clonedDeleteMap.set('fnField', function mapFn() {});
setDeleteMap(clonedDeleteMap, 'fnField');
console.log([...clonedDeleteMap.entries()]);
JSRepository: Karanjot786/TermUI Length of output: 1050 Return non-action state from snapshot history.
π€ Prompt for AI AgentsSource: Coding guidelines |
||||||||||||||||||||
| } | ||||||||||||||||||||
|
|
||||||||||||||||||||
| export function createSnapshotHistory<T extends object>( | ||||||||||||||||||||
| store: Store<T>, | ||||||||||||||||||||
| options: SnapshotHistoryOptions = {}, | ||||||||||||||||||||
|
|
@@ -27,56 +60,59 @@ export function createSnapshotHistory<T extends object>( | |||||||||||||||||||
| const snapshots: StoreSnapshot<T>[] = []; | ||||||||||||||||||||
| let cursor = -1; | ||||||||||||||||||||
| let nextId = 1; | ||||||||||||||||||||
|
|
||||||||||||||||||||
| const clone = (state: T): T => { | ||||||||||||||||||||
| if (typeof structuredClone === 'function') return structuredClone(state); | ||||||||||||||||||||
| return JSON.parse(JSON.stringify(state)) as T; | ||||||||||||||||||||
| }; | ||||||||||||||||||||
|
|
||||||||||||||||||||
|
|
||||||||||||||||||||
| const apply = (snapshot: StoreSnapshot<T>): StoreSnapshot<T> => { | ||||||||||||||||||||
| const before = clone(store.getState()); | ||||||||||||||||||||
| const before = cloneState(store.getState()); | ||||||||||||||||||||
| try { | ||||||||||||||||||||
| store.setState(clone(snapshot.state) as Partial<T>); | ||||||||||||||||||||
| store.setState(cloneState(snapshot.state) as Partial<T>); | ||||||||||||||||||||
| } catch (error) { | ||||||||||||||||||||
| store.setState(before as Partial<T>); | ||||||||||||||||||||
| throw error; | ||||||||||||||||||||
| } | ||||||||||||||||||||
| return { ...snapshot, state: clone(snapshot.state) }; | ||||||||||||||||||||
| return { ...snapshot, state: cloneState(snapshot.state) }; | ||||||||||||||||||||
| }; | ||||||||||||||||||||
|
|
||||||||||||||||||||
| return { | ||||||||||||||||||||
| capture(label) { | ||||||||||||||||||||
| snapshots.splice(cursor + 1); | ||||||||||||||||||||
| const snapshot = { id: nextId++, label, state: clone(store.getState()) }; | ||||||||||||||||||||
| const snapshot = { id: nextId++, label, state: cloneState(store.getState()) }; | ||||||||||||||||||||
| snapshots.push(snapshot); | ||||||||||||||||||||
| while (snapshots.length > limit) { | ||||||||||||||||||||
| snapshots.shift(); | ||||||||||||||||||||
| } | ||||||||||||||||||||
|
Comment on lines
80
to
82
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. π©Ί Stability & Availability | π΄ Critical | β‘ Quick win Reject negative history limits. With Proposed fix const limit = options.limit ?? 50;
+if (!Number.isSafeInteger(limit) || limit < 0) {
+ throw new RangeError('Snapshot history limit must be a non-negative integer');
+}π Committable suggestion
Suggested change
π€ Prompt for AI Agents |
||||||||||||||||||||
| cursor = snapshots.length - 1; | ||||||||||||||||||||
| return { ...snapshot, state: clone(snapshot.state) }; | ||||||||||||||||||||
| return { ...snapshot, state: cloneState(snapshot.state) }; | ||||||||||||||||||||
| }, | ||||||||||||||||||||
| restore(id) { | ||||||||||||||||||||
| const index = snapshots.findIndex(snapshot => snapshot.id === id); | ||||||||||||||||||||
| if (index === -1) throw new Error(`Unknown store snapshot: ${id}`); | ||||||||||||||||||||
| const result = apply(snapshots[index]); | ||||||||||||||||||||
| // Only advance the cursor once the state change actually succeeded β | ||||||||||||||||||||
| // otherwise a thrown setState leaves cursor out of sync with live state. | ||||||||||||||||||||
| cursor = index; | ||||||||||||||||||||
| return apply(snapshots[index]); | ||||||||||||||||||||
| return result; | ||||||||||||||||||||
| }, | ||||||||||||||||||||
| undo() { | ||||||||||||||||||||
| if (cursor <= 0) return null; | ||||||||||||||||||||
| cursor--; | ||||||||||||||||||||
| return apply(snapshots[cursor]); | ||||||||||||||||||||
| const targetIndex = cursor - 1; | ||||||||||||||||||||
| const result = apply(snapshots[targetIndex]); | ||||||||||||||||||||
| cursor = targetIndex; | ||||||||||||||||||||
| return result; | ||||||||||||||||||||
| }, | ||||||||||||||||||||
| redo() { | ||||||||||||||||||||
| if (cursor >= snapshots.length - 1) return null; | ||||||||||||||||||||
| cursor++; | ||||||||||||||||||||
| return apply(snapshots[cursor]); | ||||||||||||||||||||
| const targetIndex = cursor + 1; | ||||||||||||||||||||
| const result = apply(snapshots[targetIndex]); | ||||||||||||||||||||
| cursor = targetIndex; | ||||||||||||||||||||
| return result; | ||||||||||||||||||||
| }, | ||||||||||||||||||||
| list() { | ||||||||||||||||||||
| return snapshots.map(snapshot => ({ ...snapshot, state: clone(snapshot.state) })); | ||||||||||||||||||||
| return snapshots.map(snapshot => ({ ...snapshot, state: cloneState(snapshot.state) })); | ||||||||||||||||||||
| }, | ||||||||||||||||||||
| clear() { | ||||||||||||||||||||
| snapshots.splice(0); | ||||||||||||||||||||
| cursor = -1; | ||||||||||||||||||||
| }, | ||||||||||||||||||||
| }; | ||||||||||||||||||||
| } | ||||||||||||||||||||
|
|
||||||||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
π©Ί Stability & Availability | π Major | β‘ Quick win
Restore real timers via
afterEach, not inline at test end.vi.useRealTimers()is only called at the end of the test body (Line 64). If an assertion (Lines 58 or 62) throws first, fake timers leak into subsequent tests.As per coding guidelines: "Use
vi.useFakeTimers()for debounce or persist tests andvi.useRealTimers()inafterEachhooks; real timers should be off by default."π Proposed fix
vi.setSystemTime(500); // window has elapsed store.setState({ n: 3 }); expect(store.getHistory().past.length).toBe(2); - - vi.useRealTimers(); });π Committable suggestion
π€ Prompt for AI Agents
Source: Coding guidelines