From 2f0bb69708e224fb7b6eb90766578841112a22d2 Mon Sep 17 00:00:00 2001 From: Ben Alpert Date: Mon, 17 Feb 2014 01:24:54 -0800 Subject: [PATCH 1/2] Prevent error thrown when removing event target Fixes #1105. --- src/browser/ReactEventTopLevelCallback.js | 25 ++++++++++++-- .../ReactEventTopLevelCallback-test.js | 33 +++++++++++++++++++ 2 files changed, 55 insertions(+), 3 deletions(-) diff --git a/src/browser/ReactEventTopLevelCallback.js b/src/browser/ReactEventTopLevelCallback.js index 5d202ccb3a5..ce87dc0412c 100644 --- a/src/browser/ReactEventTopLevelCallback.js +++ b/src/browser/ReactEventTopLevelCallback.js @@ -49,6 +49,9 @@ function findParent(node) { return parent; } +// Used to store ancestor hierarchy in top level callback +var topLevelCallbackReusableArray = []; + /** * Top-level callback creator used to implement event handling using delegation. * This is used via dependency injection. @@ -89,8 +92,21 @@ var ReactEventTopLevelCallback = { getEventTarget(nativeEvent) ) || window; + var ancestors = topLevelCallbackReusableArray; + ancestors.length = 0; + // Loop through the hierarchy, in case there's any nested components. - while (topLevelTarget) { + // It's important that we build the array of ancestors before calling any + // event handlers, because event handlers can modify the DOM, leading to + // inconsistencies with ReactMount's node cache. See #1105. + var ancestor = topLevelTarget; + while (ancestor) { + ancestors.push(ancestor); + ancestor = findParent(ancestor); + } + + for (var i = 0, l = ancestors.length; i < l; i++) { + topLevelTarget = ancestors[i]; var topLevelTargetID = ReactMount.getID(topLevelTarget) || ''; ReactEventEmitter.handleTopLevel( topLevelType, @@ -98,9 +114,12 @@ var ReactEventTopLevelCallback = { topLevelTargetID, nativeEvent ); - - topLevelTarget = findParent(topLevelTarget); } + + // Emptying ancestors/topLevelCallbackReusableArray is + // not necessary for correctness, but it helps the GC reclaim + // any nodes that were left at the end of the search. + ancestors.length = 0; }; } diff --git a/src/browser/__tests__/ReactEventTopLevelCallback-test.js b/src/browser/__tests__/ReactEventTopLevelCallback-test.js index 6782d006c2b..6b7dc0cde0f 100644 --- a/src/browser/__tests__/ReactEventTopLevelCallback-test.js +++ b/src/browser/__tests__/ReactEventTopLevelCallback-test.js @@ -84,6 +84,39 @@ describe('ReactEventTopLevelCallback', function() { expect(calls[2][EVENT_TARGET_PARAM]) .toBe(grandParentControl.getDOMNode()); }); + + it('should not get confused by disappearing elements', function() { + var childContainer = document.createElement('div'); + var childControl =
Child
; + var parentContainer = document.createElement('div'); + var parentControl =
Parent
; + ReactMount.renderComponent(childControl, childContainer); + ReactMount.renderComponent(parentControl, parentContainer); + parentControl.getDOMNode().appendChild(childContainer); + + // ReactEventEmitter.handleTopLevel might remove the target from the DOM. + // Here, we have handleTopLevel remove the node when the first event + // handlers are called; we'll still expect to receive a second call for + // the parent control. + var childNode = childControl.getDOMNode(); + ReactEventEmitter.handleTopLevel.mockImplementation( + function(topLevelType, topLevelTarget, topLevelTargetID, nativeEvent) { + if (topLevelTarget === childNode) { + ReactMount.unmountComponentAtNode(childContainer); + } + } + ); + + var callback = ReactEventTopLevelCallback.createTopLevelCallback('test'); + callback({ + target: childNode + }); + + var calls = ReactEventEmitter.handleTopLevel.mock.calls; + expect(calls.length).toBe(2); + expect(calls[0][EVENT_TARGET_PARAM]).toBe(childNode); + expect(calls[1][EVENT_TARGET_PARAM]).toBe(parentControl.getDOMNode()); + }); }); it('should not fire duplicate events for a React DOM tree', function() { From d8fd1af4a0102d5ace78300cd1b5648c45ea6d66 Mon Sep 17 00:00:00 2001 From: Ben Alpert Date: Mon, 17 Feb 2014 15:07:28 -0800 Subject: [PATCH 2/2] Pool ancestors list so event system is reentrant --- src/browser/ReactEventTopLevelCallback.js | 83 ++++++++++++++--------- 1 file changed, 52 insertions(+), 31 deletions(-) diff --git a/src/browser/ReactEventTopLevelCallback.js b/src/browser/ReactEventTopLevelCallback.js index ce87dc0412c..821bf24d491 100644 --- a/src/browser/ReactEventTopLevelCallback.js +++ b/src/browser/ReactEventTopLevelCallback.js @@ -19,11 +19,13 @@ "use strict"; +var PooledClass = require('PooledClass'); var ReactEventEmitter = require('ReactEventEmitter'); var ReactInstanceHandles = require('ReactInstanceHandles'); var ReactMount = require('ReactMount'); var getEventTarget = require('getEventTarget'); +var mixInto = require('mixInto'); /** * @type {boolean} @@ -49,8 +51,52 @@ function findParent(node) { return parent; } +/** + * Calls ReactEventEmitter.handleTopLevel for each node stored in bookKeeping's + * ancestor list. Separated from createTopLevelCallback to avoid try/finally + * deoptimization. + * + * @param {string} topLevelType + * @param {DOMEvent} nativeEvent + * @param {TopLevelCallbackBookKeeping} bookKeeping + */ +function handleTopLevelImpl(topLevelType, nativeEvent, bookKeeping) { + var topLevelTarget = ReactMount.getFirstReactDOM( + getEventTarget(nativeEvent) + ) || window; + + // Loop through the hierarchy, in case there's any nested components. + // It's important that we build the array of ancestors before calling any + // event handlers, because event handlers can modify the DOM, leading to + // inconsistencies with ReactMount's node cache. See #1105. + var ancestor = topLevelTarget; + while (ancestor) { + bookKeeping.ancestors.push(ancestor); + ancestor = findParent(ancestor); + } + + for (var i = 0, l = bookKeeping.ancestors.length; i < l; i++) { + topLevelTarget = bookKeeping.ancestors[i]; + var topLevelTargetID = ReactMount.getID(topLevelTarget) || ''; + ReactEventEmitter.handleTopLevel( + topLevelType, + topLevelTarget, + topLevelTargetID, + nativeEvent + ); + } +} + // Used to store ancestor hierarchy in top level callback -var topLevelCallbackReusableArray = []; +function TopLevelCallbackBookKeeping() { + this.ancestors = []; +} +mixInto(TopLevelCallbackBookKeeping, { + destructor: function() { + this.ancestors.length = 0; + } +}); +PooledClass.addPoolingTo(TopLevelCallbackBookKeeping); /** * Top-level callback creator used to implement event handling using delegation. @@ -88,38 +134,13 @@ var ReactEventTopLevelCallback = { if (!_topLevelListenersEnabled) { return; } - var topLevelTarget = ReactMount.getFirstReactDOM( - getEventTarget(nativeEvent) - ) || window; - - var ancestors = topLevelCallbackReusableArray; - ancestors.length = 0; - - // Loop through the hierarchy, in case there's any nested components. - // It's important that we build the array of ancestors before calling any - // event handlers, because event handlers can modify the DOM, leading to - // inconsistencies with ReactMount's node cache. See #1105. - var ancestor = topLevelTarget; - while (ancestor) { - ancestors.push(ancestor); - ancestor = findParent(ancestor); - } - for (var i = 0, l = ancestors.length; i < l; i++) { - topLevelTarget = ancestors[i]; - var topLevelTargetID = ReactMount.getID(topLevelTarget) || ''; - ReactEventEmitter.handleTopLevel( - topLevelType, - topLevelTarget, - topLevelTargetID, - nativeEvent - ); + var bookKeeping = TopLevelCallbackBookKeeping.getPooled(); + try { + handleTopLevelImpl(topLevelType, nativeEvent, bookKeeping); + } finally { + TopLevelCallbackBookKeeping.release(bookKeeping); } - - // Emptying ancestors/topLevelCallbackReusableArray is - // not necessary for correctness, but it helps the GC reclaim - // any nodes that were left at the end of the search. - ancestors.length = 0; }; }