Skip to content

Commit 37e291b

Browse files
committed
plots: make relinkPrivateKeys less strict
- remove error when array container are not of the same length - this is a temporary fix until #749 is addressed.
1 parent ccbcb58 commit 37e291b

File tree

2 files changed

+87
-29
lines changed

2 files changed

+87
-29
lines changed

src/plots/plots.js

+30-29
Original file line numberDiff line numberDiff line change
@@ -585,45 +585,46 @@ plots.cleanPlot = function(newFullData, newFullLayout, oldFullData, oldFullLayou
585585
};
586586

587587
/**
588-
* Relink private _keys and keys with a function value from one layout
589-
* (usually cached) to the new fullLayout.
590-
* relink means copying if object is pass-by-value and adding a reference
591-
* if object is pass-by-ref. This prevents deepCopying massive structures like
592-
* a webgl context.
588+
* Relink private _keys and keys with a function value from one container
589+
* to the new container.
590+
* Relink means copying if object is pass-by-value and adding a reference
591+
* if object is pass-by-ref.
592+
* This prevents deepCopying massive structures like a webgl context.
593593
*/
594-
function relinkPrivateKeys(toLayout, fromLayout) {
595-
var keys = Object.keys(fromLayout);
596-
var j;
594+
function relinkPrivateKeys(toContainer, fromContainer) {
595+
var isPlainObject = Lib.isPlainObject,
596+
isArray = Array.isArray;
597+
598+
var keys = Object.keys(fromContainer);
599+
600+
for(var i = 0; i < keys.length; i++) {
601+
var k = keys[i],
602+
fromVal = fromContainer[k],
603+
toVal = toContainer[k];
604+
605+
if(k.charAt(0) === '_' || typeof fromVal === 'function') {
597606

598-
for(var i = 0; i < keys.length; ++i) {
599-
var k = keys[i];
600-
if(k.charAt(0) === '_' || typeof fromLayout[k] === 'function') {
601607
// if it already exists at this point, it's something
602608
// that we recreate each time around, so ignore it
603-
if(k in toLayout) continue;
609+
if(k in toContainer) continue;
604610

605-
toLayout[k] = fromLayout[k];
611+
toContainer[k] = fromVal;
606612
}
607-
else if(Array.isArray(fromLayout[k]) &&
608-
Array.isArray(toLayout[k]) &&
609-
fromLayout[k].length &&
610-
Lib.isPlainObject(fromLayout[k][0])) {
611-
if(fromLayout[k].length !== toLayout[k].length) {
612-
// this should be handled elsewhere, it causes
613-
// ambiguity if we try to deal with it here.
614-
throw new Error('relinkPrivateKeys needs equal ' +
615-
'length arrays');
616-
}
613+
else if(isArray(fromVal) && isArray(toVal)) {
617614

618-
for(j = 0; j < fromLayout[k].length; j++) {
619-
relinkPrivateKeys(toLayout[k][j], fromLayout[k][j]);
615+
// recurse into arrays
616+
for(var j = 0; j < fromVal.length; j++) {
617+
if(isPlainObject(fromVal[j]) && isPlainObject(toVal[j])) {
618+
relinkPrivateKeys(toVal[j], fromVal[j]);
619+
}
620620
}
621621
}
622-
else if(Lib.isPlainObject(fromLayout[k]) &&
623-
Lib.isPlainObject(toLayout[k])) {
622+
else if(isPlainObject(fromVal) && isPlainObject(toVal)) {
623+
624624
// recurse into objects, but only if they still exist
625-
relinkPrivateKeys(toLayout[k], fromLayout[k]);
626-
if(!Object.keys(toLayout[k]).length) delete toLayout[k];
625+
relinkPrivateKeys(toVal, fromVal);
626+
627+
if(!Object.keys(toVal).length) delete toContainer[k];
627628
}
628629
}
629630
}

test/jasmine/tests/plots_test.js

+57
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,63 @@ var destroyGraphDiv = require('../assets/destroy_graph_div');
77
describe('Test Plots', function() {
88
'use strict';
99

10+
describe('Plots.supplyDefaults', function() {
11+
12+
var gd;
13+
14+
it('should relink private keys', function() {
15+
var oldFullData = [{
16+
type: 'scatter3d',
17+
z: [1, 2, 3]
18+
}, {
19+
type: 'contour',
20+
_empties: [1, 2, 3]
21+
}];
22+
23+
var oldFullLayout = {
24+
_plots: { xy: {} },
25+
xaxis: { c2p: function() {} },
26+
yaxis: { _m: 20 },
27+
scene: { _scene: {} },
28+
annotations: [{ _min: 10, }, { _max: 20 }],
29+
someFunc: function() {}
30+
};
31+
32+
var newData = [{
33+
type: 'scatter3d',
34+
z: [1, 2, 3, 4]
35+
}, {
36+
type: 'contour',
37+
z: [[1, 2, 3], [2, 3, 4]]
38+
}];
39+
40+
var newLayout = {
41+
annotations: [{}, {}, {}]
42+
};
43+
44+
gd = {
45+
_fullData: oldFullData,
46+
_fullLayout: oldFullLayout,
47+
data: newData,
48+
layout: newLayout
49+
};
50+
51+
Plots.supplyDefaults(gd);
52+
53+
expect(gd._fullData[1]._empties).toBe(oldFullData[1]._empties);
54+
expect(gd._fullLayout.scene._scene).toBe(oldFullLayout.scene._scene);
55+
expect(gd._fullLayout._plots).toBe(oldFullLayout._plots);
56+
expect(gd._fullLayout.annotations[0]._min).toBe(oldFullLayout.annotations[0]._min);
57+
expect(gd._fullLayout.annotations[1]._max).toBe(oldFullLayout.annotations[1]._max);
58+
expect(gd._fullLayout.someFunc).toBe(oldFullLayout.someFunc);
59+
60+
expect(gd._fullLayout.xaxis.c2p)
61+
.not.toBe(oldFullLayout.xaxis.c2p, '(set during ax.setScale');
62+
expect(gd._fullLayout.yaxis._m)
63+
.not.toBe(oldFullLayout.yaxis._m, '(set during ax.setScale');
64+
});
65+
});
66+
1067
describe('Plots.supplyLayoutGlobalDefaults should', function() {
1168
var layoutIn,
1269
layoutOut,

0 commit comments

Comments
 (0)