stop mutating user data to add uids

- Importantly, Plotly.react users probably were trashing these every time anyway
  resulting in bigger redraws than we needed. Now we're pulling them from
  the previous fullData if possible.
- Also cleanData is now SOLELY for backward compatibility

Author: alexcjohnson

src/lib/index.js

@@ -217,21 +217,24 @@ lib.simpleMap = function(array, func, x1, x2) {
     return out;
 };
 
-// random string generator
-lib.randstr = function randstr(existing, bits, base) {
-    /*
-     * Include number of bits, the base of the string you want
-     * and an optional array of existing strings to avoid.
-     */
+/**
+ * Random string generator
+ *
+ * @param {object} existing
+ *     pass in strings to avoid as keys with truthy values
+ * @param {int} bits
+ *     bits of information in the output string, default 24
+ * @param {int} base
+ *     base of string representation, default 16. Should be a power of 2.
+ */
+lib.randstr = function randstr(existing, bits, base, _recursion) {
     if(!base) base = 16;
     if(bits === undefined) bits = 24;
     if(bits <= 0) return '0';
 
-    var digits = Math.log(Math.pow(2, bits)) / Math.log(base),
-        res = '',
-        i,
-        b,
-        x;
+    var digits = Math.log(Math.pow(2, bits)) / Math.log(base);
+    var res = '';
+    var i, b, x;
 
     for(i = 2; digits === Infinity; i *= 2) {
         digits = Math.log(Math.pow(2, bits / i)) / Math.log(base) * i;
@@ -251,9 +254,13 @@ lib.randstr = function randstr(existing, bits, base) {
     }
 
     var parsed = parseInt(res, base);
-    if((existing && (existing.indexOf(res) > -1)) ||
+    if((existing && existing[res]) ||
          (parsed !== Infinity && parsed >= Math.pow(2, bits))) {
-        return randstr(existing, bits, base);
+        if(_recursion > 10) {
+            lib.warn('randstr failed uniqueness');
+            return res;
+        }
+        return randstr(existing, bits, base, (_recursion || 0) + 1);
     }
     else return res;
 };

src/lib/svg_text_utils.js

@@ -164,7 +164,7 @@ function cleanEscapesForTex(s) {
 }
 
 function texToSVG(_texString, _config, _callback) {
-    var randomID = 'math-output-' + Lib.randstr([], 64);
+    var randomID = 'math-output-' + Lib.randstr({}, 64);
     var tmpDiv = d3.select('body').append('div')
         .attr({id: randomID})
         .style({visibility: 'hidden', position: 'absolute'})

src/plot_api/helpers.js

@@ -197,42 +197,17 @@ function cleanAxRef(container, attr) {
 }
 
 /*
- * cleanData: Make a few changes to the data right away
- * before it gets used for anything
- * Mostly for backward compatibility, modifies the data traces users provide.
+ * cleanData: Make a few changes to the data for backward compatibility
+ * before it gets used for anything. Modifies the data traces users provide.
  *
  * Important: if you're going to add something here that modifies a data array,
  * update it in place so the new array === the old one.
  */
-exports.cleanData = function(data, existingData) {
-    // Enforce unique IDs
-    var suids = [], // seen uids --- so we can weed out incoming repeats
-        uids = data.concat(Array.isArray(existingData) ? existingData : [])
-               .filter(function(trace) { return 'uid' in trace; })
-               .map(function(trace) { return trace.uid; });
-
+exports.cleanData = function(data) {
     for(var tracei = 0; tracei < data.length; tracei++) {
         var trace = data[tracei];
         var i;
 
-        // assign uids to each trace and detect collisions.
-        if(!('uid' in trace) || suids.indexOf(trace.uid) !== -1) {
-            var newUid;
-
-            for(i = 0; i < 100; i++) {
-                newUid = Lib.randstr(uids);
-                if(suids.indexOf(newUid) === -1) break;
-            }
-            trace.uid = Lib.randstr(uids);
-            uids.push(trace.uid);
-        }
-        // keep track of already seen uids, so that if there are
-        // doubles we force the trace with a repeat uid to
-        // acquire a new one
-        suids.push(trace.uid);
-
-        // BACKWARD COMPATIBILITY FIXES
-
         // use xbins to bin data in x, and ybins to bin data in y
         if(trace.type === 'histogramy' && 'xbins' in trace && !('ybins' in trace)) {
             trace.ybins = trace.xbins;

src/plot_api/plot_api.js

@@ -123,7 +123,7 @@ exports.plot = function(gd, data, layout, config) {
     // if there is already data on the graph, append the new data
     // if you only want to redraw, pass a non-array for data
     if(Array.isArray(data)) {
-        helpers.cleanData(data, gd.data);
+        helpers.cleanData(data);
 
         if(graphWasEmpty) gd.data = data;
         else gd.data.push.apply(gd.data, data);
@@ -559,7 +559,7 @@ exports.redraw = function(gd) {
         throw new Error('This element is not a Plotly plot: ' + gd);
     }
 
-    helpers.cleanData(gd.data, gd.data);
+    helpers.cleanData(gd.data);
     helpers.cleanLayout(gd.layout);
 
     gd.calcdata = undefined;
@@ -1060,7 +1060,7 @@ exports.addTraces = function addTraces(gd, traces, newIndices) {
         return Lib.extendFlat({}, trace);
     });
 
-    helpers.cleanData(traces, gd.data);
+    helpers.cleanData(traces);
 
     // add the traces to gd.data (no redrawing yet!)
     for(i = 0; i < traces.length; i++) {
@@ -2259,7 +2259,7 @@ exports.react = function(gd, data, layout, config) {
         }
 
         gd.data = data || [];
-        helpers.cleanData(gd.data, []);
+        helpers.cleanData(gd.data);
         gd.layout = layout || {};
         helpers.cleanLayout(gd.layout);
 
@@ -3257,9 +3257,9 @@ function makePlotFramework(gd) {
         .classed('main-svg', true);
 
     if(!fullLayout._uid) {
-        var otherUids = [];
+        var otherUids = {};
         d3.selectAll('defs').each(function() {
-            if(this.id) otherUids.push(this.id.split('-')[1]);
+            if(this.id) otherUids[this.id.split('-')[1]] = 1;
         });
         fullLayout._uid = Lib.randstr(otherUids);
     }

src/plots/attributes.js

@@ -73,8 +73,7 @@ module.exports = {
     uid: {
         valType: 'string',
         role: 'info',
-        dflt: '',
-        editType: 'calc'
+        editType: 'plot'
     },
     ids: {
         valType: 'data_array',

src/plots/plots.js

@@ -389,6 +389,9 @@ plots.supplyDefaults = function(gd, opts) {
     // eg set `_requestRangeslider.x2 = true` for xaxis2
     newFullLayout._requestRangeslider = {};
 
+    // pull uids from old data to use as new defaults
+    newFullLayout._traceUids = getTraceUids(oldFullData, newData);
+
     // then do the data
     newFullLayout._globalTransforms = (gd._context || {}).globalTransforms;
     plots.supplyDataDefaults(newData, newFullData, newLayout, newFullLayout);
@@ -499,6 +502,46 @@ plots.supplyDefaultsUpdateCalc = function(oldCalcdata, newFullData) {
     }
 };
 
+/**
+ * Create a list of uid strings satisfying (in this order of importance):
+ * 1. all unique, all strings
+ * 2. matches input uids if provided
+ * 3. matches previous data uids
+ */
+function getTraceUids(oldFullData, newData) {
+    var len = newData.length;
+    var oldFullInput = [];
+    var i, prevFullInput;
+    for(i = 0; i < oldFullData.length; i++) {
+        var thisFullInput = oldFullData[i]._fullInput;
+        if(thisFullInput !== prevFullInput) oldFullInput.push(thisFullInput);
+        prevFullInput = thisFullInput;
+    }
+    var oldLen = oldFullInput.length;
+    var out = new Array(len);
+    var seenUids = {};
+
+    function setUid(uid, i) {
+        out[i] = uid;
+        seenUids[uid] = 1;
+    }
+
+    function tryUid(uid, i) {
+        if(uid && typeof uid === 'string' && !seenUids[uid]) {
+            setUid(uid, i);
+            return true;
+        }
+    }
+
+    for(i = 0; i < len; i++) {
+        if(tryUid(newData[i].uid, i)) continue;
+        if(i < oldLen && tryUid(oldFullInput[i].uid, i)) continue;
+        setUid(Lib.randstr(seenUids), i);
+    }
+
+    return out;
+}
+
 /**
  * Make a container for collecting subplots we need to display.
  *
@@ -888,6 +931,8 @@ plots.supplyDataDefaults = function(dataIn, dataOut, layout, fullLayout) {
         trace = dataIn[i];
         fullTrace = plots.supplyTraceDefaults(trace, colorCnt, fullLayout, i);
 
+        fullTrace.uid = fullLayout._traceUids[i];
+
         fullTrace.index = i;
         fullTrace._input = trace;
         fullTrace._expandedIndex = cnt;
@@ -903,9 +948,9 @@ plots.supplyDataDefaults = function(dataIn, dataOut, layout, fullLayout) {
                 // that transform supply-default methods can set _ keys for future use.
                 relinkPrivateKeys(fullExpandedTrace, expandedTrace);
 
-                // mutate uid here using parent uid and expanded index
+                // set uid using parent uid and expanded index
                 // to promote consistency between update calls
-                expandedTrace.uid = fullExpandedTrace.uid = fullTrace.uid + j;
+                fullExpandedTrace.uid = fullTrace.uid + j;
 
                 // add info about parent data trace
                 fullExpandedTrace.index = i;
@@ -1045,7 +1090,6 @@ plots.supplyTraceDefaults = function(traceIn, colorIndex, layout, traceInIndex)
     var visible = coerce('visible');
 
     coerce('type');
-    coerce('uid');
     coerce('name', layout._traceWord + ' ' + traceInIndex);
 
     // we want even invisible traces to make their would-be subplots visible

test/jasmine/tests/gl2d_click_test.js

@@ -124,7 +124,6 @@ describe('@gl @flaky Test hover and click interactions', function() {
             'data', 'fullData', 'xaxis', 'yaxis'
         ]), 'event data keys');
 
-        expect(typeof pt.data.uid).toBe('string', msg + ' - uid');
         expect(pt.xaxis.domain.length).toBe(2, msg + ' - xaxis');
         expect(pt.yaxis.domain.length).toBe(2, msg + ' - yaxis');
 
@@ -149,7 +148,7 @@ describe('@gl @flaky Test hover and click interactions', function() {
             return delay(100)()
                 .then(_hover)
                 .then(function(eventData) {
-                    assertEventData(eventData, expected);
+                    assertEventData(eventData, expected, opts.msg);
 
                     var g = d3.select('g.hovertext');
                     if(g.node() === null) {

test/jasmine/tests/gl2d_pointcloud_test.js

@@ -138,12 +138,6 @@ var plotData = {
     }
 };
 
-function makePlot(gd, mock, done) {
-    return Plotly.plot(gd, mock.data, mock.layout)
-        .then(null, failTest)
-        .then(done);
-}
-
 describe('@gl pointcloud traces', function() {
 
     var gd;
@@ -157,8 +151,10 @@ describe('@gl pointcloud traces', function() {
         destroyGraphDiv();
     });
 
-    it('render without raising an error', function(done) {
-        makePlot(gd, plotData, done);
+    it('renders without raising an error', function(done) {
+        Plotly.plot(gd, plotData)
+        .catch(failTest)
+        .then(done);
     });
 
     it('should update properly', function(done) {
@@ -171,10 +167,11 @@ describe('@gl pointcloud traces', function() {
         var yBaselineMins = [{val: 0, pad: 50, extrapad: false}];
         var yBaselineMaxes = [{val: 9, pad: 50, extrapad: false}];
 
-        Plotly.plot(gd, mock.data, mock.layout).then(function() {
+        Plotly.plot(gd, mock)
+        .then(function() {
             scene2d = gd._fullLayout._plots.xy._scene2d;
 
-            expect(scene2d.traces[mock.data[0].uid].type).toEqual('pointcloud');
+            expect(scene2d.traces[gd._fullData[0].uid].type).toBe('pointcloud');
 
             expect(scene2d.xaxis._min).toEqual(xBaselineMins);
             expect(scene2d.xaxis._max).toEqual(xBaselineMaxes);
@@ -204,8 +201,8 @@ describe('@gl pointcloud traces', function() {
         }).then(function() {
             expect(scene2d.yaxis._min).toEqual(yBaselineMins);
             expect(scene2d.yaxis._max).toEqual(yBaselineMaxes);
-
-            done();
-        });
+        })
+        .catch(failTest)
+        .then(done);
     });
 });

test/jasmine/tests/plot_api_test.js

@@ -1497,37 +1497,30 @@ describe('Test plot api', function() {
         it('should work when newIndices is undefined', function() {
             Plotly.addTraces(gd, [{'name': 'c'}, {'name': 'd'}]);
             expect(gd.data[2].name).toBeDefined();
-            expect(gd.data[2].uid).toBeDefined();
             expect(gd.data[3].name).toBeDefined();
-            expect(gd.data[3].uid).toBeDefined();
             expect(plotApi.redraw).toHaveBeenCalled();
             expect(plotApi.moveTraces).not.toHaveBeenCalled();
         });
 
         it('should work when newIndices is defined', function() {
             Plotly.addTraces(gd, [{'name': 'c'}, {'name': 'd'}], [1, 3]);
             expect(gd.data[2].name).toBeDefined();
-            expect(gd.data[2].uid).toBeDefined();
             expect(gd.data[3].name).toBeDefined();
-            expect(gd.data[3].uid).toBeDefined();
             expect(plotApi.redraw).not.toHaveBeenCalled();
             expect(plotApi.moveTraces).toHaveBeenCalledWith(gd, [-2, -1], [1, 3]);
         });
 
         it('should work when newIndices has negative indices', function() {
             Plotly.addTraces(gd, [{'name': 'c'}, {'name': 'd'}], [-3, -1]);
             expect(gd.data[2].name).toBeDefined();
-            expect(gd.data[2].uid).toBeDefined();
             expect(gd.data[3].name).toBeDefined();
-            expect(gd.data[3].uid).toBeDefined();
             expect(plotApi.redraw).not.toHaveBeenCalled();
             expect(plotApi.moveTraces).toHaveBeenCalledWith(gd, [-2, -1], [-3, -1]);
         });
 
         it('should work when newIndices is an integer', function() {
             Plotly.addTraces(gd, {'name': 'c'}, 0);
             expect(gd.data[2].name).toBeDefined();
-            expect(gd.data[2].uid).toBeDefined();
             expect(plotApi.redraw).not.toHaveBeenCalled();
             expect(plotApi.moveTraces).toHaveBeenCalledWith(gd, [-1], [0]);
         });

test/jasmine/tests/plots_test.js

@@ -37,6 +37,7 @@ describe('Test Plots', function() {
                 type: 'contour',
                 _empties: [1, 2, 3]
             }];
+            oldFullData.forEach(function(trace) { trace._fullInput = trace; });
 
             var oldFullLayout = {
                 _plots: { xy: { plot: {} } },