Repository navigation
feat: Zustand editor store — block tree, selection, dirty flag (#265) - #279
Merged
Merged
Conversation
Add the Phase 1 state foundation that replaces the Phase 0 spike's shared-mutable-state + __bumpTemplate pattern. createEditorStore() returns a plain zustand store (no middleware) owning the block tree, selection, and dirty flag, with immutable insert/update/remove/move, replaceBlocks, select/clearSelection, and markDirty/markClean actions. Selectors (useBlock, useChildren, useSelection, useIsDirty) are memoized React hooks. Vitest coverage spans every action and selector including nested trees, edge cases, and round-trip mutations. Closes #265
📝 WalkthroughWalkthroughAdds a new Zustand-based editor store (blocks, selection, isDirty), immutable tree utilities, TypeScript types, React hooks, Vitest configuration and comprehensive tests covering actions and selectors. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Component as "Component\n(useBlock/useChildren)"
participant Hook as "Hook wrapper\n(useStore)"
participant Store as "Zustand Store\n(createEditorStore)"
participant Utils as "treeUtils\n(find/insert/remove/update)"
Component->>Hook: subscribe(clientId / parentId)
Hook->>Store: read blocks / selection / isDirty
Component->>Store: dispatch action (insert/update/remove/move/select)
Store->>Utils: call insertIntoTree/removeFromTree/updateAttributesInTree
Utils-->>Store: return new blocks (or same if no-op)
Store-->>Hook: update subscribers
Hook-->>Component: re-render with updated data
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@resources/js/visual-editor/editor/store/editorStore.ts`:
- Around line 23-27: The store is retaining caller-owned references: when
setting blocks from initialBlocks in createEditorStore (and in the
replaceBlocks(newBlocks) handler) you must deep-clone the incoming block
arrays/trees so the store owns its copy; update createEditorStore to clone
initialBlocks before assigning to the state (blocks) and update the
replaceBlocks implementation to deep-clone newBlocks (including nested block
objects) before set(...) so external mutations no longer bypass Zustand and
subscribers/isDirty stay in sync.
- Around line 29-38: The insertBlock action currently sets isDirty
unconditionally; change insertBlock to call insertIntoTree and compare the
returned blocks reference to the previous state.blocks and only set isDirty:
true when the returned blocks !== state.blocks (i.e., the tree actually
changed). Also modify insertIntoTree so that when the provided parentClientId is
unknown it returns the original blocks object (preserving reference identity)
rather than a new copy; this aligns insertBlock and other clientId-based actions
so stale parent ids become a true no-op.
- Around line 53-69: The removeBlock handler only clears selection when the
removed clientId exactly matches state.selection.clientId; instead, after
computing nextBlocks (via removeFromTree) check whether the currently selected
id (state.selection.clientId) still exists in nextBlocks and if not set
selection to initialSelection. Update the removeBlock function to compute
nextBlocks, detect selection existence (e.g., via a tree search helper or
existing utility) and assign nextSelection = selectionExists ? state.selection :
initialSelection before returning the new state.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 926a47f3-c1e1-4426-80bd-0f3d9a112fba
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json,!package-lock.json
📒 Files selected for processing (8)
package.jsonresources/js/visual-editor/editor/store/__tests__/editorStore.test.tsresources/js/visual-editor/editor/store/__tests__/editorStoreSelectors.test.tsxresources/js/visual-editor/editor/store/editorStore.tsresources/js/visual-editor/editor/store/index.tsresources/js/visual-editor/editor/store/treeUtils.tsresources/js/visual-editor/editor/store/types.tsvitest.config.ts
Comment on lines
+23
to
+27
| export function createEditorStore(initialBlocks: Block[] = []): EditorStore { | ||
| return createStore<EditorStoreState>((set) => ({ | ||
| blocks: initialBlocks, | ||
| selection: initialSelection, | ||
| isDirty: false, |
There was a problem hiding this comment.
Clone incoming block trees before putting them in store state.
blocks: initialBlocks and replaceBlocks(newBlocks) both keep caller-owned references alive inside the store. Any later mutation of those arrays or nested blocks will bypass Zustand entirely, so subscribers will not fire and isDirty will stay out of sync. That breaks the Phase 1 goal that the store owns the editor tree.
Also applies to: 111-117
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@resources/js/visual-editor/editor/store/editorStore.ts` around lines 23 - 27,
The store is retaining caller-owned references: when setting blocks from
initialBlocks in createEditorStore (and in the replaceBlocks(newBlocks) handler)
you must deep-clone the incoming block arrays/trees so the store owns its copy;
update createEditorStore to clone initialBlocks before assigning to the state
(blocks) and update the replaceBlocks implementation to deep-clone newBlocks
(including nested block objects) before set(...) so external mutations no longer
bypass Zustand and subscribers/isDirty stay in sync.
…remove Two CodeRabbit follow-ups on the Zustand editor store: 1. insertIntoTree used blocks.map(...) which always returned a new array, so insertBlock with an unknown parentClientId produced a fresh blocks reference and flipped isDirty. It now tracks a changed flag and returns the original blocks when nothing matched, making insertBlock a true no-op for stale parent ids. 2. removeBlock only cleared the selection when the removed clientId exactly matched the current selection. Removing an ancestor containing the selected block left a dangling clientId pointing at nothing. It now checks whether the selected block still exists in nextBlocks and resets selection to the initial value when it does not.
There was a problem hiding this comment.
♻️ Duplicate comments (1)
resources/js/visual-editor/editor/store/editorStore.ts (1)
31-34:⚠️ Potential issue | 🟠 MajorClone inbound block trees before storing them.
Line 33 and Line 128 retain caller-owned references (
initialBlocks/newBlocks). External mutation can bypass Zustand updates and desync subscribers/dirty semantics.Proposed fix
+function cloneBlocks(blocks: Block[]): Block[] { + return blocks.map((block) => ({ + ...block, + attributes: { ...block.attributes }, + innerBlocks: cloneBlocks(block.innerBlocks), + })); +} + export function createEditorStore(initialBlocks: Block[] = []): EditorStore { return createStore<EditorStoreState>((set) => ({ - blocks: initialBlocks, + blocks: cloneBlocks(initialBlocks), selection: initialSelection, isDirty: false, @@ replaceBlocks: (newBlocks: Block[]) => { + const nextBlocks = cloneBlocks(newBlocks); set((state) => ({ ...state, - blocks: newBlocks, + blocks: nextBlocks, selection: initialSelection, isDirty: true, })); },Also applies to: 125-131
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@resources/js/visual-editor/editor/store/editorStore.ts` around lines 31 - 34, The store is retaining caller-owned references (initialBlocks and newBlocks) which allows external mutation to bypass Zustand updates; fix by deep-cloning inbound block trees before storing or returning them: in createEditorStore ensure the initialBlocks assigned to EditorStoreState.blocks is a deep clone (e.g., via structuredClone or a dedicated cloneBlocks/cloneBlockTree helper) and likewise clone newBlocks in the update path (the function that writes newBlocks into state) so the store never holds external references; reference createEditorStore, initialBlocks, newBlocks, and EditorStoreState when making these changes.
🧹 Nitpick comments (1)
resources/js/visual-editor/editor/store/editorStore.ts (1)
152-168: Move selector logic into hooks with memoization to avoid unnecessary rerenders from unrelated tree mutations.
useBlockanduseChildrensubscribe to the fullstate.blockstree. Any block mutation updates the blocks reference, triggering both hooks'useMemoto recompute even when the specific block or children being searched haven't changed.Refactor sketch
export function useBlock(store: EditorStore, clientId: string | null): Block | undefined { - const blocks = useStore(store, (state) => state.blocks); - - return useMemo(() => { - if (clientId === null) { - return undefined; - } - - return findBlock(blocks, clientId); - }, [blocks, clientId]); + const selector = useMemo( + () => (state: EditorStoreState) => + clientId === null ? undefined : findBlock(state.blocks, clientId), + [clientId] + ); + + return useStore(store, selector); } export function useChildren(store: EditorStore, parentClientId: string | null): Block[] { - const blocks = useStore(store, (state) => state.blocks); - - return useMemo(() => findChildren(blocks, parentClientId), [blocks, parentClientId]); + const selector = useMemo( + () => (state: EditorStoreState) => findChildren(state.blocks, parentClientId), + [parentClientId] + ); + + return useStore(store, selector); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@resources/js/visual-editor/editor/store/editorStore.ts` around lines 152 - 168, useBlock and useChildren currently subscribe to the entire state.blocks object so any mutation triggers recomputation; change them to select only the needed slice from the store instead of the whole tree: in useBlock, call useStore(store, (state) => findBlock(state.blocks, clientId)) (or a selector that returns undefined when clientId is null) and remove the outer useMemo, and in useChildren, call useStore(store, (state) => findChildren(state.blocks, parentClientId)) (again handling null parentClientId inside the selector) so the hooks only re-render when their specific block/children result changes.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@resources/js/visual-editor/editor/store/editorStore.ts`:
- Around line 31-34: The store is retaining caller-owned references
(initialBlocks and newBlocks) which allows external mutation to bypass Zustand
updates; fix by deep-cloning inbound block trees before storing or returning
them: in createEditorStore ensure the initialBlocks assigned to
EditorStoreState.blocks is a deep clone (e.g., via structuredClone or a
dedicated cloneBlocks/cloneBlockTree helper) and likewise clone newBlocks in the
update path (the function that writes newBlocks into state) so the store never
holds external references; reference createEditorStore, initialBlocks,
newBlocks, and EditorStoreState when making these changes.
---
Nitpick comments:
In `@resources/js/visual-editor/editor/store/editorStore.ts`:
- Around line 152-168: useBlock and useChildren currently subscribe to the
entire state.blocks object so any mutation triggers recomputation; change them
to select only the needed slice from the store instead of the whole tree: in
useBlock, call useStore(store, (state) => findBlock(state.blocks, clientId)) (or
a selector that returns undefined when clientId is null) and remove the outer
useMemo, and in useChildren, call useStore(store, (state) =>
findChildren(state.blocks, parentClientId)) (again handling null parentClientId
inside the selector) so the hooks only re-render when their specific
block/children result changes.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: f28606fc-499b-40a6-b438-37405ec4e7e3
📒 Files selected for processing (3)
resources/js/visual-editor/editor/store/__tests__/editorStore.test.tsresources/js/visual-editor/editor/store/editorStore.tsresources/js/visual-editor/editor/store/treeUtils.ts
✅ Files skipped from review due to trivial changes (2)
- resources/js/visual-editor/editor/store/tests/editorStore.test.ts
- resources/js/visual-editor/editor/store/treeUtils.ts
github-project-automation
Bot
moved this from In Review
to Done
in ArtisanPack UI Overview
This was referenced Apr 13, 2026
14 of 18 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Adds the Phase 1 state foundation for the React editor migration. Introduces a plain Zustand store (no middleware) that owns the block tree, selection, and dirty flag, replacing the Phase 0 spike's shared-mutable-state + `__bumpTemplate` pattern called out as finding #1 in `PHASE-0-FINDINGS.md`.
Closes: #265
Type of Change
Related Issue
Issue: #265
Motivation and Context
Phase 0 proved the React + Tiptap architecture works, but the spike leaned on a mutable block tree and a `__bumpTemplate` callback to force preview re-renders. Every other Phase 1 issue (REST load/save, inserter, undo/redo, Tiptap wiring) needs a real state layer to build on. This PR delivers that layer with immutable updates and memoized selectors, unblocking #266 (undo/redo), #274 (persistence), and the paragraph/heading blocks.
Changes Made
How Has This Been Tested?
Testing Environment:
Tests Performed:
Accessibility Tests Run
Details: This PR ships no rendered markup, so there is nothing to keyboard- or screen-reader-test. Accessibility work lives in the shell/canvas issues that consume this store.
Tests Added
Test details:
Documentation
Documentation details: Types and action signatures are self-documenting via TypeScript. No wiki/README changes needed until the store is wired into UI in a later Phase 1 issue.
Screenshots
N/A — pure data layer.
Pre-Submission Checklist
Summary by CodeRabbit
New Features
Tests
Chores