From 95334fada0cd71c2f748bf306368f81fc4ef0d01 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sebastian=20Markb=C3=A5ge?= Date: Mon, 7 Nov 2016 11:30:34 -0800 Subject: [PATCH] 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. --- scripts/fiber/tests-failing.txt | 1 + scripts/fiber/tests-passing.txt | 4 +--- .../dom/shared/ReactBrowserEventEmitter.js | 16 +++++++++++++ .../shared/eventPlugins/SelectEventPlugin.js | 23 +++++++++++-------- .../__tests__/SelectEventPlugin-test.js | 10 ++++++++ 5 files changed, 42 insertions(+), 12 deletions(-) diff --git a/scripts/fiber/tests-failing.txt b/scripts/fiber/tests-failing.txt index 7685670ee9..8ee4590825 100644 --- a/scripts/fiber/tests-failing.txt +++ b/scripts/fiber/tests-failing.txt @@ -221,6 +221,7 @@ src/renderers/dom/shared/eventPlugins/__tests__/EnterLeaveEventPlugin-test.js * should set relatedTarget properly in iframe src/renderers/dom/shared/eventPlugins/__tests__/SelectEventPlugin-test.js +* should skip extraction if no listeners are present * should extract if an `onSelect` listener is present src/renderers/dom/shared/eventPlugins/__tests__/SimpleEventPlugin-test.js diff --git a/scripts/fiber/tests-passing.txt b/scripts/fiber/tests-passing.txt index dc27a2cea8..90abd85ab3 100644 --- a/scripts/fiber/tests-passing.txt +++ b/scripts/fiber/tests-passing.txt @@ -590,9 +590,6 @@ src/renderers/dom/shared/eventPlugins/__tests__/FallbackCompositionState-test.js * extracts when inserted within text * extracts when inserted at end of text -src/renderers/dom/shared/eventPlugins/__tests__/SelectEventPlugin-test.js -* should skip extraction if no listeners are present - src/renderers/dom/shared/eventPlugins/__tests__/SimpleEventPlugin-test.js * does not add a local click to interactive elements * adds a local click listener to non-interactive elements @@ -744,6 +741,7 @@ src/renderers/native/__tests__/ReactNativeEvents-test.js src/renderers/native/__tests__/ReactNativeMount-test.js * should be able to create and render a native component * should be able to create and update a native component +* should be able to create and update a native component src/renderers/shared/__tests__/ReactDebugTool-test.js * should add and remove hooks diff --git a/src/renderers/dom/shared/ReactBrowserEventEmitter.js b/src/renderers/dom/shared/ReactBrowserEventEmitter.js index 0d6a3b7a46..86384f561b 100644 --- a/src/renderers/dom/shared/ReactBrowserEventEmitter.js +++ b/src/renderers/dom/shared/ReactBrowserEventEmitter.js @@ -328,6 +328,22 @@ var ReactBrowserEventEmitter = Object.assign({}, ReactEventEmitterMixin, { } }, + isListeningToAllDependencies: function(registrationName, mountAt) { + var isListening = getListeningForDocument(mountAt); + var dependencies = + EventPluginRegistry.registrationNameDependencies[registrationName]; + for (var i = 0; i < dependencies.length; i++) { + var dependency = dependencies[i]; + if (!( + isListening.hasOwnProperty(dependency) && + isListening[dependency] + )) { + return false; + } + } + return true; + }, + trapBubbledEvent: function(topLevelType, handlerBaseName, handle) { return ReactBrowserEventEmitter.ReactEventListener.trapBubbledEvent( topLevelType, diff --git a/src/renderers/dom/shared/eventPlugins/SelectEventPlugin.js b/src/renderers/dom/shared/eventPlugins/SelectEventPlugin.js index a4467c72a2..2a85508fda 100644 --- a/src/renderers/dom/shared/eventPlugins/SelectEventPlugin.js +++ b/src/renderers/dom/shared/eventPlugins/SelectEventPlugin.js @@ -13,6 +13,7 @@ var EventPropagators = require('EventPropagators'); var ExecutionEnvironment = require('ExecutionEnvironment'); +var ReactBrowserEventEmitter = require('ReactBrowserEventEmitter'); var ReactDOMComponentTree = require('ReactDOMComponentTree'); var ReactInputSelection = require('ReactInputSelection'); var SyntheticEvent = require('SyntheticEvent'); @@ -21,6 +22,9 @@ var getActiveElement = require('getActiveElement'); var isTextInputElement = require('isTextInputElement'); var shallowEqual = require('shallowEqual'); +// Node type for document fragments (Node.DOCUMENT_FRAGMENT_NODE). +var DOC_FRAGMENT_TYPE = 11; + var skipSelectionChangeEvent = ( ExecutionEnvironment.canUseDOM && 'documentMode' in document && @@ -51,9 +55,10 @@ var activeElementInst = null; var lastSelection = null; var mouseDown = false; -// Track whether a listener exists for this plugin. If none exist, we do +// Track whether all listeners exists for this plugin. If none exist, we do // not extract events. See #3639. -var hasListener = false; +var isListeningToAllDependencies = + ReactBrowserEventEmitter.isListeningToAllDependencies; /** * Get an object which is a unique representation of the current selection. @@ -154,8 +159,13 @@ var SelectEventPlugin = { nativeEvent, nativeEventTarget ) { - if (!hasListener) { - return null; + if (targetInst) { + var containerInfo = targetInst._hostContainerInfo; + var isDocumentFragment = containerInfo._node && containerInfo._node.nodeType === DOC_FRAGMENT_TYPE; + var doc = isDocumentFragment ? containerInfo._node : containerInfo._ownerDocument; + if (!isListeningToAllDependencies('onSelect', doc)) { + return null; + } } var targetNode = targetInst ? @@ -209,11 +219,6 @@ var SelectEventPlugin = { return null; }, - didPutListener: function(inst, registrationName, listener) { - if (registrationName === 'onSelect') { - hasListener = true; - } - }, }; module.exports = SelectEventPlugin; diff --git a/src/renderers/dom/shared/eventPlugins/__tests__/SelectEventPlugin-test.js b/src/renderers/dom/shared/eventPlugins/__tests__/SelectEventPlugin-test.js index d8d79f7082..083dd1446e 100644 --- a/src/renderers/dom/shared/eventPlugins/__tests__/SelectEventPlugin-test.js +++ b/src/renderers/dom/shared/eventPlugins/__tests__/SelectEventPlugin-test.js @@ -46,6 +46,16 @@ describe('SelectEventPlugin', () => { var node = ReactDOM.findDOMNode(rendered); node.focus(); + // It seems that .focus() isn't triggering this event in our test + // environment so we need to ensure it gets set for this test to be valid. + var fakeNativeEvent = new function() {}; + fakeNativeEvent.target = node; + ReactTestUtils.simulateNativeEventOnNode( + 'topFocus', + node, + fakeNativeEvent + ); + var mousedown = extract(node, 'topMouseDown'); expect(mousedown).toBe(null);