Skip to content

test(workbench): add React Testing Library coverage for autosave store-swap timing #11

Description

@chrisjmendez

Intent

Three autosave bugs were introduced and fixed in PR #66 — all living in a gap that types, lint, and the existing unit tests cannot see: React effects in the same render don't observe state set by sibling effects in that render. A single integration test with @testing-library/react would have caught every one of them before any data was lost.

The three bugs this test would have caught

# What went wrong Observable symptom
1 useProjectPersistence load effect had [projectId] as its only dep. When Clerk resolved and swapped localStore → apiStore, the load never re-ran. storeRef.current intentionally defeats react-hooks/exhaustive-deps. Existing project always loaded empty canvas
2 hydrated.current never flipped true when initialState === null (new project). Logic error; types were correct. Autosave never fired; state lost on every reload
3 Save effect fired with stale canvasNodes = [] in the same render where setCanvasNodes([...real nodes...]) was called by the hydration effect. State from setCanvasNodes isn't visible until the next render. exhaustive-deps only catches missing deps, not timing-wrong extras. 0-node PUT overwrote a real row on hot-reload unmount

What's needed

  1. Add @testing-library/react (+ @testing-library/user-event) to apps/web devDependencies.
  2. Mock useUser from @clerk/nextjs and IProjectStore so tests control Clerk's load timing and store responses.
  3. Write the core assertion test:
    it("save never fires with fewer nodes than initialState after store swap", async () => {
      // mount with localStore → returns null
      // simulate Clerk resolving → swap to apiStore with 6-node project
      // assert: store.save never called with canvasNodes.length < 6
      // assert: store.save eventually called with canvasNodes.length === 6
    })
  4. Add two supporting cases: (a) new project — save fires only after user adds first node, not on load; (b) reload round-trip — mount → load N nodes → unmount → remount → same N nodes loaded, no save in between.
  5. Wire into pnpm test (already runs vitest; RTL tests belong in tests/seed-validation/ or a new tests/hooks/ directory).

Acceptance

  • pnpm test is green and includes at least the three cases above.
  • No eslint-disable react-hooks/exhaustive-deps comment is needed after the refactor (the hooks should be lint-clean by design).
  • A future regression that re-introduces any of the three bugs causes at least one of these tests to fail red before merge.

Links


Originally tracked internally as Haptic-AI/festivus#67.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    area:platformFestivus itself: UI, API, infra, agents, contribution flowenhancementNew feature or request

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions