Skip to content

Commit

Permalink
don't relink customdata, ids, or any matching objects
Browse files Browse the repository at this point in the history
  • Loading branch information
alexcjohnson committed Feb 16, 2018
1 parent 8a7ed36 commit 95d540e
Show file tree
Hide file tree
Showing 2 changed files with 112 additions and 1 deletion.
10 changes: 9 additions & 1 deletion src/lib/relink_private.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,9 @@ module.exports = function relinkPrivateKeys(toContainer, fromContainer) {
fromVal = fromContainer[k],
toVal = toContainer[k];

if(toVal === fromVal) {
continue;
}
if(k.charAt(0) === '_' || typeof fromVal === 'function') {

// if it already exists at this point, it's something
Expand All @@ -37,9 +40,14 @@ module.exports = function relinkPrivateKeys(toContainer, fromContainer) {
}
else if(isArray(fromVal) && isArray(toVal) && isPlainObject(fromVal[0])) {

// filter out data_array items that can contain user objects
// most of the time the toVal === fromVal check will catch these early
// but if the user makes new ones we also don't want to recurse in.
if(k === 'customdata' || k === 'ids') continue;

// recurse into arrays containers
for(var j = 0; j < fromVal.length; j++) {
if(isPlainObject(fromVal[j]) && isPlainObject(toVal[j])) {
if(isPlainObject(fromVal[j]) && isPlainObject(toVal[j]) && (toVal[j] !== fromVal[j])) {

This comment has been minimized.

Copy link
@brian428

brian428 Feb 16, 2018

@alexcjohnson One small suggestion: do the inequality check first? (I'm assuming the inequality check is faster than the isPlainObject() calls.)

This comment has been minimized.

Copy link
@alexcjohnson

alexcjohnson Feb 16, 2018

Author Collaborator

Correct, !== is faster, though in fact this one is very unlikely to fail now that we've filtered out customdata, given the way we construct fullData and fullLayout. Actually even the isPlainObject test becomes very unlikely to fail (though unfortunately still required) if I restrict us to the minimum length of the two arrays, and after doing that sure I might as well swap the order of tests.

relinkPrivateKeys(toVal[j], fromVal[j]);
}
}
Expand Down
103 changes: 103 additions & 0 deletions test/jasmine/tests/lib_test.js
Original file line number Diff line number Diff line change
Expand Up @@ -1984,6 +1984,109 @@ describe('Test lib.js:', function() {
});
});
});

describe('relinkPrivateKeys', function() {
it('ignores customdata and ids', function() {
var fromContainer = {
customdata: [{_x: 1, _y: 2, a: 3}],
ids: [{_i: 4, j: 5}]
};
var toContainer = {
customdata: [{a: 6}],
ids: [{j: 7}]
};

Lib.relinkPrivateKeys(toContainer, fromContainer);

expect(toContainer.customdata[0]._x).toBeUndefined();
expect(toContainer.customdata[0]._y).toBeUndefined();
expect(toContainer.ids[0]._i).toBeUndefined();
});

it('ignores any values that are ===', function() {
var accesses = 0;

var obj = {
get _x() { accesses++; return 1; },
set _x(v) { accesses++; }
};
var array = [obj];
var array2 = [obj];

var fromContainer = {
x: array,
y: array,
o: obj
};
var toContainer = {
x: array,
y: array2,
o: obj
};

Lib.relinkPrivateKeys(toContainer, fromContainer);

expect(accesses).toBe(0);

obj._x = 2;
expect(obj._x).toBe(1);
expect(accesses).toBe(2);
});

it('reinserts other private keys if they\'re not already there', function() {
var obj1 = {a: 10, _a: 11};
var obj2 = {a: 12, _a: 13};
function f1() { return 1; }
function f2() { return 2; }

var fromContainer = {
a: 1,
_a: 2,
_b: 3,
_c: obj1,
_d: obj1,
f: f1, // functions are private even without _
g: f1,
array: [{a: 3, _a: 4, _b: 5, f: f1, g: f1}],
o: {a: 6, _a: 7, _b: 8},
array2: [{a: 9, _a: 10}],
o2: {a: 11, _a: 12}
};
fromContainer._circular = fromContainer;
fromContainer._circular2 = fromContainer;
var toContainer = {
a: 21,
_a: 22,
_c: obj2,
f: f2,
array: [{a: 23, _a: 24, f: f2}],
o: {a: 26, _a: 27},
x: [28],
_x: 29
};
toContainer._circular = toContainer;

Lib.relinkPrivateKeys(toContainer, fromContainer);

var expected = {
a: 21,
_a: 22,
_b: 3,
_c: obj2,
_circular: toContainer,
_circular2: fromContainer,
_d: obj1,
f: f2,
g: f1,
array: [{a: 23, _a: 24, _b: 5, f: f2, g: f1}],
o: {a: 26, _a: 27, _b: 8},
x: [28],
_x: 29
};

expect(toContainer).toEqual(expected);
});
});
});

describe('Queue', function() {
Expand Down

0 comments on commit 95d540e

Please sign in to comment.