docs(canvas): add modularization strategy
This commit is contained in:
1
.gitignore
vendored
1
.gitignore
vendored
@@ -45,3 +45,4 @@ next-env.d.ts
|
||||
.cursor
|
||||
.cursor/*
|
||||
.kilo
|
||||
.worktrees/
|
||||
|
||||
216
docs/plans/2026-04-03-canvas-modularization-design.md
Normal file
216
docs/plans/2026-04-03-canvas-modularization-design.md
Normal file
@@ -0,0 +1,216 @@
|
||||
# Canvas Modularization Design
|
||||
|
||||
## Goal
|
||||
|
||||
Modularize `components/canvas/canvas.tsx` so the file becomes easier to read, easier to test, and easier for a second engineer to navigate without changing user-visible canvas behavior or existing public component contracts.
|
||||
|
||||
## Why Now
|
||||
|
||||
- `components/canvas/canvas.tsx` currently combines at least four different responsibilities: sync orchestration, Convex-to-React-Flow reconciliation, interaction handlers, and render composition.
|
||||
- The file is still the main knowledge bottleneck in the canvas engine even after earlier helper extractions.
|
||||
- A second developer joining during the seed phase will need clearer ownership boundaries and faster onboarding into the canvas hot path.
|
||||
|
||||
## Current Findings
|
||||
|
||||
- Sync and optimistic update orchestration are concentrated around the mutation and queue layer in `components/canvas/canvas.tsx:258` and `components/canvas/canvas.tsx:984`.
|
||||
- Convex-to-local reconciliation for nodes and edges lives in large `useLayoutEffect` blocks in `components/canvas/canvas.tsx:1922`.
|
||||
- Interaction logic for drag, connect, reconnect, drop, scissor mode, and highlighting is spread across the lower half of the file in `components/canvas/canvas.tsx:1798`, `components/canvas/canvas.tsx:2182`, `components/canvas/canvas.tsx:2555`, and `components/canvas/canvas.tsx:2730`.
|
||||
- Render composition is comparatively simple, but it is buried under orchestration code in `components/canvas/canvas.tsx:2901`.
|
||||
|
||||
## Context7 Guidance Applied
|
||||
|
||||
- React guidance: extract non-visual logic into custom hooks so components focus on rendering and explicit data flow.
|
||||
- React guidance: keep hook composition clear and stable rather than hiding too much behavior inside a generic controller.
|
||||
- React Flow guidance: keep `nodeTypes` stable outside render, preserve `ReactFlowProvider` boundaries, and keep controlled flow state explicit when splitting handlers.
|
||||
|
||||
## Chosen Approach
|
||||
|
||||
Use a thin-orchestrator refactor:
|
||||
|
||||
1. Keep `components/canvas/canvas.tsx` as the top-level canvas entry and wiring layer.
|
||||
2. Extract the largest logic clusters into domain-specific hooks with explicit inputs and outputs.
|
||||
3. Leave existing external APIs unchanged: `Canvas`, `CanvasInner`, provider usage, mutation contracts, and React Flow integration stay behaviorally identical.
|
||||
4. Prefer hook boundaries over a single giant controller or reducer in phase 1.
|
||||
|
||||
This approach gives the best trade-off between readability, testability, and migration risk.
|
||||
|
||||
## Rejected Alternatives
|
||||
|
||||
### 1. Single `useCanvasController` hook with reducer
|
||||
|
||||
- Pros: very strong test surface and centralized state transitions.
|
||||
- Cons: too risky for the current canvas because drag locks, optimistic ID handoff, offline replay, and Convex reconciliation are behavior-sensitive and already intertwined with refs and effects.
|
||||
|
||||
### 2. Minimal handler extraction only
|
||||
|
||||
- Pros: lowest refactor risk.
|
||||
- Cons: improves file length but not ownership or conceptual clarity; `canvas.tsx` would still remain the main cognitive bottleneck.
|
||||
|
||||
## Target Architecture
|
||||
|
||||
### 1. `canvas.tsx` becomes the orchestrator
|
||||
|
||||
`components/canvas/canvas.tsx` should keep only:
|
||||
|
||||
- top-level provider wiring
|
||||
- hook composition
|
||||
- final prop assembly for the rendered canvas view
|
||||
- small glue callbacks when multiple hooks need to meet
|
||||
|
||||
The file should stop owning detailed effect bodies and long imperative flows.
|
||||
|
||||
### 2. Extract domain hooks by responsibility
|
||||
|
||||
#### `components/canvas/use-canvas-sync-engine.ts`
|
||||
|
||||
Owns:
|
||||
|
||||
- online/offline state
|
||||
- queue flushing and retry scheduling
|
||||
- deferred mutation handling for optimistic nodes
|
||||
- optimistic-to-real ID remapping support
|
||||
- wrappers like `runMoveNodeMutation`, `runResizeNodeMutation`, `runUpdateNodeDataMutation`, `runCreateEdgeMutation`, and node creation variants
|
||||
|
||||
Why:
|
||||
|
||||
- This is the most operationally complex part of the file.
|
||||
- It has clear inputs and outputs and can be tested with mocked mutations and queue helpers.
|
||||
|
||||
#### `components/canvas/use-canvas-flow-reconciliation.ts`
|
||||
|
||||
Owns:
|
||||
|
||||
- Convex edges -> RF edges reconciliation
|
||||
- Convex nodes -> RF nodes reconciliation
|
||||
- carry-forward logic for optimistic edges during handoff gaps
|
||||
- local pinning and merge behavior
|
||||
- compare-node data resolution and glow-aware edge mapping
|
||||
|
||||
Why:
|
||||
|
||||
- This logic is mostly deterministic and should become easier to test if helper functions are extracted out of effect bodies.
|
||||
|
||||
#### `components/canvas/use-canvas-node-interactions.ts`
|
||||
|
||||
Owns:
|
||||
|
||||
- `onNodesChange`
|
||||
- node drag start / drag / drag stop
|
||||
- resize lock transitions
|
||||
- intersected-edge highlighting state
|
||||
|
||||
Why:
|
||||
|
||||
- Today the drag lifecycle is hard to reason about because local visual behavior and persistence behavior are mixed together.
|
||||
|
||||
#### `components/canvas/use-canvas-connections.ts`
|
||||
|
||||
Owns:
|
||||
|
||||
- `onConnect`
|
||||
- `onConnectEnd`
|
||||
- connection-drop-menu state
|
||||
- create-node-from-connection flows
|
||||
- adapter glue for reconnect handling
|
||||
|
||||
Why:
|
||||
|
||||
- New connection creation is a separate mental model from drag/drop creation and should be independently understandable.
|
||||
|
||||
#### `components/canvas/use-canvas-drop.ts`
|
||||
|
||||
Owns:
|
||||
|
||||
- `onDragOver`
|
||||
- `onDrop`
|
||||
- DnD payload parsing
|
||||
- file upload drop flow
|
||||
- create-node-on-drop orchestration
|
||||
|
||||
Why:
|
||||
|
||||
- Drop behavior has its own error handling, upload behavior, and defaults. It is a natural isolation boundary.
|
||||
|
||||
### 3. Optional view extraction in the final phase
|
||||
|
||||
Add `components/canvas/canvas-view.tsx` only after the main hook boundaries are stable.
|
||||
|
||||
It should stay presentational and receive:
|
||||
|
||||
- `nodes`, `edges`
|
||||
- overlay state
|
||||
- `ReactFlow` handlers
|
||||
- toolbar state and callbacks
|
||||
|
||||
Why not first:
|
||||
|
||||
- Extracting view first reduces file size but leaves the hard behavioral logic untouched.
|
||||
|
||||
## Ownership Model For A Two-Engineer Team
|
||||
|
||||
The proposed split creates clean ownership zones:
|
||||
|
||||
- Engineer A: sync, optimistic state, offline behavior
|
||||
- Engineer B: interactions, connection creation, drop flows
|
||||
- Shared: top-level composition in `canvas.tsx` and pure helpers in `canvas-helpers.ts`
|
||||
|
||||
This is useful even if the second engineer joins later because it documents the system in code boundaries, not only in docs.
|
||||
|
||||
## Testing Strategy
|
||||
|
||||
The refactor should raise confidence by moving logic into units that are easier to test.
|
||||
|
||||
### Priority test targets
|
||||
|
||||
- optimistic node create -> real ID remap
|
||||
- deferred `resizeNode` and `updateData` for optimistic nodes
|
||||
- edge carry during the create/handoff gap
|
||||
- drag lock prevents stale Convex snapshots from overriding local drag state
|
||||
- connection validation still rejects invalid edges
|
||||
- drag-and-drop creation still supports both node payloads and file uploads
|
||||
|
||||
### Test shape
|
||||
|
||||
- Extract deterministic logic from hook internals into pure helpers where possible.
|
||||
- Add targeted tests for those helpers first.
|
||||
- Add hook-level tests only for behavior that genuinely depends on refs, effects, or queue timing.
|
||||
- Keep manual verification for React Flow pointer interactions that are awkward to simulate precisely.
|
||||
|
||||
## Refactor Sequencing
|
||||
|
||||
Use small, behavior-preserving phases.
|
||||
|
||||
### Phase 1: Sync engine extraction
|
||||
|
||||
- Highest complexity
|
||||
- Highest test ROI
|
||||
- Unlocks simpler downstream hooks because many callbacks become imported capabilities instead of locally defined machinery
|
||||
|
||||
### Phase 2: Flow reconciliation extraction
|
||||
|
||||
- Separate mapping and merge behavior from interaction code
|
||||
- Makes the render path easier to inspect
|
||||
|
||||
### Phase 3: Connections and drop extraction
|
||||
|
||||
- Split creation pathways by intent: connect vs drop
|
||||
- Reduces the size of the lower half of `canvas.tsx`
|
||||
|
||||
### Phase 4: Optional presentational view extraction
|
||||
|
||||
- Only if the remaining orchestrator still feels too dense
|
||||
|
||||
## Guardrails
|
||||
|
||||
- Do not change `Canvas` or `CanvasInner` public props.
|
||||
- Keep `ReactFlowProvider` and `useReactFlow` usage exactly compatible with current behavior.
|
||||
- Keep `nodeTypes` outside React components.
|
||||
- Preserve ordering of side effects and queue mutations.
|
||||
- Do not introduce new dependencies.
|
||||
- Do not rewrite into a reducer architecture in this phase.
|
||||
|
||||
## Expected Outcome
|
||||
|
||||
- `components/canvas/canvas.tsx` becomes an understandable composition root instead of a monolith.
|
||||
- The highest-risk behaviors remain intact because the refactor follows existing seams rather than inventing a new runtime model.
|
||||
- A second engineer can work in the canvas codebase by area of concern instead of first learning the entire file.
|
||||
163
docs/plans/2026-04-03-canvas-modularization.md
Normal file
163
docs/plans/2026-04-03-canvas-modularization.md
Normal file
@@ -0,0 +1,163 @@
|
||||
# Canvas Modularization Implementation Plan
|
||||
|
||||
> **For Claude:** REQUIRED SUB-SKILL: Use superpowers:executing-plans to implement this plan task-by-task.
|
||||
|
||||
**Goal:** Split `components/canvas/canvas.tsx` into clear, testable domain hooks while preserving all current canvas behavior and public APIs.
|
||||
|
||||
**Architecture:** Keep `components/canvas/canvas.tsx` as the composition root and extract the largest logic clusters into domain-specific hooks. Start with sync and reconciliation because they are the most complex and most reusable, then move connection and drop flows, and only then extract a presentational view if it still adds value.
|
||||
|
||||
**Tech Stack:** Next.js 16, React 19, Convex, React Flow, TypeScript
|
||||
|
||||
---
|
||||
|
||||
### Task 1: Lock in the current modularization seams
|
||||
|
||||
**Files:**
|
||||
- Modify: `components/canvas/canvas.tsx`
|
||||
- Reference: `components/canvas/CLAUDE.md`
|
||||
- Reference: `docs/plans/2026-04-03-canvas-modularization-design.md`
|
||||
|
||||
**Step 1:** Annotate the major responsibility boundaries inside `components/canvas/canvas.tsx` by grouping the existing code into the future modules: sync engine, reconciliation, interactions, connections, drop, and render.
|
||||
|
||||
**Step 2:** List the exact state, refs, callbacks, and mutations owned by each future hook in a scratch note or temporary checklist before moving code.
|
||||
|
||||
**Step 3:** Identify cross-cutting refs that must stay shared at the orchestrator level, especially optimistic ID handoff state and React Flow instance access.
|
||||
|
||||
**Step 4:** Remove the scratch note once the extraction map is clear.
|
||||
|
||||
### Task 2: Extract pure helper logic from effect bodies first
|
||||
|
||||
**Files:**
|
||||
- Modify: `components/canvas/canvas.tsx`
|
||||
- Modify: `components/canvas/canvas-helpers.ts`
|
||||
- Create: `components/canvas/canvas-flow-reconciliation-helpers.ts`
|
||||
- Test: `components/canvas/__tests__/canvas-flow-reconciliation-helpers.test.ts`
|
||||
|
||||
**Step 1:** Move deterministic edge reconciliation helpers out of `useLayoutEffect` bodies into `components/canvas/canvas-flow-reconciliation-helpers.ts`.
|
||||
|
||||
**Step 2:** Move deterministic node reconciliation helpers out of `useLayoutEffect` bodies into `components/canvas/canvas-flow-reconciliation-helpers.ts`.
|
||||
|
||||
**Step 3:** Add tests for optimistic edge carry, endpoint remapping, and position pin cleanup.
|
||||
|
||||
**Step 4:** Update `components/canvas/canvas.tsx` to call the extracted helpers without changing behavior.
|
||||
|
||||
### Task 3: Extract the sync engine hook
|
||||
|
||||
**Files:**
|
||||
- Create: `components/canvas/use-canvas-sync-engine.ts`
|
||||
- Modify: `components/canvas/canvas.tsx`
|
||||
- Reference: `lib/canvas-op-queue.ts`
|
||||
- Reference: `lib/canvas-local-persistence.ts`
|
||||
- Test: `components/canvas/__tests__/use-canvas-sync-engine.test.ts`
|
||||
|
||||
**Step 1:** Create `useCanvasSyncEngine` with the queue state, online state, and mutation wrappers that are currently centered around `components/canvas/canvas.tsx:258` and `components/canvas/canvas.tsx:984`.
|
||||
|
||||
**Step 2:** Move deferred optimistic-node handling into the new hook, including pending resize, pending data, and pending move-after-create behavior.
|
||||
|
||||
**Step 3:** Return a clear API from the hook for create, move, resize, update-data, remove-edge, batch-remove, and split-edge flows.
|
||||
|
||||
**Step 4:** Replace the local inline sync logic in `components/canvas/canvas.tsx` with the hook.
|
||||
|
||||
**Step 5:** Add focused tests for optimistic create handoff and deferred resize/update behavior.
|
||||
|
||||
### Task 4: Extract the flow reconciliation hook
|
||||
|
||||
**Files:**
|
||||
- Create: `components/canvas/use-canvas-flow-reconciliation.ts`
|
||||
- Modify: `components/canvas/canvas.tsx`
|
||||
- Modify: `components/canvas/canvas-flow-reconciliation-helpers.ts`
|
||||
- Test: `components/canvas/__tests__/use-canvas-flow-reconciliation.test.ts`
|
||||
|
||||
**Step 1:** Create `useCanvasFlowReconciliation` to own the current Convex-to-local `useLayoutEffect` synchronization for nodes and edges.
|
||||
|
||||
**Step 2:** Pass in only the data it needs: Convex nodes, Convex edges, storage URLs, theme mode, local setters, and shared optimistic refs.
|
||||
|
||||
**Step 3:** Keep drag-lock and resize-lock behavior unchanged while moving the effects.
|
||||
|
||||
**Step 4:** Add tests or targeted assertions for the drag-lock and optimistic carry behavior that are hardest to reason about from the UI.
|
||||
|
||||
### Task 5: Extract node interaction logic
|
||||
|
||||
**Files:**
|
||||
- Create: `components/canvas/use-canvas-node-interactions.ts`
|
||||
- Modify: `components/canvas/canvas.tsx`
|
||||
- Reference: `components/canvas/canvas-node-change-helpers.ts`
|
||||
|
||||
**Step 1:** Move `onNodesChange`, edge intersection highlighting, drag start, drag, and drag stop into `useCanvasNodeInteractions`.
|
||||
|
||||
**Step 2:** Keep the returned API explicit: handlers plus any local highlight state helpers.
|
||||
|
||||
**Step 3:** Preserve all resize side effects and split-edge-on-drop behavior exactly.
|
||||
|
||||
**Step 4:** Manually verify drag, resize, and edge split interactions on the canvas page.
|
||||
|
||||
### Task 6: Extract connection flows
|
||||
|
||||
**Files:**
|
||||
- Create: `components/canvas/use-canvas-connections.ts`
|
||||
- Modify: `components/canvas/canvas.tsx`
|
||||
- Reference: `components/canvas/canvas-reconnect.ts`
|
||||
- Reference: `components/canvas/canvas-connection-drop-menu.tsx`
|
||||
|
||||
**Step 1:** Move `onConnect`, `onConnectEnd`, connection drop menu state, and `handleConnectionDropPick` into `useCanvasConnections`.
|
||||
|
||||
**Step 2:** Keep validation centralized around `validateCanvasConnection` and `validateCanvasConnectionByType`.
|
||||
|
||||
**Step 3:** Reuse the existing reconnect hook by adapting it through the new connection hook rather than rewriting reconnect behavior.
|
||||
|
||||
**Step 4:** Manually verify valid connection creation, invalid connection rejection, reconnect, and connection-drop node creation.
|
||||
|
||||
### Task 7: Extract drag-and-drop creation flows
|
||||
|
||||
**Files:**
|
||||
- Create: `components/canvas/use-canvas-drop.ts`
|
||||
- Modify: `components/canvas/canvas.tsx`
|
||||
- Reference: `components/canvas/canvas-media-utils.ts`
|
||||
|
||||
**Step 1:** Move `onDragOver` and `onDrop` into `useCanvasDrop`.
|
||||
|
||||
**Step 2:** Keep support for both sidebar/browser JSON payloads and raw node type strings.
|
||||
|
||||
**Step 3:** Preserve file upload drop behavior, including image dimension lookup and upload error toasts.
|
||||
|
||||
**Step 4:** Manually verify node drop and image file drop flows.
|
||||
|
||||
### Task 8: Decide whether a presentational view extraction is still useful
|
||||
|
||||
**Files:**
|
||||
- Create: `components/canvas/canvas-view.tsx`
|
||||
- Modify: `components/canvas/canvas.tsx`
|
||||
|
||||
**Step 1:** Measure the remaining size and readability of `components/canvas/canvas.tsx` after hook extraction.
|
||||
|
||||
**Step 2:** If the file still mixes visual structure with too much prop assembly, move the JSX tree into `components/canvas/canvas-view.tsx`.
|
||||
|
||||
**Step 3:** Keep `canvas-view.tsx` presentational only; it should not own data-fetching, queue logic, or reconciliation effects.
|
||||
|
||||
**Step 4:** Skip this task entirely if the orchestrator is already clearly readable.
|
||||
|
||||
### Task 9: Verify behavior after each extraction phase
|
||||
|
||||
**Files:**
|
||||
- Verify only
|
||||
|
||||
**Step 1:** Run `pnpm lint components/canvas/canvas.tsx components/canvas/*.ts components/canvas/*.tsx` or the project-equivalent lint command for touched files.
|
||||
|
||||
**Step 2:** Run `pnpm tsc --noEmit` or the project-equivalent type-check command.
|
||||
|
||||
**Step 3:** Run the targeted canvas tests added during the refactor.
|
||||
|
||||
**Step 4:** Manually verify these flows on the real canvas page: create node, drag node, resize node, connect nodes, reconnect edge, delete node, drop node from sidebar, and drop image file.
|
||||
|
||||
### Task 10: Finish with small, reviewable commits
|
||||
|
||||
**Files:**
|
||||
- Commit only
|
||||
|
||||
**Step 1:** Commit after Task 3 with a message like `refactor(canvas): extract sync engine hook`.
|
||||
|
||||
**Step 2:** Commit after Task 4 with a message like `refactor(canvas): extract flow reconciliation hook`.
|
||||
|
||||
**Step 3:** Commit after Tasks 5 to 7 with messages scoped to interactions, connections, and drop handling.
|
||||
|
||||
**Step 4:** If Task 8 is done, commit it separately as `refactor(canvas): extract canvas view`.
|
||||
Reference in New Issue
Block a user