Consolidate hook events (#7472)

* Remove onBeforeMountComponent hook event It is unnecessary. We now pass the element as part of onInstantiateComponent, and it can't change before mounting. * Remove onComponentHasMounted hook event It is unused after #7410. * Replace on(Begin|End)ReconcilerTimer hook events We already have onBeforeUpdateComponent. Let's just have on(Before?)(Mount|Update|Unmount)Component and stick with them. This removes double event dispatches in some hot spots. * Remove onComponentHasUpdated hook The tests still pass so presumably it was not necessary. * Add missing __DEV__ to TestUtils code * Replace on(InstantiateComponent|SetParent) with onBeforeMountComponent This lets us further consolidate hooks. The parent ID is now passed as an argument to onBeforeMountComponent() with the element. * Remove onMountRootComponent hook event It is unnecessary now that we pass the parent ID to onBeforeMountComponent. * Use parentDebugID = 0 both for roots and production This removes some awkward branching.
facebook · Aug 11, 2016 · 0e976e1 · 0e976e1
1 parent f7e0db9
commit 0e976e1
Show file tree

Hide file tree

Showing 15 changed files with 84 additions and 190 deletions.
diff --git a/src/isomorphic/hooks/ReactComponentTreeHook.js b/src/isomorphic/hooks/ReactComponentTreeHook.js
@@ -39,11 +39,11 @@ function remove(id) {
   delete itemByKey[key];
 }
 
-function create(id, element) {
+function create(id, element, parentID) {
   var key = getKeyFromID(id);
   itemByKey[key] = {
     element,
-    parentID: null,
+    parentID,
     text: null,
     childIDs: [],
     isMounted: false,
@@ -133,27 +133,21 @@ var ReactComponentTreeHook = {
       }
       invariant(
         nextChild.parentID === id,
-        'Expected onSetParent() and onSetChildren() to be consistent (%s ' +
-        'has parents %s and %s).',
+        'Expected onBeforeMountComponent() parent and onSetChildren() to ' +
+        'be consistent (%s has parents %s and %s).',
         nextChildID,
         nextChild.parentID,
         id
       );
     }
   },
 
-  onSetParent(id, parentID) {
-    var item = get(id);
-    item.parentID = parentID;
-  },
+  onBeforeMountComponent(id, element, parentID) {
+    create(id, element, parentID);
 
-  onInstantiateComponent(id, element) {
-    create(id, element);
-  },
-
-  onBeforeMountComponent(id, element) {
-    var item = get(id);
-    item.element = element;
+    if (parentID === 0) {
+      rootIDs[id] = true;
+    }
   },
 
   onBeforeUpdateComponent(id, element) {
@@ -171,10 +165,6 @@ var ReactComponentTreeHook = {
     item.isMounted = true;
   },
 
-  onMountRootComponent(id) {
-    rootIDs[id] = true;
-  },
-
   onUpdateComponent(id) {
     var item = get(id);
     if (!item || !item.isMounted) {
@@ -187,7 +177,14 @@ var ReactComponentTreeHook = {
 
   onUnmountComponent(id) {
     var item = get(id);
-    item.isMounted = false;
+    if (item) {
+      // We need to check if it exists.
+      // `item` might not exist if it is inside an error boundary, and a sibling
+      // error boundary child threw while mounting. Then this instance never
+      // got a chance to mount, but it still gets an unmounting event during
+      // the error boundary cleanup.
+      item.isMounted = false;
+    }
     unmountedIDs[id] = true;
     delete rootIDs[id];
   },

diff --git a/src/renderers/dom/client/ReactMount.js b/src/renderers/dom/client/ReactMount.js
@@ -114,7 +114,8 @@ function mountComponentIntoNode(
     transaction,
     null,
     ReactDOMContainerInfo(wrapperInstance, container),
-    context
+    context,
+    0 /* parentDebugID */
   );
 
   if (markerName) {
@@ -355,13 +356,6 @@ var ReactMount = {
     var wrapperID = componentInstance._instance.rootID;
     instancesByReactRootID[wrapperID] = componentInstance;
 
-    if (__DEV__) {
-      // The instance here is TopLevelWrapper so we report mount for its child.
-      ReactInstrumentation.debugTool.onMountRootComponent(
-        componentInstance._renderedComponent._debugID
-      );
-    }
-
     return componentInstance;
   },
 

diff --git a/src/renderers/dom/server/ReactServerRendering.js b/src/renderers/dom/server/ReactServerRendering.js
@@ -47,7 +47,8 @@ function renderToStringImpl(element, makeStaticMarkup) {
         transaction,
         null,
         ReactDOMContainerInfo(),
-        emptyObject
+        emptyObject,
+        0 /* parentDebugID */
       );
       if (__DEV__) {
         ReactInstrumentation.debugTool.onUnmountComponent(

diff --git a/src/renderers/dom/shared/ReactDOMComponent.js b/src/renderers/dom/shared/ReactDOMComponent.js
@@ -274,9 +274,7 @@ if (__DEV__) {
       ReactInstrumentation.debugTool.onBeforeUpdateComponent(contentDebugID, content);
       ReactInstrumentation.debugTool.onUpdateComponent(contentDebugID);
     } else {
-      ReactInstrumentation.debugTool.onInstantiateComponent(contentDebugID, content);
-      ReactInstrumentation.debugTool.onSetParent(contentDebugID, debugID);
-      ReactInstrumentation.debugTool.onBeforeMountComponent(contentDebugID, content);
+      ReactInstrumentation.debugTool.onBeforeMountComponent(contentDebugID, content, debugID);
       ReactInstrumentation.debugTool.onMountComponent(contentDebugID);
       ReactInstrumentation.debugTool.onSetChildren(debugID, [contentDebugID]);
     }
@@ -707,13 +705,6 @@ ReactDOMComponent.Mixin = {
         break;
     }
 
-    if (__DEV__) {
-      if (this._debugID) {
-        var callback = () => ReactInstrumentation.debugTool.onComponentHasMounted(this._debugID);
-        transaction.getReactMountReady().enqueue(callback, this);
-      }
-    }
-
     return mountImage;
   },
 
@@ -943,13 +934,6 @@ ReactDOMComponent.Mixin = {
         transaction.getReactMountReady().enqueue(postUpdateSelectWrapper, this);
         break;
     }
-
-    if (__DEV__) {
-      if (this._debugID) {
-        var callback = () => ReactInstrumentation.debugTool.onComponentHasUpdated(this._debugID);
-        transaction.getReactMountReady().enqueue(callback, this);
-      }
-    }
   },
 
   /**

diff --git a/src/renderers/native/ReactNativeMount.js b/src/renderers/native/ReactNativeMount.js
@@ -56,7 +56,8 @@ function mountComponentIntoNode(
     transaction,
     null,
     ReactNativeContainerInfo(containerTag),
-    emptyObject
+    emptyObject,
+    0 /* parentDebugID */
   );
   componentInstance._renderedComponent._topLevelWrapper = componentInstance;
   ReactNativeMount._mountImageIntoNode(markup, containerTag);
@@ -147,12 +148,6 @@ var ReactNativeMount = {
       instance,
       containerTag
     );
-    if (__DEV__) {
-      // The instance here is TopLevelWrapper so we report mount for its child.
-      ReactInstrumentation.debugTool.onMountRootComponent(
-        instance._renderedComponent._debugID
-      );
-    }
     var component = instance.getPublicInstance();
     if (callback) {
       callback.call(component);

diff --git a/src/renderers/shared/ReactDebugTool.js b/src/renderers/shared/ReactDebugTool.js
@@ -109,7 +109,10 @@ function resetMeasurements() {
   currentFlushMeasurements = [];
 }
 
-function checkDebugID(debugID) {
+function checkDebugID(debugID, allowRoot = false) {
+  if (allowRoot && debugID === 0) {
+    return;
+  }
   if (!debugID) {
     warning(false, 'ReactDebugTool: debugID may not be empty.');
   }
@@ -248,14 +251,6 @@ var ReactDebugTool = {
     endLifeCycleTimer(debugID, timerType);
     emitEvent('onEndLifeCycleTimer', debugID, timerType);
   },
-  onBeginReconcilerTimer(debugID, timerType) {
-    checkDebugID(debugID);
-    emitEvent('onBeginReconcilerTimer', debugID, timerType);
-  },
-  onEndReconcilerTimer(debugID, timerType) {
-    checkDebugID(debugID);
-    emitEvent('onEndReconcilerTimer', debugID, timerType);
-  },
   onError(debugID) {
     if (currentTimerDebugID != null) {
       endLifeCycleTimer(currentTimerDebugID, currentTimerType);
@@ -272,14 +267,6 @@ var ReactDebugTool = {
     checkDebugID(debugID);
     emitEvent('onHostOperation', debugID, type, payload);
   },
-  onComponentHasMounted(debugID) {
-    checkDebugID(debugID);
-    emitEvent('onComponentHasMounted', debugID);
-  },
-  onComponentHasUpdated(debugID) {
-    checkDebugID(debugID);
-    emitEvent('onComponentHasUpdated', debugID);
-  },
   onSetState() {
     emitEvent('onSetState');
   },
@@ -288,21 +275,10 @@ var ReactDebugTool = {
     childDebugIDs.forEach(checkDebugID);
     emitEvent('onSetChildren', debugID, childDebugIDs);
   },
-  onSetParent(debugID, parentDebugID) {
-    checkDebugID(debugID);
-    emitEvent('onSetParent', debugID, parentDebugID);
-  },
-  onInstantiateComponent(debugID, element) {
-    checkDebugID(debugID);
-    emitEvent('onInstantiateComponent', debugID, element);
-  },
-  onMountRootComponent(debugID) {
+  onBeforeMountComponent(debugID, element, parentDebugID) {
     checkDebugID(debugID);
-    emitEvent('onMountRootComponent', debugID);
-  },
-  onBeforeMountComponent(debugID, element) {
-    checkDebugID(debugID);
-    emitEvent('onBeforeMountComponent', debugID, element);
+    checkDebugID(parentDebugID, true);
+    emitEvent('onBeforeMountComponent', debugID, element, parentDebugID);
   },
   onMountComponent(debugID) {
     checkDebugID(debugID);
@@ -316,6 +292,10 @@ var ReactDebugTool = {
     checkDebugID(debugID);
     emitEvent('onUpdateComponent', debugID);
   },
+  onBeforeUnmountComponent(debugID) {
+    checkDebugID(debugID);
+    emitEvent('onBeforeUnmountComponent', debugID);
+  },
   onUnmountComponent(debugID) {
     checkDebugID(debugID);
     emitEvent('onUnmountComponent', debugID);

diff --git a/src/renderers/shared/hooks/ReactChildrenMutationWarningHook.js b/src/renderers/shared/hooks/ReactChildrenMutationWarningHook.js
@@ -50,7 +50,7 @@ var ReactChildrenMutationWarningHook = {
   onMountComponent(debugID) {
     handleElement(debugID, ReactComponentTreeHook.getElement(debugID));
   },
-  onComponentHasUpdated(debugID) {
+  onUpdateComponent(debugID) {
     handleElement(debugID, ReactComponentTreeHook.getElement(debugID));
   },
 };

diff --git a/src/renderers/shared/stack/reconciler/ReactChildReconciler.js b/src/renderers/shared/stack/reconciler/ReactChildReconciler.js
@@ -75,7 +75,7 @@ var ReactChildReconciler = {
     nestedChildNodes,
     transaction,
     context,
-    selfDebugID // __DEV__ only
+    selfDebugID // 0 in production and for roots
   ) {
     if (nestedChildNodes == null) {
       return null;
@@ -117,7 +117,9 @@ var ReactChildReconciler = {
     transaction,
     hostParent,
     hostContainerInfo,
-    context) {
+    context,
+    selfDebugID // 0 in production and for roots
+  ) {
     // We currently don't have a way to track moves here but if we use iterators
     // instead of for..in we can zip the iterators and check if an item has
     // moved.
@@ -156,7 +158,8 @@ var ReactChildReconciler = {
           transaction,
           hostParent,
           hostContainerInfo,
-          context
+          context,
+          selfDebugID
         );
         mountImages.push(nextChildMountImage);
       }

diff --git a/src/renderers/shared/stack/reconciler/ReactCompositeComponent.js b/src/renderers/shared/stack/reconciler/ReactCompositeComponent.js
@@ -370,13 +370,6 @@ var ReactCompositeComponentMixin = {
       }
     }
 
-    if (__DEV__) {
-      if (this._debugID) {
-        var callback = (component) => ReactInstrumentation.debugTool.onComponentHasMounted(this._debugID);
-        transaction.getReactMountReady().enqueue(callback, this);
-      }
-    }
-
     return markup;
   },
 
@@ -532,21 +525,18 @@ var ReactCompositeComponentMixin = {
       nodeType !== ReactNodeTypes.EMPTY /* shouldHaveDebugID */
     );
     this._renderedComponent = child;
+
+    var selfDebugID = 0;
     if (__DEV__) {
-      if (child._debugID !== 0 && this._debugID !== 0) {
-        ReactInstrumentation.debugTool.onSetParent(
-          child._debugID,
-          this._debugID
-        );
-      }
+      selfDebugID = this._debugID;
     }
-
     var markup = ReactReconciler.mountComponent(
       child,
       transaction,
       hostParent,
       hostContainerInfo,
-      this._processChildContext(context)
+      this._processChildContext(context),
+      selfDebugID
     );
 
     if (__DEV__) {
@@ -1024,13 +1014,6 @@ var ReactCompositeComponentMixin = {
         );
       }
     }
-
-    if (__DEV__) {
-      if (this._debugID) {
-        var callback = () => ReactInstrumentation.debugTool.onComponentHasUpdated(this._debugID);
-        transaction.getReactMountReady().enqueue(callback, this);
-      }
-    }
   },
 
   /**
@@ -1061,21 +1044,18 @@ var ReactCompositeComponentMixin = {
         nodeType !== ReactNodeTypes.EMPTY /* shouldHaveDebugID */
       );
       this._renderedComponent = child;
+
+      var selfDebugID = 0;
       if (__DEV__) {
-        if (child._debugID !== 0 && this._debugID !== 0) {
-          ReactInstrumentation.debugTool.onSetParent(
-            child._debugID,
-            this._debugID
-          );
-        }
+        selfDebugID = this._debugID;
       }
-
       var nextMarkup = ReactReconciler.mountComponent(
         child,
         transaction,
         this._hostParent,
         this._hostContainerInfo,
-        this._processChildContext(context)
+        this._processChildContext(context),
+        selfDebugID
       );
 
       if (__DEV__) {