Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 1 commit into from
Jul 11, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions doc/CHANGES.md
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down
112 changes: 112 additions & 0 deletions spec/utils-spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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'}});
});
});
});
12 changes: 6 additions & 6 deletions src/gridstack.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand All @@ -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);
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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
Expand Down
29 changes: 29 additions & 0 deletions src/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
}