@samitouri / QOS-React / commits / 95334fada0

Track if SelectEventPlugin is attached on a per document basis (#8190)

This gets rid of the global flag on if something has listened to onSelect and instead reads the isListening map if all the events are covered. This is required if we want to attach events locally at roots. Could be slower perf wise to handle events. An alternative solution would be to attach a special flag on the listener map for the document so we don't have to check the full dependency list. However, my favorite solution would be to just eagerly attach all event listeners (except maybe wheel). Then we don't have to do any of this stuff on a per element basis.

Sebastian Markbåge committed Nov 7, 2016 at 11:30 UTC 95334fada0cd71c2f748bf306368f81fc4ef0d01
5 files changed +42 -12
scripts/fiber/tests-failing.txt
+1
@@ -221,6 +221,7 @@ src/renderers/dom/shared/eventPlugins/__tests__/EnterLeaveEventPlugin-test.js
221 * should set relatedTarget properly in iframe
222
223 src/renderers/dom/shared/eventPlugins/__tests__/SelectEventPlugin-test.js
224 +* should skip extraction if no listeners are present
225 * should extract if an `onSelect` listener is present
226
227 src/renderers/dom/shared/eventPlugins/__tests__/SimpleEventPlugin-test.js
scripts/fiber/tests-passing.txt
+1 -3
@@ -590,9 +590,6 @@ src/renderers/dom/shared/eventPlugins/__tests__/FallbackCompositionState-test.js
590 * extracts when inserted within text
591 * extracts when inserted at end of text
592
593 -src/renderers/dom/shared/eventPlugins/__tests__/SelectEventPlugin-test.js
594 -* should skip extraction if no listeners are present
595 -
593 src/renderers/dom/shared/eventPlugins/__tests__/SimpleEventPlugin-test.js
594 * does not add a local click to interactive elements
595 * adds a local click listener to non-interactive elements
@@ -744,6 +741,7 @@ src/renderers/native/__tests__/ReactNativeEvents-test.js
741 src/renderers/native/__tests__/ReactNativeMount-test.js
742 * should be able to create and render a native component
743 * should be able to create and update a native component
744 +* should be able to create and update a native component
745
746 src/renderers/shared/__tests__/ReactDebugTool-test.js
747 * should add and remove hooks
src/renderers/dom/shared/ReactBrowserEventEmitter.js
+16
@@ -328,6 +328,22 @@ var ReactBrowserEventEmitter = Object.assign({}, ReactEventEmitterMixin, {
328 }
329 },
330
331 + isListeningToAllDependencies: function(registrationName, mountAt) {
332 + var isListening = getListeningForDocument(mountAt);
333 + var dependencies =
334 + EventPluginRegistry.registrationNameDependencies[registrationName];
335 + for (var i = 0; i < dependencies.length; i++) {
336 + var dependency = dependencies[i];
337 + if (!(
338 + isListening.hasOwnProperty(dependency) &&
339 + isListening[dependency]
340 + )) {
341 + return false;
342 + }
343 + }
344 + return true;
345 + },
346 +
347 trapBubbledEvent: function(topLevelType, handlerBaseName, handle) {
348 return ReactBrowserEventEmitter.ReactEventListener.trapBubbledEvent(
349 topLevelType,
src/renderers/dom/shared/eventPlugins/SelectEventPlugin.js
+14 -9
@@ -13,6 +13,7 @@
13
14 var EventPropagators = require('EventPropagators');
15 var ExecutionEnvironment = require('ExecutionEnvironment');
16 +var ReactBrowserEventEmitter = require('ReactBrowserEventEmitter');
17 var ReactDOMComponentTree = require('ReactDOMComponentTree');
18 var ReactInputSelection = require('ReactInputSelection');
19 var SyntheticEvent = require('SyntheticEvent');
@@ -21,6 +22,9 @@ var getActiveElement = require('getActiveElement');
22 var isTextInputElement = require('isTextInputElement');
23 var shallowEqual = require('shallowEqual');
24
25 +// Node type for document fragments (Node.DOCUMENT_FRAGMENT_NODE).
26 +var DOC_FRAGMENT_TYPE = 11;
27 +
28 var skipSelectionChangeEvent = (
29 ExecutionEnvironment.canUseDOM &&
30 'documentMode' in document &&
@@ -51,9 +55,10 @@ var activeElementInst = null;
55 var lastSelection = null;
56 var mouseDown = false;
57
54 -// Track whether a listener exists for this plugin. If none exist, we do
58 +// Track whether all listeners exists for this plugin. If none exist, we do
59 // not extract events. See #3639.
56 -var hasListener = false;
60 +var isListeningToAllDependencies =
61 + ReactBrowserEventEmitter.isListeningToAllDependencies;
62
63 /**
64 * Get an object which is a unique representation of the current selection.
@@ -154,8 +159,13 @@ var SelectEventPlugin = {
159 nativeEvent,
160 nativeEventTarget
161 ) {
157 - if (!hasListener) {
158 - return null;
162 + if (targetInst) {
163 + var containerInfo = targetInst._hostContainerInfo;
164 + var isDocumentFragment = containerInfo._node && containerInfo._node.nodeType === DOC_FRAGMENT_TYPE;
165 + var doc = isDocumentFragment ? containerInfo._node : containerInfo._ownerDocument;
166 + if (!isListeningToAllDependencies('onSelect', doc)) {
167 + return null;
168 + }
169 }
170
171 var targetNode = targetInst ?
@@ -209,11 +219,6 @@ var SelectEventPlugin = {
219 return null;
220 },
221
212 - didPutListener: function(inst, registrationName, listener) {
213 - if (registrationName === 'onSelect') {
214 - hasListener = true;
215 - }
216 - },
222 };
223
224 module.exports = SelectEventPlugin;
src/renderers/dom/shared/eventPlugins/__tests__/SelectEventPlugin-test.js
+10
@@ -46,6 +46,16 @@ describe('SelectEventPlugin', () => {
46 var node = ReactDOM.findDOMNode(rendered);
47 node.focus();
48
49 + // It seems that .focus() isn't triggering this event in our test
50 + // environment so we need to ensure it gets set for this test to be valid.
51 + var fakeNativeEvent = new function() {};
52 + fakeNativeEvent.target = node;
53 + ReactTestUtils.simulateNativeEventOnNode(
54 + 'topFocus',
55 + node,
56 + fakeNativeEvent
57 + );
58 +
59 var mousedown = extract(node, 'topMouseDown');
60 expect(mousedown).toBe(null);
61