From 1d2b54c1e13add5cc21e15667584e0360b2b6413 Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Mon, 7 Nov 2016 11:29:25 -0800 Subject: [PATCH] Track if SelectEventPlugin is attached on a per document basis 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 7685670ee914..8ee4590825ba 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 dc27a2cea888..90abd85ab38a 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 0d6a3b7a46db..86384f561bc9 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 a4467c72a287..2a85508fdad8 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 d8d79f708225..083dd1446e04 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);