Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
46 changes: 46 additions & 0 deletions packages/core/src/store/use-scene-commits.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -170,6 +170,52 @@ describe('scene commit boundary', () => {
expect(useScene.temporal.getState().pastStates).toHaveLength(1)
})

test('excludes fresh placement subtrees until they become a committed undo step', () => {
const draftLevel = LevelNode.parse({
id: 'level_fresh_placement',
parentId: BUILDING_ID,
children: [],
level: 1,
metadata: { isNew: true },
})
const draftWall = WallNode.parse({
id: 'wall_fresh_placement',
parentId: draftLevel.id,
start: [0, 0],
end: [4, 0],
})
const commits: SceneCommit[] = []
unsubscribe = subscribeSceneCommits((commit) => commits.push(commit))

useScene.getState().createNodes([
{ node: draftLevel, parentId: BUILDING_ID },
{ node: draftWall, parentId: draftLevel.id },
])
useScene.getState().updateNode(draftWall.id, { end: [5, 0] })

expect(useScene.getState().nodes[draftLevel.id]).toBeDefined()
expect(useScene.getState().nodes[draftWall.id]).toBeDefined()
expect(commits).toHaveLength(0)
expect(useScene.temporal.getState().pastStates).toHaveLength(0)

useScene.getState().updateNode(draftLevel.id, { metadata: {} })

expect(commits).toHaveLength(1)
expect(commits[0]?.before.nodes[draftLevel.id]).toBeUndefined()
expect(commits[0]?.before.nodes[draftWall.id]).toBeUndefined()
expect(commits[0]?.current.nodes[draftLevel.id]).toBeDefined()
expect(commits[0]?.current.nodes[draftWall.id]).toBeDefined()
expect(useScene.temporal.getState().pastStates).toHaveLength(1)

useScene.temporal.getState().undo()
expect(useScene.getState().nodes[draftLevel.id]).toBeUndefined()
expect(useScene.getState().nodes[draftWall.id]).toBeUndefined()

useScene.temporal.getState().redo()
expect(useScene.getState().nodes[draftLevel.id]).toBeDefined()
expect(useScene.getState().nodes[draftWall.id]).toBeDefined()
})

test('coalesces a compound transaction into one commit and one undo step', () => {
const commits: SceneCommit[] = []
unsubscribe = subscribeSceneCommits((commit) => commits.push(commit))
Expand Down
75 changes: 74 additions & 1 deletion packages/core/src/store/use-scene.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1088,7 +1088,80 @@ function sceneHistorySnapshotFromState(
>,
): SceneSnapshot {
const { nodes, rootNodeIds, collections, materials, installedPlugins } = state
return { nodes, rootNodeIds, collections, materials, installedPlugins }
// Fresh placement nodes are renderable drafts, not document history. Excluding their
// entire subtree here protects both local undo and external commit subscribers.
const transientNodeIds = new Set<AnyNodeId>()
for (const node of Object.values(nodes)) {
const metadata = node.metadata
if (
metadata &&
typeof metadata === 'object' &&
!Array.isArray(metadata) &&
(metadata as Record<string, unknown>).isNew === true
) {
transientNodeIds.add(node.id)
}
}

if (transientNodeIds.size === 0) {
return { nodes, rootNodeIds, collections, materials, installedPlugins }
}

const childIdsByParentId = new Map<AnyNodeId, Set<AnyNodeId>>()
const addChild = (parentId: AnyNodeId, childId: AnyNodeId) => {
const childIds = childIdsByParentId.get(parentId) ?? new Set<AnyNodeId>()
childIds.add(childId)
childIdsByParentId.set(parentId, childIds)
}
for (const node of Object.values(nodes)) {
if (node.parentId) addChild(node.parentId as AnyNodeId, node.id)
for (const childId of getNodeChildIds(node)) addChild(node.id, childId)
}

const pendingIds = [...transientNodeIds]
while (pendingIds.length > 0) {
const parentId = pendingIds.pop()
if (!parentId) continue
for (const childId of childIdsByParentId.get(parentId) ?? []) {
if (transientNodeIds.has(childId)) continue
transientNodeIds.add(childId)
pendingIds.push(childId)
}
}

const historyNodes = {} as Record<AnyNodeId, AnyNode>
for (const [id, node] of Object.entries(nodes) as [AnyNodeId, AnyNode][]) {
if (transientNodeIds.has(id)) continue
if (!('children' in node && Array.isArray(node.children))) {
historyNodes[id] = node
continue
}
const children = (node.children as AnyNodeId[]).filter(
(childId) => !transientNodeIds.has(childId),
)
historyNodes[id] =
children.length === node.children.length ? node : ({ ...node, children } as AnyNode)
}

const historyCollections = {} as Record<CollectionId, Collection>
for (const [id, collection] of Object.entries(collections) as [CollectionId, Collection][]) {
const nodeIds = collection.nodeIds.filter((nodeId) => !transientNodeIds.has(nodeId))
if (collection.controlNodeId && transientNodeIds.has(collection.controlNodeId)) {
const { controlNodeId: _controlNodeId, ...rest } = collection
historyCollections[id] = { ...rest, nodeIds }
} else {
historyCollections[id] =
nodeIds.length === collection.nodeIds.length ? collection : { ...collection, nodeIds }
}
}

return {
nodes: historyNodes,
rootNodeIds: rootNodeIds.filter((id) => !transientNodeIds.has(id)),
collections: historyCollections,
materials,
installedPlugins,
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Marking live nodes new deletes history

High Severity

sceneHistorySnapshotFromState drops any node once metadata.isNew becomes true. Paths that first create a normal node and then set isNew (door/window paste) therefore emit a second local commit and undo step shaped like a delete: before still has the node, current does not. External commit subscribers and undo can then remove a just-pasted node.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 71147a3. Configure here.

}

const useScene: UseSceneStore = create<SceneState>()(
Expand Down
Loading