diff --git a/packages/core/src/dom_components/model/Component.ts b/packages/core/src/dom_components/model/Component.ts index 58c689047..61306246f 100644 --- a/packages/core/src/dom_components/model/Component.ts +++ b/packages/core/src/dom_components/model/Component.ts @@ -316,7 +316,7 @@ export default class Component extends StyleableModel { }; 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 { const current = list[id]; if (!current) { - // Insert in list list[id] = model; } else if (current !== model) { - // Create new ID - const nextId = Component.getIncrementId(id, list); - model.setId(nextId); - list[nextId] = model; + const currentPage = current.page; + const modelPage = model.page; + const samePage = !!currentPage && !!modelPage && currentPage === modelPage; + + 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; + } = {}, + ) { const list = Component.getList(model); const { idMap = {} } = opts; - let { id } = model.get('attributes')!; - let nextId; + const attrs = model.get('attributes') || {}; + 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) { - nextId = Component.getIncrementId(id, list, opts); - model.setId(nextId); - if (id !== nextId) idMap[id] = nextId; + list[nextId] = model; + } + } } 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}`; diff --git a/packages/core/test/specs/data_sources/model/data_collection/ComponentDataCollection.ts b/packages/core/test/specs/data_sources/model/data_collection/ComponentDataCollection.ts index 5c2b85934..d9a033291 100644 --- a/packages/core/test/specs/data_sources/model/data_collection/ComponentDataCollection.ts +++ b/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}`); expect(cmp.getEl()?.innerHTML).toBe(innerHTML); + expect(JSON.parse(JSON.stringify(cmp.toJSON()))).toEqual({ tagName: cmp.tagName, dataResolver: cmp.get('dataResolver'), diff --git a/packages/core/test/specs/pages/pages-same-id-on-different-pages.ts b/packages/core/test/specs/pages/pages-same-id-on-different-pages.ts new file mode 100644 index 000000000..328b931f7 --- /dev/null +++ b/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'); + }); +});