Merge pull request #2805 from plotly/transform-removal-from-react-fix

Transform removal from react fix

Author: etpinard

src/plots/plots.js

@@ -2515,6 +2515,8 @@ plots.doCalcdata = function(gd, traces) {
 
         if(trace.visible === true) {
 
+            // clear existing ref in case it got relinked
+            delete trace._indexToPoints;
             // keep ref of index-to-points map object of the *last* enabled transform,
             // this index-to-points map object is required to determine the calcdata indices
             // that correspond to input indices (e.g. from 'selectedpoints')

test/jasmine/tests/transform_filter_test.js

@@ -1274,4 +1274,27 @@ describe('filter transforms interactions', function() {
         .then(done);
     });
 
+    it('should clear indexToPoints on removal', function(done) {
+        var gd = createGraphDiv();
+
+        Plotly.react(gd, [{
+            y: [1, 2, 3, 1, 2, 3],
+            transforms: [{
+                type: 'filter',
+                target: 'y',
+                operation: '<',
+                value: 3
+            }]
+        }])
+        .then(function() {
+            expect(gd._fullData[0]._indexToPoints).toEqual({0: [0], 1: [1], 2: [3], 3: [4]});
+            return Plotly.react(gd, [{ y: [1, 2, 3, 1, 2, 3] }]);
+        })
+        .then(function() {
+            expect(gd._fullData[0]._indexToPoints).toBeUndefined();
+        })
+        .catch(failTest)
+        .then(done);
+    });
+
 });

test/jasmine/tests/transform_multi_test.js

@@ -6,6 +6,7 @@ var Lib = require('@src/lib');
 
 var createGraphDiv = require('../assets/create_graph_div');
 var destroyGraphDiv = require('../assets/destroy_graph_div');
+var failTest = require('../assets/fail_test');
 var customAssertions = require('../assets/custom_assertions');
 var supplyAllDefaults = require('../assets/supply_defaults');
 
@@ -434,9 +435,9 @@ describe('multiple transforms:', function() {
             expect(gd._fullData[0].transforms[0]._indexToPoints).toEqual({0: [1], 1: [3], 2: [4]});
             expect(gd._fullData[0].transforms[1]._indexToPoints).toEqual({0: [1, 3], 1: [4]});
             expect(gd._fullData[0].transforms[2]._indexToPoints).toEqual({0: [4]});
-
-            done();
-        });
+        })
+        .catch(failTest)
+        .then(done);
     });
 
 
@@ -455,9 +456,9 @@ describe('multiple transforms:', function() {
             expect(gd._fullData[1].y).toEqual([2, 3]);
 
             assertDims([2, 2]);
-
-            done();
-        });
+        })
+        .catch(failTest)
+        .then(done);
     });
 
     it('Plotly.plot should plot the transform traces (reverse case)', function(done) {
@@ -477,9 +478,9 @@ describe('multiple transforms:', function() {
             expect(gd._fullData[1].y).toEqual([2, 3]);
 
             assertDims([2, 2]);
-
-            done();
-        });
+        })
+        .catch(failTest)
+        .then(done);
     });
 
     it('Plotly.restyle should work', function(done) {
@@ -529,9 +530,9 @@ describe('multiple transforms:', function() {
                 ['rgb(0, 128, 0)', 'rgb(255, 0, 0)'],
                 [0.4, 0.4]
             );
-
-            done();
-        });
+        })
+        .catch(failTest)
+        .then(done);
     });
 
     it('Plotly.extendTraces should work', function(done) {
@@ -573,9 +574,9 @@ describe('multiple transforms:', function() {
             return Plotly.deleteTraces(gd, [0]);
         }).then(function() {
             assertDims([]);
-
-            done();
-        });
+        })
+        .catch(failTest)
+        .then(done);
     });
 
     it('toggling trace visibility should work', function(done) {
@@ -595,12 +596,12 @@ describe('multiple transforms:', function() {
             return Plotly.restyle(gd, 'visible', [true, true]);
         }).then(function() {
             assertDims([2, 2, 2, 2]);
-
-            done();
-        });
+        })
+        .catch(failTest)
+        .then(done);
     });
 
-    it('executes filter and aggregate in the order given', function() {
+    it('executes filter and aggregate in the order given', function(done) {
         // filter and aggregate do not commute!
 
         var trace1 = {
@@ -624,18 +625,21 @@ describe('multiple transforms:', function() {
         var trace2 = Lib.extendDeep({}, trace1);
         trace2.transforms.reverse();
 
-        Plotly.newPlot(gd, [trace1, trace2]);
+        Plotly.newPlot(gd, [trace1, trace2]).then(function() {
+            var trace1Out = gd._fullData[0];
+            expect(trace1Out.x).toEqual([2]);
+            expect(trace1Out.y).toEqual([5]);
 
-        var trace1Out = gd._fullData[0];
-        expect(trace1Out.x).toEqual([2]);
-        expect(trace1Out.y).toEqual([5]);
+            var trace2Out = gd._fullData[1];
+            expect(trace2Out.x).toEqual([4, -5]);
+            expect(trace2Out.y).toEqual([5, 4]);
 
-        var trace2Out = gd._fullData[1];
-        expect(trace2Out.x).toEqual([4, -5]);
-        expect(trace2Out.y).toEqual([5, 4]);
+        })
+        .catch(failTest)
+        .then(done);
     });
 
-    it('always executes groupby before aggregate', function() {
+    it('always executes groupby before aggregate', function(done) {
         // aggregate and groupby wouldn't commute, but groupby always happens first
         // because it has a `transform`, and aggregate has a `calcTransform`
 
@@ -658,29 +662,31 @@ describe('multiple transforms:', function() {
         var trace2 = Lib.extendDeep({}, trace1);
         trace2.transforms.reverse();
 
-        Plotly.newPlot(gd, [trace1, trace2]);
-
-        var t1g1 = gd._fullData[0];
-        var t1g2 = gd._fullData[1];
-        var t2g1 = gd._fullData[2];
-        var t2g2 = gd._fullData[3];
-
-        expect(t1g1.x).toEqual([1, 2]);
-        expect(t1g1.y).toEqual([2, 4]);
-        // group 2 has its aggregations switched, since group 2 comes first
-        expect(t1g2.x).toEqual([3, 9]);
-        expect(t1g2.y).toEqual([6, 9]);
-
-        // if we had done aggregation first, we'd implicitly get the first val
-        // for each of the groupby groups, which is [1, 1]
-        // so we'd only make 1 output trace, and it would look like:
-        // {x: [10, 5], y: [20/3, 5]}
-        // (and if we got some other groupby groups values, the most it could do
-        // is break ^^ into two separate traces)
-        expect(t2g1.x).toEqual(t1g1.x);
-        expect(t2g1.y).toEqual(t1g1.y);
-        expect(t2g2.x).toEqual(t1g2.x);
-        expect(t2g2.y).toEqual(t1g2.y);
+        Plotly.newPlot(gd, [trace1, trace2]).then(function() {
+            var t1g1 = gd._fullData[0];
+            var t1g2 = gd._fullData[1];
+            var t2g1 = gd._fullData[2];
+            var t2g2 = gd._fullData[3];
+
+            expect(t1g1.x).toEqual([1, 2]);
+            expect(t1g1.y).toEqual([2, 4]);
+            // group 2 has its aggregations switched, since group 2 comes first
+            expect(t1g2.x).toEqual([3, 9]);
+            expect(t1g2.y).toEqual([6, 9]);
+
+            // if we had done aggregation first, we'd implicitly get the first val
+            // for each of the groupby groups, which is [1, 1]
+            // so we'd only make 1 output trace, and it would look like:
+            // {x: [10, 5], y: [20/3, 5]}
+            // (and if we got some other groupby groups values, the most it could do
+            // is break ^^ into two separate traces)
+            expect(t2g1.x).toEqual(t1g1.x);
+            expect(t2g1.y).toEqual(t1g1.y);
+            expect(t2g2.x).toEqual(t1g2.x);
+            expect(t2g2.y).toEqual(t1g2.y);
+        })
+        .catch(failTest)
+        .then(done);
     });
 });
 
@@ -699,8 +705,9 @@ describe('invalid transforms', function() {
             transforms: [{}]
         }]).then(function() {
             expect(gd._fullData[0].transforms.length).toEqual(1);
-            done();
-        });
+        })
+        .catch(failTest)
+        .then(done);
     });
 });
 
@@ -761,9 +768,9 @@ describe('multiple traces with transforms:', function() {
             expect(gd._fullData[2].y).toEqual([3, 5, 2]);
 
             assertDims([2, 3, 3]);
-
-            done();
-        });
+        })
+        .catch(failTest)
+        .then(done);
     });
 
     it('Plotly.restyle should work', function(done) {
@@ -816,9 +823,9 @@ describe('multiple traces with transforms:', function() {
                 ['rgb(0, 128, 0)', 'rgb(0, 128, 0)', 'rgb(255, 0, 0)'],
                 [0.4, 0.6, 0.6]
             );
-
-            done();
-        });
+        })
+        .catch(failTest)
+        .then(done);
     });
 
     it('Plotly.extendTraces should work', function(done) {
@@ -843,9 +850,9 @@ describe('multiple traces with transforms:', function() {
             }, [0]);
         }).then(function() {
             assertDims([5, 4, 4]);
-
-            done();
-        });
+        })
+        .catch(failTest)
+        .then(done);
     });
 
     it('Plotly.deleteTraces should work', function(done) {
@@ -863,9 +870,9 @@ describe('multiple traces with transforms:', function() {
             return Plotly.deleteTraces(gd, [0]);
         }).then(function() {
             assertDims([]);
-
-            done();
-        });
+        })
+        .catch(failTest)
+        .then(done);
     });
 
     it('toggling trace visibility should work', function(done) {
@@ -891,9 +898,10 @@ describe('multiple traces with transforms:', function() {
             return Plotly.restyle(gd, 'visible', 'legendonly', [0]);
         }).then(function() {
             assertDims([3, 3]);
+        })
+        .catch(failTest)
+        .then(done);
 
-            done();
-        });
     });
 });
 
@@ -959,6 +967,7 @@ describe('restyle applied on transforms:', function() {
             expect(gd.data[0].transforms).toBeUndefined(msg);
             expect(gd._fullData[0].y).toEqual([2, 1, 2], msg);
         })
+        .catch(failTest)
         .then(done);
     });