Merge branch 'fix/id-conflict' of https://github.com/IhorKaleniuk666/grapesjs into IhorKaleniuk666-fix/id-conflict

packages/core/src/dom_components/model/Component.ts

@@ -316,7 +316,7 @@ export default class Component extends StyleableModel<ComponentProperties> {
     };
     const attrs = this.dataResolverWatchers.getValueOrResolver('attributes', defaultAttrs);
     this.setAttributes(attrs);
-    this.ccid = Component.createId(this, opt);
+    this.ccid = Component.createId(this, opt as any);
     this.preInit();
     this.initClasses();
     this.initComponents();
@@ -2066,39 +2066,82 @@ export default class Component extends StyleableModel<ComponentProperties> {
     const current = list[id];
 
     if (!current) {
-      // Insert in list
       list[id] = model;
     } else if (current !== model) {
-      // Create new ID
+      const currentPage = current.page;
-      const nextId = Component.getIncrementId(id, list);
+      const modelPage = model.page;
-      model.setId(nextId);
+      const samePage = !!currentPage && !!modelPage && currentPage === modelPage;
-      list[nextId] = model;
+
+      if (samePage) {
+        const nextId = Component.getIncrementId(id, list);
+        model.setId(nextId);
+        list[nextId] = model;
+      } else {
+        const baseId = (model as any).ccid || id;
+        let nextId = baseId;
+
+        while (list[nextId] && list[nextId] !== model) {
+          nextId = Component.getIncrementId(nextId, list);
+        }
+
+        (model as any).ccid = nextId;
+        list[nextId] = model;
+      }
     }
 
     model.components().forEach((i) => Component.ensureInList(i));
   }
 
-  static createId(model: Component, opts: any = {}) {
+  static createId(
+    model: Component,
+    opts: {
+      keepIds?: string[];
+      idMap?: PrevToNewIdMap;
+      updatedIds?: Record<string, ComponentDefinitionDefined[]>;
+    } = {},
+  ) {
     const list = Component.getList(model);
     const { idMap = {} } = opts;
-    let { id } = model.get('attributes')!;
+    const attrs = model.get('attributes') || {};
-    let nextId;
+    const attrId = attrs.id as string | undefined;
+    let nextId: string;
+
+    if (attrId) {
+      const existing = list[attrId] as Component | undefined;
+
+      if (!existing || existing === model) {
+        nextId = attrId;
+        if (!list[nextId]) {
+          list[nextId] = model;
+        }
+      } else {
+        const existingPage = existing.page;
+        const newPage = model.page;
+        const samePage = !!existingPage && !!newPage && existingPage === newPage;
+
+        if (samePage) {
+          nextId = Component.getIncrementId(attrId, list, opts);
+          model.setId(nextId);
+          if (attrId !== nextId) {
+            idMap[attrId] = nextId;
+          }
+          list[nextId] = model;
+        } else {
+          nextId = Component.getIncrementId(attrId, list, opts);
 
-    if (id) {
+          list[nextId] = model;
-      nextId = Component.getIncrementId(id, list, opts);
+        }
-      model.setId(nextId);
+      }
-      if (id !== nextId) idMap[id] = nextId;
     } else {
       nextId = Component.getNewId(list);
+      list[nextId] = model;
     }
 
-    list[nextId] = model;
     return nextId;
   }
 
   static getNewId(list: ObjectAny) {
     const count = Object.keys(list).length;
-    // Testing 1000000 components with `+ 2` returns 0 collisions
     const ilen = count.toString().length + 2;
     const uid = (Math.random() + 1.1).toString(36).slice(-ilen);
     let newId = `i${uid}`;

packages/core/test/specs/data_sources/model/data_collection/ComponentDataCollection.ts

@@ -104,6 +104,7 @@ describe('Collection component', () => {
         expect(cmp.getInnerHTML()).toBe(innerHTML);
         expect(cmp.toHTML()).toBe(`<${tagName} id="${cmp.getId()}">${innerHTML}</${tagName}>`);
         expect(cmp.getEl()?.innerHTML).toBe(innerHTML);
+
         expect(JSON.parse(JSON.stringify(cmp.toJSON()))).toEqual({
           tagName: cmp.tagName,
           dataResolver: cmp.get('dataResolver'),

packages/core/test/specs/pages/pages-same-id-on-different-pages.ts

@@ -0,0 +1,111 @@
+import Editor from '../../../src/editor';
+import EditorModel from '../../../src/editor/model/Editor';
+import ComponentWrapper from '../../../src/dom_components/model/ComponentWrapper';
+
+describe('Pages with same component ids across pages', () => {
+  let editor: Editor;
+  let em: EditorModel;
+  let pm: Editor['Pages'];
+  let domc: Editor['Components'];
+  const getTitle = (wrapper: ComponentWrapper) => wrapper.components().at(0)!.components().at(0)!;
+
+  beforeAll(() => {
+    editor = new Editor({
+      pageManager: {
+        pages: [
+          {
+            id: 'p1',
+            frames: [
+              {
+                component: {
+                  type: 'wrapper',
+                  attributes: { id: 'body' },
+                  components: [
+                    {
+                      tagName: 'section',
+                      components: [
+                        {
+                          tagName: 'h1',
+                          type: 'text',
+                          attributes: { id: 'main-title' },
+                          components: [{ type: 'textnode', content: 'A' }],
+                        },
+                      ],
+                    },
+                  ],
+                },
+              },
+            ],
+          },
+          {
+            id: 'p2',
+            frames: [
+              {
+                component: {
+                  type: 'wrapper',
+                  attributes: { id: 'body' },
+                  components: [
+                    {
+                      tagName: 'section',
+                      components: [
+                        {
+                          tagName: 'h1',
+                          type: 'text',
+                          attributes: { id: 'main-title' },
+                          components: [{ type: 'textnode', content: 'B' }],
+                        },
+                      ],
+                    },
+                  ],
+                },
+              },
+            ],
+          },
+        ],
+      },
+    });
+
+    em = editor.getModel();
+    pm = em.Pages;
+    domc = em.Components;
+    pm.onLoad();
+  });
+
+  afterAll(() => {
+    editor.destroy();
+  });
+
+  test('Handles pages with components having the same id across pages', () => {
+    const pages = pm.getAll();
+    expect(pages.length).toBe(2);
+
+    const p1 = pages[0];
+    const p2 = pages[1];
+
+    const w1 = p1.getMainComponent();
+    const w2 = p2.getMainComponent();
+
+    expect(w1.getId()).toBe('body');
+    expect(w2.getId()).toBe('body');
+
+    const t1 = getTitle(w1);
+    const t2 = getTitle(w2);
+
+    // IDs should be preserved per page but stored uniquely in the shared map
+    expect(t1.getId()).toBe('main-title');
+    expect(t2.getId()).toBe('main-title');
+
+    const all = domc.allById();
+
+    expect(all['body']).toBe(w1);
+    expect(all['body-2']).toBe(w2);
+    expect(all['main-title']).toBe(t1);
+    expect(all['main-title-2']).toBe(t2);
+
+    const html1 = editor.getHtml({ component: w1 });
+    const html2 = editor.getHtml({ component: w2 });
+
+    expect(html1).toContain('A');
+    expect(html2).toContain('B');
+  });
+});