Merge branch 'unique-id' into dev

src/dom_components/index.js

@@ -38,6 +38,7 @@ module.exports = () => {
   const ComponentView = require('./view/ComponentView');
   const Components = require('./model/Components');
   const ComponentsView = require('./view/ComponentsView');
+  const componentsById = {};
 
   var component, componentView;
   var componentTypes = [
@@ -137,6 +138,8 @@ module.exports = () => {
 
     componentTypes,
 
+    componentsById,
+
     /**
      * Name of the module
      * @type {String}
@@ -231,7 +234,8 @@ module.exports = () => {
       component = new Component(wrapper, {
         em,
         config: c,
-        componentTypes
+        componentTypes,
+        domc: this
       });
       component.set({ attributes: { id: 'wrapper' } });
 

src/dom_components/model/Component.js

@@ -169,11 +169,11 @@ const Component = Backbone.Model.extend(Styleable).extend(
       this.opt = opt;
       this.em = em;
       this.config = opt.config || {};
-      this.ccid = Component.createId(this);
       this.set('attributes', {
         ...(this.defaults.attributes || {}),
         ...(this.get('attributes') || {})
       });
+      this.ccid = Component.createId(this);
       this.initClasses();
       this.initTraits();
       this.initComponents();
@@ -1037,17 +1037,84 @@ const Component = Backbone.Model.extend(Styleable).extend(
      * @private
      */
     createId(model) {
-      componentIndex++;
+      const list = Component.getList(model);
+      let { id } = model.get('attributes');
+      let nextId;
+
+      if (id) {
+        nextId = Component.getIncrementId(id, list);
+        model.setId(nextId);
+      } else {
+        nextId = Component.getNewId(list);
+      }
+
+      list[nextId] = model;
+      return nextId;
+    },
+
+    getNewId(list) {
+      const count = Object.keys(list).length;
       // Testing 1000000 components with `+ 2` returns 0 collisions
-      const ilen = componentIndex.toString().length + 2;
+      const ilen = count.toString().length + 2;
       const uid = (Math.random() + 1.1).toString(36).slice(-ilen);
-      const nextId = 'i' + uid;
-      componentList[nextId] = model;
-      return nextId;
+      let newId = `i${uid}`;
+
+      while (list[newId]) {
+        newId = Component.getNewId(list);
+      }
+
+      return newId;
     },
 
-    getList() {
-      return componentList;
+    getIncrementId(id, list) {
+      let counter = 1;
+      let newId = id;
+
+      while (list[newId]) {
+        counter++;
+        newId = `${id}-${counter}`;
+      }
+
+      return newId;
+    },
+
+    /**
+     * The list of components is taken from the Components module.
+     * Initially, the list, was set statically on the Component object but it was
+     * not ok, as it was shared between multiple editor instances
+     */
+    getList(model) {
+      const domc = model.opt && model.opt.domc;
+      return domc ? domc.componentsById : {};
+    },
+
+    /**
+     * This method checks, for each parsed component and style object
+     * (are not Components/CSSRules yet), for duplicated id and fixes them
+     * This method is used in Components.js just after the parsing
+     */
+    checkId(components, styles = [], list = {}) {
+      const comps = isArray(components) ? components : [components];
+      comps.forEach(comp => {
+        const { attributes = {}, components } = comp;
+        const { id } = attributes;
+
+        // Check if we have collisions with current components
+        if (id && list[id]) {
+          const newId = Component.getIncrementId(id, list);
+          attributes.id = newId;
+          // Update passed styles
+          isArray(styles) &&
+            styles.forEach(style => {
+              const { selectors } = style;
+              selectors.forEach((sel, idx) => {
+                if (sel === `#${id}`) selectors[idx] = `#${newId}`;
+              });
+            });
+        }
+
+        components && Component.checkId(components, styles, list);
+      });
     }
   }
 );

src/dom_components/model/Components.js

@@ -1,9 +1,11 @@
 import { isEmpty, isArray, isString } from 'underscore';
 
 const Backbone = require('backbone');
+let Component;
 
 module.exports = Backbone.Collection.extend({
   initialize(models, opt = {}) {
+    this.opt = opt;
     this.listenTo(this, 'add', this.onAdd);
     this.config = opt.config;
     this.em = opt.em;
@@ -14,6 +16,7 @@ module.exports = Backbone.Collection.extend({
       options.em = opt.em;
       options.config = opt.config;
       options.componentTypes = df;
+      options.domc = opt.domc;
 
       for (var it = 0; it < df.length; it++) {
         var dfId = df[it].id;
@@ -36,6 +39,9 @@ module.exports = Backbone.Collection.extend({
     const { em } = this;
     const cssc = em.get('CssComposer');
     const parsed = em.get('Parser').parseHtml(value);
+    // We need this to avoid duplicate IDs
+    if (!Component) Component = require('./Component');
+    Component.checkId(parsed.html, parsed.css, this.opt.domc.componentsById);
 
     if (parsed.css && cssc && !opt.temporary) {
       cssc.addCollection(parsed.css, {

test/specs/dom_components/model/Component.js

@@ -24,7 +24,8 @@ module.exports = {
         dcomp = new DomComponents();
         compOpts = {
           em,
-          componentTypes: dcomp.componentTypes
+          componentTypes: dcomp.componentTypes,
+          domc: dcomp
         };
         obj = new Component({}, compOpts);
       });
@@ -526,6 +527,7 @@ module.exports = {
 
     describe('Components', () => {
       beforeEach(() => {
+        em = new Editor({});
         dcomp = new DomComponents();
         compOpts = {
           componentTypes: dcomp.componentTypes
@@ -549,6 +551,91 @@ module.exports = {
         var m = c.add({ type: 'text' });
         expect(m instanceof ComponentText).toEqual(true);
       });
+
+      test('Avoid conflicting components with the same ID', () => {
+        const em = new Editor({});
+        dcomp = new DomComponents();
+        dcomp.init({ em });
+        const id = 'myid';
+        const idB = 'myid2';
+        const block = `
+          <div id="${id}">
+            <div id="${idB}"></div>
+          </div>
+          <style>
+            #${id} {
+              color: red;
+            }
+            #${id}:hover {
+              color: blue;
+            }
+            #${idB} {
+              color: yellow;
+            }
+          </style>
+        `;
+        const added = dcomp.addComponent(block);
+        // Let's check if everthing is working as expected
+        expect(Object.keys(dcomp.componentsById).length).toBe(3); // + 1 wrapper
+        expect(added.getId()).toBe(id);
+        expect(
+          added
+            .components()
+            .at(0)
+            .getId()
+        ).toBe(idB);
+        const cc = em.get('CssComposer');
+        expect(cc.getAll().length).toBe(3);
+        expect(
+          cc
+            .getAll()
+            .at(0)
+            .selectorsToString()
+        ).toBe(`#${id}`);
+        expect(
+          cc
+            .getAll()
+            .at(1)
+            .selectorsToString()
+        ).toBe(`#${id}:hover`);
+        expect(
+          cc
+            .getAll()
+            .at(2)
+            .selectorsToString()
+        ).toBe(`#${idB}`);
+        // Now let's add the same block
+        const added2 = dcomp.addComponent(block);
+        const id2 = added2.getId();
+        const newId = `${id}-2`;
+        const newIdB = `${idB}-2`;
+        expect(id2).toBe(newId);
+        expect(
+          added2
+            .components()
+            .at(0)
+            .getId()
+        ).toBe(newIdB);
+        expect(cc.getAll().length).toBe(6);
+        expect(
+          cc
+            .getAll()
+            .at(3)
+            .selectorsToString()
+        ).toBe(`#${newId}`);
+        expect(
+          cc
+            .getAll()
+            .at(4)
+            .selectorsToString()
+        ).toBe(`#${newId}:hover`);
+        expect(
+          cc
+            .getAll()
+            .at(5)
+            .selectorsToString()
+        ).toBe(`#${newIdB}`);
+      });
     });
   }
 };

test/specs/grapesjs/index.js

@@ -465,6 +465,7 @@ describe('GrapesJS', () => {
         el1 = wrapper.find('#el1')[0];
         el2 = wrapper.find('#el2')[0];
         el3 = wrapper.find('#el3')[0];
+        // console.log('wrapper', wrapper.getEl().innerHTML);
       });
 
       test('Select a single component', () => {