Fix modal draft state overwriting store-managed children
Problem: Child nodes created via NodePicker while editing a parent node would disappear when clicking "Done" on the modal. This caused: - Child nodes not appearing in tree after closing modal - Validation errors about non-existent nodes - Tree unable to save Root cause: Modal used structuredClone() to create local draft state, which included a stale `children: []` array. When saving, this overwrote the actual children that were added to the store via addNode(). Fix: Exclude `children` from the draft when saving, since children are managed separately by addNode/deleteNode store actions. Also documented this critical pattern in LESSONS-LEARNED.md for future reference when implementing modals with local draft state. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
This commit is contained in:
@@ -2,7 +2,7 @@
|
|||||||
|
|
||||||
> **Purpose:** This file documents bugs, fixes, and gotchas encountered during development.
|
> **Purpose:** This file documents bugs, fixes, and gotchas encountered during development.
|
||||||
> **For Claude Code:** Read this file at the start of each session to avoid repeating past mistakes.
|
> **For Claude Code:** Read this file at the start of each session to avoid repeating past mistakes.
|
||||||
> **Last Updated:** January 28, 2026
|
> **Last Updated:** January 30, 2026
|
||||||
|
|
||||||
---
|
---
|
||||||
|
|
||||||
@@ -185,6 +185,51 @@ setEditingNodeId(node.id)
|
|||||||
|
|
||||||
---
|
---
|
||||||
|
|
||||||
|
### Modal Draft State: Don't Overwrite Store-Managed Fields ⚠️ CRITICAL
|
||||||
|
**Problem:** Child nodes created while a modal is open disappear when clicking "Done". Tree validation fails with errors about non-existent nodes.
|
||||||
|
|
||||||
|
**Cause:** When using local draft state for Cancel/Done functionality in a modal:
|
||||||
|
1. Modal opens and clones the entire node (including `children: []`) into local state
|
||||||
|
2. User creates new child nodes via a dropdown (e.g., NodePicker) - these are added to the **store**
|
||||||
|
3. User clicks "Done" → modal saves draft back to store
|
||||||
|
4. The draft's stale `children: []` **overwrites** the store's actual children array
|
||||||
|
5. Newly created nodes are deleted
|
||||||
|
|
||||||
|
**Symptoms:**
|
||||||
|
- Child nodes don't appear in the tree after closing modal
|
||||||
|
- Validation errors: "Option references non-existent node"
|
||||||
|
- Tree won't save due to validation failures
|
||||||
|
|
||||||
|
**Broken pattern:**
|
||||||
|
```tsx
|
||||||
|
// Modal with local draft state
|
||||||
|
const [draft, setDraft] = useState(() => structuredClone(node))
|
||||||
|
|
||||||
|
const handleSave = () => {
|
||||||
|
// BAD: This overwrites children with stale snapshot!
|
||||||
|
updateNode(node.id, draft)
|
||||||
|
onClose()
|
||||||
|
}
|
||||||
|
```
|
||||||
|
|
||||||
|
**Solution:** Exclude fields that are managed by other store actions (like `children` managed by `addNode`/`deleteNode`):
|
||||||
|
```tsx
|
||||||
|
const handleSave = () => {
|
||||||
|
// GOOD: Exclude 'children' - it's managed separately by addNode/deleteNode
|
||||||
|
const { children, ...draftWithoutChildren } = draft
|
||||||
|
updateNode(node.id, draftWithoutChildren)
|
||||||
|
onClose()
|
||||||
|
}
|
||||||
|
```
|
||||||
|
|
||||||
|
**General rule:** When a modal uses local draft state, identify which fields are:
|
||||||
|
- **Editable by the form** → include in draft save
|
||||||
|
- **Managed by other actions** (addNode, deleteNode, etc.) → exclude from draft save
|
||||||
|
|
||||||
|
**Files affected:** `NodeEditorModal.tsx`, any modal that edits objects with nested collections
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
### Modal Scroll/Overflow with Fixed Header and Footer
|
### Modal Scroll/Overflow with Fixed Header and Footer
|
||||||
**Problem:** Modal content extends beyond screen when there's too much content, pushing close button and action buttons off-screen.
|
**Problem:** Modal content extends beyond screen when there's too much content, pushing close button and action buttons off-screen.
|
||||||
|
|
||||||
|
|||||||
@@ -31,7 +31,10 @@ export function NodeEditorModal({ node, onClose, isNewNode = false }: NodeEditor
|
|||||||
|
|
||||||
const handleSave = () => {
|
const handleSave = () => {
|
||||||
// Commit all draft changes to the store
|
// Commit all draft changes to the store
|
||||||
updateNode(node.id, draft)
|
// IMPORTANT: Exclude 'children' from the update - children are managed separately
|
||||||
|
// by addNode/deleteNode and we don't want to overwrite them with stale draft data
|
||||||
|
const { children, ...draftWithoutChildren } = draft
|
||||||
|
updateNode(node.id, draftWithoutChildren)
|
||||||
onClose()
|
onClose()
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user