fix(builder): regenerate node ids on duplicate/paste to prevent state corruption
Craft.js duplicate (ContextMenu + keyboard shortcut) and paste were reusing the original node's toNodeTree() output verbatim, so addNodeTree() inserted duplicate node ids into the editor tree. Added regenerateTreeIds() which deep-clones a NodeTree and remaps rootNodeId, node map keys, node.id, internal node.data.parent, node.data.nodes, and node.data.linkedNodes via Craft.js's own getRandomId(). Also fixed pasteNode to insert as a sibling of the right-clicked node (using its parent) instead of using a leaf node as the new parent, which previously threw. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -1,6 +1,7 @@
|
|||||||
import { useEffect } from 'react';
|
import { useEffect } from 'react';
|
||||||
import { useEditor } from '@craftjs/core';
|
import { useEditor } from '@craftjs/core';
|
||||||
import { findDeletableTarget } from '../utils/craft-helpers';
|
import { findDeletableTarget } from '../utils/craft-helpers';
|
||||||
|
import { regenerateTreeIds } from '../utils/craft-tree';
|
||||||
|
|
||||||
function isInputFocused(): boolean {
|
function isInputFocused(): boolean {
|
||||||
const el = document.activeElement;
|
const el = document.activeElement;
|
||||||
@@ -73,7 +74,7 @@ export function useKeyboardShortcuts() {
|
|||||||
const node = query.node(nodeId).get();
|
const node = query.node(nodeId).get();
|
||||||
const parentId = node?.data?.parent;
|
const parentId = node?.data?.parent;
|
||||||
if (parentId) {
|
if (parentId) {
|
||||||
const tree = query.node(nodeId).toNodeTree();
|
const tree = regenerateTreeIds(query.node(nodeId).toNodeTree());
|
||||||
actions.addNodeTree(tree, parentId);
|
actions.addNodeTree(tree, parentId);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -3,6 +3,7 @@ import { useEditor } from '@craftjs/core';
|
|||||||
import { findDeletableTarget } from '../../utils/craft-helpers';
|
import { findDeletableTarget } from '../../utils/craft-helpers';
|
||||||
import { useSitesmithModal } from '../../state/SitesmithContext';
|
import { useSitesmithModal } from '../../state/SitesmithContext';
|
||||||
import { buildSitesmithTarget } from '../../utils/sitesmith-target';
|
import { buildSitesmithTarget } from '../../utils/sitesmith-target';
|
||||||
|
import { regenerateTreeIds } from '../../utils/craft-tree';
|
||||||
|
|
||||||
interface ContextMenuProps {
|
interface ContextMenuProps {
|
||||||
visible: boolean;
|
visible: boolean;
|
||||||
@@ -65,15 +66,11 @@ export const ContextMenu: React.FC<ContextMenuProps> = ({
|
|||||||
const duplicate = useCallback(() => {
|
const duplicate = useCallback(() => {
|
||||||
if (!nodeId || nodeId === 'ROOT') return;
|
if (!nodeId || nodeId === 'ROOT') return;
|
||||||
try {
|
try {
|
||||||
const tree = query.node(nodeId).toSerializedNode();
|
|
||||||
const parentId = getParentId();
|
const parentId = getParentId();
|
||||||
if (!parentId) return;
|
if (!parentId) return;
|
||||||
|
|
||||||
// Get the full subtree
|
const tree = regenerateTreeIds(query.node(nodeId).toNodeTree());
|
||||||
const freshTree = query.node(nodeId).toNodeTree();
|
actions.addNodeTree(tree, parentId);
|
||||||
const clonedTree = query.parseSerializedNode(freshTree.nodes[freshTree.rootNodeId].data).toNode();
|
|
||||||
|
|
||||||
actions.addNodeTree(freshTree, parentId);
|
|
||||||
} catch (e) {
|
} catch (e) {
|
||||||
console.error('Duplicate failed:', e);
|
console.error('Duplicate failed:', e);
|
||||||
}
|
}
|
||||||
@@ -92,10 +89,25 @@ export const ContextMenu: React.FC<ContextMenuProps> = ({
|
|||||||
|
|
||||||
const pasteNode = useCallback(() => {
|
const pasteNode = useCallback(() => {
|
||||||
const sourceId = clipboardRef.current;
|
const sourceId = clipboardRef.current;
|
||||||
if (!sourceId) return;
|
if (!sourceId) {
|
||||||
|
onClose();
|
||||||
|
return;
|
||||||
|
}
|
||||||
try {
|
try {
|
||||||
const targetParent = nodeId || 'ROOT';
|
if (!query.node(sourceId).get()) {
|
||||||
const tree = query.node(sourceId).toNodeTree();
|
onClose();
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
|
||||||
|
// Paste as a SIBLING of the right-clicked node, not as its child --
|
||||||
|
// using the clicked node itself as the parent throws when it's a leaf.
|
||||||
|
let targetParent = 'ROOT';
|
||||||
|
if (nodeId && nodeId !== 'ROOT') {
|
||||||
|
const clickedNode = query.node(nodeId).get();
|
||||||
|
targetParent = clickedNode?.data?.parent || 'ROOT';
|
||||||
|
}
|
||||||
|
|
||||||
|
const tree = regenerateTreeIds(query.node(sourceId).toNodeTree());
|
||||||
actions.addNodeTree(tree, targetParent);
|
actions.addNodeTree(tree, targetParent);
|
||||||
} catch (e) {
|
} catch (e) {
|
||||||
console.error('Paste failed:', e);
|
console.error('Paste failed:', e);
|
||||||
|
|||||||
@@ -0,0 +1,132 @@
|
|||||||
|
import { describe, test, expect } from 'vitest';
|
||||||
|
import type { NodeTree, Node } from '@craftjs/core';
|
||||||
|
import { regenerateTreeIds } from './craft-tree';
|
||||||
|
|
||||||
|
function makeNode(overrides: Partial<Node>): Node {
|
||||||
|
return {
|
||||||
|
id: 'placeholder',
|
||||||
|
data: {
|
||||||
|
props: {},
|
||||||
|
type: 'div',
|
||||||
|
name: 'div',
|
||||||
|
displayName: 'div',
|
||||||
|
isCanvas: false,
|
||||||
|
parent: null,
|
||||||
|
linkedNodes: {},
|
||||||
|
nodes: [],
|
||||||
|
hidden: false,
|
||||||
|
},
|
||||||
|
info: {},
|
||||||
|
events: { selected: false, dragged: false, hovered: false },
|
||||||
|
dom: null,
|
||||||
|
related: {},
|
||||||
|
rules: {
|
||||||
|
canDrag: () => true,
|
||||||
|
canDrop: () => true,
|
||||||
|
canMoveIn: () => true,
|
||||||
|
canMoveOut: () => true,
|
||||||
|
},
|
||||||
|
_hydrationTimestamp: 0,
|
||||||
|
...overrides,
|
||||||
|
} as Node;
|
||||||
|
}
|
||||||
|
|
||||||
|
function makeTree(): NodeTree {
|
||||||
|
// root -> [childA, childB]; root also has a linkedNodes entry -> childB
|
||||||
|
const root = makeNode({
|
||||||
|
id: 'root-1',
|
||||||
|
data: {
|
||||||
|
...makeNode({}).data,
|
||||||
|
parent: null,
|
||||||
|
nodes: ['child-a-1'],
|
||||||
|
linkedNodes: { slot: 'child-b-1' },
|
||||||
|
},
|
||||||
|
});
|
||||||
|
const childA = makeNode({
|
||||||
|
id: 'child-a-1',
|
||||||
|
data: {
|
||||||
|
...makeNode({}).data,
|
||||||
|
parent: 'root-1',
|
||||||
|
nodes: [],
|
||||||
|
linkedNodes: {},
|
||||||
|
},
|
||||||
|
});
|
||||||
|
const childB = makeNode({
|
||||||
|
id: 'child-b-1',
|
||||||
|
data: {
|
||||||
|
...makeNode({}).data,
|
||||||
|
parent: 'root-1',
|
||||||
|
nodes: [],
|
||||||
|
linkedNodes: {},
|
||||||
|
},
|
||||||
|
});
|
||||||
|
|
||||||
|
return {
|
||||||
|
rootNodeId: 'root-1',
|
||||||
|
nodes: {
|
||||||
|
'root-1': root,
|
||||||
|
'child-a-1': childA,
|
||||||
|
'child-b-1': childB,
|
||||||
|
},
|
||||||
|
};
|
||||||
|
}
|
||||||
|
|
||||||
|
describe('regenerateTreeIds', () => {
|
||||||
|
test('produces an id set fully disjoint from the input', () => {
|
||||||
|
const input = makeTree();
|
||||||
|
const inputIds = Object.keys(input.nodes);
|
||||||
|
const output = regenerateTreeIds(input);
|
||||||
|
const outputIds = Object.keys(output.nodes);
|
||||||
|
|
||||||
|
for (const id of outputIds) {
|
||||||
|
expect(inputIds).not.toContain(id);
|
||||||
|
}
|
||||||
|
});
|
||||||
|
|
||||||
|
test('rootNodeId is a key in nodes and equals that node id', () => {
|
||||||
|
const output = regenerateTreeIds(makeTree());
|
||||||
|
expect(output.nodes[output.rootNodeId]).toBeDefined();
|
||||||
|
expect(output.nodes[output.rootNodeId].id).toBe(output.rootNodeId);
|
||||||
|
});
|
||||||
|
|
||||||
|
test('every child data.parent points to a NEW id that exists in the output', () => {
|
||||||
|
const output = regenerateTreeIds(makeTree());
|
||||||
|
for (const id of Object.keys(output.nodes)) {
|
||||||
|
const node = output.nodes[id];
|
||||||
|
if (node.data.parent !== null) {
|
||||||
|
expect(output.nodes[node.data.parent]).toBeDefined();
|
||||||
|
}
|
||||||
|
}
|
||||||
|
// specifically the two children under the (new) root
|
||||||
|
const newRoot = output.nodes[output.rootNodeId];
|
||||||
|
expect(newRoot.data.nodes.length).toBe(1);
|
||||||
|
const newChildAId = newRoot.data.nodes[0];
|
||||||
|
expect(output.nodes[newChildAId].data.parent).toBe(output.rootNodeId);
|
||||||
|
});
|
||||||
|
|
||||||
|
test('linkedNodes values are remapped to existing new ids', () => {
|
||||||
|
const output = regenerateTreeIds(makeTree());
|
||||||
|
const newRoot = output.nodes[output.rootNodeId];
|
||||||
|
const linkedId = newRoot.data.linkedNodes.slot;
|
||||||
|
expect(linkedId).toBeDefined();
|
||||||
|
expect(output.nodes[linkedId]).toBeDefined();
|
||||||
|
// linked node's parent should still reference the new root
|
||||||
|
expect(output.nodes[linkedId].data.parent).toBe(output.rootNodeId);
|
||||||
|
});
|
||||||
|
|
||||||
|
test('structure/count is preserved', () => {
|
||||||
|
const input = makeTree();
|
||||||
|
const output = regenerateTreeIds(input);
|
||||||
|
expect(Object.keys(output.nodes).length).toBe(Object.keys(input.nodes).length);
|
||||||
|
expect(output.nodes[output.rootNodeId].data.nodes.length).toBe(
|
||||||
|
input.nodes[input.rootNodeId].data.nodes.length
|
||||||
|
);
|
||||||
|
});
|
||||||
|
|
||||||
|
test('does not mutate the input tree', () => {
|
||||||
|
const input = makeTree();
|
||||||
|
const snapshot = JSON.parse(JSON.stringify(input));
|
||||||
|
regenerateTreeIds(input);
|
||||||
|
expect(JSON.parse(JSON.stringify(input))).toEqual(snapshot);
|
||||||
|
});
|
||||||
|
});
|
||||||
@@ -0,0 +1,64 @@
|
|||||||
|
import type { NodeTree, Node, NodeId } from '@craftjs/core';
|
||||||
|
import { getRandomId } from '@craftjs/utils';
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Returns a deep-cloned NodeTree where every node id in the tree
|
||||||
|
* (rootNodeId, the `nodes` map keys, each `node.id`, each internal
|
||||||
|
* `node.data.parent`, every `node.data.nodes` entry, and every
|
||||||
|
* `node.data.linkedNodes` value) has been remapped to a fresh,
|
||||||
|
* collision-free id generated by Craft.js's own id generator.
|
||||||
|
*
|
||||||
|
* The subtree root's `data.parent` is deliberately left untouched --
|
||||||
|
* `actions.addNodeTree(tree, parentId)` sets that itself when the tree is
|
||||||
|
* attached to its new parent.
|
||||||
|
*
|
||||||
|
* The input tree is not mutated.
|
||||||
|
*/
|
||||||
|
export function regenerateTreeIds(tree: NodeTree): NodeTree {
|
||||||
|
const oldIds = Object.keys(tree.nodes);
|
||||||
|
|
||||||
|
// Build old -> new id map first so all cross-references can be rewritten
|
||||||
|
// consistently regardless of iteration order.
|
||||||
|
const idMap = new Map<NodeId, NodeId>();
|
||||||
|
for (const oldId of oldIds) {
|
||||||
|
idMap.set(oldId, getRandomId());
|
||||||
|
}
|
||||||
|
|
||||||
|
const remapId = (id: NodeId): NodeId => idMap.get(id) ?? id;
|
||||||
|
|
||||||
|
const newNodes: Record<NodeId, Node> = {};
|
||||||
|
for (const oldId of oldIds) {
|
||||||
|
const oldNode = tree.nodes[oldId];
|
||||||
|
const newId = idMap.get(oldId)!;
|
||||||
|
|
||||||
|
const newParent =
|
||||||
|
oldId === tree.rootNodeId
|
||||||
|
? oldNode.data.parent // subtree root's parent is set by the caller
|
||||||
|
: oldNode.data.parent !== null && oldNode.data.parent !== undefined
|
||||||
|
? remapId(oldNode.data.parent)
|
||||||
|
: oldNode.data.parent;
|
||||||
|
|
||||||
|
const newLinkedNodes: Record<string, NodeId> = {};
|
||||||
|
for (const [slot, linkedId] of Object.entries(oldNode.data.linkedNodes || {})) {
|
||||||
|
newLinkedNodes[slot] = remapId(linkedId);
|
||||||
|
}
|
||||||
|
|
||||||
|
const newNode: Node = {
|
||||||
|
...oldNode,
|
||||||
|
id: newId,
|
||||||
|
data: {
|
||||||
|
...oldNode.data,
|
||||||
|
parent: newParent,
|
||||||
|
nodes: (oldNode.data.nodes || []).map(remapId),
|
||||||
|
linkedNodes: newLinkedNodes,
|
||||||
|
},
|
||||||
|
};
|
||||||
|
|
||||||
|
newNodes[newId] = newNode;
|
||||||
|
}
|
||||||
|
|
||||||
|
return {
|
||||||
|
rootNodeId: remapId(tree.rootNodeId),
|
||||||
|
nodes: newNodes,
|
||||||
|
};
|
||||||
|
}
|
||||||
Reference in New Issue
Block a user