toggleNode (src/store/modelerStore.ts:260) mutates the document without recording it:
toggleNode: (id) => {
const { tree } = get();
if (!tree) return;
const newTree = updateInTree(tree, id, (node) => ({ ...node, enabled: !node.enabled }));
set({ tree: newTree }); // no history push
},Every other document mutation in the store follows a four-line ritual — slice(0, historyIndex + 1), push cloneTree(newTree), set tree/history/historyIndex — repeated at roughly thirteen sites. toggleNode is the one that skips it.
Consequence
The invariant tree is structurally equal to history[historyIndex] is broken as soon as a node is toggled. Concretely: toggle a node off, then undo. Undo assigns tree = history[historyIndex - 1], a snapshot that predates the toggle and predates whatever edit came before it — so the toggle silently vanishes along with one more edit than the user asked to undo. Redo then reintroduces a state that never existed as such.
Disabling a node changes the rendered geometry and the exported mesh, so this is a document mutation, not view state. Compare toggleExpanded (:266) and setSelectedNode, which are genuinely view-only and correctly stay out of history.
Fix
Either push history in toggleNode like every other mutation, or — better, given thirteen hand-rolled copies — extract a commit(newTree) helper that does the slice/clone/push/set in one place and route every mutating action through it. That removes the class of bug rather than this instance.
immer is already a declared dependency and unused in this file; a helper could use it to avoid the JSON.parse(JSON.stringify(...)) deep clone at :75.
Related
While reading this: undo (:682) and redo (:690) restore only tree and historyIndex, not selectedNodeId. After undoing a node creation, selectedNodeId can dangle at a node no longer in the tree. Consumers mostly do findNode(...) and bail on null so it degrades quietly, but removeNode (:254) is the only action that clears a stale selection. Worth handling in the same change.
toggleNode(src/store/modelerStore.ts:260) mutates the document without recording it:Every other document mutation in the store follows a four-line ritual —
slice(0, historyIndex + 1), pushcloneTree(newTree), settree/history/historyIndex— repeated at roughly thirteen sites.toggleNodeis the one that skips it.Consequence
The invariant
treeis structurally equal tohistory[historyIndex]is broken as soon as a node is toggled. Concretely: toggle a node off, then undo. Undo assignstree = history[historyIndex - 1], a snapshot that predates the toggle and predates whatever edit came before it — so the toggle silently vanishes along with one more edit than the user asked to undo. Redo then reintroduces a state that never existed as such.Disabling a node changes the rendered geometry and the exported mesh, so this is a document mutation, not view state. Compare
toggleExpanded(:266) andsetSelectedNode, which are genuinely view-only and correctly stay out of history.Fix
Either push history in
toggleNodelike every other mutation, or — better, given thirteen hand-rolled copies — extract acommit(newTree)helper that does the slice/clone/push/set in one place and route every mutating action through it. That removes the class of bug rather than this instance.immeris already a declared dependency and unused in this file; a helper could use it to avoid theJSON.parse(JSON.stringify(...))deep clone at:75.Related
While reading this:
undo(:682) andredo(:690) restore onlytreeandhistoryIndex, notselectedNodeId. After undoing a node creation,selectedNodeIdcan dangle at a node no longer in the tree. Consumers mostly dofindNode(...)and bail on null so it degrades quietly, butremoveNode(:254) is the only action that clears a stale selection. Worth handling in the same change.