Merge pull request #8491 from sebmarkbage/fibercurrenteventhandlers

[Fiber] Read Event Handlers from the "Current" Fiber

scripts/fiber/tests-passing.txt

@@ -572,6 +572,7 @@ src/renderers/dom/shared/__tests__/ReactBrowserEventEmitter-test.js
 * should invoke a simple handler registered on a node
 * should not invoke handlers if ReactBrowserEventEmitter is disabled
 * should bubble simply
+* should bubble to the right handler after an update
 * should continue bubbling if an error is thrown
 * should set currentTarget
 * should support stopPropagation()

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

@@ -75,11 +75,21 @@ describe('ReactBrowserEventEmitter', () => {
     var PARENT_PROPS = {};
     var CHILD_PROPS = {};
 
+    function Child(props) {
+      return <div ref={(c) => CHILD = c} {...props} />;
+    }
+
+    class ChildWrapper extends React.PureComponent {
+      render() {
+        return <Child {...this.props} />;
+      }
+    }
+
     function renderTree() {
       ReactDOM.render(
         <div ref={(c) => GRANDPARENT = c} {...GRANDPARENT_PROPS}>
           <div ref={(c) => PARENT = c} {...PARENT_PROPS}>
-            <div ref={(c) => CHILD = c} {...CHILD_PROPS} />
+            <ChildWrapper {...CHILD_PROPS} />
           </div>
         </div>,
         container
@@ -193,6 +203,46 @@ describe('ReactBrowserEventEmitter', () => {
     expect(idCallOrder[2]).toBe(GRANDPARENT);
   });
 
+  it('should bubble to the right handler after an update', () => {
+    putListener(
+      GRANDPARENT,
+      ON_CLICK_KEY,
+      recordID.bind(null, 'GRANDPARENT')
+    );
+    putListener(
+      PARENT,
+      ON_CLICK_KEY,
+      recordID.bind(null, 'PARENT')
+    );
+    putListener(
+      CHILD,
+      ON_CLICK_KEY,
+      recordID.bind(null, 'CHILD')
+    );
+    ReactTestUtils.Simulate.click(CHILD);
+    expect(idCallOrder).toEqual([
+      'CHILD',
+      'PARENT',
+      'GRANDPARENT',
+    ]);
+
+    idCallOrder = [];
+
+    // Update just the grand parent without updating the child.
+    putListener(
+      GRANDPARENT,
+      ON_CLICK_KEY,
+      recordID.bind(null, 'UPDATED_GRANDPARENT')
+    );
+
+    ReactTestUtils.Simulate.click(CHILD);
+    expect(idCallOrder).toEqual([
+      'CHILD',
+      'PARENT',
+      'UPDATED_GRANDPARENT',
+    ]);
+  });
+
   it('should continue bubbling if an error is thrown', () => {
     putListener(
       CHILD,

src/renderers/shared/shared/ReactTreeTraversal.js

@@ -12,6 +12,7 @@
 'use strict';
 
 var { HostComponent } = require('ReactTypeOfWork');
+var { getNodeFromInstance, getInstanceFromNode } = require('EventPluginUtils');
 
 function getParent(inst) {
   if (inst._hostParent !== undefined) {
@@ -22,9 +23,18 @@ function getParent(inst) {
       inst = inst.return;
       // TODO: If this is a HostRoot we might want to bail out.
       // That is depending on if we want nested subtrees (layers) to bubble
-      // events to their parent.
+      // events to their parent. We could also go through parentNode on the
+      // host node but that wouldn't work for React Native and doesn't let us
+      // do the portal feature.
     } while (inst && inst.tag !== HostComponent);
-    return inst;
+    // Going through the Host Node will guarantee that we get the "current"
+    // Fiber, instead of the alternate because that pointer is updated when
+    // props update.
+    // TODO: This is a bit hacky and possibly slow. We should ideally have
+    // something in the reconciler that allow us to do this safely.
+    if (inst) {
+      return getInstanceFromNode(getNodeFromInstance(inst));
+    }
   }
   return null;
 }

src/renderers/shared/shared/event/EventPluginHub.js

@@ -126,11 +126,6 @@ var EventPluginHub = {
     // TODO: shouldPreventMouseEvent is DOM-specific and definitely should not
     // live here; needs to be moved to a better place soon
     if (typeof inst.tag === 'number') {
-      // TODO: This is not safe because we might want the *other* Fiber's
-      // props depending on which is the current one. This will usually be the
-      // current Fiber but if we're walking up the tree using TreeTraversal for
-      // bubbling, we will not be guaranteed to walk up the current tree when
-      // a Fiber has been reused.
       const props = inst.memoizedProps;
       listener = props[registrationName];
       if (shouldPreventMouseEvent(registrationName, inst.type, props)) {

src/renderers/shared/shared/event/EventPluginUtils.js

@@ -11,7 +11,6 @@
 
 'use strict';
 
-var ReactTreeTraversal = require('ReactTreeTraversal');
 var ReactErrorUtils = require('ReactErrorUtils');
 
 var invariant = require('invariant');
@@ -226,21 +225,6 @@ var EventPluginUtils = {
   getNodeFromInstance: function(node) {
     return ComponentTree.getNodeFromInstance(node);
   },
-  isAncestor: function(a, b) {
-    return ReactTreeTraversal.isAncestor(a, b);
-  },
-  getLowestCommonAncestor: function(a, b) {
-    return ReactTreeTraversal.getLowestCommonAncestor(a, b);
-  },
-  getParentInstance: function(inst) {
-    return ReactTreeTraversal.getParentInstance(inst);
-  },
-  traverseTwoPhase: function(target, fn, arg) {
-    return ReactTreeTraversal.traverseTwoPhase(target, fn, arg);
-  },
-  traverseEnterLeave: function(from, to, fn, argFrom, argTo) {
-    return ReactTreeTraversal.traverseEnterLeave(from, to, fn, argFrom, argTo);
-  },
 
   injection: injection,
 };

src/renderers/shared/shared/event/EventPropagators.js

@@ -12,7 +12,7 @@
 'use strict';
 
 var EventPluginHub = require('EventPluginHub');
-var EventPluginUtils = require('EventPluginUtils');
+var ReactTreeTraversal = require('ReactTreeTraversal');
 
 var accumulateInto = require('accumulateInto');
 var forEachAccumulated = require('forEachAccumulated');
@@ -62,7 +62,7 @@ function accumulateDirectionalDispatches(inst, phase, event) {
  */
 function accumulateTwoPhaseDispatchesSingle(event) {
   if (event && event.dispatchConfig.phasedRegistrationNames) {
-    EventPluginUtils.traverseTwoPhase(
+    ReactTreeTraversal.traverseTwoPhase(
       event._targetInst,
       accumulateDirectionalDispatches,
       event
@@ -77,8 +77,8 @@ function accumulateTwoPhaseDispatchesSingleSkipTarget(event) {
   if (event && event.dispatchConfig.phasedRegistrationNames) {
     var targetInst = event._targetInst;
     var parentInst =
-      targetInst ? EventPluginUtils.getParentInstance(targetInst) : null;
-    EventPluginUtils.traverseTwoPhase(
+      targetInst ? ReactTreeTraversal.getParentInstance(targetInst) : null;
+    ReactTreeTraversal.traverseTwoPhase(
       parentInst,
       accumulateDirectionalDispatches,
       event
@@ -124,7 +124,7 @@ function accumulateTwoPhaseDispatchesSkipTarget(events) {
 }
 
 function accumulateEnterLeaveDispatches(leave, enter, from, to) {
-  EventPluginUtils.traverseEnterLeave(
+  ReactTreeTraversal.traverseEnterLeave(
     from,
     to,
     accumulateDispatches,

src/renderers/shared/shared/event/eventPlugins/ResponderEventPlugin.js

@@ -13,6 +13,7 @@
 
 var EventPluginUtils = require('EventPluginUtils');
 var EventPropagators = require('EventPropagators');
+var ReactTreeTraversal = require('ReactTreeTraversal');
 var ResponderSyntheticEvent = require('ResponderSyntheticEvent');
 var ResponderTouchHistoryStore = require('ResponderTouchHistoryStore');
 
@@ -331,7 +332,7 @@ function setResponderAndExtractTransfer(
   // TODO: stop one short of the current responder.
   var bubbleShouldSetFrom = !responderInst ?
     targetInst :
-    EventPluginUtils.getLowestCommonAncestor(responderInst, targetInst);
+    ReactTreeTraversal.getLowestCommonAncestor(responderInst, targetInst);
 
   // When capturing/bubbling the "shouldSet" event, we want to skip the target
   // (deepest ID) if it happens to be the current responder. The reasoning:
@@ -454,7 +455,7 @@ function noResponderTouches(nativeEvent) {
     if (target !== null && target !== undefined && target !== 0) {
       // Is the original touch location inside of the current responder?
       var targetInst = EventPluginUtils.getInstanceFromNode(target);
-      if (EventPluginUtils.isAncestor(responderInst, targetInst)) {
+      if (ReactTreeTraversal.isAncestor(responderInst, targetInst)) {
         return false;
       }
     }