Update rules id with remapped duplicate components in Pages. Closes #…

…5425

src/canvas/model/Frame.ts

@@ -1,12 +1,13 @@
-import { forEach, isEmpty, isNumber, isString, result } from 'underscore';
+import { forEach, isEmpty, isNumber, isString, keys, result } from 'underscore';
 import CanvasModule from '..';
 import { ModuleModel } from '../../abstract';
-import { BoxRect } from '../../common';
+import { BoxRect, PrevToNewIdMap } from '../../common';
 import ComponentWrapper from '../../dom_components/model/ComponentWrapper';
 import Page from '../../pages/model/Page';
 import { createId, isComponent, isObject } from '../../utils/mixins';
 import FrameView from '../view/FrameView';
 import Frames from './Frames';
+import { CssRuleJSON } from '../../css_composer/model/CssRule';
 
 const keyAutoW = '__aw';
 const keyAutoH = '__ah';
@@ -63,7 +64,7 @@ export default class Frame extends ModuleModel<CanvasModule> {
     const domc = em.Components;
     const conf = domc.getConfig();
     const allRules = em.Css.getAll();
-    const idMap: any = {};
+    const idMap: PrevToNewIdMap = {};
     const modOpts = { em, config: conf, frame: this, idMap };
 
     if (!isComponent(component)) {
@@ -76,27 +77,15 @@ export default class Frame extends ModuleModel<CanvasModule> {
     if (!styles) {
       this.set('styles', allRules);
     } else if (!isObject(styles)) {
+      let newStyles = styles as string | CssRuleJSON[];
+
       // Avoid losing styles on remapped components
-      const idMapKeys = Object.keys(idMap);
-      if (idMapKeys.length && Array.isArray(styles)) {
-        styles.forEach(style => {
-          const sel = style.selectors;
-          if (sel && sel.length == 1) {
-            const sSel = sel[0];
-            const idSel = sSel.name && sSel.type === 2 && sSel;
-            if (idSel && idMap[idSel.name]) {
-              idSel.name = idMap[idSel.name];
-            } else if (isString(sSel) && sSel[0] === '#') {
-              const prevId = sSel.substring(1);
-              if (prevId && idMap[prevId]) {
-                sel[0] = `#${idMap[prevId]}`;
-              }
-            }
-          }
-        });
+      if (keys(idMap).length) {
+        newStyles = isString(newStyles) ? em.Parser.parseCss(newStyles) : newStyles;
+        em.Css.checkId(newStyles, { idMap });
       }
 
-      allRules.add(styles);
+      allRules.add(newStyles);
       this.set('styles', allRules);
     }
 

src/common/index.ts

@@ -79,3 +79,5 @@ export const DEFAULT_BOXRECT: BoxRect = {
   width: 0,
   height: 0,
 };
+
+export type PrevToNewIdMap = Record<string, string>;

src/css_composer/index.ts

@@ -38,7 +38,7 @@ import CssRulesView from './view/CssRulesView';
 import { ItemManagerModule } from '../abstract/Module';
 import EditorModel from '../editor/model/Editor';
 import Component from '../dom_components/model/Component';
-import { ObjectAny } from '../common';
+import { ObjectAny, PrevToNewIdMap } from '../common';
 
 /** @private */
 interface RuleOptions {
@@ -518,6 +518,41 @@ export default class CssComposer extends ItemManagerModule<CssComposerConfig & {
     return this.rulesView.render().el;
   }
 
+  checkId(rule: CssRuleJSON | CssRuleJSON[], opts: { idMap?: PrevToNewIdMap } = {}) {
+    const { idMap = {} } = opts;
+    const changed: CssRuleJSON[] = [];
+
+    if (!Object.keys(idMap).length) return changed;
+
+    const rules = Array.isArray(rule) ? rule : [rule];
+    rules.forEach(rule => {
+      const sel = rule.selectors;
+
+      if (sel && sel.length == 1) {
+        const sSel = sel[0];
+
+        if (isString(sSel)) {
+          if (sSel[0] === '#') {
+            const prevId = sSel.substring(1);
+            const newId = idMap[prevId];
+            if (prevId && newId) {
+              sel[0] = `#${newId}`;
+              changed.push(rule);
+            }
+          }
+        } else if (sSel.name && sSel.type === Selector.TYPE_ID) {
+          const newId = idMap[sSel.name];
+          if (newId) {
+            sSel.name = newId;
+            changed.push(rule);
+          }
+        }
+      }
+    });
+
+    return changed;
+  }
+
   destroy() {
     this.rules.reset();
     this.rules.stopListening();

src/dom_components/model/Component.ts

@@ -32,7 +32,7 @@ import {
 import Frame from '../../canvas/model/Frame';
 import { DomComponentsConfig } from '../config/config';
 import ComponentView from '../view/ComponentView';
-import { AddOptions, ExtractMethods, ObjectAny, ObjectStrings, SetOptions } from '../../common';
+import { AddOptions, ExtractMethods, ObjectAny, PrevToNewIdMap, SetOptions } from '../../common';
 import CssRule, { CssRuleJSON } from '../../css_composer/model/CssRule';
 import Trait, { TraitProperties } from '../../trait_manager/model/Trait';
 import { ToolbarButtonProps } from './ToolbarButton';
@@ -2096,10 +2096,10 @@ export default class Component extends StyleableModel<ComponentProperties> {
     components: ComponentDefinitionDefined | ComponentDefinitionDefined[],
     styles: CssRuleJSON[] = [],
     list: ObjectAny = {},
-    opts: { keepIds?: string[] } = {}
+    opts: { keepIds?: string[]; idMap?: PrevToNewIdMap } = {}
   ) {
     const comps = isArray(components) ? components : [components];
-    const { keepIds = [] } = opts;
+    const { keepIds = [], idMap = {} } = opts;
     comps.forEach(comp => {
       comp.attributes;
       const { attributes = {}, components } = comp;
@@ -2108,6 +2108,7 @@ export default class Component extends StyleableModel<ComponentProperties> {
       // Check if we have collisions with current components
       if (id && list[id] && keepIds.indexOf(id) < 0) {
         const newId = Component.getIncrementId(id, list);
+        idMap[id] = newId;
         attributes.id = newId;
         // Update passed styles
         isArray(styles) &&

test/specs/pages/index.ts

@@ -233,4 +233,35 @@ describe('Managing pages', () => {
     expect(event).toBeCalledTimes(1);
     expect(event).toBeCalledWith(page, up, opts);
   });
+
+  test('Prevent duplicate ids in components and styles', () => {
+    const id = 'myid';
+    const idSel = `#${id}`;
+    pm.add({
+      component: `<div id="${id}">My Page</div>`,
+      styles: `${idSel} { color: red }`,
+    })!;
+    pm.add({
+      component: `<div id="${id}">My Page</div>`,
+      styles: `${idSel} { color: blue }`,
+    })!;
+
+    expect(pm.getAll().length).toBe(3);
+
+    // Check component/rule from the first page
+    const cmp1 = domc.allById()[id];
+    const rule1 = em.Css.getRule(idSel)!;
+    expect(cmp1.getId()).toBe(id);
+    expect(rule1.getSelectorsString()).toBe(idSel);
+    expect(rule1.getStyle()).toEqual({ color: 'red' });
+
+    // Check component/rule from the second page
+    const id2 = 'myid-2';
+    const idSel2 = `#${id2}`;
+    const cmp2 = domc.allById()[id2];
+    const rule2 = em.Css.getRule(idSel2)!;
+    expect(cmp2.getId()).toBe(id2);
+    expect(rule2.getSelectorsString()).toBe(idSel2);
+    expect(rule2.getStyle()).toEqual({ color: 'blue' });
+  });
 });