From 02d024cf6f00fa44d0298d453ff3659066fe4c07 Mon Sep 17 00:00:00 2001 From: Cheng Lou Date: Fri, 15 Aug 2014 12:39:42 -0700 Subject: [PATCH] [RFC] Per React container event listening/dispatching Work in progress. `enterleave` plugin (and maybe `analyticsPlugin`) is broken because it relied on the old behavior. But wanted to put this out here for suggestions. There are also some comments that need to be changed, I'll do it when the code is finalized. If we attach the event listening/dispatching at container level, it'll benefit the case of `` (both are container roots), since `Plugin1` won't disturb `Editor`. We also detach those listeners now. There wasn't really a need in the past. Fixes #2043 Should help with #1964 --- src/browser/ReactBrowserEventEmitter.js | 102 ++++++++++++------ .../ReactBrowserEventEmitter-test.js | 54 +++++++--- src/browser/ui/ReactDOMComponent.js | 11 +- src/browser/ui/ReactMount.js | 2 + src/vendor/stubs/EventListener.js | 2 + 5 files changed, 111 insertions(+), 60 deletions(-) diff --git a/src/browser/ReactBrowserEventEmitter.js b/src/browser/ReactBrowserEventEmitter.js index 677b0769863..cb026adb7ef 100644 --- a/src/browser/ReactBrowserEventEmitter.js +++ b/src/browser/ReactBrowserEventEmitter.js @@ -25,8 +25,10 @@ var EventPluginRegistry = require('EventPluginRegistry'); var ReactEventEmitterMixin = require('ReactEventEmitterMixin'); var ViewportMetrics = require('ViewportMetrics'); +var invariant = require('invariant'); var isEventSupported = require('isEventSupported'); var merge = require('merge'); +var warning = require('warning'); /** * Summary of `ReactBrowserEventEmitter` event handling: @@ -83,9 +85,7 @@ var merge = require('merge'); * React Core . General Purpose Event Plugin System */ -var alreadyListeningTo = {}; var isMonitoringScrollValue = false; -var reactTopListenersCounter = 0; // For events like 'submit' which don't consistently bubble (which we trap at a // lower node than `document`), binding at `document` would cause duplicate @@ -130,19 +130,28 @@ var topEventMapping = { topWheel: 'wheel' }; -/** - * To ensure no conflicts with other potential React instances on the page - */ -var topListenersIDKey = "_reactListenersID" + String(Math.random()).slice(2); +// TODO: (chenglou) Alternatively, we could use an internal +// map> +var eventsKey = '_reactEvents'; -function getListeningForDocument(mountAt) { +function getListenedEvents(mountAt) { // In IE8, `mountAt` is a host object and doesn't have `hasOwnProperty` // directly. - if (!Object.prototype.hasOwnProperty.call(mountAt, topListenersIDKey)) { - mountAt[topListenersIDKey] = reactTopListenersCounter++; - alreadyListeningTo[mountAt[topListenersIDKey]] = {}; + if (!Object.prototype.hasOwnProperty.call(mountAt, eventsKey)) { + mountAt[eventsKey] = {}; + } + return mountAt[eventsKey]; +} + +function removeListenedEvents(mountAt) { + if (!Object.prototype.hasOwnProperty.call(mountAt, eventsKey)) { + warning( + true, + 'Tried to remove a React root level listener, but it was not found.' + ); + return; } - return alreadyListeningTo[mountAt[topListenersIDKey]]; + delete mountAt[eventsKey]; } /** @@ -196,7 +205,7 @@ var ReactBrowserEventEmitter = merge(ReactEventEmitterMixin, { }, /** - * We listen for bubbled touch events on the document object. + * We listen for bubbled touch events on a root container. * * Firefox v8.01 (and possibly others) exhibited strange behavior when * mounting `onmousemove` events at some node that was not the document @@ -216,28 +225,29 @@ var ReactBrowserEventEmitter = merge(ReactEventEmitterMixin, { * @param {string} registrationName Name of listener (e.g. `onClick`). * @param {object} contentDocumentHandle Document which owns the container */ - listenTo: function(registrationName, contentDocumentHandle) { - var mountAt = contentDocumentHandle; - var isListening = getListeningForDocument(mountAt); - var dependencies = EventPluginRegistry. - registrationNameDependencies[registrationName]; + listenTo: function(registrationName, mountAt) { + var events = getListenedEvents(mountAt); + var dependencies = + EventPluginRegistry.registrationNameDependencies[registrationName]; var topLevelTypes = EventConstants.topLevelTypes; for (var i = 0, l = dependencies.length; i < l; i++) { + // `events` is a mapping of dependency -> event. The map does two + // things: store the fact that a dependency has already been registered, + // and store the event for later removal when the node's unmounted. var dependency = dependencies[i]; - if (!( - isListening.hasOwnProperty(dependency) && - isListening[dependency] - )) { + var event; + + if (!events.hasOwnProperty(dependency) || events[dependency] == null) { if (dependency === topLevelTypes.topWheel) { if (isEventSupported('wheel')) { - ReactBrowserEventEmitter.ReactEventListener.trapBubbledEvent( + event = ReactBrowserEventEmitter.trapBubbledEvent( topLevelTypes.topWheel, 'wheel', mountAt ); } else if (isEventSupported('mousewheel')) { - ReactBrowserEventEmitter.ReactEventListener.trapBubbledEvent( + event = ReactBrowserEventEmitter.trapBubbledEvent( topLevelTypes.topWheel, 'mousewheel', mountAt @@ -245,7 +255,7 @@ var ReactBrowserEventEmitter = merge(ReactEventEmitterMixin, { } else { // Firefox needs to capture a different mouse scroll event. // @see http://www.quirksmode.org/dom/events/tests/scroll.html - ReactBrowserEventEmitter.ReactEventListener.trapBubbledEvent( + event = ReactBrowserEventEmitter.trapBubbledEvent( topLevelTypes.topWheel, 'DOMMouseScroll', mountAt @@ -254,28 +264,29 @@ var ReactBrowserEventEmitter = merge(ReactEventEmitterMixin, { } else if (dependency === topLevelTypes.topScroll) { if (isEventSupported('scroll', true)) { - ReactBrowserEventEmitter.ReactEventListener.trapCapturedEvent( + event = ReactBrowserEventEmitter.trapCapturedEvent( topLevelTypes.topScroll, 'scroll', mountAt ); } else { - ReactBrowserEventEmitter.ReactEventListener.trapBubbledEvent( + event = ReactBrowserEventEmitter.trapBubbledEvent( topLevelTypes.topScroll, 'scroll', - ReactBrowserEventEmitter.ReactEventListener.WINDOW_HANDLE + ReactBrowserEventEmitter.WINDOW_HANDLE ); } } else if (dependency === topLevelTypes.topFocus || dependency === topLevelTypes.topBlur) { + var event2; if (isEventSupported('focus', true)) { - ReactBrowserEventEmitter.ReactEventListener.trapCapturedEvent( + event = ReactBrowserEventEmitter.trapCapturedEvent( topLevelTypes.topFocus, 'focus', mountAt ); - ReactBrowserEventEmitter.ReactEventListener.trapCapturedEvent( + event2 = ReactBrowserEventEmitter.trapCapturedEvent( topLevelTypes.topBlur, 'blur', mountAt @@ -283,12 +294,12 @@ var ReactBrowserEventEmitter = merge(ReactEventEmitterMixin, { } else if (isEventSupported('focusin')) { // IE has `focusin` and `focusout` events which bubble. // @see http://www.quirksmode.org/blog/archives/2008/04/delegating_the.html - ReactBrowserEventEmitter.ReactEventListener.trapBubbledEvent( + event = ReactBrowserEventEmitter.trapBubbledEvent( topLevelTypes.topFocus, 'focusin', mountAt ); - ReactBrowserEventEmitter.ReactEventListener.trapBubbledEvent( + event2 = ReactBrowserEventEmitter.trapBubbledEvent( topLevelTypes.topBlur, 'focusout', mountAt @@ -296,19 +307,40 @@ var ReactBrowserEventEmitter = merge(ReactEventEmitterMixin, { } // to make sure blur and focus event listeners are only attached once - isListening[topLevelTypes.topBlur] = true; - isListening[topLevelTypes.topFocus] = true; + events[topLevelTypes.topFocus] = event; + events[topLevelTypes.topBlur] = event2; } else if (topEventMapping.hasOwnProperty(dependency)) { - ReactBrowserEventEmitter.ReactEventListener.trapBubbledEvent( + event = ReactBrowserEventEmitter.trapBubbledEvent( dependency, topEventMapping[dependency], mountAt ); } + // As mentioned above, events like `submit` don't bubble to document and + // thus are not attached to it. In that case, there's no `event` (and a + // `remove`) to store. We'll put a `true` placeholder here. + events[dependency] = event || true; + } + } + }, - isListening[dependency] = true; + removeListenedEvents: function(container) { + var events = getListenedEvents(container); + if (!events) { + // Might be that no event was (lazily) added in the first place. + return; + } + for (var key in events) { + if (!events.hasOwnProperty(key)) { + continue; + } + if (events[key].remove) { + // See `listenTo`. The event might be a `true` placeholder for things + // like `onSubmit`. + events[key].remove(); } } + removeListenedEvents(container); }, trapBubbledEvent: function(topLevelType, handlerBaseName, handle) { diff --git a/src/browser/__tests__/ReactBrowserEventEmitter-test.js b/src/browser/__tests__/ReactBrowserEventEmitter-test.js index 68e2ba0c9c7..981832980fb 100644 --- a/src/browser/__tests__/ReactBrowserEventEmitter-test.js +++ b/src/browser/__tests__/ReactBrowserEventEmitter-test.js @@ -19,14 +19,15 @@ "use strict"; require('mock-modules') - .dontMock('EventPluginHub') - .dontMock('ReactMount') - .dontMock('ReactBrowserEventEmitter') - .dontMock('ReactInstanceHandles') - .dontMock('EventPluginHub') - .dontMock('TapEventPlugin') - .dontMock('TouchEventUtils') - .dontMock('keyOf'); + .dontMock('EventListener') + .dontMock('EventPluginHub') + .dontMock('keyOf') + .dontMock('ReactBrowserEventEmitter') + .dontMock('ReactEventListener') + .dontMock('ReactInstanceHandles') + .dontMock('ReactMount') + .dontMock('TapEventPlugin') + .dontMock('TouchEventUtils'); var keyOf = require('keyOf'); @@ -373,26 +374,47 @@ describe('ReactBrowserEventEmitter', function() { expect(idCallOrder.length).toBe(0); }); + it('should attach the event to the root container', function() { + var div = document.createElement('div'); + ReactBrowserEventEmitter.listenTo(ON_CLICK_KEY, div); + expect(div._reactEvents.topClick.remove).toBeDefined(); + }); + + it('should be able to remove listeners on the root container', function() { + var div = document.createElement('div'); + spyOn(div, 'removeEventListener').andCallThrough(); + ReactBrowserEventEmitter.listenTo(ON_CLICK_KEY, div); + ReactBrowserEventEmitter.listenTo(ON_CHANGE_KEY, div); + ReactBrowserEventEmitter.removeListenedEvents(div); + // Once for click, 7 times for change. + expect(div.removeEventListener.argsForCall.length).toBe(8); + expect(div._reactEvents).toBe(undefined); + }); + + it('should listen to events only once', function() { - spyOn(EventListener, 'listen'); - ReactBrowserEventEmitter.listenTo(ON_CLICK_KEY, document); - ReactBrowserEventEmitter.listenTo(ON_CLICK_KEY, document); + spyOn(EventListener, 'listen').andCallThrough(); + var div = document.createElement('div'); + ReactBrowserEventEmitter.listenTo(ON_CLICK_KEY, div); + ReactBrowserEventEmitter.listenTo(ON_CLICK_KEY, div); expect(EventListener.listen.callCount).toBe(1); }); it('should work with event plugins without dependencies', function() { spyOn(EventListener, 'listen'); - ReactBrowserEventEmitter.listenTo(ON_CLICK_KEY, document); + var div = document.createElement('div'); + ReactBrowserEventEmitter.listenTo(ON_CLICK_KEY, div); expect(EventListener.listen.argsForCall[0][1]).toBe('click'); }); it('should work with event plugins with dependencies', function() { - spyOn(EventListener, 'listen'); - spyOn(EventListener, 'capture'); + spyOn(EventListener, 'listen').andCallThrough(); + spyOn(EventListener, 'capture').andCallThrough(); - ReactBrowserEventEmitter.listenTo(ON_CHANGE_KEY, document); + var div = document.createElement('div'); + ReactBrowserEventEmitter.listenTo(ON_CHANGE_KEY, div); var setEventListeners = []; var listenCalls = EventListener.listen.argsForCall; @@ -405,7 +427,7 @@ describe('ReactBrowserEventEmitter', function() { } var module = - ReactBrowserEventEmitter.registrationNameModules[ON_CHANGE_KEY]; + ReactBrowserEventEmitter.registrationNameModules[ON_CHANGE_KEY]; var dependencies = module.eventTypes.change.dependencies; expect(setEventListeners.length).toEqual(dependencies.length); diff --git a/src/browser/ui/ReactDOMComponent.js b/src/browser/ui/ReactDOMComponent.js index 70c0ceb9e0a..685bf4de1a0 100644 --- a/src/browser/ui/ReactDOMComponent.js +++ b/src/browser/ui/ReactDOMComponent.js @@ -35,8 +35,6 @@ var keyOf = require('keyOf'); var merge = require('merge'); var mixInto = require('mixInto'); -var deleteListener = ReactBrowserEventEmitter.deleteListener; -var listenTo = ReactBrowserEventEmitter.listenTo; var registrationNameModules = ReactBrowserEventEmitter.registrationNameModules; // For quickly matching children type, to test if can be treated as content. @@ -44,8 +42,6 @@ var CONTENT_TYPES = {'string': true, 'number': true}; var STYLE = keyOf({style: null}); -var ELEMENT_NODE_TYPE = 1; - /** * @param {?object} props */ @@ -68,10 +64,7 @@ function assertValidProps(props) { function putListener(id, registrationName, listener, transaction) { var container = ReactMount.findReactContainerForID(id); if (container) { - var doc = container.nodeType === ELEMENT_NODE_TYPE ? - container.ownerDocument : - container; - listenTo(registrationName, doc); + ReactBrowserEventEmitter.listenTo(registrationName, container); } transaction.getPutListenerQueue().enqueuePutListener( id, @@ -283,7 +276,7 @@ ReactDOMComponent.Mixin = { } } } else if (registrationNameModules.hasOwnProperty(propKey)) { - deleteListener(this._rootNodeID, propKey); + ReactBrowserEventEmitter.deleteListener(this._rootNodeID, propKey); } else if ( DOMProperty.isStandardName[propKey] || DOMProperty.isCustomAttribute(propKey)) { diff --git a/src/browser/ui/ReactMount.js b/src/browser/ui/ReactMount.js index 581a904427d..149999b3478 100644 --- a/src/browser/ui/ReactMount.js +++ b/src/browser/ui/ReactMount.js @@ -461,6 +461,8 @@ var ReactMount = { if (!component) { return false; } + + ReactBrowserEventEmitter.removeListenedEvents(container); ReactMount.unmountComponentFromNode(component, container); delete instancesByReactRootID[reactRootID]; delete containersByReactRootID[reactRootID]; diff --git a/src/vendor/stubs/EventListener.js b/src/vendor/stubs/EventListener.js index c3ddd3186ea..c69061de030 100644 --- a/src/vendor/stubs/EventListener.js +++ b/src/vendor/stubs/EventListener.js @@ -3,6 +3,8 @@ * @typechecks */ +'use strict'; + var emptyFunction = require('emptyFunction'); /**