Skip to content

Undo/redo: impure setState updaters double-push history under StrictMode; importing a project doesn't clear the stacks (undo bleeds across projects) #98

Description

@dompm

1. Side effects inside setState updaters

main.tsx enables <StrictMode>, and useProject.ts performs side effects inside state updaters:

  • useProject.ts:189-203setUndoStack/setRedoStack calls and timer scheduling inside the setProject updater
  • undo/redo (useProject.ts:205-233) — nested setProject inside setUndoStack/setRedoStack updaters
  • selectPiece (:393-399) — setActiveSheetId inside a setProject updater; addPieceFromBox (:674,684) — setSelectedPieceIds inside the updater

React double-invokes updaters in dev StrictMode, so every edit pushes onto the undo stack twice — in dev, each Ctrl+Z must be pressed twice and canUndo is wrong. Beyond dev mode, this pattern is explicitly unsupported and breaks under concurrent rendering.

Fix: move history into a single reducer — { past, present, future } — so one dispatch atomically updates project + history. This also gives selection/undo consistency for free.

2. loadProjectData doesn't reset history — and can overwrite the previous project's file

useProject.ts:849-856 — unlike switchProject/createNewProject (which reset both stacks), loadProjectData (used by JSON import, drag-drop import, and the tutorial at App.tsx:387) leaves the old project's states in undoStack. Import project B while editing project A, press Ctrl+Z → the canvas snaps to A's old state and the scheduled persist(prev, prev.name) (useProject.ts:213) writes that stale state over A's file in OPFS. Two projects desynced by one keystroke.

3. No schema version or validation on imported/loaded JSON

Project (types.ts:80-93) has no version field; loadProjectFromOPFS blind-casts JSON.parse (opfs.ts:21); loadProjectData accepts raw user JSON from file-open/drag-drop with zero validation (App.tsx:719, 984). A truncated or hand-edited file doesn't fail at load — it crashes later deep in render (e.g. prev.patternCrop spread at useProject.ts:372), and the broken object can get autosaved over the OPFS copy. The schema has clearly evolved (lampConfig, curvePoints); add version: number + a small migration ladder + structural validation at the import boundary, and reset undo/redo there.

4. Minor: sheet IDs from Date.now()

useProject.ts:34,922,959 — pieces use crypto.randomUUID() but sheets use sheet-${Date.now()}; two sheets in the same millisecond (or import + tutorial defaults sheet-1..n) can collide, breaking piece→sheet references and React keys. Use randomUUID() everywhere.

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions