Workspace 'Save to Gallery' silently no-ops after Run Auto-Digitize — closure captures stale appState='digitizing' #102

Closed
opened 2026-09-18 23:39:33 +00:00 by Mark · 1 comment
Owner

Bug 1: Save to Gallery silent no-op after digitize

Severity

High (data-loss-impacting). The user-facing "Save to Gallery" button does nothing
after the recent runAutoDigitize rewrite.

Reproduction

  1. Bring up easystitch-service on :8080 (live FastAPI) and the ThreadCraft dev server on :3000.
  2. Navigate to /workspace.
  3. Upload tests/fixtures/flower.png.
  4. Click Run Auto-Digitize. Wait for the workspace state to advance to digitized.
  5. Click Save to Gallery (the data-testid="save-to-gallery" button in AppHeader).
  6. Open DevTools → Application → Local Storage → tc_gallery_designs. It's still null.

Expected: a Design entry appears under tc_gallery_designs in localStorage.
Actual: addDesign(design) is never called. localStorage.setItem is never invoked.

Evidence

Reproduced by shim-instrumenting saveCurrentDesign in
src/components/workspace/useWorkspace.ts to log the closure state on entry:

[SHIM-SAVE] entered; uploadedImage=SET appState=digitizing stitchObjects=0 threadStops=0

The closure still sees appState='digitizing' at click time, even though the
canvas is mounted (meaning the actual appState in Redux is 'digitized'). The
second guard at useWorkspace.ts:765-768:

if (state.appState === 'digitizing') {
  addToast('error', 'Wait for auto-digitize to finish before saving');
  return null;
}

fires before addDesign(design) is reached, returning null silently.

This was confirmed two ways:

  • Storage.prototype.setItem was shimmed to log every write — zero writes
    occurred between Save click and the end of the test.
  • localStorage.getItem('tc_gallery_designs') is null before AND after Save
    in a clean session.

Root cause

saveCurrentDesign is a useCallback whose dep array is
[state.uploadedImage, state.appState, state.designSettings.fileName, state.designSettings.hoopSize, state.stitchObjects, state.threadStops, state.colorRegions, addToast]
(useWorkspace.ts:815). After commitDigitizedResult dispatches
SET_STATE: 'digitizing', a new closure is created.

pages/Workspace.tsx:254 passes onSave={saveCurrentDesign} down to
components/workspace/AppHeader.tsx. The <button onClick={onSave}> at
AppHeader.tsx:170 captures onSave at render time. Because the AppHeader's
props are deep-equal on the prior render (the closure identity changed but the
prop value didn't), React's reconciler may not re-render the button — the DOM
keeps the previous onclick reference.

When the user clicks Save, the previous closure fires, and it sees
appState='digitizing' because it was captured before the digitize started.

Suggested fix

Wrap the callback in a ref pattern so AppHeader always reads the latest closure:

const saveCurrentDesignRef = useRef(saveCurrentDesign);
saveCurrentDesignRef.current = saveCurrentDesign;
// then in AppHeader: onClick={() => saveCurrentDesignRef.current()}

Alternative: useEvent from react-use, or context.

  • #63 (closed) — originally "no Save Session button"; button was added but the
    wiring landed broken from day one.
  • #65 (closed) — gallery persistence; this bug dodges that fix entirely because
    designs never reach tc_gallery_designs.
  • #74 (closed) — added "Open in Workspace" in Gallery. Cannot exercise that
    button end-to-end until Save works.
# Bug 1: Save to Gallery silent no-op after digitize ## Severity High (data-loss-impacting). The user-facing "Save to Gallery" button does nothing after the recent runAutoDigitize rewrite. ## Reproduction 1. Bring up easystitch-service on :8080 (live FastAPI) and the ThreadCraft dev server on :3000. 2. Navigate to `/workspace`. 3. Upload `tests/fixtures/flower.png`. 4. Click **Run Auto-Digitize**. Wait for the workspace state to advance to `digitized`. 5. Click **Save to Gallery** (the `data-testid="save-to-gallery"` button in AppHeader). 6. Open DevTools → Application → Local Storage → `tc_gallery_designs`. It's still `null`. Expected: a `Design` entry appears under `tc_gallery_designs` in localStorage. Actual: `addDesign(design)` is never called. `localStorage.setItem` is never invoked. ## Evidence Reproduced by shim-instrumenting `saveCurrentDesign` in `src/components/workspace/useWorkspace.ts` to log the closure state on entry: ``` [SHIM-SAVE] entered; uploadedImage=SET appState=digitizing stitchObjects=0 threadStops=0 ``` The closure **still sees `appState='digitizing'`** at click time, even though the canvas is mounted (meaning the actual appState in Redux is `'digitized'`). The second guard at `useWorkspace.ts:765-768`: ```ts if (state.appState === 'digitizing') { addToast('error', 'Wait for auto-digitize to finish before saving'); return null; } ``` fires before `addDesign(design)` is reached, returning `null` silently. This was confirmed two ways: - `Storage.prototype.setItem` was shimmed to log every write — **zero** writes occurred between Save click and the end of the test. - `localStorage.getItem('tc_gallery_designs')` is `null` before AND after Save in a clean session. ## Root cause `saveCurrentDesign` is a `useCallback` whose dep array is `[state.uploadedImage, state.appState, state.designSettings.fileName, state.designSettings.hoopSize, state.stitchObjects, state.threadStops, state.colorRegions, addToast]` (`useWorkspace.ts:815`). After `commitDigitizedResult` dispatches `SET_STATE: 'digitizing'`, a new closure is created. `pages/Workspace.tsx:254` passes `onSave={saveCurrentDesign}` down to `components/workspace/AppHeader.tsx`. The `<button onClick={onSave}>` at `AppHeader.tsx:170` captures `onSave` at render time. Because the AppHeader's props are deep-equal on the prior render (the closure identity changed but the prop value didn't), React's reconciler may not re-render the button — the DOM keeps the previous `onclick` reference. When the user clicks Save, the **previous** closure fires, and it sees `appState='digitizing'` because it was captured before the digitize started. ## Suggested fix Wrap the callback in a ref pattern so AppHeader always reads the latest closure: ```ts const saveCurrentDesignRef = useRef(saveCurrentDesign); saveCurrentDesignRef.current = saveCurrentDesign; // then in AppHeader: onClick={() => saveCurrentDesignRef.current()} ``` Alternative: `useEvent` from `react-use`, or context. ## Related - #63 (closed) — originally "no Save Session button"; button was added but the wiring landed broken from day one. - #65 (closed) — gallery persistence; this bug dodges that fix entirely because designs never reach `tc_gallery_designs`. - #74 (closed) — added "Open in Workspace" in Gallery. Cannot exercise that button end-to-end until Save works.
Mark closed this issue 2026-09-18 23:58:27 +00:00
Author
Owner

Fixed by 11752b2 — AppHeader dispatches Save/Export through latest-callback refs so the button always reads the freshest onSave closure rather than the one captured at first render. SHIM [SHIM-SAVE] debug log also removed (was added during E2E reproduction, safe to drop). Regression tests added to AppHeader.test.tsx ((#102) clicking Save to Gallery calls the LATEST onSave closure, not a stale one and the matching Export assertion) — these fail against the old code path and pass against the fix.

Reproduction was: upload → Run Auto-Digitize → wait for state=digitized → click Save to Gallery → addDesign(design) never called, localStorage.setItem never invoked, closure still saw appState=digitizing. Fix verified at unit-test level (7/7 AppHeader tests pass, 330/330 total across 38 test files). CI run #87 (workflow run id 7108) was queued on push — local npm test, tsc -b, and eslint . all clean on the changed files.

Fixed by 11752b2 — AppHeader dispatches Save/Export through latest-callback refs so the button always reads the freshest `onSave` closure rather than the one captured at first render. SHIM `[SHIM-SAVE]` debug log also removed (was added during E2E reproduction, safe to drop). Regression tests added to `AppHeader.test.tsx` (`(#102) clicking Save to Gallery calls the LATEST onSave closure, not a stale one` and the matching Export assertion) — these fail against the old code path and pass against the fix. Reproduction was: upload → Run Auto-Digitize → wait for state=`digitized` → click Save to Gallery → `addDesign(design)` never called, `localStorage.setItem` never invoked, closure still saw `appState=digitizing`. Fix verified at unit-test level (7/7 AppHeader tests pass, 330/330 total across 38 test files). CI run #87 (workflow run id 7108) was queued on push — local `npm test`, `tsc -b`, and `eslint .` all clean on the changed files.
Sign in to join this conversation.
No description provided.