Preserve Node IDs When Grouping Nodes

code/CHANGELOG.md

@@ -13,6 +13,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [Unreleased]
 
+### Changed
+
+- [#1112](https://github.com/InditexTech/weavejs/issues/1112) Preserve Node IDs When Grouping Nodes
+
 ## [5.0.0] - 2026-06-18
 
 ### Added

code/packages/react/src/components/provider.tsx

@@ -10,6 +10,7 @@ import type {
   WeaveRenderer,
   WeaveStore,
   WeavePerformanceConfig,
+  WeaveFontsPreloadFunction,
 } from '@inditextech/weave-sdk';
 import { Weave } from '@inditextech/weave-sdk';
 import {
@@ -25,7 +26,7 @@ import { useWeave } from './store';
 
 type WeaveProviderType = {
   getContainer: () => HTMLElement;
-  fonts?: WeaveFont[] | (() => Promise<WeaveFont[]>);
+  fonts?: WeaveFont[] | WeaveFontsPreloadFunction;
   store: WeaveStore;
   renderer: WeaveRenderer;
   nodes?: WeaveNode[];

code/packages/sdk/src/actions/images-tool/__tests__/images-tool.test.ts

@@ -45,6 +45,7 @@ vi.mock('@/utils/utils', () => ({
   mergeExceptArrays: vi.fn(
     (a: Record<string, unknown>, b: Record<string, unknown>) => ({ ...a, ...b })
   ),
+  sleep: vi.fn().mockResolvedValue(undefined),
 }));
 
 if (typeof (globalThis as Record<string, unknown>)['window'] === 'undefined') {
@@ -395,18 +396,17 @@ describe('WeaveImagesToolAction', () => {
       expect((action as unknown as R)['uploadImageFunction']).toBe(uploadFn);
     });
 
-    it('5.2 nodesIds reset to [] inside FILE block', () => {
-      (action as unknown as R)['nodesIds'] = ['old-id'];
+    it('5.2 nodesIds reset to new Set() inside FILE block', () => {
+      (action as unknown as R)['nodesIds'] = new Set(['old-id']);
       action.trigger(vi.fn(), {
         type: WEAVE_IMAGES_TOOL_UPLOAD_TYPE.FILE,
         images: [makeImageFile()],
         uploadImageFunction: uploadFn,
         onStartUploading: onStart,
         onFinishedUploading: onFinish,
       });
-      // nodesIds is reset in FILE block then populated by addImages
-      // immediately after trigger, addImages is called but async; nodesIds may be []
-      expect(Array.isArray((action as unknown as R)['nodesIds'])).toBe(true);
+      // nodesIds is reset to a new Set in FILE block then populated by addImages
+      expect((action as unknown as R)['nodesIds']).toBeInstanceOf(Set);
     });
 
     it('5.3 addImages called → state transitions toward DEFINING_POSITION', async () => {
@@ -897,8 +897,8 @@ describe('WeaveImagesToolAction', () => {
 
     it('13.1 nodeId in nodesIds → handleImageAdded called', async () => {
       const checkAdded = await setupHandleAdding();
-      const nodesIds = (action as unknown as R)['nodesIds'] as string[];
-      const nodeId = nodesIds[0];
+      const nodesIds = (action as unknown as R)['nodesIds'] as Set<string>;
+      const nodeId = Array.from(nodesIds)[0];
       (action as unknown as R)['toAdd'] = 5; // keep > 0 so cleanup doesn't happen
       const handleImageAddedSpy = vi.spyOn(action, 'handleImageAdded');
       checkAdded?.({ nodeId });
@@ -915,14 +915,14 @@ describe('WeaveImagesToolAction', () => {
 
     it('13.3 getImagesAdded()<=0 → setState(FINISHED) + cancelAction + removeEventListener + emitEvent', async () => {
       const checkAdded = await setupHandleAdding();
-      const nodesIds = (action as unknown as R)['nodesIds'] as string[];
+      const nodesIds = (action as unknown as R)['nodesIds'] as Set<string>;
       (action as unknown as R)['toAdd'] = 1; // will become 0 after handleImageAdded
       const cancelFn = (action as unknown as R)['cancelAction'] as ReturnType<typeof vi.fn>;
-      checkAdded?.({ nodeId: nodesIds[0] });
+      checkAdded?.({ nodeId: Array.from(nodesIds)[0] });
       expect((action as unknown as R)['state']).toBe(WEAVE_IMAGES_TOOL_STATE.FINISHED);
       expect(cancelFn).toHaveBeenCalled();
       expect(mockWeave.removeEventListener).toHaveBeenCalledWith('onAddedImage', expect.any(Function));
-      expect(mockWeave.emitEvent).toHaveBeenCalledWith('onAddedImages', { nodesIds });
+      expect(mockWeave.emitEvent).toHaveBeenCalledWith('onAddedImages', { nodesIds: Array.from(nodesIds) });
     });
 
     it('13.4 getImagesAdded()>0 → no setState/cancelAction', async () => {
@@ -1010,7 +1010,7 @@ describe('WeaveImagesToolAction', () => {
   describe('Suite 16: cleanup', () => {
     beforeEach(() => {
       setupAction();
-      (action as unknown as R)['nodesIds'] = ['node-1', 'node-2'];
+      (action as unknown as R)['nodesIds'] = new Set(['node-1', 'node-2']);
     });
 
     it('16.1 tempPointerFeedbackNode present → destroy + null', () => {
@@ -1078,7 +1078,7 @@ describe('WeaveImagesToolAction', () => {
       expect((action as unknown as R)['initialCursor']).toBeNull();
       expect((action as unknown as R)['container']).toBeUndefined();
       expect((action as unknown as R)['clickPoint']).toBeNull();
-      expect((action as unknown as R)['nodesIds']).toEqual([]);
+      expect((action as unknown as R)['nodesIds']).toEqual(new Set());
       expect((action as unknown as R)['toAdd']).toBe(0);
       expect((action as unknown as R)['state']).toBe(WEAVE_IMAGES_TOOL_STATE.IDLE);
     });

code/packages/sdk/src/actions/images-tool/images-tool.ts

@@ -54,7 +54,7 @@ export class WeaveImagesToolAction extends WeaveAction {
   protected pointers!: Map<number, Vector2d>;
   protected tempPointerFeedbackNode!: Konva.Group | null;
   protected container: Konva.Layer | Konva.Group | undefined;
-  protected nodesIds: string[] = [];
+  protected nodesIds: Set<string> = new Set();
   private toAdd: number = 0;
   protected imagesFile: WeaveImagesFile[] = [];
   protected imagesURL: WeaveImagesURL[] = [];
@@ -412,11 +412,11 @@ export class WeaveImagesToolAction extends WeaveAction {
     let imagePositionY = originPoint.y;
     let maxHeight = 0;
 
-    this.nodesIds = [];
+    this.nodesIds = new Set();
     const images: Promise<void>[] = [];
 
     const checkAddedImages = ({ nodeId }: { nodeId: string }) => {
-      if (this.nodesIds.includes(nodeId)) {
+      if (this.nodesIds.has(nodeId)) {
         this.handleImageAdded();
       }
 
@@ -429,7 +429,7 @@ export class WeaveImagesToolAction extends WeaveAction {
 
         this.instance.emitEvent<WeaveImagesToolActionOnAddedEvent>(
           'onAddedImages',
-          { nodesIds: this.nodesIds }
+          { nodesIds: Array.from(this.nodesIds) }
         );
       }
     };
@@ -509,7 +509,7 @@ export class WeaveImagesToolAction extends WeaveAction {
           handleImage(nodeId, image, { x: imagePositionX, y: imagePositionY })
         );
 
-        this.nodesIds.push(nodeId);
+        this.nodesIds.add(nodeId);
 
         maxHeight = Math.max(maxHeight, height);
 
@@ -564,7 +564,7 @@ export class WeaveImagesToolAction extends WeaveAction {
           handleImage(nodeId, image, { x: imagePositionX, y: imagePositionY })
         );
 
-        this.nodesIds.push(nodeId);
+        this.nodesIds.add(nodeId);
 
         maxHeight = Math.max(maxHeight, image.height);
 
@@ -577,8 +577,8 @@ export class WeaveImagesToolAction extends WeaveAction {
       }
     }
 
-    if (this.nodesIds.length > 0) {
-      this.toAdd = this.nodesIds.length;
+    if (this.nodesIds.size > 0) {
+      this.toAdd = this.nodesIds.size;
 
       await Promise.allSettled(images);
     } else {
@@ -616,21 +616,21 @@ export class WeaveImagesToolAction extends WeaveAction {
       this.container = params.container;
     }
 
-    this.nodesIds = [];
+    this.nodesIds = new Set();
     this.forceMainContainer = params.forceMainContainer ?? false;
 
     if (params.type === WEAVE_IMAGES_TOOL_UPLOAD_TYPE.FILE) {
       this.uploadType = WEAVE_IMAGES_TOOL_UPLOAD_TYPE.FILE;
       this.onStartUploading = params.onStartUploading;
       this.onFinishedUploading = params.onFinishedUploading;
       this.uploadImageFunction = params.uploadImageFunction;
-      this.nodesIds = [];
+      this.nodesIds = new Set();
       this.imagesFile = params.images;
     }
 
     if (params.type === WEAVE_IMAGES_TOOL_UPLOAD_TYPE.IMAGE_URL) {
       this.uploadType = WEAVE_IMAGES_TOOL_UPLOAD_TYPE.IMAGE_URL;
-      this.nodesIds = [];
+      this.nodesIds = new Set();
       this.imagesURL = params.images;
     }
 
@@ -665,19 +665,16 @@ export class WeaveImagesToolAction extends WeaveAction {
     }
   }
 
-  cleanup(): void {
+  async waitForImagesToBeAdded(nodesIds: Set<string>) {
     const stage = this.instance.getStage();
 
-    this.tempPointerFeedbackNode?.destroy();
-    this.tempPointerFeedbackNode = null;
-    this.instance.getUtilityLayer()?.batchDraw();
-
     const selectionPlugin =
       this.instance.getPlugin<WeaveNodesSelectionPlugin>('nodesSelection');
+
     if (selectionPlugin) {
       const addedNodes = [];
 
-      for (const nodeId of this.nodesIds) {
+      for (const nodeId of Array.from(nodesIds)) {
         const node = stage.findOne(`#${nodeId}`);
 
         if (node) {
@@ -688,6 +685,19 @@ export class WeaveImagesToolAction extends WeaveAction {
       selectionPlugin.setSelectedNodes(addedNodes);
       this.instance.triggerAction(SELECTION_TOOL_ACTION_NAME);
     }
+  }
+
+  cleanup(): void {
+    const stage = this.instance.getStage();
+
+    this.tempPointerFeedbackNode?.destroy();
+    this.tempPointerFeedbackNode = null;
+    this.instance.getUtilityLayer()?.batchDraw();
+
+    const nodesIds = this.nodesIds;
+    this.nodesIds = new Set();
+    this.toAdd = 0;
+    this.waitForImagesToBeAdded(nodesIds);
 
     this.instance.emitEvent<undefined>('onFinishedImages');
 
@@ -701,8 +711,6 @@ export class WeaveImagesToolAction extends WeaveAction {
     this.container = undefined;
     this.stageClickPoint = null;
     this.clickPoint = null;
-    this.nodesIds = [];
-    this.toAdd = 0;
     this.setState(WEAVE_IMAGES_TOOL_STATE.IDLE);
   }
 

code/packages/sdk/src/managers/__tests__/groups.test.ts

@@ -388,11 +388,11 @@ describe('WeaveGroupsManager', () => {
     beforeEach(() => { vi.useFakeTimers(); });
     afterEach(() => { vi.useRealTimers(); });
 
-    it('empty nodes: group() does not crash and calls removeNodes with []', () => {
+    it('empty nodes: group() does not crash and does not call removeNodeNT', () => {
       const { weave } = makeMockWeave();
       const mgr = new WeaveGroupsManager(weave);
       mgr.group([]);
-      expect(weave.removeNodes).toHaveBeenCalledWith([]);
+      expect(weave.removeNodeNT).not.toHaveBeenCalled();
     });
 
     it('no frames: realNodes = nodes, all nodes are processed', () => {
@@ -448,27 +448,35 @@ describe('WeaveGroupsManager', () => {
       });
       const mgr = new WeaveGroupsManager(weave);
       mgr.group([frameEl, childEl, otherEl]);
-      // childEl should be excluded; removeNodes should not contain childEl
-      expect(weave.removeNodes).toHaveBeenCalledWith(
-        expect.not.arrayContaining([childEl])
-      );
+      // childEl should be excluded from realNodes; removeNodeNT should not be called with it
+      expect(weave.removeNodeNT).not.toHaveBeenCalledWith(childEl, expect.anything());
     });
 
     it('frame child NOT excluded when parent has no nodeId in framesIds', () => {
       const mainLayer = new Konva.Layer();
       const frameKonva = new Konva.Group({ nodeType: 'frame' });
-      // Child whose parent has nodeId NOT in framesIds → should be included
+      // Child whose parent has nodeId NOT in framesIds → should be included.
+      // Give childKonva an explicit id so mainLayer.findOne('#child-key') resolves
+      // correctly (Konva assigns empty-string ids in jsdom without an explicit id).
+      // The stage mock proxy simulates childKonva having parentWithOtherNodeId
+      // as its parent, which is what allNodesInSameParent uses for filtering.
       const parentWithOtherNodeId = new Konva.Group({ nodeId: 'other-id' });
-      const childKonva = new Konva.Rect({ nodeType: 'rect' });
-      parentWithOtherNodeId.add(childKonva);
+      const childKonva = new Konva.Rect({ id: 'child-key', nodeType: 'rect' });
       mainLayer.add(frameKonva);
+      mainLayer.add(childKonva);
 
       const frameEl = { key: 'frame-key', type: 'node', props: { id: 'frame-key', nodeType: 'frame' } } as unknown as WeaveStateElement;
-      const childEl = { key: childKonva.id(), type: 'node', props: { id: childKonva.id(), nodeType: 'rect' } } as unknown as WeaveStateElement;
+      const childEl = { key: 'child-key', type: 'node', props: { id: 'child-key', nodeType: 'rect' } } as unknown as WeaveStateElement;
 
-      const stageMap: Record<string, Konva.Node> = {
+      // Proxy for childKonva in the stage mock: simulates being a child of
+      // parentWithOtherNodeId (nodeId='other-id', which is NOT in framesIds)
+      const childKonvaProxy = {
+        getAttrs: () => ({ nodeType: 'rect' }),
+        getParent: () => parentWithOtherNodeId,
+      };
+      const stageMap: Record<string, Konva.Node | object> = {
         'frame-key': frameKonva,
-        [childKonva.id()]: childKonva,
+        'child-key': childKonvaProxy as unknown as Konva.Node,
       };
       const groupHandler = makeGroupHandler();
       const nodeHandler = makeNodeHandler();
@@ -480,9 +488,7 @@ describe('WeaveGroupsManager', () => {
       const mgr = new WeaveGroupsManager(weave);
       mgr.group([frameEl, childEl]);
       // childEl should be included (parent.nodeId = 'other-id' not in framesIds)
-      expect(weave.removeNodes).toHaveBeenCalledWith(
-        expect.arrayContaining([childEl])
-      );
+      expect(weave.removeNodeNT).toHaveBeenCalledWith(childEl, expect.anything());
     });
 
     it('single parent with nodeId → parentId = nodeId used in addNodeNT', () => {