fix: resolve state management issues and memory leaks (#152)

This commit fixes several issues in the core state management:

1. Fix deleteNodesAction recursive call issue
   - Previously, the function recursively called get().deleteNodes()
     inside the set() callback, which could cause state inconsistencies.
   - Now collects all descendant IDs first in a separate pass, then
     deletes them all in a single batch operation.

2. Fix memory leak in use-scene temporal tracking
   - Module-level variables (prevPastLength, prevFutureLength,
     prevNodesSnapshot) persisted across store re-creations.
   - Now cleared in unloadScene() to release stale node references.
   - Added clearTemporalTracking() helper function.

3. Remove unused variable in wall-system
   - Removed debug variable 'useFrameNb' that was declared but never used.

4. Fix requestAnimationFrame in updateNodesAction
   - Previously used RAF without cleanup, causing multiple queued
     callbacks when updates happened rapidly.
   - Now uses a single tracked RAF with cancellation support.

Co-authored-by: hobostay <hobostay@users.noreply.github.com>
Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

packages/core/src/store/actions/node-actions.ts

@@ -4,6 +4,10 @@ import type { SceneState } from '../use-scene'
 
 type AnyContainerNode = AnyNode & { children: string[] }
 
+// Track pending RAF for updateNodesAction to prevent multiple queued callbacks
+let pendingRafId: number | null = null
+let pendingUpdates: Set<AnyNodeId> = new Set()
+
 export const createNodesAction = (
   set: (fn: (state: SceneState) => Partial<SceneState>) => void,
   get: () => SceneState,
@@ -58,6 +62,7 @@ export const updateNodesAction = (
   updates: { id: AnyNodeId; data: Partial<AnyNode> }[],
 ) => {
   const parentsToUpdate = new Set<AnyNodeId>()
+  const idsToMarkDirty = new Set<AnyNodeId>()
 
   set((state) => {
     const nextNodes = { ...state.nodes }
@@ -98,14 +103,25 @@ export const updateNodesAction = (
     return { nodes: nextNodes }
   })
 
-  // Mark dirty after the next frame to ensure React renders complete
-  requestAnimationFrame(() => {
-    updates.forEach((u) => {
-      get().markDirty(u.id)
-    })
-    parentsToUpdate.forEach((pId) => {
-      get().markDirty(pId)
+  // Collect all IDs that need to be marked dirty
+  updates.forEach((u) => idsToMarkDirty.add(u.id))
+  parentsToUpdate.forEach((pId) => idsToMarkDirty.add(pId))
+
+  // Add to pending updates set
+  idsToMarkDirty.forEach((id) => pendingUpdates.add(id))
+
+  // Cancel any pending RAF and schedule a new one
+  if (pendingRafId !== null) {
+    cancelAnimationFrame(pendingRafId)
+  }
+
+  pendingRafId = requestAnimationFrame(() => {
+    // Mark all pending updates as dirty
+    pendingUpdates.forEach((id) => {
+      get().markDirty(id)
     })
+    pendingUpdates.clear()
+    pendingRafId = null
   })
 }
 
@@ -121,7 +137,26 @@ export const deleteNodesAction = (
     const nextCollections = { ...state.collections }
     let nextRootIds = [...state.rootNodeIds]
 
+    // Collect all IDs to delete (including descendants) in a first pass
+    // This avoids issues with recursive calls during state mutation
+    const allIdsToDelete = new Set<AnyNodeId>()
+    const collectDescendants = (id: AnyNodeId) => {
+      const node = nextNodes[id]
+      if (!node) return
+      allIdsToDelete.add(id)
+      if ('children' in node && node.children) {
+        for (const childId of node.children as AnyNodeId[]) {
+          collectDescendants(childId)
+        }
+      }
+    }
+
     for (const id of ids) {
+      collectDescendants(id)
+    }
+
+    // Now process all nodes for deletion
+    for (const id of allIdsToDelete) {
       const node = nextNodes[id]
       if (!node) continue
 
@@ -153,12 +188,6 @@ export const deleteNodesAction = (
 
       // 4. Delete the node itself
       delete nextNodes[id]
-
-      // Inside the deleteNodes loop
-      if ('children' in node && node.children.length > 0) {
-        // Recursively delete all children first
-        get().deleteNodes(node.children as AnyNodeId[])
-      }
     }
 
     return { nodes: nextNodes, rootNodeIds: nextRootIds, collections: nextCollections }

packages/core/src/store/use-scene.ts

@@ -115,6 +115,11 @@ const useScene: UseSceneStore = create<SceneState>()(
       collections: {} as Record<CollectionId, Collection>,
 
       unloadScene: () => {
+        // Clear temporal tracking to prevent memory leaks from stale node references
+        prevPastLength = 0
+        prevFutureLength = 0
+        prevNodesSnapshot = null
+
         set({
           nodes: {},
           rootNodeIds: [],
@@ -305,13 +310,21 @@ let prevPastLength = 0
 let prevFutureLength = 0
 let prevNodesSnapshot: Record<AnyNodeId, AnyNode> | null = null
 
-export function clearSceneHistory() {
-  useScene.temporal.getState().clear()
+/**
+ * Clears temporal history tracking variables to prevent memory leaks.
+ * Should be called when unloading a scene to release node references.
+ */
+export function clearTemporalTracking() {
   prevPastLength = 0
   prevFutureLength = 0
   prevNodesSnapshot = null
 }
 
+export function clearSceneHistory() {
+  useScene.temporal.getState().clear()
+  clearTemporalTracking()
+}
+
 // Subscribe to the temporal store (Undo/Redo events)
 useScene.temporal.subscribe((state) => {
   const currentPastLength = state.pastStates.length

packages/core/src/systems/wall/wall-system.tsx

@@ -22,7 +22,6 @@ const csgEvaluator = new Evaluator()
 // WALL SYSTEM
 // ============================================================================
 
-let useFrameNb = 0
 export const WallSystem = () => {
   const dirtyNodes = useScene((state) => state.dirtyNodes)
   const clearDirty = useScene((state) => state.clearDirty)
@@ -35,7 +34,6 @@ export const WallSystem = () => {
     // Collect dirty walls and their levels
     const dirtyWallsByLevel = new Map<string, Set<string>>()
 
-    useFrameNb += 1
     dirtyNodes.forEach((id) => {
       const node = nodes[id]
       if (!node || node.type !== 'wall') return