Skip to content

Commit 02d024c

Browse files
committed
[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 `<Editor/><Plugin1/>` (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
1 parent 5b4e2be commit 02d024c

5 files changed

Lines changed: 111 additions & 60 deletions

File tree

src/browser/ReactBrowserEventEmitter.js

Lines changed: 67 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -25,8 +25,10 @@ var EventPluginRegistry = require('EventPluginRegistry');
2525
var ReactEventEmitterMixin = require('ReactEventEmitterMixin');
2626
var ViewportMetrics = require('ViewportMetrics');
2727

28+
var invariant = require('invariant');
2829
var isEventSupported = require('isEventSupported');
2930
var merge = require('merge');
31+
var warning = require('warning');
3032

3133
/**
3234
* Summary of `ReactBrowserEventEmitter` event handling:
@@ -83,9 +85,7 @@ var merge = require('merge');
8385
* React Core . General Purpose Event Plugin System
8486
*/
8587

86-
var alreadyListeningTo = {};
8788
var isMonitoringScrollValue = false;
88-
var reactTopListenersCounter = 0;
8989

9090
// For events like 'submit' which don't consistently bubble (which we trap at a
9191
// lower node than `document`), binding at `document` would cause duplicate
@@ -130,19 +130,28 @@ var topEventMapping = {
130130
topWheel: 'wheel'
131131
};
132132

133-
/**
134-
* To ensure no conflicts with other potential React instances on the page
135-
*/
136-
var topListenersIDKey = "_reactListenersID" + String(Math.random()).slice(2);
133+
// TODO: (chenglou) Alternatively, we could use an internal
134+
// map<IDOfRootNodeInsideContainer, map<eventRegistrationName, eventPlugin>>
135+
var eventsKey = '_reactEvents';
137136

138-
function getListeningForDocument(mountAt) {
137+
function getListenedEvents(mountAt) {
139138
// In IE8, `mountAt` is a host object and doesn't have `hasOwnProperty`
140139
// directly.
141-
if (!Object.prototype.hasOwnProperty.call(mountAt, topListenersIDKey)) {
142-
mountAt[topListenersIDKey] = reactTopListenersCounter++;
143-
alreadyListeningTo[mountAt[topListenersIDKey]] = {};
140+
if (!Object.prototype.hasOwnProperty.call(mountAt, eventsKey)) {
141+
mountAt[eventsKey] = {};
142+
}
143+
return mountAt[eventsKey];
144+
}
145+
146+
function removeListenedEvents(mountAt) {
147+
if (!Object.prototype.hasOwnProperty.call(mountAt, eventsKey)) {
148+
warning(
149+
true,
150+
'Tried to remove a React root level listener, but it was not found.'
151+
);
152+
return;
144153
}
145-
return alreadyListeningTo[mountAt[topListenersIDKey]];
154+
delete mountAt[eventsKey];
146155
}
147156

148157
/**
@@ -196,7 +205,7 @@ var ReactBrowserEventEmitter = merge(ReactEventEmitterMixin, {
196205
},
197206

198207
/**
199-
* We listen for bubbled touch events on the document object.
208+
* We listen for bubbled touch events on a root container.
200209
*
201210
* Firefox v8.01 (and possibly others) exhibited strange behavior when
202211
* mounting `onmousemove` events at some node that was not the document
@@ -216,36 +225,37 @@ var ReactBrowserEventEmitter = merge(ReactEventEmitterMixin, {
216225
* @param {string} registrationName Name of listener (e.g. `onClick`).
217226
* @param {object} contentDocumentHandle Document which owns the container
218227
*/
219-
listenTo: function(registrationName, contentDocumentHandle) {
220-
var mountAt = contentDocumentHandle;
221-
var isListening = getListeningForDocument(mountAt);
222-
var dependencies = EventPluginRegistry.
223-
registrationNameDependencies[registrationName];
228+
listenTo: function(registrationName, mountAt) {
229+
var events = getListenedEvents(mountAt);
230+
var dependencies =
231+
EventPluginRegistry.registrationNameDependencies[registrationName];
224232

225233
var topLevelTypes = EventConstants.topLevelTypes;
226234
for (var i = 0, l = dependencies.length; i < l; i++) {
235+
// `events` is a mapping of dependency -> event. The map does two
236+
// things: store the fact that a dependency has already been registered,
237+
// and store the event for later removal when the node's unmounted.
227238
var dependency = dependencies[i];
228-
if (!(
229-
isListening.hasOwnProperty(dependency) &&
230-
isListening[dependency]
231-
)) {
239+
var event;
240+
241+
if (!events.hasOwnProperty(dependency) || events[dependency] == null) {
232242
if (dependency === topLevelTypes.topWheel) {
233243
if (isEventSupported('wheel')) {
234-
ReactBrowserEventEmitter.ReactEventListener.trapBubbledEvent(
244+
event = ReactBrowserEventEmitter.trapBubbledEvent(
235245
topLevelTypes.topWheel,
236246
'wheel',
237247
mountAt
238248
);
239249
} else if (isEventSupported('mousewheel')) {
240-
ReactBrowserEventEmitter.ReactEventListener.trapBubbledEvent(
250+
event = ReactBrowserEventEmitter.trapBubbledEvent(
241251
topLevelTypes.topWheel,
242252
'mousewheel',
243253
mountAt
244254
);
245255
} else {
246256
// Firefox needs to capture a different mouse scroll event.
247257
// @see http://www.quirksmode.org/dom/events/tests/scroll.html
248-
ReactBrowserEventEmitter.ReactEventListener.trapBubbledEvent(
258+
event = ReactBrowserEventEmitter.trapBubbledEvent(
249259
topLevelTypes.topWheel,
250260
'DOMMouseScroll',
251261
mountAt
@@ -254,61 +264,83 @@ var ReactBrowserEventEmitter = merge(ReactEventEmitterMixin, {
254264
} else if (dependency === topLevelTypes.topScroll) {
255265

256266
if (isEventSupported('scroll', true)) {
257-
ReactBrowserEventEmitter.ReactEventListener.trapCapturedEvent(
267+
event = ReactBrowserEventEmitter.trapCapturedEvent(
258268
topLevelTypes.topScroll,
259269
'scroll',
260270
mountAt
261271
);
262272
} else {
263-
ReactBrowserEventEmitter.ReactEventListener.trapBubbledEvent(
273+
event = ReactBrowserEventEmitter.trapBubbledEvent(
264274
topLevelTypes.topScroll,
265275
'scroll',
266-
ReactBrowserEventEmitter.ReactEventListener.WINDOW_HANDLE
276+
ReactBrowserEventEmitter.WINDOW_HANDLE
267277
);
268278
}
269279
} else if (dependency === topLevelTypes.topFocus ||
270280
dependency === topLevelTypes.topBlur) {
281+
var event2;
271282

272283
if (isEventSupported('focus', true)) {
273-
ReactBrowserEventEmitter.ReactEventListener.trapCapturedEvent(
284+
event = ReactBrowserEventEmitter.trapCapturedEvent(
274285
topLevelTypes.topFocus,
275286
'focus',
276287
mountAt
277288
);
278-
ReactBrowserEventEmitter.ReactEventListener.trapCapturedEvent(
289+
event2 = ReactBrowserEventEmitter.trapCapturedEvent(
279290
topLevelTypes.topBlur,
280291
'blur',
281292
mountAt
282293
);
283294
} else if (isEventSupported('focusin')) {
284295
// IE has `focusin` and `focusout` events which bubble.
285296
// @see http://www.quirksmode.org/blog/archives/2008/04/delegating_the.html
286-
ReactBrowserEventEmitter.ReactEventListener.trapBubbledEvent(
297+
event = ReactBrowserEventEmitter.trapBubbledEvent(
287298
topLevelTypes.topFocus,
288299
'focusin',
289300
mountAt
290301
);
291-
ReactBrowserEventEmitter.ReactEventListener.trapBubbledEvent(
302+
event2 = ReactBrowserEventEmitter.trapBubbledEvent(
292303
topLevelTypes.topBlur,
293304
'focusout',
294305
mountAt
295306
);
296307
}
297308

298309
// to make sure blur and focus event listeners are only attached once
299-
isListening[topLevelTypes.topBlur] = true;
300-
isListening[topLevelTypes.topFocus] = true;
310+
events[topLevelTypes.topFocus] = event;
311+
events[topLevelTypes.topBlur] = event2;
301312
} else if (topEventMapping.hasOwnProperty(dependency)) {
302-
ReactBrowserEventEmitter.ReactEventListener.trapBubbledEvent(
313+
event = ReactBrowserEventEmitter.trapBubbledEvent(
303314
dependency,
304315
topEventMapping[dependency],
305316
mountAt
306317
);
307318
}
319+
// As mentioned above, events like `submit` don't bubble to document and
320+
// thus are not attached to it. In that case, there's no `event` (and a
321+
// `remove`) to store. We'll put a `true` placeholder here.
322+
events[dependency] = event || true;
323+
}
324+
}
325+
},
308326

309-
isListening[dependency] = true;
327+
removeListenedEvents: function(container) {
328+
var events = getListenedEvents(container);
329+
if (!events) {
330+
// Might be that no event was (lazily) added in the first place.
331+
return;
332+
}
333+
for (var key in events) {
334+
if (!events.hasOwnProperty(key)) {
335+
continue;
336+
}
337+
if (events[key].remove) {
338+
// See `listenTo`. The event might be a `true` placeholder for things
339+
// like `onSubmit`.
340+
events[key].remove();
310341
}
311342
}
343+
removeListenedEvents(container);
312344
},
313345

314346
trapBubbledEvent: function(topLevelType, handlerBaseName, handle) {

src/browser/__tests__/ReactBrowserEventEmitter-test.js

Lines changed: 38 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -19,14 +19,15 @@
1919
"use strict";
2020

2121
require('mock-modules')
22-
.dontMock('EventPluginHub')
23-
.dontMock('ReactMount')
24-
.dontMock('ReactBrowserEventEmitter')
25-
.dontMock('ReactInstanceHandles')
26-
.dontMock('EventPluginHub')
27-
.dontMock('TapEventPlugin')
28-
.dontMock('TouchEventUtils')
29-
.dontMock('keyOf');
22+
.dontMock('EventListener')
23+
.dontMock('EventPluginHub')
24+
.dontMock('keyOf')
25+
.dontMock('ReactBrowserEventEmitter')
26+
.dontMock('ReactEventListener')
27+
.dontMock('ReactInstanceHandles')
28+
.dontMock('ReactMount')
29+
.dontMock('TapEventPlugin')
30+
.dontMock('TouchEventUtils');
3031

3132

3233
var keyOf = require('keyOf');
@@ -373,26 +374,47 @@ describe('ReactBrowserEventEmitter', function() {
373374
expect(idCallOrder.length).toBe(0);
374375
});
375376

377+
it('should attach the event to the root container', function() {
378+
var div = document.createElement('div');
379+
ReactBrowserEventEmitter.listenTo(ON_CLICK_KEY, div);
380+
expect(div._reactEvents.topClick.remove).toBeDefined();
381+
});
382+
383+
it('should be able to remove listeners on the root container', function() {
384+
var div = document.createElement('div');
385+
spyOn(div, 'removeEventListener').andCallThrough();
386+
ReactBrowserEventEmitter.listenTo(ON_CLICK_KEY, div);
387+
ReactBrowserEventEmitter.listenTo(ON_CHANGE_KEY, div);
388+
ReactBrowserEventEmitter.removeListenedEvents(div);
389+
// Once for click, 7 times for change.
390+
expect(div.removeEventListener.argsForCall.length).toBe(8);
391+
expect(div._reactEvents).toBe(undefined);
392+
});
393+
394+
376395
it('should listen to events only once', function() {
377-
spyOn(EventListener, 'listen');
378-
ReactBrowserEventEmitter.listenTo(ON_CLICK_KEY, document);
379-
ReactBrowserEventEmitter.listenTo(ON_CLICK_KEY, document);
396+
spyOn(EventListener, 'listen').andCallThrough();
397+
var div = document.createElement('div');
398+
ReactBrowserEventEmitter.listenTo(ON_CLICK_KEY, div);
399+
ReactBrowserEventEmitter.listenTo(ON_CLICK_KEY, div);
380400
expect(EventListener.listen.callCount).toBe(1);
381401
});
382402

383403
it('should work with event plugins without dependencies', function() {
384404
spyOn(EventListener, 'listen');
385405

386-
ReactBrowserEventEmitter.listenTo(ON_CLICK_KEY, document);
406+
var div = document.createElement('div');
407+
ReactBrowserEventEmitter.listenTo(ON_CLICK_KEY, div);
387408

388409
expect(EventListener.listen.argsForCall[0][1]).toBe('click');
389410
});
390411

391412
it('should work with event plugins with dependencies', function() {
392-
spyOn(EventListener, 'listen');
393-
spyOn(EventListener, 'capture');
413+
spyOn(EventListener, 'listen').andCallThrough();
414+
spyOn(EventListener, 'capture').andCallThrough();
394415

395-
ReactBrowserEventEmitter.listenTo(ON_CHANGE_KEY, document);
416+
var div = document.createElement('div');
417+
ReactBrowserEventEmitter.listenTo(ON_CHANGE_KEY, div);
396418

397419
var setEventListeners = [];
398420
var listenCalls = EventListener.listen.argsForCall;
@@ -405,7 +427,7 @@ describe('ReactBrowserEventEmitter', function() {
405427
}
406428

407429
var module =
408-
ReactBrowserEventEmitter.registrationNameModules[ON_CHANGE_KEY];
430+
ReactBrowserEventEmitter.registrationNameModules[ON_CHANGE_KEY];
409431
var dependencies = module.eventTypes.change.dependencies;
410432
expect(setEventListeners.length).toEqual(dependencies.length);
411433

src/browser/ui/ReactDOMComponent.js

Lines changed: 2 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -35,17 +35,13 @@ var keyOf = require('keyOf');
3535
var merge = require('merge');
3636
var mixInto = require('mixInto');
3737

38-
var deleteListener = ReactBrowserEventEmitter.deleteListener;
39-
var listenTo = ReactBrowserEventEmitter.listenTo;
4038
var registrationNameModules = ReactBrowserEventEmitter.registrationNameModules;
4139

4240
// For quickly matching children type, to test if can be treated as content.
4341
var CONTENT_TYPES = {'string': true, 'number': true};
4442

4543
var STYLE = keyOf({style: null});
4644

47-
var ELEMENT_NODE_TYPE = 1;
48-
4945
/**
5046
* @param {?object} props
5147
*/
@@ -68,10 +64,7 @@ function assertValidProps(props) {
6864
function putListener(id, registrationName, listener, transaction) {
6965
var container = ReactMount.findReactContainerForID(id);
7066
if (container) {
71-
var doc = container.nodeType === ELEMENT_NODE_TYPE ?
72-
container.ownerDocument :
73-
container;
74-
listenTo(registrationName, doc);
67+
ReactBrowserEventEmitter.listenTo(registrationName, container);
7568
}
7669
transaction.getPutListenerQueue().enqueuePutListener(
7770
id,
@@ -283,7 +276,7 @@ ReactDOMComponent.Mixin = {
283276
}
284277
}
285278
} else if (registrationNameModules.hasOwnProperty(propKey)) {
286-
deleteListener(this._rootNodeID, propKey);
279+
ReactBrowserEventEmitter.deleteListener(this._rootNodeID, propKey);
287280
} else if (
288281
DOMProperty.isStandardName[propKey] ||
289282
DOMProperty.isCustomAttribute(propKey)) {

src/browser/ui/ReactMount.js

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -461,6 +461,8 @@ var ReactMount = {
461461
if (!component) {
462462
return false;
463463
}
464+
465+
ReactBrowserEventEmitter.removeListenedEvents(container);
464466
ReactMount.unmountComponentFromNode(component, container);
465467
delete instancesByReactRootID[reactRootID];
466468
delete containersByReactRootID[reactRootID];

src/vendor/stubs/EventListener.js

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,8 @@
33
* @typechecks
44
*/
55

6+
'use strict';
7+
68
var emptyFunction = require('emptyFunction');
79

810
/**

0 commit comments

Comments
 (0)