From fbbcd20f65d1b61373d6625973a8885382816e82 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20T=C3=A9treault-Pinard?= Date: Fri, 5 Jul 2019 14:43:42 -0400 Subject: [PATCH 1/2] :hocho: unnecessary `0` arg in slice(0) calls --- src/plot_api/plot_api.js | 2 +- src/plots/command.js | 2 +- src/plots/plots.js | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/plot_api/plot_api.js b/src/plot_api/plot_api.js index b32dcb59748..05aaf6838cb 100644 --- a/src/plot_api/plot_api.js +++ b/src/plot_api/plot_api.js @@ -3620,7 +3620,7 @@ function deleteFrames(gd, frameList) { } } - frameList = frameList.slice(0); + frameList = frameList.slice(); frameList.sort(); for(i = frameList.length - 1; i >= 0; i--) { diff --git a/src/plots/command.js b/src/plots/command.js index b04897b4cb4..0987f81c065 100644 --- a/src/plots/command.js +++ b/src/plots/command.js @@ -372,7 +372,7 @@ function computeDataBindings(gd, args) { thisTraces[j] = traces ? traces[j] : j; } } else { - thisTraces = traces ? traces.slice(0) : null; + thisTraces = traces ? traces.slice() : null; } // Convert [7] to just 7 when traces is null: diff --git a/src/plots/plots.js b/src/plots/plots.js index 79633ff6cc8..705e8aa614d 100644 --- a/src/plots/plots.js +++ b/src/plots/plots.js @@ -2737,7 +2737,7 @@ plots.doCalcdata = function(gd, traces) { // XXX: Is this correct? Needs a closer look so that *some* traces can be recomputed without // *all* needing doCalcdata: var calcdata = new Array(fullData.length); - var oldCalcdata = (gd.calcdata || []).slice(0); + var oldCalcdata = (gd.calcdata || []).slice(); gd.calcdata = calcdata; // extra helper variables From f2a03beed0c4b9d3eefdb0619ac4d19f59586af3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20T=C3=A9treault-Pinard?= Date: Fri, 5 Jul 2019 14:48:19 -0400 Subject: [PATCH 2/2] fix #4002 - don't mutate array 'attr' .. as this can have erroneous repercussions e.g. when fullLayout.sliders[i].steps[j].args is linked to the same ref as in gd.layout. --- src/plots/command.js | 9 +++++-- test/jasmine/tests/command_test.js | 38 ++++++++++++++++++++++++++---- 2 files changed, 40 insertions(+), 7 deletions(-) diff --git a/src/plots/command.js b/src/plots/command.js index 0987f81c065..22e6257f8e7 100644 --- a/src/plots/command.js +++ b/src/plots/command.js @@ -360,9 +360,13 @@ function computeDataBindings(gd, args) { traces = null; } - crawl(aobj, function(path, attrName, attr) { + crawl(aobj, function(path, attrName, _attr) { var thisTraces; - if(Array.isArray(attr)) { + var attr; + + if(Array.isArray(_attr)) { + attr = _attr.slice(); + var nAttr = Math.min(attr.length, gd.data.length); if(traces) { nAttr = Math.min(nAttr, traces.length); @@ -372,6 +376,7 @@ function computeDataBindings(gd, args) { thisTraces[j] = traces ? traces[j] : j; } } else { + attr = _attr; thisTraces = traces ? traces.slice() : null; } diff --git a/test/jasmine/tests/command_test.js b/test/jasmine/tests/command_test.js index d860fe1f68d..c21bfadb4c5 100644 --- a/test/jasmine/tests/command_test.js +++ b/test/jasmine/tests/command_test.js @@ -240,28 +240,56 @@ describe('Plots.computeAPICommandBindings', function() { }); it('with an array value', function() { - var result = Plots.computeAPICommandBindings(gd, 'restyle', ['marker.size', [7], [1]]); + var value = [7]; + var traces = [1]; + var result = Plots.computeAPICommandBindings(gd, 'restyle', ['marker.size', value, traces]); expect(result).toEqual([{prop: 'marker.size', traces: [1], type: 'data', value: [7]}]); + expect(result[0].value).not.toBe(value, 'should not mutate value array'); + expect(result[0].traces).not.toBe(traces, 'should not mutate traces array'); }); it('with two array values and two traces specified', function() { - var result = Plots.computeAPICommandBindings(gd, 'restyle', ['marker.size', [7, 5], [0, 1]]); + var value = [7, 5]; + var traces = [0, 1]; + var result = Plots.computeAPICommandBindings(gd, 'restyle', ['marker.size', value, traces]); expect(result).toEqual([{prop: 'marker.size', traces: [0, 1], type: 'data', value: [7, 5]}]); + expect(result[0].value).not.toBe(value, 'should not mutate value array'); + expect(result[0].traces).not.toBe(traces, 'should not mutate traces array'); }); it('with traces specified in reverse order', function() { - var result = Plots.computeAPICommandBindings(gd, 'restyle', ['marker.size', [7, 5], [1, 0]]); + var value = [7, 5]; + var traces = [1, 0]; + var result = Plots.computeAPICommandBindings(gd, 'restyle', ['marker.size', value, traces]); expect(result).toEqual([{prop: 'marker.size', traces: [1, 0], type: 'data', value: [7, 5]}]); + expect(result[0].value).not.toBe(value, 'should not mutate value array'); + expect(result[0].traces).not.toBe(traces, 'should not mutate traces array'); }); it('with two values and a single trace specified', function() { - var result = Plots.computeAPICommandBindings(gd, 'restyle', ['marker.size', [7, 5], [0]]); + var value = [7, 5]; + var traces = [0]; + var result = Plots.computeAPICommandBindings(gd, 'restyle', ['marker.size', value, traces]); expect(result).toEqual([{prop: 'marker.size', traces: [0], type: 'data', value: [7]}]); + expect(result[0].value).not.toBe(value, 'should not mutate value array'); + expect(result[0].traces).not.toBe(traces, 'should not mutate traces array'); }); it('with two values and a different trace specified', function() { - var result = Plots.computeAPICommandBindings(gd, 'restyle', ['marker.size', [7, 5], [1]]); + var value = [7, 5]; + var traces = [1]; + var result = Plots.computeAPICommandBindings(gd, 'restyle', ['marker.size', value, traces]); expect(result).toEqual([{prop: 'marker.size', traces: [1], type: 'data', value: [7]}]); + expect(result[0].value).not.toBe(value, 'should not mutate value array'); + expect(result[0].traces).not.toBe(traces, 'should not mutate traces array'); + }); + + it('with two values and no trace specified', function() { + gd.data.length = 0; + var value = [false, true]; + var result = Plots.computeAPICommandBindings(gd, 'restyle', ['visible', value]); + expect(result).toEqual([{prop: 'visible', traces: [], type: 'data', value: []}]); + expect(result[0].value).not.toBe(value, 'should not mutate value array'); }); }); });