save(false, true) + enable() error fix

#fix 1793
* `save(false, true)` followed by enable() throws error.
* we now have new `Utils.cloneDeep()` (needed for grid options) and added test cases.

Author: adumesny

doc/CHANGES.md

@@ -65,6 +65,7 @@ Change log
 * fix [#1791](https://github.com/gridstack/gridstack.js/pull/1791) removed drag flicker and scroll issue. Thanks [@nelsieborja](https://github.com/nelsieborja)
 * fix [#1795](https://github.com/gridstack/gridstack.js/issues/1795) `save(false)` will no longer have `.content` field (removed existing one if present)
 * fix [#1782](https://github.com/gridstack/gridstack.js/issues/1782) `save(false, false)` now correctly saves nested grids
+* fix [#1793](https://github.com/gridstack/gridstack.js/issues/1793) `save(false, true)` followed by enable() throws error. we now have new `Utils.cloneDeep()`
 
 ## 4.2.5 (2021-5-31)
 

spec/utils-spec.ts

@@ -129,4 +129,116 @@ describe('gridstack utils', function() {
     });
   });
 
+  describe('clone', () => {
+    const a: any = {first: 1, second: 'text'};
+    const b: any = {first: 1, second: {third: 3}};
+    const c: any = {first: 1, second: [1, 2, 3], third: { fourth: {fifth: 5}}};
+    it('Should have the same values', () => {
+      const z = Utils.clone(a);
+      expect(z).toEqual({first: 1, second: 'text'});
+    });
+    it('Should have 2 in first key, and original unchanged', () => {
+      const z = Utils.clone(a);
+      z.first = 2;
+      expect(a).toEqual({first: 1, second: 'text'});
+      expect(z).toEqual({first: 2, second: 'text'});
+    });
+    it('Should have new string in second key, and original unchanged', () => {
+      const z = Utils.clone(a);
+      z.second = 2;
+      expect(a).toEqual({first: 1, second: 'text'});
+      expect(z).toEqual({first: 1, second: 2});
+    });
+    it('new string in both cases - use cloneDeep instead', () => {
+      const z = Utils.clone(b);
+      z.second.third = 'share';
+      expect(b).toEqual({first: 1, second: {third: 'share'}});
+      expect(z).toEqual({first: 1, second: {third: 'share'}});
+    });
+    it('Array Should match', () => {
+      const z = Utils.clone(c);
+      expect(c).toEqual({first: 1, second: [1, 2, 3], third: { fourth: {fifth: 5}}});
+      expect(z).toEqual({first: 1, second: [1, 2, 3], third: { fourth: {fifth: 5}}});
+    });
+    it('Array[0] changed in both cases - use cloneDeep instead', () => {
+      const z = Utils.clone(c);
+      z.second[0] = 0;
+      expect(c).toEqual({first: 1, second: [0, 2, 3], third: { fourth: {fifth: 5}}});
+      expect(z).toEqual({first: 1, second: [0, 2, 3], third: { fourth: {fifth: 5}}});
+    });
+    it('fifth changed in both cases - use cloneDeep instead', () => {
+      const z = Utils.clone(c);
+      z.third.fourth.fifth = 'share';
+      expect(c).toEqual({first: 1, second: [0, 2, 3], third: { fourth: {fifth: 'share'}}});
+      expect(z).toEqual({first: 1, second: [0, 2, 3], third: { fourth: {fifth: 'share'}}});
+    });
+  });
+  describe('cloneDeep', () => {
+    // reset our test cases
+    const a: any = {first: 1, second: 'text'};
+    const b: any = {first: 1, second: {third: 3}};
+    const c: any = {first: 1, second: [1, 2, 3], third: { fourth: {fifth: 5}}};
+    const d: any = {first: [1, [2, 3], ['four', 'five', 'six']]};
+    const e: any = {first: 1, __skip: {second: 2}};
+    const f: any = {first: 1, _dontskip: {second: 2}};
+  
+    it('Should have the same values', () => {
+      const z = Utils.cloneDeep(a);
+      expect(z).toEqual({first: 1, second: 'text'});
+    });
+    it('Should have 2 in first key, and original unchanged', () => {
+      const z = Utils.cloneDeep(a);
+      z.first = 2;
+      expect(a).toEqual({first: 1, second: 'text'});
+      expect(z).toEqual({first: 2, second: 'text'});
+    });
+    it('Should have new string in second key, and original unchanged', () => {
+      const z = Utils.cloneDeep(a);
+      z.second = 2;
+      expect(a).toEqual({first: 1, second: 'text'});
+      expect(z).toEqual({first: 1, second: 2});
+    });
+    it('Should have new string nested object, and original unchanged', () => {
+      const z = Utils.cloneDeep(b);
+      z.second.third = 'diff';
+      expect(b).toEqual({first: 1, second: {third: 3}});
+      expect(z).toEqual({first: 1, second: {third: 'diff'}});
+    });
+    it('Array Should match', () => {
+      const z = Utils.cloneDeep(c);
+      expect(c).toEqual({first: 1, second: [1, 2, 3], third: { fourth: {fifth: 5}}});
+      expect(z).toEqual({first: 1, second: [1, 2, 3], third: { fourth: {fifth: 5}}});
+    });
+    it('Array[0] changed in z only', () => {
+      const z = Utils.cloneDeep(c);
+      z.second[0] = 0;
+      expect(c).toEqual({first: 1, second: [1, 2, 3], third: { fourth: {fifth: 5}}});
+      expect(z).toEqual({first: 1, second: [0, 2, 3], third: { fourth: {fifth: 5}}});
+    });
+    it('nested firth element changed only in z', () => {
+      const z = Utils.cloneDeep(c);
+      z.third.fourth.fifth = 'diff';
+      expect(c).toEqual({first: 1, second: [1, 2, 3], third: { fourth: {fifth: 5}}});
+      expect(z).toEqual({first: 1, second: [1, 2, 3], third: { fourth: {fifth: 'diff'}}});
+    });
+    it('nested array only has one item changed', () => {
+      const z = Utils.cloneDeep(d);
+      z.first[1] = 'two';
+      z.first[2][2] = 6;
+      expect(d).toEqual({first: [1, [2, 3], ['four', 'five', 'six']]});
+      expect(z).toEqual({first: [1, 'two',  ['four', 'five', 6]]});
+    });
+    it('skip __ items so it mods both instance', () => {
+      const z = Utils.cloneDeep(e);
+      z.__skip.second = 'two';
+      expect(e).toEqual({first: 1, __skip: {second: 'two'}}); // TODO support clone deep of function workaround
+      expect(z).toEqual({first: 1, __skip: {second: 'two'}});
+    });
+    it('correctly copy _ item', () => {
+      const z = Utils.cloneDeep(f);
+      z._dontskip.second = 'two';
+      expect(f).toEqual({first: 1, _dontskip: {second: 2}});
+      expect(z).toEqual({first: 1, _dontskip: {second: 'two'}});
+    });
+  });  
 });

src/gridstack.ts

@@ -121,7 +121,7 @@ export class GridStack {
       return null;
     }
     if (!el.gridstack) {
-      el.gridstack = new GridStack(el, {...options});
+      el.gridstack = new GridStack(el, Utils.cloneDeep(options));
     }
     return el.gridstack
   }
@@ -139,7 +139,7 @@ export class GridStack {
     let grids: GridStack[] = [];
     GridStack.getGridElements(selector).forEach(el => {
       if (!el.gridstack) {
-        el.gridstack = new GridStack(el, {...options});
+        el.gridstack = new GridStack(el, Utils.cloneDeep(options));
         delete options.dragIn; delete options.dragInOptions; // only need to be done once (really a static global thing, not per grid)
       }
       grids.push(el.gridstack);
@@ -247,7 +247,7 @@ export class GridStack {
     let rowAttr = Utils.toNumber(el.getAttribute('gs-row'));
 
     // elements attributes override any passed options (like CSS style) - merge the two together
-    let defaults: GridStackOptions = {...GridDefaults,
+    let defaults: GridStackOptions = {...Utils.cloneDeep(GridDefaults),
       column: Utils.toNumber(el.getAttribute('gs-column')) || 12,
       minRow: rowAttr ? rowAttr : Utils.toNumber(el.getAttribute('gs-min-row')) || 0,
       maxRow: rowAttr ? rowAttr : Utils.toNumber(el.getAttribute('gs-max-row')) || 0,
@@ -412,7 +412,7 @@ export class GridStack {
     // as the actual value are filled in when _prepareElement() calls el.getAttribute('gs-xyz) before adding the node.
     // So make sure we load any DOM attributes that are not specified in passed in options (which override)
     let domAttr = this._readAttr(el);
-    options = {...(options || {})};  // make a copy before we modify in case caller re-uses it
+    options = Utils.cloneDeep(options) || {};  // make a copy before we modify in case caller re-uses it
     Utils.defaults(options, domAttr);
     let node = this.engine.prepareNode(options);
     this._writeAttr(el, options);
@@ -470,7 +470,7 @@ export class GridStack {
 
     // check if save entire grid options (needed for recursive) + children...
     if (saveGridOpt) {
-      let o: GridStackOptions = {...this.opts};
+      let o: GridStackOptions = Utils.cloneDeep(this.opts);
       // delete default values that will be recreated on launch
       if (o.marginBottom === o.marginTop && o.marginRight === o.marginLeft && o.marginTop === o.marginRight) {
         o.margin = o.marginTop;
@@ -980,7 +980,7 @@ export class GridStack {
     GridStack.getElements(els).forEach(el => {
       if (!el || !el.gridstackNode) return;
       let n = el.gridstackNode;
-      let w = {...opt}; // make a copy we can modify in case they re-use it or multiple items
+      let w = Utils.cloneDeep(opt); // make a copy we can modify in case they re-use it or multiple items
       delete w.autoPosition;
 
       // move/resize widget if anything changed

src/utils.ts

@@ -370,5 +370,34 @@ export class Utils {
       scrollEl.scrollBy({ behavior: 'smooth', top: distance - (height - pointerPosY)});
     }
   }
+
+  /** single level clone, returning a new object with same top fields. This will share sub objects and arrays */
+  static clone<T>(obj: T): T {
+    if (obj === null || obj === undefined || typeof(obj) !== 'object') {
+      return obj;
+    }
+    // return Object.assign({}, obj);
+    if (obj instanceof Array) {
+      // eslint-disable-next-line @typescript-eslint/no-explicit-any
+      return [...obj] as any;
+    }
+    return {...obj};
+  }
+
+  /**
+   * Recursive clone version that returns a full copy, checking for nested objects and arrays ONLY.
+   * Note: this will use as-is any key starting with double __ (and not copy inside) some lib have circular dependencies.
+   */
+  static cloneDeep<T>(obj: T): T {
+    // return JSON.parse(JSON.stringify(obj)); // doesn't work with date format ?
+    const ret = Utils.clone(obj);
+    for (const key in ret) {
+      // NOTE: we don't support function/circular dependencies so skip those properties for now...
+      if (ret.hasOwnProperty(key) && typeof(ret[key]) === 'object' && key.substring(0, 2) !== '__') {
+        ret[key] = Utils.cloneDeep(obj[key]);
+      }
+    }
+    return ret;
+  }
 }