From 216026418cf47787f5393e4db734281ae9b6c5c8 Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Fri, 19 Jun 2015 21:53:25 -0700 Subject: [PATCH] Simplify event dispatching This simplifies event dispatching by removing the `return false` special case for the SimpleEventPlugin which allow us to inline much more here and the code becomes easier to follow. --- .../ReactBrowserEventEmitter-test.js | 13 ++---- .../client/eventPlugins/SimpleEventPlugin.js | 26 ----------- src/renderers/shared/event/EventPluginHub.js | 8 +--- .../shared/event/EventPluginUtils.js | 46 +++++++------------ 4 files changed, 23 insertions(+), 70 deletions(-) diff --git a/src/renderers/dom/client/__tests__/ReactBrowserEventEmitter-test.js b/src/renderers/dom/client/__tests__/ReactBrowserEventEmitter-test.js index fc6af66049..b36c8d5468 100644 --- a/src/renderers/dom/client/__tests__/ReactBrowserEventEmitter-test.js +++ b/src/renderers/dom/client/__tests__/ReactBrowserEventEmitter-test.js @@ -264,7 +264,7 @@ describe('ReactBrowserEventEmitter', function() { expect(idCallOrder[0]).toBe(getID(CHILD)); }); - it('should stopPropagation if false is returned, but warn', function() { + it('should not stopPropagation if false is returned', function() { ReactBrowserEventEmitter.putListener( getID(CHILD), ON_CLICK_KEY, @@ -282,14 +282,11 @@ describe('ReactBrowserEventEmitter', function() { ); spyOn(console, 'error'); ReactTestUtils.Simulate.click(CHILD); - expect(idCallOrder.length).toBe(1); + expect(idCallOrder.length).toBe(3); expect(idCallOrder[0]).toBe(getID(CHILD)); - expect(console.error.calls.length).toEqual(1); - expect(console.error.calls[0].args[0]).toBe( - 'Warning: Returning `false` from an event handler is deprecated and ' + - 'will be ignored in a future release. Instead, manually call ' + - 'e.stopPropagation() or e.preventDefault(), as appropriate.' - ); + expect(idCallOrder[1]).toBe(getID(PARENT)); + expect(idCallOrder[2]).toBe(getID(GRANDPARENT)); + expect(console.error.calls.length).toEqual(0); }); /** diff --git a/src/renderers/dom/client/eventPlugins/SimpleEventPlugin.js b/src/renderers/dom/client/eventPlugins/SimpleEventPlugin.js index cc4ddde826..03c1c1eaa8 100644 --- a/src/renderers/dom/client/eventPlugins/SimpleEventPlugin.js +++ b/src/renderers/dom/client/eventPlugins/SimpleEventPlugin.js @@ -13,7 +13,6 @@ var EventConstants = require('EventConstants'); var EventListener = require('EventListener'); -var EventPluginUtils = require('EventPluginUtils'); var EventPropagators = require('EventPropagators'); var ReactMount = require('ReactMount'); var SyntheticClipboardEvent = require('SyntheticClipboardEvent'); @@ -30,7 +29,6 @@ var emptyFunction = require('emptyFunction'); var getEventCharCode = require('getEventCharCode'); var invariant = require('invariant'); var keyOf = require('keyOf'); -var warning = require('warning'); var topLevelTypes = EventConstants.topLevelTypes; @@ -452,30 +450,6 @@ var SimpleEventPlugin = { eventTypes: eventTypes, - /** - * Same as the default implementation, except cancels the event when return - * value is false. This behavior will be disabled in a future release. - * - * @param {object} event Event to be dispatched. - * @param {function} listener Application-level callback. - * @param {string} domID DOM ID to pass to the callback. - */ - executeDispatch: function(event, listener, domID) { - var returnValue = EventPluginUtils.executeDispatch(event, listener, domID); - - warning( - typeof returnValue !== 'boolean', - 'Returning `false` from an event handler is deprecated and will be ' + - 'ignored in a future release. Instead, manually call ' + - 'e.stopPropagation() or e.preventDefault(), as appropriate.' - ); - - if (returnValue === false) { - event.stopPropagation(); - event.preventDefault(); - } - }, - /** * @param {string} topLevelType Record from `EventConstants`. * @param {DOMEventTarget} topLevelTarget The listening component root node. diff --git a/src/renderers/shared/event/EventPluginHub.js b/src/renderers/shared/event/EventPluginHub.js index 81faff015a..ce448b701d 100644 --- a/src/renderers/shared/event/EventPluginHub.js +++ b/src/renderers/shared/event/EventPluginHub.js @@ -39,13 +39,7 @@ var eventQueue = null; */ var executeDispatchesAndRelease = function(event) { if (event) { - var executeDispatch = EventPluginUtils.executeDispatch; - // Plugins can provide custom behavior when dispatching events. - var PluginModule = EventPluginRegistry.getPluginModuleForEvent(event); - if (PluginModule && PluginModule.executeDispatch) { - executeDispatch = PluginModule.executeDispatch; - } - EventPluginUtils.executeDispatchesInOrder(event, executeDispatch); + EventPluginUtils.executeDispatchesInOrder(event); if (!event.isPersistent()) { event.constructor.release(event); diff --git a/src/renderers/shared/event/EventPluginUtils.js b/src/renderers/shared/event/EventPluginUtils.js index dbdef8b95a..20b5998e7e 100644 --- a/src/renderers/shared/event/EventPluginUtils.js +++ b/src/renderers/shared/event/EventPluginUtils.js @@ -78,11 +78,22 @@ if (__DEV__) { } /** - * Invokes `cb(event, listener, id)`. Avoids using call if no scope is - * provided. The `(listener,id)` pair effectively forms the "dispatch" but are - * kept separate to conserve memory. + * Dispatch the event to the listener. + * @param {SyntheticEvent} event SyntheticEvent to handle + * @param {function} listener Application-level callback + * @param {string} domID DOM id to pass to the callback. */ -function forEachEventDispatch(event, cb) { +function executeDispatch(event, listener, domID) { + var type = event.type || 'unknown-event'; + event.currentTarget = injection.Mount.getNode(domID); + ReactErrorUtils.invokeGuardedCallback(type, listener, event, domID); + event.currentTarget = null; +} + +/** + * Standard/simple iteration through an event's collected dispatches. + */ +function executeDispatchesInOrder(event) { var dispatchListeners = event._dispatchListeners; var dispatchIDs = event._dispatchIDs; if (__DEV__) { @@ -94,33 +105,11 @@ function forEachEventDispatch(event, cb) { break; } // Listeners and IDs are two parallel arrays that are always in sync. - cb(event, dispatchListeners[i], dispatchIDs[i]); + executeDispatch(event, dispatchListeners[i], dispatchIDs[i]); } } else if (dispatchListeners) { - cb(event, dispatchListeners, dispatchIDs); + executeDispatch(event, dispatchListeners, dispatchIDs); } -} - -/** - * Default implementation of PluginModule.executeDispatch(). - * @param {SyntheticEvent} event SyntheticEvent to handle - * @param {function} listener Application-level callback - * @param {string} domID DOM id to pass to the callback. - */ -function executeDispatch(event, listener, domID) { - event.currentTarget = injection.Mount.getNode(domID); - var type = event.type || 'unknown-event'; - var returnValue = - ReactErrorUtils.invokeGuardedCallback(type, listener, event, domID); - event.currentTarget = null; - return returnValue; -} - -/** - * Standard/simple iteration through an event's collected dispatches. - */ -function executeDispatchesInOrder(event, cb) { - forEachEventDispatch(event, cb); event._dispatchListeners = null; event._dispatchIDs = null; } @@ -210,7 +199,6 @@ var EventPluginUtils = { isStartish: isStartish, executeDirectDispatch: executeDirectDispatch, - executeDispatch: executeDispatch, executeDispatchesInOrder: executeDispatchesInOrder, executeDispatchesInOrderStopAtTrue: executeDispatchesInOrderStopAtTrue, hasDispatches: hasDispatches,