diff --git a/scripts/fiber/tests-passing.txt b/scripts/fiber/tests-passing.txt index a1401beb4089..21ebd152e7fb 100644 --- a/scripts/fiber/tests-passing.txt +++ b/scripts/fiber/tests-passing.txt @@ -556,6 +556,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() diff --git a/src/renderers/dom/shared/__tests__/ReactBrowserEventEmitter-test.js b/src/renderers/dom/shared/__tests__/ReactBrowserEventEmitter-test.js index 6462df35f6cd..0a6244e6f340 100644 --- a/src/renderers/dom/shared/__tests__/ReactBrowserEventEmitter-test.js +++ b/src/renderers/dom/shared/__tests__/ReactBrowserEventEmitter-test.js @@ -75,11 +75,21 @@ describe('ReactBrowserEventEmitter', () => { var PARENT_PROPS = {}; var CHILD_PROPS = {}; + function Child(props) { + return
CHILD = c} {...props} />; + } + + class ChildWrapper extends React.PureComponent { + render() { + return ; + } + } + function renderTree() { ReactDOM.render(
GRANDPARENT = c} {...GRANDPARENT_PROPS}>
PARENT = c} {...PARENT_PROPS}> -
CHILD = c} {...CHILD_PROPS} /> +
, 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, diff --git a/src/renderers/shared/shared/ReactTreeTraversal.js b/src/renderers/shared/shared/ReactTreeTraversal.js index 22a72b0a672f..a53eaf7b4c3f 100644 --- a/src/renderers/shared/shared/ReactTreeTraversal.js +++ b/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; } diff --git a/src/renderers/shared/shared/event/EventPluginHub.js b/src/renderers/shared/shared/event/EventPluginHub.js index 402335427822..57d80423340c 100644 --- a/src/renderers/shared/shared/event/EventPluginHub.js +++ b/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)) { diff --git a/src/renderers/shared/shared/event/EventPluginUtils.js b/src/renderers/shared/shared/event/EventPluginUtils.js index c2861ddbf5ba..a616562807e0 100644 --- a/src/renderers/shared/shared/event/EventPluginUtils.js +++ b/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, }; diff --git a/src/renderers/shared/shared/event/EventPropagators.js b/src/renderers/shared/shared/event/EventPropagators.js index da1b23841665..75cd7d4e45db 100644 --- a/src/renderers/shared/shared/event/EventPropagators.js +++ b/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, diff --git a/src/renderers/shared/shared/event/eventPlugins/ResponderEventPlugin.js b/src/renderers/shared/shared/event/eventPlugins/ResponderEventPlugin.js index 1973592d8a56..d4057833c26a 100644 --- a/src/renderers/shared/shared/event/eventPlugins/ResponderEventPlugin.js +++ b/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; } }