Fix ReactPerf.printOperations() crash

src/isomorphic/ReactDebugTool.js

@@ -90,6 +90,10 @@ function resetMeasurements() {
   }
 }
 
+function checkDebugID(debugID) {
+  warning(debugID, 'ReactDebugTool: debugID may not be empty.');
+}
+
 var ReactDebugTool = {
   addDevtool(devtool) {
     eventHandlers.push(devtool);
@@ -143,6 +147,7 @@ var ReactDebugTool = {
     emitEvent('onEndFlush');
   },
   onBeginLifeCycleTimer(debugID, timerType) {
+    checkDebugID(debugID);
     emitEvent('onBeginLifeCycleTimer', debugID, timerType);
     if (__DEV__) {
       if (isProfiling && currentFlushNesting > 0) {
@@ -162,6 +167,7 @@ var ReactDebugTool = {
     }
   },
   onEndLifeCycleTimer(debugID, timerType) {
+    checkDebugID(debugID);
     if (__DEV__) {
       if (isProfiling && currentFlushNesting > 0) {
         warning(
@@ -186,9 +192,11 @@ var ReactDebugTool = {
     emitEvent('onEndLifeCycleTimer', debugID, timerType);
   },
   onBeginReconcilerTimer(debugID, timerType) {
+    checkDebugID(debugID);
     emitEvent('onBeginReconcilerTimer', debugID, timerType);
   },
   onEndReconcilerTimer(debugID, timerType) {
+    checkDebugID(debugID);
     emitEvent('onEndReconcilerTimer', debugID, timerType);
   },
   onBeginProcessingChildContext() {
@@ -198,33 +206,42 @@ var ReactDebugTool = {
     emitEvent('onEndProcessingChildContext');
   },
   onNativeOperation(debugID, type, payload) {
+    checkDebugID(debugID);
     emitEvent('onNativeOperation', debugID, type, payload);
   },
   onSetState() {
     emitEvent('onSetState');
   },
   onSetDisplayName(debugID, displayName) {
+    checkDebugID(debugID);
     emitEvent('onSetDisplayName', debugID, displayName);
   },
   onSetChildren(debugID, childDebugIDs) {
+    checkDebugID(debugID);
     emitEvent('onSetChildren', debugID, childDebugIDs);
   },
   onSetOwner(debugID, ownerDebugID) {
+    checkDebugID(debugID);
     emitEvent('onSetOwner', debugID, ownerDebugID);
   },
   onSetText(debugID, text) {
+    checkDebugID(debugID);
     emitEvent('onSetText', debugID, text);
   },
   onMountRootComponent(debugID) {
+    checkDebugID(debugID);
     emitEvent('onMountRootComponent', debugID);
   },
   onMountComponent(debugID) {
+    checkDebugID(debugID);
     emitEvent('onMountComponent', debugID);
   },
   onUpdateComponent(debugID) {
+    checkDebugID(debugID);
     emitEvent('onUpdateComponent', debugID);
   },
   onUnmountComponent(debugID) {
+    checkDebugID(debugID);
     emitEvent('onUnmountComponent', debugID);
   },
   onTestEvent() {

src/isomorphic/__tests__/ReactPerf-test.js

@@ -67,6 +67,13 @@ describe('ReactPerf', function() {
     ReactPerf.start();
     fn();
     ReactPerf.stop();
+
+    // Make sure none of the methods crash.
+    ReactPerf.getWasted();
+    ReactPerf.getInclusive();
+    ReactPerf.getExclusive();
+    ReactPerf.getOperations();
+
     return ReactPerf.getLastMeasurements();
   }
 
@@ -208,6 +215,32 @@ describe('ReactPerf', function() {
     });
   });
 
+  it('should not count replacing null with a native as waste', function() {
+    var element = null;
+    function Foo() {
+      return element;
+    }
+    var container = document.createElement('div');
+    ReactDOM.render(<Foo />, container);
+    expectNoWaste(() => {
+      element = <div />;
+      ReactDOM.render(<Foo />, container);
+    });
+  });
+
+  it('should not count replacing a native with null as waste', function() {
+    var element = <div />;
+    function Foo() {
+      return element;
+    }
+    var container = document.createElement('div');
+    ReactDOM.render(<Foo />, container);
+    expectNoWaste(() => {
+      element = null;
+      ReactDOM.render(<Foo />, container);
+    });
+  });
+
   it('warns once when using getMeasurementsSummaryMap', function() {
     var measurements = measure(() => {});
     spyOn(console, 'error');

src/isomorphic/devtools/__tests__/ReactNativeOperationHistoryDevtool-test.js

@@ -65,19 +65,38 @@ describe('ReactNativeOperationHistoryDevtool', () => {
       }]);
     });
 
-    it('gets recorded for composite roots that return null', () => {
+    it('gets ignored for composite roots that return null', () => {
       function Foo() {
         return null;
       }
       var node = document.createElement('div');
       ReactDOM.render(<Foo />, node);
+
+      // Empty DOM components should be invisible to devtools.
+      assertHistoryMatches([]);
+    });
+
+    it('gets recorded when a native is mounted deeply instead of null', () => {
+      var element;
+      function Foo() {
+        return element;
+      }
+
+      var node = document.createElement('div');
+      element = null;
+      ReactDOM.render(<Foo />, node);
+
+      ReactNativeOperationHistoryDevtool.clearHistory();
+      element = <span />;
+      ReactDOM.render(<Foo />, node);
       var inst = ReactDOMComponentTree.getInstanceFromNode(node.firstChild);
+
+      // Since empty components should be invisible to devtools,
+      // we record a "mount" event rather than a "replace with".
       assertHistoryMatches([{
         instanceID: inst._debugID,
         type: 'mount',
-        payload: ReactDOMFeatureFlags.useCreateElement ?
-          '#comment' :
-          '<!-- react-empty: 1 -->',
+        payload: 'SPAN',
       }]);
     });
   });
@@ -500,6 +519,27 @@ describe('ReactNativeOperationHistoryDevtool', () => {
       }]);
     });
 
+    it('gets recorded when composite renders to null after a native', () => {
+      var element;
+      function Foo() {
+        return element;
+      }
+
+      var node = document.createElement('div');
+      element = <span />;
+      ReactDOM.render(<Foo />, node);
+      var inst = ReactDOMComponentTree.getInstanceFromNode(node.firstChild);
+
+      ReactNativeOperationHistoryDevtool.clearHistory();
+      element = null;
+      ReactDOM.render(<Foo />, node);
+      assertHistoryMatches([{
+        instanceID: inst._debugID,
+        type: 'replace with',
+        payload: '#comment',
+      }]);
+    });
+
     it('gets ignored if the type has not changed', () => {
       var element;
       function Foo() {

src/renderers/dom/client/ReactMount.js

@@ -701,11 +701,14 @@ var ReactMount = {
     }
 
     if (__DEV__) {
-      ReactInstrumentation.debugTool.onNativeOperation(
-        ReactDOMComponentTree.getInstanceFromNode(container.firstChild)._debugID,
-        'mount',
-        markup.toString()
-      );
+      var nativeNode = ReactDOMComponentTree.getInstanceFromNode(container.firstChild);
+      if (nativeNode._debugID !== 0) {
+        ReactInstrumentation.debugTool.onNativeOperation(
+          nativeNode._debugID,
+          'mount',
+          markup.toString()
+        );
+      }
     }
   },
 };

src/renderers/dom/client/__tests__/ReactDOMIDOperations-test.js

@@ -18,7 +18,7 @@ describe('ReactDOMIDOperations', function() {
 
   it('should update innerHTML and preserve whitespace', function() {
     var stubNode = document.createElement('div');
-    var stubInstance = {};
+    var stubInstance = {_debugID: 1};
     ReactDOMComponentTree.precacheNode(stubInstance, stubNode);
 
     var html = '\n  \t  <span>  \n  testContent  \t  </span>  \n  \t';

src/renderers/dom/client/utils/DOMChildrenOperations.js

@@ -135,11 +135,22 @@ var dangerouslyReplaceNodeWithMarkup = Danger.dangerouslyReplaceNodeWithMarkup;
 if (__DEV__) {
   dangerouslyReplaceNodeWithMarkup = function(oldChild, markup, prevInstance) {
     Danger.dangerouslyReplaceNodeWithMarkup(oldChild, markup);
-    ReactInstrumentation.debugTool.onNativeOperation(
-      prevInstance._debugID,
-      'replace with',
-      markup.toString()
-    );
+    if (prevInstance._debugID !== 0) {
+      ReactInstrumentation.debugTool.onNativeOperation(
+        prevInstance._debugID,
+        'replace with',
+        markup.toString()
+      );
+    } else {
+      var nextInstance = ReactDOMComponentTree.getInstanceFromNode(markup.node);
+      if (nextInstance._debugID !== 0) {
+        ReactInstrumentation.debugTool.onNativeOperation(
+          nextInstance._debugID,
+          'mount',
+          markup.toString()
+        );
+      }
+    }
   };
 }
 

src/renderers/dom/shared/__tests__/DOMPropertyOperations-test.js

@@ -174,10 +174,12 @@ describe('DOMPropertyOperations', function() {
 
   describe('setValueForProperty', function() {
     var stubNode;
+    var stubInstance;
 
     beforeEach(function() {
       stubNode = document.createElement('div');
-      ReactDOMComponentTree.precacheNode({}, stubNode);
+      stubInstance = {_debugID: 1};
+      ReactDOMComponentTree.precacheNode(stubInstance, stubNode);
     });
 
     it('should set values as properties by default', function() {
@@ -226,7 +228,7 @@ describe('DOMPropertyOperations', function() {
 
     it('should not remove empty attributes for special properties', function() {
       stubNode = document.createElement('input');
-      ReactDOMComponentTree.precacheNode({}, stubNode);
+      ReactDOMComponentTree.precacheNode(stubInstance, stubNode);
 
       DOMPropertyOperations.setValueForProperty(stubNode, 'value', '');
       // JSDOM does not behave correctly for attributes/properties
@@ -348,10 +350,12 @@ describe('DOMPropertyOperations', function() {
 
   describe('deleteValueForProperty', function() {
     var stubNode;
+    var stubInstance;
 
     beforeEach(function() {
       stubNode = document.createElement('div');
-      ReactDOMComponentTree.precacheNode({}, stubNode);
+      stubInstance = {_debugID: 1};
+      ReactDOMComponentTree.precacheNode(stubInstance, stubNode);
     });
 
     it('should remove attributes for normal properties', function() {
@@ -367,7 +371,7 @@ describe('DOMPropertyOperations', function() {
 
     it('should not remove attributes for special properties', function() {
       stubNode = document.createElement('input');
-      ReactDOMComponentTree.precacheNode({}, stubNode);
+      ReactDOMComponentTree.precacheNode(stubInstance, stubNode);
 
       stubNode.setAttribute('value', 'foo');
 
@@ -379,7 +383,7 @@ describe('DOMPropertyOperations', function() {
 
     it('should not leave all options selected when deleting multiple', function() {
       stubNode = document.createElement('select');
-      ReactDOMComponentTree.precacheNode({}, stubNode);
+      ReactDOMComponentTree.precacheNode(stubInstance, stubNode);
 
       stubNode.multiple = true;
       stubNode.appendChild(document.createElement('option'));

src/renderers/shared/stack/reconciler/instantiateReactComponent.js

@@ -139,11 +139,13 @@ function instantiateReactComponent(node) {
     var debugID = isEmpty ? 0 : nextDebugID++;
     instance._debugID = debugID;
 
-    var displayName = getDisplayName(instance);
-    ReactInstrumentation.debugTool.onSetDisplayName(debugID, displayName);
-    var owner = node && node._owner;
-    if (owner) {
-      ReactInstrumentation.debugTool.onSetOwner(debugID, owner._debugID);
+    if (debugID !== 0) {
+      var displayName = getDisplayName(instance);
+      ReactInstrumentation.debugTool.onSetDisplayName(debugID, displayName);
+      var owner = node && node._owner;
+      if (owner) {
+        ReactInstrumentation.debugTool.onSetOwner(debugID, owner._debugID);
+      }
     }
   }
 

src/test/ReactTestUtils.js

@@ -376,9 +376,12 @@ ReactShallowRenderer.prototype.getMountedInstance = function() {
   return this._instance ? this._instance._instance : null;
 };
 
+var nextDebugID = 1;
+
 var NoopInternalComponent = function(element) {
   this._renderedOutput = element;
   this._currentElement = element;
+  this._debugID = nextDebugID++;
 };
 
 NoopInternalComponent.prototype = {
@@ -404,6 +407,7 @@ NoopInternalComponent.prototype = {
 };
 
 var ShallowComponentWrapper = function(element) {
+  this._debugID = nextDebugID++;
   this.construct(element);
 };
 Object.assign(