From ee4984980e7f7b030e2a4b371bb45db294924f3b Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Fri, 2 Dec 2016 20:03:09 -0800 Subject: [PATCH] Get the "current" Fiber from each node along the path This fixes the issue where using the .return pointer isn't guaranteed to return the current Fiber so we might read the wrong props when we try to get the current event. --- scripts/fiber/tests-failing.txt | 4 ---- scripts/fiber/tests-passing.txt | 2 ++ src/renderers/shared/shared/ReactTreeTraversal.js | 14 ++++++++++++-- .../shared/shared/event/EventPluginHub.js | 5 ----- 4 files changed, 14 insertions(+), 11 deletions(-) diff --git a/scripts/fiber/tests-failing.txt b/scripts/fiber/tests-failing.txt index 9b46b63ed5..16dd1717af 100644 --- a/scripts/fiber/tests-failing.txt +++ b/scripts/fiber/tests-failing.txt @@ -27,10 +27,6 @@ src/renderers/art/__tests__/ReactART-test.js src/renderers/dom/__tests__/ReactDOMProduction-test.js * should throw with an error code in production -src/renderers/dom/shared/__tests__/ReactBrowserEventEmitter-test.js -* should bubble to the right handler after an update -* should invoke handlers that were removed while bubbling - src/renderers/dom/shared/__tests__/ReactDOM-test.js * throws in render() if the mount callback is not a function * throws in render() if the update callback is not a function diff --git a/scripts/fiber/tests-passing.txt b/scripts/fiber/tests-passing.txt index 7fdf742564..21ebd152e7 100644 --- a/scripts/fiber/tests-passing.txt +++ b/scripts/fiber/tests-passing.txt @@ -556,11 +556,13 @@ 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() * should stop after first dispatch if stopPropagation * should not stopPropagation if false is returned +* should invoke handlers that were removed while bubbling * should not invoke newly inserted handlers while bubbling * should have mouse enter simulated by test utils * should infer onTouchTap from a touchStart/End diff --git a/src/renderers/shared/shared/ReactTreeTraversal.js b/src/renderers/shared/shared/ReactTreeTraversal.js index 22a72b0a67..a53eaf7b4c 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 4023354278..57d8042334 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)) {