diff --git a/scripts/fiber/tests-failing.txt b/scripts/fiber/tests-failing.txt index 7955d43a14..ab6ebb9e4c 100644 --- a/scripts/fiber/tests-failing.txt +++ b/scripts/fiber/tests-failing.txt @@ -147,6 +147,8 @@ src/renderers/dom/shared/__tests__/ReactDOMComponent-test.js * should validate against invalid styles * should track input values * should track textarea values +* should execute custom event plugin listening behavior +* should handle null and missing properly with event hooks * should warn for children on void elements * should support custom elements which extend native elements * should warn against children for void elements @@ -155,6 +157,7 @@ src/renderers/dom/shared/__tests__/ReactDOMComponent-test.js * should warn about contentEditable and children * should validate against invalid styles * should report component containing invalid styles +* should clean up listeners * should clean up input value tracking * should clean up input textarea tracking * should warn about the `onScroll` issue when unsupported (IE8) @@ -379,11 +382,13 @@ src/renderers/dom/stack/server/__tests__/ReactServerRendering-test.js * should generate simple markup for self-closing tags * should generate simple markup for attribute with `>` symbol * should generate comment markup for component returns null +* should not register event listeners * should render composite components * should only execute certain lifecycle methods * should have the correct mounting behavior * should not put checksum and React ID on components * should not put checksum and React ID on text components +* should not register event listeners * should only execute certain lifecycle methods * allows setState in componentWillMount without using DOM * renders components with different batching strategies @@ -498,9 +503,6 @@ src/renderers/shared/shared/__tests__/ReactTreeTraversal-test.js * should leave to the window * should leave to the window from the shallowest -src/renderers/shared/shared/event/__tests__/EventPluginHub-test.js -* should prevent non-function listeners, at dispatch - src/renderers/shared/stack/reconciler/__tests__/ReactChildReconciler-test.js * warns for duplicated keys * warns for duplicated keys with component stack info diff --git a/scripts/fiber/tests-passing.txt b/scripts/fiber/tests-passing.txt index 055be3e8d9..08d7f5dd29 100644 --- a/scripts/fiber/tests-passing.txt +++ b/scripts/fiber/tests-passing.txt @@ -955,6 +955,9 @@ src/renderers/shared/shared/__tests__/ReactTreeTraversal-test.js * should not traverse if enter/leave the same node * should determine the first common ancestor correctly +src/renderers/shared/shared/event/__tests__/EventPluginHub-test.js +* should prevent non-function listeners + src/renderers/shared/shared/event/__tests__/EventPluginRegistry-test.js * should be able to inject ordering before plugins * should be able to inject plugins before and after ordering @@ -1059,6 +1062,7 @@ src/renderers/shared/stack/reconciler/__tests__/ReactErrorBoundaries-test.js * propagates errors on retry on mounting * propagates errors inside boundary during componentWillMount * propagates errors inside boundary while rendering error state +* does not register event handlers for unmounted children * does not call componentWillUnmount when aborting initial mount * resets refs if mounting aborts * successfully mounts if no error occurs diff --git a/src/renderers/dom/shared/ReactBrowserEventEmitter.js b/src/renderers/dom/shared/ReactBrowserEventEmitter.js index c92804cd47..86384f561b 100644 --- a/src/renderers/dom/shared/ReactBrowserEventEmitter.js +++ b/src/renderers/dom/shared/ReactBrowserEventEmitter.js @@ -163,6 +163,16 @@ function getListeningForDocument(mountAt) { return alreadyListeningTo[mountAt[topListenersIDKey]]; } +/** + * `ReactBrowserEventEmitter` is used to attach top-level event listeners. For + * example: + * + * EventPluginHub.putListener('myID', 'onClick', myFunction); + * + * This would allocate a "registration" of `('onClick', myFunction)` on 'myID'. + * + * @internal + */ var ReactBrowserEventEmitter = Object.assign({}, ReactEventEmitterMixin, { /** diff --git a/src/renderers/dom/shared/__tests__/ReactBrowserEventEmitter-test.js b/src/renderers/dom/shared/__tests__/ReactBrowserEventEmitter-test.js index ea2ed81c94..d76d4868a3 100644 --- a/src/renderers/dom/shared/__tests__/ReactBrowserEventEmitter-test.js +++ b/src/renderers/dom/shared/__tests__/ReactBrowserEventEmitter-test.js @@ -15,9 +15,8 @@ var EventListener; var EventPluginHub; var EventPluginRegistry; var React; -var ReactDOM; -var ReactDOMComponentTree; var ReactBrowserEventEmitter; +var ReactDOMComponentTree; var ReactTestUtils; var TapEventPlugin; @@ -44,17 +43,18 @@ var GRANDPARENT; var PARENT; var CHILD; -var getListener; -var putListener; -var deleteAllListeners; - function registerSimpleTestHandler() { - putListener(CHILD, ON_CLICK_KEY, LISTENER); - var listener = getListener(CHILD, ON_CLICK_KEY); + EventPluginHub.putListener(getInternal(CHILD), ON_CLICK_KEY, LISTENER); + var listener = EventPluginHub.getListener(getInternal(CHILD), ON_CLICK_KEY); expect(listener).toEqual(LISTENER); - return getListener(CHILD, ON_CLICK_KEY); + return EventPluginHub.getListener(getInternal(CHILD), ON_CLICK_KEY); } +function getInternal(node) { + return ReactDOMComponentTree.getInstanceFromNode(node); +} + + describe('ReactBrowserEventEmitter', () => { beforeEach(() => { jest.resetModuleRegistry(); @@ -63,67 +63,18 @@ describe('ReactBrowserEventEmitter', () => { EventPluginHub = require('EventPluginHub'); EventPluginRegistry = require('EventPluginRegistry'); React = require('React'); - ReactDOM = require('ReactDOM'); - ReactDOMComponentTree = require('ReactDOMComponentTree'); ReactBrowserEventEmitter = require('ReactBrowserEventEmitter'); + ReactDOMComponentTree = require('ReactDOMComponentTree'); ReactTestUtils = require('ReactTestUtils'); TapEventPlugin = require('TapEventPlugin'); - var container = document.createElement('div'); - - var GRANDPARENT_PROPS = {}; - var PARENT_PROPS = {}; - var CHILD_PROPS = {}; - - function renderTree() { - ReactDOM.render( -
GRANDPARENT = c} {...GRANDPARENT_PROPS}> -
PARENT = c} {...PARENT_PROPS}> -
CHILD = c} {...CHILD_PROPS} /> -
-
, - container - ); - } - - renderTree(); - - getListener = function(node, eventName) { - var inst = ReactDOMComponentTree.getInstanceFromNode(node); - return EventPluginHub.getListener( - inst, - eventName - ); - }; - putListener = function(node, eventName, listener) { - switch (node) { - case CHILD: - CHILD_PROPS[eventName] = listener; - break; - case PARENT: - PARENT_PROPS[eventName] = listener; - break; - case GRANDPARENT: - GRANDPARENT_PROPS[eventName] = listener; - break; - } - // Rerender with new event listeners - renderTree(); - }; - deleteAllListeners = function(node) { - switch (node) { - case CHILD: - CHILD_PROPS = {}; - break; - case PARENT: - PARENT_PROPS = {}; - break; - case GRANDPARENT: - GRANDPARENT_PROPS = {}; - break; - } - renderTree(); - }; + ReactTestUtils.renderIntoDocument( +
GRANDPARENT = c}> +
PARENT = c}> +
CHILD = c} /> +
+
+ ); idCallOrder = []; tapMoveThreshold = TapEventPlugin.tapMoveThreshold; @@ -134,20 +85,20 @@ describe('ReactBrowserEventEmitter', () => { it('should store a listener correctly', () => { registerSimpleTestHandler(); - var listener = getListener(CHILD, ON_CLICK_KEY); + var listener = EventPluginHub.getListener(getInternal(CHILD), ON_CLICK_KEY); expect(listener).toBe(LISTENER); }); it('should retrieve a listener correctly', () => { registerSimpleTestHandler(); - var listener = getListener(CHILD, ON_CLICK_KEY); + var listener = EventPluginHub.getListener(getInternal(CHILD), ON_CLICK_KEY); expect(listener).toEqual(LISTENER); }); it('should clear all handlers when asked to', () => { registerSimpleTestHandler(); - deleteAllListeners(CHILD); - var listener = getListener(CHILD, ON_CLICK_KEY); + EventPluginHub.deleteAllListeners(getInternal(CHILD)); + var listener = EventPluginHub.getListener(getInternal(CHILD), ON_CLICK_KEY); expect(listener).toBe(undefined); }); @@ -171,153 +122,153 @@ describe('ReactBrowserEventEmitter', () => { ); it('should bubble simply', () => { - putListener( - CHILD, + EventPluginHub.putListener( + getInternal(CHILD), ON_CLICK_KEY, - recordID.bind(null, CHILD) + recordID.bind(null, getInternal(CHILD)) ); - putListener( - PARENT, + EventPluginHub.putListener( + getInternal(PARENT), ON_CLICK_KEY, - recordID.bind(null, PARENT) + recordID.bind(null, getInternal(PARENT)) ); - putListener( - GRANDPARENT, + EventPluginHub.putListener( + getInternal(GRANDPARENT), ON_CLICK_KEY, - recordID.bind(null, GRANDPARENT) + recordID.bind(null, getInternal(GRANDPARENT)) ); ReactTestUtils.Simulate.click(CHILD); expect(idCallOrder.length).toBe(3); - expect(idCallOrder[0]).toBe(CHILD); - expect(idCallOrder[1]).toBe(PARENT); - expect(idCallOrder[2]).toBe(GRANDPARENT); + expect(idCallOrder[0]).toBe(getInternal(CHILD)); + expect(idCallOrder[1]).toBe(getInternal(PARENT)); + expect(idCallOrder[2]).toBe(getInternal(GRANDPARENT)); }); it('should continue bubbling if an error is thrown', () => { - putListener( - CHILD, + EventPluginHub.putListener( + getInternal(CHILD), ON_CLICK_KEY, - recordID.bind(null, CHILD) + recordID.bind(null, getInternal(CHILD)) ); - putListener( - PARENT, + EventPluginHub.putListener( + getInternal(PARENT), ON_CLICK_KEY, function() { - recordID(PARENT); + recordID(getInternal(PARENT)); throw new Error('Handler interrupted'); } ); - putListener( - GRANDPARENT, + EventPluginHub.putListener( + getInternal(GRANDPARENT), ON_CLICK_KEY, - recordID.bind(null, GRANDPARENT) + recordID.bind(null, getInternal(GRANDPARENT)) ); expect(function() { ReactTestUtils.Simulate.click(CHILD); }).toThrow(); expect(idCallOrder.length).toBe(3); - expect(idCallOrder[0]).toBe(CHILD); - expect(idCallOrder[1]).toBe(PARENT); - expect(idCallOrder[2]).toBe(GRANDPARENT); + expect(idCallOrder[0]).toBe(getInternal(CHILD)); + expect(idCallOrder[1]).toBe(getInternal(PARENT)); + expect(idCallOrder[2]).toBe(getInternal(GRANDPARENT)); }); it('should set currentTarget', () => { - putListener( - CHILD, + EventPluginHub.putListener( + getInternal(CHILD), ON_CLICK_KEY, function(event) { - recordID(CHILD); + recordID(getInternal(CHILD)); expect(event.currentTarget).toBe(CHILD); } ); - putListener( - PARENT, + EventPluginHub.putListener( + getInternal(PARENT), ON_CLICK_KEY, function(event) { - recordID(PARENT); + recordID(getInternal(PARENT)); expect(event.currentTarget).toBe(PARENT); } ); - putListener( - GRANDPARENT, + EventPluginHub.putListener( + getInternal(GRANDPARENT), ON_CLICK_KEY, function(event) { - recordID(GRANDPARENT); + recordID(getInternal(GRANDPARENT)); expect(event.currentTarget).toBe(GRANDPARENT); } ); ReactTestUtils.Simulate.click(CHILD); expect(idCallOrder.length).toBe(3); - expect(idCallOrder[0]).toBe(CHILD); - expect(idCallOrder[1]).toBe(PARENT); - expect(idCallOrder[2]).toBe(GRANDPARENT); + expect(idCallOrder[0]).toBe(getInternal(CHILD)); + expect(idCallOrder[1]).toBe(getInternal(PARENT)); + expect(idCallOrder[2]).toBe(getInternal(GRANDPARENT)); }); it('should support stopPropagation()', () => { - putListener( - CHILD, + EventPluginHub.putListener( + getInternal(CHILD), ON_CLICK_KEY, - recordID.bind(null, CHILD) + recordID.bind(null, getInternal(CHILD)) ); - putListener( - PARENT, + EventPluginHub.putListener( + getInternal(PARENT), ON_CLICK_KEY, - recordIDAndStopPropagation.bind(null, PARENT) + recordIDAndStopPropagation.bind(null, getInternal(PARENT)) ); - putListener( - GRANDPARENT, + EventPluginHub.putListener( + getInternal(GRANDPARENT), ON_CLICK_KEY, - recordID.bind(null, GRANDPARENT) + recordID.bind(null, getInternal(GRANDPARENT)) ); ReactTestUtils.Simulate.click(CHILD); expect(idCallOrder.length).toBe(2); - expect(idCallOrder[0]).toBe(CHILD); - expect(idCallOrder[1]).toBe(PARENT); + expect(idCallOrder[0]).toBe(getInternal(CHILD)); + expect(idCallOrder[1]).toBe(getInternal(PARENT)); }); it('should stop after first dispatch if stopPropagation', () => { - putListener( - CHILD, + EventPluginHub.putListener( + getInternal(CHILD), ON_CLICK_KEY, - recordIDAndStopPropagation.bind(null, CHILD) + recordIDAndStopPropagation.bind(null, getInternal(CHILD)) ); - putListener( - PARENT, + EventPluginHub.putListener( + getInternal(PARENT), ON_CLICK_KEY, - recordID.bind(null, PARENT) + recordID.bind(null, getInternal(PARENT)) ); - putListener( - GRANDPARENT, + EventPluginHub.putListener( + getInternal(GRANDPARENT), ON_CLICK_KEY, - recordID.bind(null, GRANDPARENT) + recordID.bind(null, getInternal(GRANDPARENT)) ); ReactTestUtils.Simulate.click(CHILD); expect(idCallOrder.length).toBe(1); - expect(idCallOrder[0]).toBe(CHILD); + expect(idCallOrder[0]).toBe(getInternal(CHILD)); }); it('should not stopPropagation if false is returned', () => { - putListener( - CHILD, + EventPluginHub.putListener( + getInternal(CHILD), ON_CLICK_KEY, - recordIDAndReturnFalse.bind(null, CHILD) + recordIDAndReturnFalse.bind(null, getInternal(CHILD)) ); - putListener( - PARENT, + EventPluginHub.putListener( + getInternal(PARENT), ON_CLICK_KEY, - recordID.bind(null, PARENT) + recordID.bind(null, getInternal(PARENT)) ); - putListener( - GRANDPARENT, + EventPluginHub.putListener( + getInternal(GRANDPARENT), ON_CLICK_KEY, - recordID.bind(null, GRANDPARENT) + recordID.bind(null, getInternal(GRANDPARENT)) ); spyOn(console, 'error'); ReactTestUtils.Simulate.click(CHILD); expect(idCallOrder.length).toBe(3); - expect(idCallOrder[0]).toBe(CHILD); - expect(idCallOrder[1]).toBe(PARENT); - expect(idCallOrder[2]).toBe(GRANDPARENT); + expect(idCallOrder[0]).toBe(getInternal(CHILD)); + expect(idCallOrder[1]).toBe(getInternal(PARENT)); + expect(idCallOrder[2]).toBe(getInternal(GRANDPARENT)); expect(console.error.calls.count()).toEqual(0); }); @@ -333,15 +284,15 @@ describe('ReactBrowserEventEmitter', () => { it('should invoke handlers that were removed while bubbling', () => { var handleParentClick = jest.fn(); var handleChildClick = function(event) { - deleteAllListeners(PARENT); + EventPluginHub.deleteAllListeners(getInternal(PARENT)); }; - putListener( - CHILD, + EventPluginHub.putListener( + getInternal(CHILD), ON_CLICK_KEY, handleChildClick ); - putListener( - PARENT, + EventPluginHub.putListener( + getInternal(PARENT), ON_CLICK_KEY, handleParentClick ); @@ -352,14 +303,14 @@ describe('ReactBrowserEventEmitter', () => { it('should not invoke newly inserted handlers while bubbling', () => { var handleParentClick = jest.fn(); var handleChildClick = function(event) { - putListener( - PARENT, + EventPluginHub.putListener( + getInternal(PARENT), ON_CLICK_KEY, handleParentClick ); }; - putListener( - CHILD, + EventPluginHub.putListener( + getInternal(CHILD), ON_CLICK_KEY, handleChildClick ); @@ -368,21 +319,21 @@ describe('ReactBrowserEventEmitter', () => { }); it('should have mouse enter simulated by test utils', () => { - putListener( - CHILD, + EventPluginHub.putListener( + getInternal(CHILD), ON_MOUSE_ENTER_KEY, - recordID.bind(null, CHILD) + recordID.bind(null, getInternal(CHILD)) ); ReactTestUtils.Simulate.mouseEnter(CHILD); expect(idCallOrder.length).toBe(1); - expect(idCallOrder[0]).toBe(CHILD); + expect(idCallOrder[0]).toBe(getInternal(CHILD)); }); it('should infer onTouchTap from a touchStart/End', () => { - putListener( - CHILD, + EventPluginHub.putListener( + getInternal(CHILD), ON_TOUCH_TAP_KEY, - recordID.bind(null, CHILD) + recordID.bind(null, getInternal(CHILD)) ); ReactTestUtils.SimulateNative.touchStart( CHILD, @@ -393,14 +344,14 @@ describe('ReactBrowserEventEmitter', () => { ReactTestUtils.nativeTouchData(0, 0) ); expect(idCallOrder.length).toBe(1); - expect(idCallOrder[0]).toBe(CHILD); + expect(idCallOrder[0]).toBe(getInternal(CHILD)); }); it('should infer onTouchTap from when dragging below threshold', () => { - putListener( - CHILD, + EventPluginHub.putListener( + getInternal(CHILD), ON_TOUCH_TAP_KEY, - recordID.bind(null, CHILD) + recordID.bind(null, getInternal(CHILD)) ); ReactTestUtils.SimulateNative.touchStart( CHILD, @@ -411,14 +362,14 @@ describe('ReactBrowserEventEmitter', () => { ReactTestUtils.nativeTouchData(0, tapMoveThreshold - 1) ); expect(idCallOrder.length).toBe(1); - expect(idCallOrder[0]).toBe(CHILD); + expect(idCallOrder[0]).toBe(getInternal(CHILD)); }); it('should not onTouchTap from when dragging beyond threshold', () => { - putListener( - CHILD, + EventPluginHub.putListener( + getInternal(CHILD), ON_TOUCH_TAP_KEY, - recordID.bind(null, CHILD) + recordID.bind(null, getInternal(CHILD)) ); ReactTestUtils.SimulateNative.touchStart( CHILD, @@ -472,20 +423,20 @@ describe('ReactBrowserEventEmitter', () => { }); it('should bubble onTouchTap', () => { - putListener( - CHILD, + EventPluginHub.putListener( + getInternal(CHILD), ON_TOUCH_TAP_KEY, - recordID.bind(null, CHILD) + recordID.bind(null, getInternal(CHILD)) ); - putListener( - PARENT, + EventPluginHub.putListener( + getInternal(PARENT), ON_TOUCH_TAP_KEY, - recordID.bind(null, PARENT) + recordID.bind(null, getInternal(PARENT)) ); - putListener( - GRANDPARENT, + EventPluginHub.putListener( + getInternal(GRANDPARENT), ON_TOUCH_TAP_KEY, - recordID.bind(null, GRANDPARENT) + recordID.bind(null, getInternal(GRANDPARENT)) ); ReactTestUtils.SimulateNative.touchStart( CHILD, @@ -496,9 +447,9 @@ describe('ReactBrowserEventEmitter', () => { ReactTestUtils.nativeTouchData(0, 0) ); expect(idCallOrder.length).toBe(3); - expect(idCallOrder[0]).toBe(CHILD); - expect(idCallOrder[1]).toBe(PARENT); - expect(idCallOrder[2]).toBe(GRANDPARENT); + expect(idCallOrder[0]).toBe(getInternal(CHILD)); + expect(idCallOrder[1]).toBe(getInternal(PARENT)); + expect(idCallOrder[2]).toBe(getInternal(GRANDPARENT)); }); it('should not crash ensureScrollValueMonitoring when createEvent returns null', () => { diff --git a/src/renderers/dom/shared/__tests__/ReactDOMComponent-test.js b/src/renderers/dom/shared/__tests__/ReactDOMComponent-test.js index 826be8e7b9..00d0bd445d 100644 --- a/src/renderers/dom/shared/__tests__/ReactDOMComponent-test.js +++ b/src/renderers/dom/shared/__tests__/ReactDOMComponent-test.js @@ -972,6 +972,61 @@ describe('ReactDOMComponent', () => { expect(tracker.getValue()).toEqual('foo'); }); + it('should execute custom event plugin listening behavior', () => { + var SimpleEventPlugin = require('SimpleEventPlugin'); + + SimpleEventPlugin.didPutListener = jest.fn(); + SimpleEventPlugin.willDeleteListener = jest.fn(); + + var container = document.createElement('div'); + ReactDOM.render( +
true} />, + container + ); + + expect(SimpleEventPlugin.didPutListener.mock.calls.length).toBe(1); + + ReactDOM.unmountComponentAtNode(container); + + expect(SimpleEventPlugin.willDeleteListener.mock.calls.length).toBe(1); + }); + + it('should handle null and missing properly with event hooks', () => { + var SimpleEventPlugin = require('SimpleEventPlugin'); + + SimpleEventPlugin.didPutListener = jest.fn(); + SimpleEventPlugin.willDeleteListener = jest.fn(); + var container = document.createElement('div'); + + ReactDOM.render(
, container); + expect(SimpleEventPlugin.didPutListener.mock.calls.length).toBe(0); + expect(SimpleEventPlugin.willDeleteListener.mock.calls.length).toBe(0); + + ReactDOM.render(
, container); + expect(SimpleEventPlugin.didPutListener.mock.calls.length).toBe(0); + expect(SimpleEventPlugin.willDeleteListener.mock.calls.length).toBe(0); + + ReactDOM.render(
'apple'} />, container); + expect(SimpleEventPlugin.didPutListener.mock.calls.length).toBe(1); + expect(SimpleEventPlugin.willDeleteListener.mock.calls.length).toBe(0); + + ReactDOM.render(
'banana'} />, container); + expect(SimpleEventPlugin.didPutListener.mock.calls.length).toBe(2); + expect(SimpleEventPlugin.willDeleteListener.mock.calls.length).toBe(0); + + ReactDOM.render(
, container); + expect(SimpleEventPlugin.didPutListener.mock.calls.length).toBe(2); + expect(SimpleEventPlugin.willDeleteListener.mock.calls.length).toBe(1); + + ReactDOM.render(
, container); + expect(SimpleEventPlugin.didPutListener.mock.calls.length).toBe(2); + expect(SimpleEventPlugin.willDeleteListener.mock.calls.length).toBe(1); + + ReactDOM.unmountComponentAtNode(container); + expect(SimpleEventPlugin.didPutListener.mock.calls.length).toBe(2); + expect(SimpleEventPlugin.willDeleteListener.mock.calls.length).toBe(1); + }); + it('should warn for children on void elements', () => { class X extends React.Component { render() { @@ -1102,6 +1157,31 @@ describe('ReactDOMComponent', () => { }); describe('unmountComponent', () => { + it('should clean up listeners', () => { + var EventPluginHub = require('EventPluginHub'); + var ReactDOMComponentTree = require('ReactDOMComponentTree'); + + var container = document.createElement('div'); + document.body.appendChild(container); + + var callback = function() {}; + var instance =
; + instance = ReactDOM.render(instance, container); + + var rootNode = ReactDOM.findDOMNode(instance); + var inst = ReactDOMComponentTree.getInstanceFromNode(rootNode); + expect( + EventPluginHub.getListener(inst, 'onClick') + ).toBe(callback); + expect(rootNode).toBe(ReactDOM.findDOMNode(instance)); + + ReactDOM.unmountComponentAtNode(container); + + expect( + EventPluginHub.getListener(inst, 'onClick') + ).toBe(undefined); + }); + it('should clean up input value tracking', () => { var container = document.createElement('div'); var node = ReactDOM.render(, container); diff --git a/src/renderers/dom/stack/client/ReactDOMComponent.js b/src/renderers/dom/stack/client/ReactDOMComponent.js index cebcc047e8..e0fd44e83d 100644 --- a/src/renderers/dom/stack/client/ReactDOMComponent.js +++ b/src/renderers/dom/stack/client/ReactDOMComponent.js @@ -19,6 +19,7 @@ var DOMLazyTree = require('DOMLazyTree'); var DOMNamespaces = require('DOMNamespaces'); var DOMProperty = require('DOMProperty'); var DOMPropertyOperations = require('DOMPropertyOperations'); +var EventPluginHub = require('EventPluginHub'); var EventPluginRegistry = require('EventPluginRegistry'); var ReactBrowserEventEmitter = require('ReactBrowserEventEmitter'); var ReactDOMComponentFlags = require('ReactDOMComponentFlags'); @@ -42,6 +43,7 @@ var warning = require('warning'); var didWarnShadyDOM = false; var Flags = ReactDOMComponentFlags; +var deleteListener = EventPluginHub.deleteListener; var getNode = ReactDOMComponentTree.getNodeFromInstance; var listenTo = ReactBrowserEventEmitter.listenTo; var registrationNameModules = EventPluginRegistry.registrationNameModules; @@ -203,6 +205,30 @@ function assertValidProps(component, props) { ); } +function enqueuePutListener(inst, registrationName, listener, transaction) { + if (transaction instanceof ReactServerRenderingTransaction) { + return; + } + if (__DEV__) { + // IE8 has no API for event capturing and the `onScroll` event doesn't + // bubble. + warning( + registrationName !== 'onScroll' || isEventSupported('scroll', true), + 'This browser doesn\'t support the `onScroll` event' + ); + } + var containerInfo = inst._hostContainerInfo; + var isDocumentFragment = containerInfo._node && containerInfo._node.nodeType === DOC_FRAGMENT_TYPE; + var doc = isDocumentFragment ? containerInfo._node : containerInfo._ownerDocument; + listenTo(registrationName, doc); + transaction.getReactMountReady().enqueue(putListener, { + inst: inst, + registrationName: registrationName, + listener: listener, + }); +} + +// TODO: This is coming from future #8192. Dedupe this and enqueuePutListener. function ensureListeningTo(inst, registrationName, transaction) { if (transaction instanceof ReactServerRenderingTransaction) { return; @@ -221,6 +247,15 @@ function ensureListeningTo(inst, registrationName, transaction) { listenTo(registrationName, doc); } +function putListener() { + var listenerToPut = this; + EventPluginHub.putListener( + listenerToPut.inst, + listenerToPut.registrationName, + listenerToPut.listener + ); +} + function inputPostMount() { var inst = this; ReactDOMInput.postMountWrapper(inst); @@ -757,7 +792,7 @@ ReactDOMComponent.Mixin = { } if (registrationNameModules.hasOwnProperty(propKey)) { if (propValue) { - ensureListeningTo(this, propKey, transaction); + enqueuePutListener(this, propKey, propValue, transaction); } } else { if (propKey === STYLE) { @@ -1012,7 +1047,12 @@ ReactDOMComponent.Mixin = { } this._previousStyleCopy = null; } else if (registrationNameModules.hasOwnProperty(propKey)) { - // Do nothing for event names. + if (lastProps[propKey]) { + // Only call deleteListener if there was a listener previously or + // else willDeleteListener gets called when there wasn't actually a + // listener (e.g., onClick={null}) + deleteListener(this, propKey); + } } else if (isCustomComponent(this._tag, lastProps)) { if (!RESERVED_PROPS.hasOwnProperty(propKey)) { DOMPropertyOperations.deleteValueForAttribute( @@ -1073,7 +1113,9 @@ ReactDOMComponent.Mixin = { } } else if (registrationNameModules.hasOwnProperty(propKey)) { if (nextProp) { - ensureListeningTo(this, propKey, transaction); + enqueuePutListener(this, propKey, nextProp, transaction); + } else if (lastProp) { + deleteListener(this, propKey); } } else if (isCustomComponentTag) { if (!RESERVED_PROPS.hasOwnProperty(propKey)) { @@ -1222,6 +1264,7 @@ ReactDOMComponent.Mixin = { this.unmountChildren(safely, skipLifecycle); ReactDOMComponentTree.uncacheNode(this); + EventPluginHub.deleteAllListeners(this); this._rootNodeID = 0; this._domID = 0; this._wrapperState = null; diff --git a/src/renderers/dom/stack/server/__tests__/ReactServerRendering-test.js b/src/renderers/dom/stack/server/__tests__/ReactServerRendering-test.js index fc074ec716..8572dbe15d 100644 --- a/src/renderers/dom/stack/server/__tests__/ReactServerRendering-test.js +++ b/src/renderers/dom/stack/server/__tests__/ReactServerRendering-test.js @@ -85,7 +85,15 @@ describe('ReactServerRendering', () => { expect(response).toBe(''); }); - // TODO: Test that listeners are not registered onto any document/container. + it('should not register event listeners', () => { + var EventPluginHub = require('EventPluginHub'); + var cb = jest.fn(); + + ReactServerRendering.renderToString( + hello world + ); + expect(EventPluginHub.__getListenerBank()).toEqual({}); + }); it('should render composite components', () => { class Parent extends React.Component { @@ -314,6 +322,16 @@ describe('ReactServerRendering', () => { expect(response).toBe('hello world'); }); + it('should not register event listeners', () => { + var EventPluginHub = require('EventPluginHub'); + var cb = jest.fn(); + + ReactServerRendering.renderToStaticMarkup( + hello world + ); + expect(EventPluginHub.__getListenerBank()).toEqual({}); + }); + it('should only execute certain lifecycle methods', () => { function runTest() { var lifecycle = []; diff --git a/src/renderers/native/ReactNativeBaseComponent.js b/src/renderers/native/ReactNativeBaseComponent.js index 32dc122350..1d47e9dbdf 100644 --- a/src/renderers/native/ReactNativeBaseComponent.js +++ b/src/renderers/native/ReactNativeBaseComponent.js @@ -14,12 +14,18 @@ var NativeMethodsMixin = require('NativeMethodsMixin'); var ReactNativeAttributePayload = require('ReactNativeAttributePayload'); var ReactNativeComponentTree = require('ReactNativeComponentTree'); +var ReactNativeEventEmitter = require('ReactNativeEventEmitter'); var ReactNativeTagHandles = require('ReactNativeTagHandles'); var ReactMultiChild = require('ReactMultiChild'); var UIManager = require('UIManager'); var deepFreezeAndThrowOnMutationInDev = require('deepFreezeAndThrowOnMutationInDev'); +var registrationNames = ReactNativeEventEmitter.registrationNames; +var putListener = ReactNativeEventEmitter.putListener; +var deleteListener = ReactNativeEventEmitter.deleteListener; +var deleteAllListeners = ReactNativeEventEmitter.deleteAllListeners; + type ReactNativeBaseComponentViewConfig = { validAttributes: Object; uiViewClassName: string; @@ -51,6 +57,7 @@ ReactNativeBaseComponent.Mixin = { unmountComponent: function(safely, skipLifecycle) { ReactNativeComponentTree.uncacheNode(this); + deleteAllListeners(this); this.unmountChildren(safely, skipLifecycle); this._rootNodeID = 0; }, @@ -116,9 +123,44 @@ ReactNativeBaseComponent.Mixin = { ); } + this._reconcileListenersUponUpdate( + prevElement.props, + nextElement.props + ); this.updateChildren(nextElement.props.children, transaction, context); }, + /** + * @param {object} initialProps Native component props. + */ + _registerListenersUponCreation: function(initialProps) { + for (var key in initialProps) { + // NOTE: The check for `!props[key]`, is only possible because this method + // registers listeners the *first* time a component is created. + if (registrationNames[key] && initialProps[key]) { + var listener = initialProps[key]; + putListener(this, key, listener); + } + } + }, + + /** + * Reconciles event listeners, adding or removing if necessary. + * @param {object} prevProps Native component props including events. + * @param {object} nextProps Next native component props including events. + */ + _reconcileListenersUponUpdate: function(prevProps, nextProps) { + for (var key in nextProps) { + if (registrationNames[key] && (nextProps[key] !== prevProps[key])) { + if (nextProps[key]) { + putListener(this, key, nextProps[key]); + } else { + deleteListener(this, key); + } + } + } + }, + /** * Currently this still uses IDs for reconciliation so this can return null. * @@ -165,6 +207,7 @@ ReactNativeBaseComponent.Mixin = { ReactNativeComponentTree.precacheNode(this, tag); + this._registerListenersUponCreation(this._currentElement.props); this.initializeChildren( this._currentElement.props.children, tag, diff --git a/src/renderers/native/ReactNativeEventEmitter.js b/src/renderers/native/ReactNativeEventEmitter.js index 6100bee894..87906b458b 100644 --- a/src/renderers/native/ReactNativeEventEmitter.js +++ b/src/renderers/native/ReactNativeEventEmitter.js @@ -78,14 +78,29 @@ var removeTouchesAtIndices = function( return rippedOut; }; +/** + * `ReactNativeEventEmitter` is used to attach top-level event listeners. For example: + * + * ReactNativeEventEmitter.putListener('myID', 'onClick', myFunction); + * + * This would allocate a "registration" of `('onClick', myFunction)` on 'myID'. + * + * @internal + */ var ReactNativeEventEmitter = { ...ReactEventEmitterMixin, registrationNames: EventPluginRegistry.registrationNameModules, + putListener: EventPluginHub.putListener, + getListener: EventPluginHub.getListener, + deleteListener: EventPluginHub.deleteListener, + + deleteAllListeners: EventPluginHub.deleteAllListeners, + /** * Internal version of `receiveEvent` in terms of normalized (non-tag) * `rootNodeID`. diff --git a/src/renderers/shared/shared/event/EventPluginHub.js b/src/renderers/shared/shared/event/EventPluginHub.js index b0c2d0164f..ecff02ebf3 100644 --- a/src/renderers/shared/shared/event/EventPluginHub.js +++ b/src/renderers/shared/shared/event/EventPluginHub.js @@ -19,6 +19,11 @@ var accumulateInto = require('accumulateInto'); var forEachAccumulated = require('forEachAccumulated'); var invariant = require('invariant'); +/** + * Internal store for event listeners + */ +var listenerBank = {}; + /** * Internal queue of events that have accumulated their dispatches and are * waiting to have their dispatches executed. @@ -48,6 +53,12 @@ var executeDispatchesAndReleaseTopLevel = function(e) { return executeDispatchesAndRelease(e, false); }; +var getDictionaryKey = function(inst) { + // Prevents V8 performance issue: + // https://github.com/facebook/react/pull/7232 + return '.' + inst._rootNodeID; +}; + /** * This is a unified interface for event plugins to be installed and configured. * @@ -90,22 +101,88 @@ var EventPluginHub = { }, + /** + * Stores `listener` at `listenerBank[registrationName][key]`. Is idempotent. + * + * @param {object} inst The instance, which is the source of events. + * @param {string} registrationName Name of listener (e.g. `onClick`). + * @param {function} listener The callback to store. + */ + putListener: function(inst, registrationName, listener) { + invariant( + typeof listener === 'function', + 'Expected %s listener to be a function, instead got type %s', + registrationName, typeof listener + ); + + var key = getDictionaryKey(inst); + var bankForRegistrationName = + listenerBank[registrationName] || (listenerBank[registrationName] = {}); + bankForRegistrationName[key] = listener; + + var PluginModule = + EventPluginRegistry.registrationNameModules[registrationName]; + if (PluginModule && PluginModule.didPutListener) { + PluginModule.didPutListener(inst, registrationName, listener); + } + }, + /** * @param {object} inst The instance, which is the source of events. * @param {string} registrationName Name of listener (e.g. `onClick`). * @return {?function} The stored callback. */ getListener: function(inst, registrationName) { - var listener = inst._currentElement.props[registrationName]; - if (listener !== undefined) { - invariant( - typeof listener === 'function', - 'Expected %s listener to be a function, instead got type %s', - registrationName, - typeof listener - ); + var bankForRegistrationName = listenerBank[registrationName]; + var key = getDictionaryKey(inst); + return bankForRegistrationName && bankForRegistrationName[key]; + }, + + /** + * Deletes a listener from the registration bank. + * + * @param {object} inst The instance, which is the source of events. + * @param {string} registrationName Name of listener (e.g. `onClick`). + */ + deleteListener: function(inst, registrationName) { + var PluginModule = + EventPluginRegistry.registrationNameModules[registrationName]; + if (PluginModule && PluginModule.willDeleteListener) { + PluginModule.willDeleteListener(inst, registrationName); + } + + var bankForRegistrationName = listenerBank[registrationName]; + // TODO: This should never be null -- when is it? + if (bankForRegistrationName) { + var key = getDictionaryKey(inst); + delete bankForRegistrationName[key]; + } + }, + + /** + * Deletes all listeners for the DOM element with the supplied ID. + * + * @param {object} inst The instance, which is the source of events. + */ + deleteAllListeners: function(inst) { + var key = getDictionaryKey(inst); + for (var registrationName in listenerBank) { + if (!listenerBank.hasOwnProperty(registrationName)) { + continue; + } + + if (!listenerBank[registrationName][key]) { + continue; + } + + var PluginModule = + EventPluginRegistry.registrationNameModules[registrationName]; + if (PluginModule && PluginModule.willDeleteListener) { + PluginModule.willDeleteListener(inst, registrationName); + } + + delete listenerBank[registrationName][key]; } - return listener; }, /** @@ -183,6 +260,17 @@ var EventPluginHub = { ReactErrorUtils.rethrowCaughtError(); }, + /** + * These are needed for tests only. Do not use! + */ + __purge: function() { + listenerBank = {}; + }, + + __getListenerBank: function() { + return listenerBank; + }, + }; module.exports = EventPluginHub; diff --git a/src/renderers/shared/shared/event/EventPluginRegistry.js b/src/renderers/shared/shared/event/EventPluginRegistry.js index 1f09f23f83..f44593eab8 100644 --- a/src/renderers/shared/shared/event/EventPluginRegistry.js +++ b/src/renderers/shared/shared/event/EventPluginRegistry.js @@ -130,7 +130,8 @@ function publishEventForPlugin( } /** - * Publishes a registration name that is used to identify dispatched events. + * Publishes a registration name that is used to identify dispatched events and + * can be used with `EventPluginHub.putListener` to register listeners. * * @param {string} registrationName Registration name to add. * @param {object} PluginModule Plugin publishing the event. diff --git a/src/renderers/shared/shared/event/PluginModuleType.js b/src/renderers/shared/shared/event/PluginModuleType.js index 1d9044d6bd..ce00d28f94 100644 --- a/src/renderers/shared/shared/event/PluginModuleType.js +++ b/src/renderers/shared/shared/event/PluginModuleType.js @@ -36,5 +36,14 @@ export type PluginModule = { nativeTarget: NativeEvent, nativeEventTarget: EventTarget, ) => null | ReactSyntheticEvent, + didPutListener?: ( + inst: ReactInstance, + registrationName: string, + listener: () => void, + ) => void, + willDeleteListener?: ( + inst: ReactInstance, + registrationName: string, + ) => void, tapMoveThreshold?: number, }; diff --git a/src/renderers/shared/shared/event/__tests__/EventPluginHub-test.js b/src/renderers/shared/shared/event/__tests__/EventPluginHub-test.js index 48c859b8ac..2e49bc4f43 100644 --- a/src/renderers/shared/shared/event/__tests__/EventPluginHub-test.js +++ b/src/renderers/shared/shared/event/__tests__/EventPluginHub-test.js @@ -14,21 +14,19 @@ jest.mock('isEventSupported'); describe('EventPluginHub', () => { - var React; - var ReactTestUtils; + var EventPluginHub; + var isEventSupported; beforeEach(() => { jest.resetModuleRegistry(); - React = require('React'); - ReactTestUtils = require('ReactTestUtils'); + EventPluginHub = require('EventPluginHub'); + isEventSupported = require('isEventSupported'); + isEventSupported.mockReturnValueOnce(false); }); - it('should prevent non-function listeners, at dispatch', () => { - var node = ReactTestUtils.renderIntoDocument( -
- ); + it('should prevent non-function listeners', () => { expect(function() { - ReactTestUtils.SimulateNative.click(node); + EventPluginHub.putListener(1, 'onClick', 'not a function'); }).toThrowError( 'Expected onClick listener to be a function, instead got type string' ); diff --git a/src/renderers/shared/shared/event/eventPlugins/__tests__/ResponderEventPlugin-test.js b/src/renderers/shared/shared/event/eventPlugins/__tests__/ResponderEventPlugin-test.js index a68867e3bd..557049d31c 100644 --- a/src/renderers/shared/shared/event/eventPlugins/__tests__/ResponderEventPlugin-test.js +++ b/src/renderers/shared/shared/event/eventPlugins/__tests__/ResponderEventPlugin-test.js @@ -233,7 +233,7 @@ var registerTestHandlers = function(eventTestConfig, readableIDToID) { } return config.returnVal; }.bind(null, readableID, nodeConfig); - putListener(getInstanceFromNode(id), registrationName, handler); + EventPluginHub.putListener(getInstanceFromNode(id), registrationName, handler); } }; for (var eventName in eventTestConfig) { @@ -258,6 +258,9 @@ var registerTestHandlers = function(eventTestConfig, readableIDToID) { return runs; }; + + + var run = function(config, hierarchyConfig, nativeEventConfig) { var max = NA; var searchForMax = function(nodeConfig) { @@ -306,30 +309,10 @@ var PARENT_HOST_NODE = { }; var CHILD_HOST_NODE = { }; var CHILD_HOST_NODE2 = { }; -var GRANDPARENT_INST = { - _hostParent: null, - _rootNodeID: '1', - _hostNode: GRANDPARENT_HOST_NODE, - _currentElement: { props: {} }, -}; -var PARENT_INST = { - _hostParent: GRANDPARENT_INST, - _rootNodeID: '2', - _hostNode: PARENT_HOST_NODE, - _currentElement: { props: {} }, -}; -var CHILD_INST = { - _hostParent: PARENT_INST, - _rootNodeID: '3', - _hostNode: CHILD_HOST_NODE, - _currentElement: { props: {} }, -}; -var CHILD_INST2 = { - _hostParent: PARENT_INST, - _rootNodeID: '4', - _hostNode: CHILD_HOST_NODE2, - _currentElement: { props: {} }, -}; +var GRANDPARENT_INST = { _hostParent: null, _rootNodeID: '1', _hostNode: GRANDPARENT_HOST_NODE }; +var PARENT_INST = { _hostParent: GRANDPARENT_INST, _rootNodeID: '2', _hostNode: PARENT_HOST_NODE }; +var CHILD_INST = { _hostParent: PARENT_INST, _rootNodeID: '3', _hostNode: CHILD_HOST_NODE }; +var CHILD_INST2 = { _hostParent: PARENT_INST, _rootNodeID: '4', _hostNode: CHILD_HOST_NODE2 }; GRANDPARENT_HOST_NODE._reactInstance = GRANDPARENT_INST; PARENT_HOST_NODE._reactInstance = PARENT_INST; @@ -356,14 +339,6 @@ function getNodeFromInstance(inst) { return inst._hostNode; } -function putListener(node, registrationName, handler) { - node._currentElement.props[registrationName] = handler; -} - -function deleteAllListeners(node) { - node._currentElement.props = {}; -} - describe('ResponderEventPlugin', () => { beforeEach(() => { @@ -373,11 +348,6 @@ describe('ResponderEventPlugin', () => { EventPluginUtils = require('EventPluginUtils'); ResponderEventPlugin = require('ResponderEventPlugin'); - deleteAllListeners(GRANDPARENT_INST); - deleteAllListeners(PARENT_INST); - deleteAllListeners(CHILD_INST); - deleteAllListeners(CHILD_INST2); - EventPluginUtils.injection.injectComponentTree({ getInstanceFromNode, getNodeFromInstance, diff --git a/src/renderers/shared/stack/reconciler/__tests__/ReactErrorBoundaries-test.js b/src/renderers/shared/stack/reconciler/__tests__/ReactErrorBoundaries-test.js index 06a410c614..405d6567c1 100644 --- a/src/renderers/shared/stack/reconciler/__tests__/ReactErrorBoundaries-test.js +++ b/src/renderers/shared/stack/reconciler/__tests__/ReactErrorBoundaries-test.js @@ -879,6 +879,26 @@ describe('ReactErrorBoundaries', () => { ]); }); + it('does not register event handlers for unmounted children', () => { + var EventPluginHub = require('EventPluginHub'); + var container = document.createElement('div'); + EventPluginHub.putListener = jest.fn(); + ReactDOM.render( + + + + , + container + ); + expect(EventPluginHub.putListener).not.toBeCalled(); + + log.length = 0; + ReactDOM.unmountComponentAtNode(container); + expect(log).toEqual([ + 'ErrorBoundary componentWillUnmount', + ]); + }); + it('does not call componentWillUnmount when aborting initial mount', () => { var container = document.createElement('div'); ReactDOM.render(