Merge pull request #4023 from plotly/command-observer-mutate-fix

Command observer (bad) mutation fix

Author: etpinard

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--) {

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,7 +376,8 @@ function computeDataBindings(gd, args) {
                 thisTraces[j] = traces ? traces[j] : j;
             }
         } else {
-            thisTraces = traces ? traces.slice(0) : null;
+            attr = _attr;
+            thisTraces = traces ? traces.slice() : null;
         }
 
         // Convert [7] to just 7 when traces is null:

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

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');
                 });
             });
         });