From 55c583a368df3d170947214d2e4628c4e87cbad5 Mon Sep 17 00:00:00 2001 From: Dominic Gannaway Date: Thu, 21 Mar 2019 15:54:03 +0000 Subject: [PATCH 01/12] Add part of the event responder system --- packages/events/EventBatching.js | 66 ++++++++++ packages/events/EventPluginHub.js | 65 +--------- packages/react-dom/src/client/ReactDOM.js | 6 +- .../src/client/ReactDOMHostConfig.js | 12 ++ .../src/events/DOMEventResponderSystem.js | 114 ++++++++++++++++++ .../src/events/ReactDOMEventListener.js | 30 +++-- .../DOMEventResponderSystem-test.internal.js | 88 ++++++++++++++ packages/react-dom/src/fire/ReactFire.js | 7 +- .../src/ReactFabricEventEmitter.js | 7 +- .../src/ReactNativeEventEmitter.js | 7 +- .../ReactFiberEvents-test-internal.js | 2 +- 11 files changed, 322 insertions(+), 82 deletions(-) create mode 100644 packages/events/EventBatching.js create mode 100644 packages/react-dom/src/events/DOMEventResponderSystem.js create mode 100644 packages/react-dom/src/events/__tests__/DOMEventResponderSystem-test.internal.js diff --git a/packages/events/EventBatching.js b/packages/events/EventBatching.js new file mode 100644 index 0000000000000..bffc978ad6e21 --- /dev/null +++ b/packages/events/EventBatching.js @@ -0,0 +1,66 @@ +/** + * Copyright (c) Facebook, Inc. and its affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + * @flow + */ + +import invariant from 'shared/invariant'; +import {rethrowCaughtError} from 'shared/ReactErrorUtils'; + +import type {ReactSyntheticEvent} from './ReactSyntheticEventType'; +import accumulateInto from './accumulateInto'; +import forEachAccumulated from './forEachAccumulated'; +import {executeDispatchesInOrder} from './EventPluginUtils'; + +/** + * Internal queue of events that have accumulated their dispatches and are + * waiting to have their dispatches executed. + */ +let eventQueue: ?(Array | ReactSyntheticEvent) = null; + +/** + * Dispatches an event and releases it back into the pool, unless persistent. + * + * @param {?object} event Synthetic event to be dispatched. + * @private + */ +const executeDispatchesAndRelease = function(event: ReactSyntheticEvent) { + if (event) { + executeDispatchesInOrder(event); + + if (!event.isPersistent()) { + event.constructor.release(event); + } + } +}; +const executeDispatchesAndReleaseTopLevel = function(e) { + return executeDispatchesAndRelease(e); +}; + +export function runEventsInBatch( + events: Array | ReactSyntheticEvent | null, +) { + if (events !== null) { + eventQueue = accumulateInto(eventQueue, events); + } + + // Set `eventQueue` to null before processing it so that we can tell if more + // events get enqueued while processing. + const processingEventQueue = eventQueue; + eventQueue = null; + + if (!processingEventQueue) { + return; + } + + forEachAccumulated(processingEventQueue, executeDispatchesAndReleaseTopLevel); + invariant( + !eventQueue, + 'processEventQueue(): Additional events were enqueued while processing ' + + 'an event queue. Support for this has not yet been implemented.', + ); + // This would be a good time to rethrow if any of the event handlers threw. + rethrowCaughtError(); +} diff --git a/packages/events/EventPluginHub.js b/packages/events/EventPluginHub.js index e639110713481..297d0cbdcc44a 100644 --- a/packages/events/EventPluginHub.js +++ b/packages/events/EventPluginHub.js @@ -6,7 +6,6 @@ * @flow */ -import {rethrowCaughtError} from 'shared/ReactErrorUtils'; import invariant from 'shared/invariant'; import { @@ -14,12 +13,9 @@ import { injectEventPluginsByName, plugins, } from './EventPluginRegistry'; -import { - executeDispatchesInOrder, - getFiberCurrentPropsFromNode, -} from './EventPluginUtils'; +import {getFiberCurrentPropsFromNode} from './EventPluginUtils'; import accumulateInto from './accumulateInto'; -import forEachAccumulated from './forEachAccumulated'; +import {runEventsInBatch} from './EventBatching'; import type {PluginModule} from './PluginModuleType'; import type {ReactSyntheticEvent} from './ReactSyntheticEventType'; @@ -27,31 +23,6 @@ import type {Fiber} from 'react-reconciler/src/ReactFiber'; import type {AnyNativeEvent} from './PluginModuleType'; import type {TopLevelType} from './TopLevelEventTypes'; -/** - * Internal queue of events that have accumulated their dispatches and are - * waiting to have their dispatches executed. - */ -let eventQueue: ?(Array | ReactSyntheticEvent) = null; - -/** - * Dispatches an event and releases it back into the pool, unless persistent. - * - * @param {?object} event Synthetic event to be dispatched. - * @private - */ -const executeDispatchesAndRelease = function(event: ReactSyntheticEvent) { - if (event) { - executeDispatchesInOrder(event); - - if (!event.isPersistent()) { - event.constructor.release(event); - } - } -}; -const executeDispatchesAndReleaseTopLevel = function(e) { - return executeDispatchesAndRelease(e); -}; - function isInteractive(tag) { return ( tag === 'button' || @@ -158,7 +129,7 @@ export function getListener(inst: Fiber, registrationName: string) { * @return {*} An accumulation of synthetic events. * @internal */ -function extractEvents( +function extractPluginEvents( topLevelType: TopLevelType, targetInst: null | Fiber, nativeEvent: AnyNativeEvent, @@ -183,39 +154,13 @@ function extractEvents( return events; } -export function runEventsInBatch( - events: Array | ReactSyntheticEvent | null, -) { - if (events !== null) { - eventQueue = accumulateInto(eventQueue, events); - } - - // Set `eventQueue` to null before processing it so that we can tell if more - // events get enqueued while processing. - const processingEventQueue = eventQueue; - eventQueue = null; - - if (!processingEventQueue) { - return; - } - - forEachAccumulated(processingEventQueue, executeDispatchesAndReleaseTopLevel); - invariant( - !eventQueue, - 'processEventQueue(): Additional events were enqueued while processing ' + - 'an event queue. Support for this has not yet been implemented.', - ); - // This would be a good time to rethrow if any of the event handlers threw. - rethrowCaughtError(); -} - -export function runExtractedEventsInBatch( +export function runExtractedPluginEventsInBatch( topLevelType: TopLevelType, targetInst: null | Fiber, nativeEvent: AnyNativeEvent, nativeEventTarget: EventTarget, ) { - const events = extractEvents( + const events = extractPluginEvents( topLevelType, targetInst, nativeEvent, diff --git a/packages/react-dom/src/client/ReactDOM.js b/packages/react-dom/src/client/ReactDOM.js index 0eac062bed74f..e226843322f78 100644 --- a/packages/react-dom/src/client/ReactDOM.js +++ b/packages/react-dom/src/client/ReactDOM.js @@ -44,10 +44,8 @@ import { enqueueStateRestore, restoreStateIfNeeded, } from 'events/ReactControlledComponent'; -import { - injection as EventPluginHubInjection, - runEventsInBatch, -} from 'events/EventPluginHub'; +import {injection as EventPluginHubInjection} from 'events/EventPluginHub'; +import {runEventsInBatch} from 'events/EventBatching'; import {eventNameDispatchConfigs} from 'events/EventPluginRegistry'; import { accumulateTwoPhaseDispatches, diff --git a/packages/react-dom/src/client/ReactDOMHostConfig.js b/packages/react-dom/src/client/ReactDOMHostConfig.js index baea3cbeec5af..e2177b517a3c9 100644 --- a/packages/react-dom/src/client/ReactDOMHostConfig.js +++ b/packages/react-dom/src/client/ReactDOMHostConfig.js @@ -44,6 +44,7 @@ import { import dangerousStyleValue from '../shared/dangerousStyleValue'; import type {DOMContainer} from './ReactDOM'; +import {currentEventComponentFibers} from '../events/DOMEventResponderSystem'; import type {ReactEventResponder} from 'shared/ReactTypes'; import { REACT_EVENT_COMPONENT_TYPE, @@ -51,6 +52,7 @@ import { REACT_EVENT_TARGET_TOUCH_HIT, } from 'shared/ReactSymbols'; import getElementFromTouchHitTarget from 'shared/getElementFromTouchHitTarget'; +import type {Fiber} from 'react-reconciler/src/ReactFiber'; export type Type = string; export type Props = { @@ -862,6 +864,16 @@ export function handleEventComponent( rootContainerInstance: Container, internalInstanceHandle: Object, ): void { + // We keep the event responder hub up-to-do with the state of + // event component fiber tree (it uses this to know what events to fire). + // To do this, we update the current event component fiber and remove + // the alternate fiber if it exists. + const eventComponetFiber = ((internalInstanceHandle: any): Fiber); + currentEventComponentFibers.add(eventComponetFiber); + const alternateFiber = eventComponetFiber.alternate; + if (alternateFiber !== null) { + currentEventComponentFibers.delete(alternateFiber); + } const rootElement = rootContainerInstance.ownerDocument; listenToEventResponderEvents(eventResponder, rootElement); } diff --git a/packages/react-dom/src/events/DOMEventResponderSystem.js b/packages/react-dom/src/events/DOMEventResponderSystem.js new file mode 100644 index 0000000000000..f458aa9ebb2e3 --- /dev/null +++ b/packages/react-dom/src/events/DOMEventResponderSystem.js @@ -0,0 +1,114 @@ +/** + * Copyright (c) Facebook, Inc. and its affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + * @flow + */ + +import {type EventSystemFlags} from 'events/EventSystemFlags'; +import type {AnyNativeEvent} from 'events/PluginModuleType'; +import {EventComponent} from 'shared/ReactWorkTags'; +import type {DOMTopLevelEventType} from 'events/TopLevelEventTypes'; +import type {ReactEventResponder} from 'shared/ReactTypes'; +import warning from 'shared/warning'; +import type {Fiber} from 'react-reconciler/src/ReactFiber'; + +// We track the active component fibers so we can traverse through +// the fiber tree and find the relative current fibers. We need to +// do this because an update might have switched an event component +// fiber to its alternate fiber. +export const currentEventComponentFibers: Set = new Set(); + +// Event responders provide us an array of target event types. +// To ensure we fire the right responders for given events, we check +// if the incoming event type is actually relevant for an event +// responder. Instead of doing an O(n) lookup on the event responder +// target event types array each time, we instead create a Set for +// faster O(1) lookups. +export const eventResponderValidEventTypes: Map< + ReactEventResponder, + Set, +> = new Map(); + +function createValidEventTypeSet(targetEventTypes): Set { + const eventTypeSet = new Set(); + // Go through each target event type of the event responder + for (let i = 0, length = targetEventTypes.length; i < length; ++i) { + const targetEventType = targetEventTypes[i]; + + if (typeof targetEventType === 'string') { + eventTypeSet.add(((targetEventType: any): DOMTopLevelEventType)); + } else { + if (__DEV__) { + warning( + typeof targetEventType === 'object' && targetEventType !== null, + 'Event Responder: invalid entry in targetEventTypes array. ' + + 'Entry must be string or an object. Instead, got %s.', + targetEventType, + ); + } + const targetEventConfigObject = ((targetEventType: any): { + name: DOMTopLevelEventType, + passive?: boolean, + capture?: boolean, + }); + eventTypeSet.add(targetEventConfigObject.name); + } + } + return eventTypeSet; +} + +function handleTopLevelType( + topLevelType: DOMTopLevelEventType, + fiber: Fiber, + context: Object, +): void { + const responder: ReactEventResponder = fiber.type.responder; + const props = fiber.memoizedProps; + const stateNode = fiber.stateNode; + let validEventTypesForResponder = eventResponderValidEventTypes.get( + responder, + ); + + if (validEventTypesForResponder === undefined) { + validEventTypesForResponder = createValidEventTypeSet( + responder.targetEventTypes, + ); + eventResponderValidEventTypes.set(responder, validEventTypesForResponder); + } + if (!validEventTypesForResponder.has(topLevelType)) { + return; + } + let state = stateNode.get(responder); + if (state === undefined && responder.createInitialState !== undefined) { + state = responder.createInitialState(props); + stateNode.set(responder, state); + } + // TODO provide all the props for handleEvent + responder.handleEvent(context, props, state); +} + +export function runResponderEventsInBatch( + topLevelType: DOMTopLevelEventType, + targetFiber: Fiber, + nativeEvent: AnyNativeEvent, + nativeEventTarget: EventTarget, + eventSystemFlags: EventSystemFlags, +): void { + // TODO add proper event context + let context = ({}: any); + let node = targetFiber; + // Traverse up the fiber tree till we find event component fibers. + while (node !== null) { + if (node.tag === EventComponent) { + if (node.alternate !== null && !currentEventComponentFibers.has(node)) { + node = node.alternate; + } + // TODO create a responder context and pass it through + handleTopLevelType(topLevelType, node, context); + } + node = node.return; + } + // TODO dispatch extracted events from context (with batching) +} diff --git a/packages/react-dom/src/events/ReactDOMEventListener.js b/packages/react-dom/src/events/ReactDOMEventListener.js index 32da3c6106147..8751f2138c410 100644 --- a/packages/react-dom/src/events/ReactDOMEventListener.js +++ b/packages/react-dom/src/events/ReactDOMEventListener.js @@ -12,7 +12,8 @@ import type {Fiber} from 'react-reconciler/src/ReactFiber'; import type {DOMTopLevelEventType} from 'events/TopLevelEventTypes'; import {batchedUpdates, interactiveUpdates} from 'events/ReactGenericBatching'; -import {runExtractedEventsInBatch} from 'events/EventPluginHub'; +import {runExtractedPluginEventsInBatch} from 'events/EventPluginHub'; +import {runResponderEventsInBatch} from '../events/DOMEventResponderSystem'; import {isFiberMounted} from 'react-reconciler/reflection'; import {HostRoot} from 'shared/ReactWorkTags'; import { @@ -130,16 +131,27 @@ function handleTopLevel(bookKeeping: BookKeepingInstance) { for (let i = 0; i < bookKeeping.ancestors.length; i++) { targetInst = bookKeeping.ancestors[i]; - if (bookKeeping.eventSystemFlags === PLUGIN_EVENT_SYSTEM) { - runExtractedEventsInBatch( - ((bookKeeping.topLevelType: any): DOMTopLevelEventType), + const eventSystemFlags = bookKeeping.eventSystemFlags; + const eventTarget = getEventTarget(bookKeeping.nativeEvent); + const topLevelType = ((bookKeeping.topLevelType: any): DOMTopLevelEventType); + const nativeEvent = ((bookKeeping.nativeEvent: any): AnyNativeEvent); + + if (eventSystemFlags === PLUGIN_EVENT_SYSTEM) { + runExtractedPluginEventsInBatch( + topLevelType, targetInst, - ((bookKeeping.nativeEvent: any): AnyNativeEvent), - getEventTarget(bookKeeping.nativeEvent), + nativeEvent, + eventTarget, + ); + } else if (enableEventAPI && targetInst !== null) { + // Responder event system (experimental event API) + runResponderEventsInBatch( + topLevelType, + targetInst, + nativeEvent, + eventTarget, + eventSystemFlags, ); - } else { - // RESPONDER_EVENT_SYSTEM - // TODO: Add implementation } } } diff --git a/packages/react-dom/src/events/__tests__/DOMEventResponderSystem-test.internal.js b/packages/react-dom/src/events/__tests__/DOMEventResponderSystem-test.internal.js new file mode 100644 index 0000000000000..81b2b9f2f4314 --- /dev/null +++ b/packages/react-dom/src/events/__tests__/DOMEventResponderSystem-test.internal.js @@ -0,0 +1,88 @@ +/** + * Copyright (c) Facebook, Inc. and its affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + * + * @emails react-core + */ + +'use strict'; + +let React; +let ReactFeatureFlags; +let ReactDOM; + +function createReactEventComponent(targetEventTypes, handleEvent) { + const testEventResponder = { + targetEventTypes, + handleEvent, + }; + + return { + $$typeof: Symbol.for('react.event_component'), + props: null, + responder: testEventResponder, + }; +} + +function dispatchClickEvent(element) { + const clickEvent = document.createEvent('Event'); + clickEvent.initEvent('click', true, true); + element.dispatchEvent(clickEvent); +} + +// This is a new feature in Fiber so I put it in its own test file. It could +// probably move to one of the other test files once it is official. +describe('DOMEventResponderSystem', () => { + let container; + + beforeEach(() => { + jest.resetModules(); + ReactFeatureFlags = require('shared/ReactFeatureFlags'); + ReactFeatureFlags.enableEventAPI = true; + React = require('react'); + ReactDOM = require('react-dom'); + container = document.createElement('div'); + document.body.appendChild(container); + }); + + afterEach(() => { + document.body.removeChild(container); + container = null; + }); + + it('the event responder handleEvent() function should fire on click event', () => { + let eventResponderFiredCount = 0; + const buttonRef = React.createRef(); + + const ClickEventComponent = createReactEventComponent(['click'], () => { + eventResponderFiredCount++; + }); + + const Test = () => ( + + + + ); + + ReactDOM.render(, container); + expect(container.innerHTML).toBe(''); + + // Clicking the button should trigger the event responder handleEvent() + let buttonElement = buttonRef.current; + dispatchClickEvent(buttonElement); + expect(eventResponderFiredCount).toBe(1); + + // Unmounting the container and clicking should not increment anything + ReactDOM.render(null, container); + dispatchClickEvent(buttonElement); + expect(eventResponderFiredCount).toBe(1); + + // Re-rendering the container and clicking should increase the counter again + ReactDOM.render(, container); + buttonElement = buttonRef.current; + dispatchClickEvent(buttonElement); + expect(eventResponderFiredCount).toBe(2); + }); +}); diff --git a/packages/react-dom/src/fire/ReactFire.js b/packages/react-dom/src/fire/ReactFire.js index 3e2f00b21f9f6..525f935882254 100644 --- a/packages/react-dom/src/fire/ReactFire.js +++ b/packages/react-dom/src/fire/ReactFire.js @@ -49,10 +49,9 @@ import { enqueueStateRestore, restoreStateIfNeeded, } from 'events/ReactControlledComponent'; -import { - injection as EventPluginHubInjection, - runEventsInBatch, -} from 'events/EventPluginHub'; +import {injection as EventPluginHubInjection} from 'events/EventPluginHub'; + +import {runEventsInBatch} from 'events/EventBatching'; import {eventNameDispatchConfigs} from 'events/EventPluginRegistry'; import { accumulateTwoPhaseDispatches, diff --git a/packages/react-native-renderer/src/ReactFabricEventEmitter.js b/packages/react-native-renderer/src/ReactFabricEventEmitter.js index 7e016f8ce8c51..eead4ba1d10a2 100644 --- a/packages/react-native-renderer/src/ReactFabricEventEmitter.js +++ b/packages/react-native-renderer/src/ReactFabricEventEmitter.js @@ -9,7 +9,10 @@ import type {Fiber} from 'react-reconciler/src/ReactFiber'; -import {getListener, runExtractedEventsInBatch} from 'events/EventPluginHub'; +import { + getListener, + runExtractedPluginEventsInBatch, +} from 'events/EventPluginHub'; import {registrationNameModules} from 'events/EventPluginRegistry'; import {batchedUpdates} from 'events/ReactGenericBatching'; @@ -25,7 +28,7 @@ export function dispatchEvent( ) { const targetFiber = (target: null | Fiber); batchedUpdates(function() { - runExtractedEventsInBatch( + runExtractedPluginEventsInBatch( topLevelType, targetFiber, nativeEvent, diff --git a/packages/react-native-renderer/src/ReactNativeEventEmitter.js b/packages/react-native-renderer/src/ReactNativeEventEmitter.js index a5ec6e298f13d..912291cd457a6 100644 --- a/packages/react-native-renderer/src/ReactNativeEventEmitter.js +++ b/packages/react-native-renderer/src/ReactNativeEventEmitter.js @@ -7,7 +7,10 @@ * @flow */ -import {getListener, runExtractedEventsInBatch} from 'events/EventPluginHub'; +import { + getListener, + runExtractedPluginEventsInBatch, +} from 'events/EventPluginHub'; import {registrationNameModules} from 'events/EventPluginRegistry'; import {batchedUpdates} from 'events/ReactGenericBatching'; import warningWithoutStack from 'shared/warningWithoutStack'; @@ -95,7 +98,7 @@ function _receiveRootNodeIDEvent( const nativeEvent = nativeEventParam || EMPTY_NATIVE_EVENT; const inst = getInstanceFromNode(rootNodeID); batchedUpdates(function() { - runExtractedEventsInBatch( + runExtractedPluginEventsInBatch( topLevelType, inst, nativeEvent, diff --git a/packages/react-reconciler/src/__tests__/ReactFiberEvents-test-internal.js b/packages/react-reconciler/src/__tests__/ReactFiberEvents-test-internal.js index 9e7bcf180471f..0740cb39a106a 100644 --- a/packages/react-reconciler/src/__tests__/ReactFiberEvents-test-internal.js +++ b/packages/react-reconciler/src/__tests__/ReactFiberEvents-test-internal.js @@ -53,7 +53,7 @@ function initTestRenderer() { // This is a new feature in Fiber so I put it in its own test file. It could // probably move to one of the other test files once it is official. -describe('ReactTopLevelText', () => { +describe('ReactFiberEvents', () => { describe('NoopRenderer', () => { beforeEach(() => { initNoopRenderer(); From 5d74ddedb7df3cce09a9198442a3daf3758cc3c3 Mon Sep 17 00:00:00 2001 From: Dominic Gannaway Date: Thu, 21 Mar 2019 17:44:25 +0000 Subject: [PATCH 02/12] Fix failing test --- .../events/__tests__/ResponderEventPlugin-test.internal.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/events/__tests__/ResponderEventPlugin-test.internal.js b/packages/events/__tests__/ResponderEventPlugin-test.internal.js index e828ae21e4ba7..84d132993889e 100644 --- a/packages/events/__tests__/ResponderEventPlugin-test.internal.js +++ b/packages/events/__tests__/ResponderEventPlugin-test.internal.js @@ -11,7 +11,7 @@ const {HostComponent} = require('shared/ReactWorkTags'); -let EventPluginHub; +let EventBatching; let EventPluginUtils; let ResponderEventPlugin; @@ -321,7 +321,7 @@ const run = function(config, hierarchyConfig, nativeEventConfig) { // At this point the negotiation events have been dispatched as part of the // extraction process, but not the side effectful events. Below, we dispatch // side effectful events. - EventPluginHub.runEventsInBatch(extractedEvents); + EventBatching.runEventsInBatch(extractedEvents); // Ensure that every event that declared an `order`, was actually dispatched. expect('number of events dispatched:' + runData.dispatchCount).toBe( @@ -403,7 +403,7 @@ describe('ResponderEventPlugin', () => { jest.resetModules(); const ReactDOMUnstableNativeDependencies = require('react-dom/unstable-native-dependencies'); - EventPluginHub = require('events/EventPluginHub'); + EventBatching = require('events/EventBatching'); EventPluginUtils = require('events/EventPluginUtils'); ResponderEventPlugin = ReactDOMUnstableNativeDependencies.ResponderEventPlugin; From b16cc896d8551e19f8aa1a4a9527d0e840056468 Mon Sep 17 00:00:00 2001 From: Dominic Gannaway Date: Thu, 21 Mar 2019 18:07:45 +0000 Subject: [PATCH 03/12] Fix import --- packages/react-dom/src/events/ChangeEventPlugin.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/react-dom/src/events/ChangeEventPlugin.js b/packages/react-dom/src/events/ChangeEventPlugin.js index 9e7ba2953edd8..214becb6732a6 100644 --- a/packages/react-dom/src/events/ChangeEventPlugin.js +++ b/packages/react-dom/src/events/ChangeEventPlugin.js @@ -5,7 +5,7 @@ * LICENSE file in the root directory of this source tree. */ -import {runEventsInBatch} from 'events/EventPluginHub'; +import {runEventsInBatch} from 'events/EventBatching'; import {accumulateTwoPhaseDispatches} from 'events/EventPropagators'; import {enqueueStateRestore} from 'events/ReactControlledComponent'; import {batchedUpdates} from 'events/ReactGenericBatching'; From 38b98c7db69e3f63206d86a56061c32e78c124c9 Mon Sep 17 00:00:00 2001 From: Dominic Gannaway Date: Thu, 21 Mar 2019 20:22:31 +0000 Subject: [PATCH 04/12] Updated code to add a long explanation plus DCE wrappers --- .../src/client/ReactDOMHostConfig.js | 54 ++++++++++--------- .../src/events/DOMEventResponderSystem.js | 21 +++++++- 2 files changed, 49 insertions(+), 26 deletions(-) diff --git a/packages/react-dom/src/client/ReactDOMHostConfig.js b/packages/react-dom/src/client/ReactDOMHostConfig.js index e2177b517a3c9..8d97879daf4b6 100644 --- a/packages/react-dom/src/client/ReactDOMHostConfig.js +++ b/packages/react-dom/src/client/ReactDOMHostConfig.js @@ -864,18 +864,20 @@ export function handleEventComponent( rootContainerInstance: Container, internalInstanceHandle: Object, ): void { - // We keep the event responder hub up-to-do with the state of - // event component fiber tree (it uses this to know what events to fire). - // To do this, we update the current event component fiber and remove - // the alternate fiber if it exists. - const eventComponetFiber = ((internalInstanceHandle: any): Fiber); - currentEventComponentFibers.add(eventComponetFiber); - const alternateFiber = eventComponetFiber.alternate; - if (alternateFiber !== null) { - currentEventComponentFibers.delete(alternateFiber); + if (enableEventAPI) { + // We keep the event responder hub up-to-do with the state of + // event component fiber tree (it uses this to know what events to fire). + // To do this, we update the current event component fiber and remove + // the alternate fiber if it exists. + const eventComponetFiber = ((internalInstanceHandle: any): Fiber); + currentEventComponentFibers.add(eventComponetFiber); + const alternateFiber = eventComponetFiber.alternate; + if (alternateFiber !== null) { + currentEventComponentFibers.delete(alternateFiber); + } + const rootElement = rootContainerInstance.ownerDocument; + listenToEventResponderEvents(eventResponder, rootElement); } - const rootElement = rootContainerInstance.ownerDocument; - listenToEventResponderEvents(eventResponder, rootElement); } export function handleEventTarget( @@ -883,20 +885,22 @@ export function handleEventTarget( props: Props, internalInstanceHandle: Object, ): void { - // Touch target hit slop handling - if (type === REACT_EVENT_TARGET_TOUCH_HIT) { - // Validates that there is a single element - const element = getElementFromTouchHitTarget(internalInstanceHandle); - if (element !== null) { - // We update the event target state node to be that of the element. - // We can then diff this entry to determine if we need to add the - // hit slop element, or change the dimensions of the hit slop. - const lastElement = internalInstanceHandle.stateNode; - if (lastElement !== element) { - internalInstanceHandle.stateNode = element; - // TODO: Create the hit slop element and attach it to the element - } else { - // TODO: Diff the left, top, right, bottom props + if (enableEventAPI) { + // Touch target hit slop handling + if (type === REACT_EVENT_TARGET_TOUCH_HIT) { + // Validates that there is a single element + const element = getElementFromTouchHitTarget(internalInstanceHandle); + if (element !== null) { + // We update the event target state node to be that of the element. + // We can then diff this entry to determine if we need to add the + // hit slop element, or change the dimensions of the hit slop. + const lastElement = internalInstanceHandle.stateNode; + if (lastElement !== element) { + internalInstanceHandle.stateNode = element; + // TODO: Create the hit slop element and attach it to the element + } else { + // TODO: Diff the left, top, right, bottom props + } } } } diff --git a/packages/react-dom/src/events/DOMEventResponderSystem.js b/packages/react-dom/src/events/DOMEventResponderSystem.js index f458aa9ebb2e3..151b2c79f2314 100644 --- a/packages/react-dom/src/events/DOMEventResponderSystem.js +++ b/packages/react-dom/src/events/DOMEventResponderSystem.js @@ -11,6 +11,7 @@ import type {AnyNativeEvent} from 'events/PluginModuleType'; import {EventComponent} from 'shared/ReactWorkTags'; import type {DOMTopLevelEventType} from 'events/TopLevelEventTypes'; import type {ReactEventResponder} from 'shared/ReactTypes'; +import invariant from 'shared/invariant'; import warning from 'shared/warning'; import type {Fiber} from 'react-reconciler/src/ReactFiber'; @@ -102,7 +103,25 @@ export function runResponderEventsInBatch( // Traverse up the fiber tree till we find event component fibers. while (node !== null) { if (node.tag === EventComponent) { - if (node.alternate !== null && !currentEventComponentFibers.has(node)) { + // When we traverse the fiber tree from the target fiber, we will + // ecounter event component fibers that might not be the current + // fiber. This will happen frequently because of how ReactDOM + // stores elements relative to their fibers. When we create or + // mount elements, we store their fiber on the element. We never + // update the fiber when the element updates to its alternate fiber, + // we only update the props for the fiber. Furthermore, we also + // never update the props if the element doesn't need an update. + // That means that an element target might point to a fiber tree + // that is stale and not the current tree. To get around this, we + // always store the current event component in a Set and use this + // logic to determine when we need to swith to the event component + // fiber alternate. + if (!currentEventComponentFibers.has(node)) { + invariant( + node.alternate !== null, + 'runResponderEventsInBatch failed to find the active fiber. ' + + 'This is most definitely a bug in React.', + ); node = node.alternate; } // TODO create a responder context and pass it through From c3556e39086d2a59455e340356c60ee484351944 Mon Sep 17 00:00:00 2001 From: Dominic Gannaway Date: Fri, 22 Mar 2019 11:38:30 +0000 Subject: [PATCH 05/12] Add passive event checks in tests --- .../src/events/DOMEventResponderSystem.js | 36 +++++++++-- .../DOMEventResponderSystem-test.internal.js | 61 ++++++++++++++++++- 2 files changed, 90 insertions(+), 7 deletions(-) diff --git a/packages/react-dom/src/events/DOMEventResponderSystem.js b/packages/react-dom/src/events/DOMEventResponderSystem.js index 151b2c79f2314..ef53a880268ee 100644 --- a/packages/react-dom/src/events/DOMEventResponderSystem.js +++ b/packages/react-dom/src/events/DOMEventResponderSystem.js @@ -6,7 +6,11 @@ * @flow */ -import {type EventSystemFlags} from 'events/EventSystemFlags'; +import { + type EventSystemFlags, + IS_PASSIVE, + PASSIVE_NOT_SUPPORTED, +} from 'events/EventSystemFlags'; import type {AnyNativeEvent} from 'events/PluginModuleType'; import {EventComponent} from 'shared/ReactWorkTags'; import type {DOMTopLevelEventType} from 'events/TopLevelEventTypes'; @@ -32,6 +36,27 @@ export const eventResponderValidEventTypes: Map< Set, > = new Map(); +// TODO add context methods for dispatching events +function DOMEventResponderContext( + topLevelType: DOMTopLevelEventType, + nativeEvent: AnyNativeEvent, + nativeEventTarget: EventTarget, + eventSystemFlags: EventSystemFlags, +) { + this.event = nativeEvent; + this.eventType = topLevelType; + this.eventTarget = nativeEventTarget; + this._flags = eventSystemFlags; +} + +DOMEventResponderContext.prototype.isPassive = function(): boolean { + return (this._flags & IS_PASSIVE) !== 0; +}; + +DOMEventResponderContext.prototype.isPassiveSupported = function(): boolean { + return (this._flags & PASSIVE_NOT_SUPPORTED) === 0; +}; + function createValidEventTypeSet(targetEventTypes): Set { const eventTypeSet = new Set(); // Go through each target event type of the event responder @@ -97,8 +122,12 @@ export function runResponderEventsInBatch( nativeEventTarget: EventTarget, eventSystemFlags: EventSystemFlags, ): void { - // TODO add proper event context - let context = ({}: any); + let context = new DOMEventResponderContext( + topLevelType, + nativeEvent, + nativeEventTarget, + eventSystemFlags, + ); let node = targetFiber; // Traverse up the fiber tree till we find event component fibers. while (node !== null) { @@ -124,7 +153,6 @@ export function runResponderEventsInBatch( ); node = node.alternate; } - // TODO create a responder context and pass it through handleTopLevelType(topLevelType, node, context); } node = node.return; diff --git a/packages/react-dom/src/events/__tests__/DOMEventResponderSystem-test.internal.js b/packages/react-dom/src/events/__tests__/DOMEventResponderSystem-test.internal.js index 81b2b9f2f4314..b0b022b064df8 100644 --- a/packages/react-dom/src/events/__tests__/DOMEventResponderSystem-test.internal.js +++ b/packages/react-dom/src/events/__tests__/DOMEventResponderSystem-test.internal.js @@ -54,11 +54,20 @@ describe('DOMEventResponderSystem', () => { it('the event responder handleEvent() function should fire on click event', () => { let eventResponderFiredCount = 0; + let eventLog = []; const buttonRef = React.createRef(); - const ClickEventComponent = createReactEventComponent(['click'], () => { - eventResponderFiredCount++; - }); + const ClickEventComponent = createReactEventComponent( + ['click'], + (context, props) => { + eventResponderFiredCount++; + eventLog.push({ + name: context.eventType, + passive: context.isPassive(), + passiveSupported: context.isPassiveSupported(), + }); + }, + ); const Test = () => ( @@ -73,6 +82,13 @@ describe('DOMEventResponderSystem', () => { let buttonElement = buttonRef.current; dispatchClickEvent(buttonElement); expect(eventResponderFiredCount).toBe(1); + expect(eventLog.length).toBe(1); + // JSDOM does not support passive events, so this will be false + expect(eventLog[0]).toEqual({ + name: 'click', + passive: false, + passiveSupported: false, + }); // Unmounting the container and clicking should not increment anything ReactDOM.render(null, container); @@ -85,4 +101,43 @@ describe('DOMEventResponderSystem', () => { dispatchClickEvent(buttonElement); expect(eventResponderFiredCount).toBe(2); }); + + it('the event responder handleEvent() function should fire on click event (passive events forced)', () => { + // JSDOM does not support passive events, so this manually overrides the value to be true + const checkPassiveEvents = require('react-dom/src/events/checkPassiveEvents'); + checkPassiveEvents.passiveBrowserEventsSupported = true; + + let eventLog = []; + const buttonRef = React.createRef(); + + const ClickEventComponent = createReactEventComponent( + ['click'], + (context, props) => { + eventLog.push({ + name: context.eventType, + passive: context.isPassive(), + passiveSupported: context.isPassiveSupported(), + }); + }, + ); + + const Test = () => ( + + + + ); + + ReactDOM.render(, container); + + // Clicking the button should trigger the event responder handleEvent() + let buttonElement = buttonRef.current; + dispatchClickEvent(buttonElement); + expect(eventLog.length).toBe(1); + // JSDOM does not support passive events, so this will be false + expect(eventLog[0]).toEqual({ + name: 'click', + passive: true, + passiveSupported: true, + }); + }); }); From 006fcebba92196fc091697ecb6c5110004b0dfbd Mon Sep 17 00:00:00 2001 From: Dominic Gannaway Date: Fri, 22 Mar 2019 14:52:45 +0000 Subject: [PATCH 06/12] Add more tests --- .../DOMEventResponderSystem-test.internal.js | 81 ++++++++++++++++++- 1 file changed, 80 insertions(+), 1 deletion(-) diff --git a/packages/react-dom/src/events/__tests__/DOMEventResponderSystem-test.internal.js b/packages/react-dom/src/events/__tests__/DOMEventResponderSystem-test.internal.js index b0b022b064df8..0ddd3463be5f1 100644 --- a/packages/react-dom/src/events/__tests__/DOMEventResponderSystem-test.internal.js +++ b/packages/react-dom/src/events/__tests__/DOMEventResponderSystem-test.internal.js @@ -133,11 +133,90 @@ describe('DOMEventResponderSystem', () => { let buttonElement = buttonRef.current; dispatchClickEvent(buttonElement); expect(eventLog.length).toBe(1); - // JSDOM does not support passive events, so this will be false expect(eventLog[0]).toEqual({ name: 'click', passive: true, passiveSupported: true, }); }); + + it('nested event responders and their handleEvent() function should fire multiple times', () => { + let eventResponderFiredCount = 0; + let eventLog = []; + const buttonRef = React.createRef(); + + const ClickEventComponent = createReactEventComponent( + ['click'], + (context, props) => { + eventResponderFiredCount++; + eventLog.push({ + name: context.eventType, + passive: context.isPassive(), + passiveSupported: context.isPassiveSupported(), + }); + }, + ); + + const Test = () => ( + + + + + + ); + + ReactDOM.render(, container); + + // Clicking the button should trigger the event responder handleEvent() + let buttonElement = buttonRef.current; + dispatchClickEvent(buttonElement); + expect(eventResponderFiredCount).toBe(2); + expect(eventLog.length).toBe(2); + // JSDOM does not support passive events, so this will be false + expect(eventLog[0]).toEqual({ + name: 'click', + passive: false, + passiveSupported: false, + }); + expect(eventLog[1]).toEqual({ + name: 'click', + passive: false, + passiveSupported: false, + }); + }); + + it('nested event responders and their handleEvent() should fire in the correct order', () => { + let eventLog = []; + const buttonRef = React.createRef(); + + const ClickEventComponentA = createReactEventComponent( + ['click'], + (context, props) => { + eventLog.push('A'); + }, + ); + + const ClickEventComponentB = createReactEventComponent( + ['click'], + (context, props) => { + eventLog.push('B'); + }, + ); + + const Test = () => ( + + + + + + ); + + ReactDOM.render(, container); + + // Clicking the button should trigger the event responder handleEvent() + let buttonElement = buttonRef.current; + dispatchClickEvent(buttonElement); + + expect(eventLog).toEqual(['B', 'A']); + }); }); From 04ac91bfea1de1be966478730e4a13cac1c4eefc Mon Sep 17 00:00:00 2001 From: Dominic Gannaway Date: Mon, 25 Mar 2019 11:39:19 -0700 Subject: [PATCH 07/12] Adds event dispatching logic --- .../src/events/DOMEventResponderSystem.js | 86 ++++++++++++++++++- .../src/events/ReactDOMEventListener.js | 5 +- 2 files changed, 84 insertions(+), 7 deletions(-) diff --git a/packages/react-dom/src/events/DOMEventResponderSystem.js b/packages/react-dom/src/events/DOMEventResponderSystem.js index ef53a880268ee..674b3ae746246 100644 --- a/packages/react-dom/src/events/DOMEventResponderSystem.js +++ b/packages/react-dom/src/events/DOMEventResponderSystem.js @@ -13,12 +13,18 @@ import { } from 'events/EventSystemFlags'; import type {AnyNativeEvent} from 'events/PluginModuleType'; import {EventComponent} from 'shared/ReactWorkTags'; -import type {DOMTopLevelEventType} from 'events/TopLevelEventTypes'; import type {ReactEventResponder} from 'shared/ReactTypes'; import invariant from 'shared/invariant'; import warning from 'shared/warning'; +import type {DOMTopLevelEventType} from 'events/TopLevelEventTypes'; +import accumulateInto from 'events/accumulateInto'; +import SyntheticEvent from 'events/SyntheticEvent'; +import {runEventsInBatch} from 'events/ReactGenericBatching'; +import {interactiveUpdates} from 'events/ReactGenericBatching'; import type {Fiber} from 'react-reconciler/src/ReactFiber'; +import {getClosestInstanceFromNode} from '../client/ReactDOMComponentTree'; + // We track the active component fibers so we can traverse through // the fiber tree and find the relative current fibers. We need to // do this because an update might have switched an event component @@ -36,6 +42,8 @@ export const eventResponderValidEventTypes: Map< Set, > = new Map(); +type EventListener = (event: SyntheticEvent) => void; + // TODO add context methods for dispatching events function DOMEventResponderContext( topLevelType: DOMTopLevelEventType, @@ -47,6 +55,10 @@ function DOMEventResponderContext( this.eventType = topLevelType; this.eventTarget = nativeEventTarget; this._flags = eventSystemFlags; + this._fiber = null; + this._responder = null; + this._discreteEvents = []; + this._nonDiscreteEvents = []; } DOMEventResponderContext.prototype.isPassive = function(): boolean { @@ -57,6 +69,73 @@ DOMEventResponderContext.prototype.isPassiveSupported = function(): boolean { return (this._flags & PASSIVE_NOT_SUPPORTED) === 0; }; +function copyEventProperties(eventData, syntheticEvent) { + for (let propName in eventData) { + syntheticEvent[propName] = eventData[propName]; + } +} + +DOMEventResponderContext.prototype.dispatchEvent = function( + name: string, + eventListener: EventListener, + eventTarget: AnyNativeEvent, + capture: boolean, + discrete: boolean, + extraProperties?: Object, +): void { + const eventTargetFiber = getClosestInstanceFromNode(eventTarget); + const syntheticEvent = SyntheticEvent.getPooled( + null, + eventTargetFiber, + this.event, + eventTarget, + ); + if (extraProperties !== undefined) { + copyEventProperties(extraProperties, syntheticEvent); + } + syntheticEvent.type = name; + syntheticEvent._dispatchInstances = [eventTargetFiber]; + syntheticEvent._dispatchListeners = [eventListener]; + syntheticEvent.capture = capture; + if (discrete) { + this._discreteEvents.push(syntheticEvent); + } else { + this._nonDiscreteEvents.push(syntheticEvent); + } +}; + +function accumulateTwoPhaseEvents( + events: Array, +): Array { + let i; + // Capture phase + for (i = events.length; i-- > 0; ) { + const syntheticEvent = events[i]; + if (syntheticEvent.capture) { + events = accumulateInto(events, syntheticEvent); + } + } + // Bubble phase + for (i = 0; i < events.length; i++) { + const syntheticEvent = events[i]; + if (!syntheticEvent.capture) { + events = accumulateInto(events, syntheticEvent); + } + } + return events; +} + +DOMEventResponderContext.prototype._runEventsInBatch = function(): void { + if (this._discreteEvents.length > 0) { + interactiveUpdates(() => { + runEventsInBatch(accumulateTwoPhaseEvents(this._discreteEvents)); + }); + } + if (this._nonDiscreteEvents.length > 0) { + runEventsInBatch(accumulateTwoPhaseEvents(this._nonDiscreteEvents)); + } +}; + function createValidEventTypeSet(targetEventTypes): Set { const eventTypeSet = new Set(); // Go through each target event type of the event responder @@ -111,7 +190,8 @@ function handleTopLevelType( state = responder.createInitialState(props); stateNode.set(responder, state); } - // TODO provide all the props for handleEvent + context._fiber = fiber; + context._responder = responder; responder.handleEvent(context, props, state); } @@ -157,5 +237,5 @@ export function runResponderEventsInBatch( } node = node.return; } - // TODO dispatch extracted events from context (with batching) + context._runEventsInBatch(); } diff --git a/packages/react-dom/src/events/ReactDOMEventListener.js b/packages/react-dom/src/events/ReactDOMEventListener.js index 0b5e634d38482..5591646b19807 100644 --- a/packages/react-dom/src/events/ReactDOMEventListener.js +++ b/packages/react-dom/src/events/ReactDOMEventListener.js @@ -188,9 +188,6 @@ export function trapEventForResponderEventSystem( passive: boolean, ): void { if (enableEventAPI) { - const dispatch = isInteractiveTopLevelEventType(topLevelType) - ? dispatchInteractiveEvent - : dispatchEvent; const rawEventName = getRawEventName(topLevelType); let eventFlags = RESPONDER_EVENT_SYSTEM; @@ -210,7 +207,7 @@ export function trapEventForResponderEventSystem( eventFlags |= IS_ACTIVE; } // Check if interactive and wrap in interactiveUpdates - const listener = dispatch.bind(null, topLevelType, eventFlags); + const listener = dispatchEvent.bind(null, topLevelType, eventFlags); addEventListener(element, rawEventName, listener, { capture, passive, From 81ceb25dcb50b7501c02760ce1e9691e2b50d7bd Mon Sep 17 00:00:00 2001 From: Dominic Gannaway Date: Mon, 25 Mar 2019 22:35:04 -0700 Subject: [PATCH 08/12] Fix build --- packages/react-dom/src/events/DOMEventResponderSystem.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/react-dom/src/events/DOMEventResponderSystem.js b/packages/react-dom/src/events/DOMEventResponderSystem.js index 674b3ae746246..8893471274ae4 100644 --- a/packages/react-dom/src/events/DOMEventResponderSystem.js +++ b/packages/react-dom/src/events/DOMEventResponderSystem.js @@ -19,7 +19,7 @@ import warning from 'shared/warning'; import type {DOMTopLevelEventType} from 'events/TopLevelEventTypes'; import accumulateInto from 'events/accumulateInto'; import SyntheticEvent from 'events/SyntheticEvent'; -import {runEventsInBatch} from 'events/ReactGenericBatching'; +import {runEventsInBatch} from 'events/EventBatching'; import {interactiveUpdates} from 'events/ReactGenericBatching'; import type {Fiber} from 'react-reconciler/src/ReactFiber'; From 989b65df1b0afe4ab6364ed893450c4b433fbc6c Mon Sep 17 00:00:00 2001 From: Dominic Gannaway Date: Tue, 26 Mar 2019 08:51:23 -0700 Subject: [PATCH 09/12] Address PR feedback --- .../src/client/ReactDOMHostConfig.js | 12 ------ .../src/events/DOMEventResponderSystem.js | 37 ++----------------- packages/react-reconciler/src/ReactFiber.js | 5 ++- .../src/ReactFiberCompleteWork.js | 2 + 4 files changed, 10 insertions(+), 46 deletions(-) diff --git a/packages/react-dom/src/client/ReactDOMHostConfig.js b/packages/react-dom/src/client/ReactDOMHostConfig.js index 8d97879daf4b6..52541406190dd 100644 --- a/packages/react-dom/src/client/ReactDOMHostConfig.js +++ b/packages/react-dom/src/client/ReactDOMHostConfig.js @@ -44,7 +44,6 @@ import { import dangerousStyleValue from '../shared/dangerousStyleValue'; import type {DOMContainer} from './ReactDOM'; -import {currentEventComponentFibers} from '../events/DOMEventResponderSystem'; import type {ReactEventResponder} from 'shared/ReactTypes'; import { REACT_EVENT_COMPONENT_TYPE, @@ -52,7 +51,6 @@ import { REACT_EVENT_TARGET_TOUCH_HIT, } from 'shared/ReactSymbols'; import getElementFromTouchHitTarget from 'shared/getElementFromTouchHitTarget'; -import type {Fiber} from 'react-reconciler/src/ReactFiber'; export type Type = string; export type Props = { @@ -865,16 +863,6 @@ export function handleEventComponent( internalInstanceHandle: Object, ): void { if (enableEventAPI) { - // We keep the event responder hub up-to-do with the state of - // event component fiber tree (it uses this to know what events to fire). - // To do this, we update the current event component fiber and remove - // the alternate fiber if it exists. - const eventComponetFiber = ((internalInstanceHandle: any): Fiber); - currentEventComponentFibers.add(eventComponetFiber); - const alternateFiber = eventComponetFiber.alternate; - if (alternateFiber !== null) { - currentEventComponentFibers.delete(alternateFiber); - } const rootElement = rootContainerInstance.ownerDocument; listenToEventResponderEvents(eventResponder, rootElement); } diff --git a/packages/react-dom/src/events/DOMEventResponderSystem.js b/packages/react-dom/src/events/DOMEventResponderSystem.js index 8893471274ae4..86b3d8bea414e 100644 --- a/packages/react-dom/src/events/DOMEventResponderSystem.js +++ b/packages/react-dom/src/events/DOMEventResponderSystem.js @@ -14,7 +14,6 @@ import { import type {AnyNativeEvent} from 'events/PluginModuleType'; import {EventComponent} from 'shared/ReactWorkTags'; import type {ReactEventResponder} from 'shared/ReactTypes'; -import invariant from 'shared/invariant'; import warning from 'shared/warning'; import type {DOMTopLevelEventType} from 'events/TopLevelEventTypes'; import accumulateInto from 'events/accumulateInto'; @@ -25,12 +24,6 @@ import type {Fiber} from 'react-reconciler/src/ReactFiber'; import {getClosestInstanceFromNode} from '../client/ReactDOMComponentTree'; -// We track the active component fibers so we can traverse through -// the fiber tree and find the relative current fibers. We need to -// do this because an update might have switched an event component -// fiber to its alternate fiber. -export const currentEventComponentFibers: Set = new Set(); - // Event responders provide us an array of target event types. // To ensure we fire the right responders for given events, we check // if the incoming event type is actually relevant for an event @@ -170,8 +163,7 @@ function handleTopLevelType( context: Object, ): void { const responder: ReactEventResponder = fiber.type.responder; - const props = fiber.memoizedProps; - const stateNode = fiber.stateNode; + const {props, stateMap} = fiber.stateNode; let validEventTypesForResponder = eventResponderValidEventTypes.get( responder, ); @@ -185,10 +177,10 @@ function handleTopLevelType( if (!validEventTypesForResponder.has(topLevelType)) { return; } - let state = stateNode.get(responder); + let state = stateMap.get(responder); if (state === undefined && responder.createInitialState !== undefined) { state = responder.createInitialState(props); - stateNode.set(responder, state); + stateMap.set(responder, state); } context._fiber = fiber; context._responder = responder; @@ -202,7 +194,7 @@ export function runResponderEventsInBatch( nativeEventTarget: EventTarget, eventSystemFlags: EventSystemFlags, ): void { - let context = new DOMEventResponderContext( + const context = new DOMEventResponderContext( topLevelType, nativeEvent, nativeEventTarget, @@ -212,27 +204,6 @@ export function runResponderEventsInBatch( // Traverse up the fiber tree till we find event component fibers. while (node !== null) { if (node.tag === EventComponent) { - // When we traverse the fiber tree from the target fiber, we will - // ecounter event component fibers that might not be the current - // fiber. This will happen frequently because of how ReactDOM - // stores elements relative to their fibers. When we create or - // mount elements, we store their fiber on the element. We never - // update the fiber when the element updates to its alternate fiber, - // we only update the props for the fiber. Furthermore, we also - // never update the props if the element doesn't need an update. - // That means that an element target might point to a fiber tree - // that is stale and not the current tree. To get around this, we - // always store the current event component in a Set and use this - // logic to determine when we need to swith to the event component - // fiber alternate. - if (!currentEventComponentFibers.has(node)) { - invariant( - node.alternate !== null, - 'runResponderEventsInBatch failed to find the active fiber. ' + - 'This is most definitely a bug in React.', - ); - node = node.alternate; - } handleTopLevelType(topLevelType, node, context); } node = node.return; diff --git a/packages/react-reconciler/src/ReactFiber.js b/packages/react-reconciler/src/ReactFiber.js index b5a8b249a54c3..fe4891e35c0b0 100644 --- a/packages/react-reconciler/src/ReactFiber.js +++ b/packages/react-reconciler/src/ReactFiber.js @@ -623,7 +623,10 @@ export function createFiberFromEventComponent( const fiber = createFiber(EventComponent, pendingProps, key, mode); fiber.elementType = eventComponent; fiber.type = eventComponent; - fiber.stateNode = new Map(); + fiber.stateNode = { + props: pendingProps, + stateMap: new Map(), + }; fiber.expirationTime = expirationTime; return fiber; } diff --git a/packages/react-reconciler/src/ReactFiberCompleteWork.js b/packages/react-reconciler/src/ReactFiberCompleteWork.js index 8a16dbe43befd..1a0fd30232262 100644 --- a/packages/react-reconciler/src/ReactFiberCompleteWork.js +++ b/packages/react-reconciler/src/ReactFiberCompleteWork.js @@ -774,6 +774,8 @@ function completeWork( popHostContext(workInProgress); const rootContainerInstance = getRootHostContainer(); const responder = workInProgress.type.responder; + // Update the props on the event component state node + workInProgress.stateNode.props = newProps; handleEventComponent(responder, rootContainerInstance, workInProgress); } break; From 7cc5404a6f66b46edbcd3dbd8894740578c158b1 Mon Sep 17 00:00:00 2001 From: Dominic Gannaway Date: Tue, 26 Mar 2019 14:28:55 -0700 Subject: [PATCH 10/12] Fix a few bugs and added a test --- .../src/events/DOMEventResponderSystem.js | 75 +++++++++++-------- .../DOMEventResponderSystem-test.internal.js | 36 +++++++++ 2 files changed, 79 insertions(+), 32 deletions(-) diff --git a/packages/react-dom/src/events/DOMEventResponderSystem.js b/packages/react-dom/src/events/DOMEventResponderSystem.js index 86b3d8bea414e..5f6656d4bc456 100644 --- a/packages/react-dom/src/events/DOMEventResponderSystem.js +++ b/packages/react-dom/src/events/DOMEventResponderSystem.js @@ -16,7 +16,6 @@ import {EventComponent} from 'shared/ReactWorkTags'; import type {ReactEventResponder} from 'shared/ReactTypes'; import warning from 'shared/warning'; import type {DOMTopLevelEventType} from 'events/TopLevelEventTypes'; -import accumulateInto from 'events/accumulateInto'; import SyntheticEvent from 'events/SyntheticEvent'; import {runEventsInBatch} from 'events/EventBatching'; import {interactiveUpdates} from 'events/ReactGenericBatching'; @@ -37,6 +36,13 @@ export const eventResponderValidEventTypes: Map< type EventListener = (event: SyntheticEvent) => void; +function createEventQueue() { + return { + bubble: [], + capture: [], + }; +} + // TODO add context methods for dispatching events function DOMEventResponderContext( topLevelType: DOMTopLevelEventType, @@ -50,8 +56,8 @@ function DOMEventResponderContext( this._flags = eventSystemFlags; this._fiber = null; this._responder = null; - this._discreteEvents = []; - this._nonDiscreteEvents = []; + this._discreteEvents = null; + this._nonDiscreteEvents = null; } DOMEventResponderContext.prototype.isPassive = function(): boolean { @@ -69,11 +75,11 @@ function copyEventProperties(eventData, syntheticEvent) { } DOMEventResponderContext.prototype.dispatchEvent = function( - name: string, + eventName: string, eventListener: EventListener, eventTarget: AnyNativeEvent, - capture: boolean, - discrete: boolean, + capture?: boolean, + discrete?: boolean, extraProperties?: Object, ): void { const eventTargetFiber = getClosestInstanceFromNode(eventTarget); @@ -86,46 +92,51 @@ DOMEventResponderContext.prototype.dispatchEvent = function( if (extraProperties !== undefined) { copyEventProperties(extraProperties, syntheticEvent); } - syntheticEvent.type = name; + syntheticEvent.type = eventName; syntheticEvent._dispatchInstances = [eventTargetFiber]; syntheticEvent._dispatchListeners = [eventListener]; - syntheticEvent.capture = capture; + + let events; if (discrete) { - this._discreteEvents.push(syntheticEvent); + events = this._discreteEvents; + if (events === null) { + events = this._nonDiscreteEvents = createEventQueue(); + } + if (capture) { + events.capture.push(syntheticEvent); + } else { + events.bubble.push(syntheticEvent); + } } else { - this._nonDiscreteEvents.push(syntheticEvent); + events = this._nonDiscreteEvents; + if (events === null) { + events = this._nonDiscreteEvents = createEventQueue(); + } + if (capture) { + events.capture.push(syntheticEvent); + } else { + events.bubble.push(syntheticEvent); + } } }; -function accumulateTwoPhaseEvents( - events: Array, -): Array { - let i; - // Capture phase - for (i = events.length; i-- > 0; ) { - const syntheticEvent = events[i]; - if (syntheticEvent.capture) { - events = accumulateInto(events, syntheticEvent); - } - } - // Bubble phase - for (i = 0; i < events.length; i++) { - const syntheticEvent = events[i]; - if (!syntheticEvent.capture) { - events = accumulateInto(events, syntheticEvent); - } +function runTwoPhaseEvents({bubble, capture}) { + if (capture.length > 0) { + // We always fire capture events in LIFO order + capture.reverse(); } - return events; + runEventsInBatch(capture); + runEventsInBatch(bubble); } DOMEventResponderContext.prototype._runEventsInBatch = function(): void { - if (this._discreteEvents.length > 0) { + if (this._discreteEvents !== null) { interactiveUpdates(() => { - runEventsInBatch(accumulateTwoPhaseEvents(this._discreteEvents)); + runTwoPhaseEvents(this._discreteEvents); }); } - if (this._nonDiscreteEvents.length > 0) { - runEventsInBatch(accumulateTwoPhaseEvents(this._nonDiscreteEvents)); + if (this._nonDiscreteEvents !== null) { + runTwoPhaseEvents(this._nonDiscreteEvents); } }; diff --git a/packages/react-dom/src/events/__tests__/DOMEventResponderSystem-test.internal.js b/packages/react-dom/src/events/__tests__/DOMEventResponderSystem-test.internal.js index 0ddd3463be5f1..6ffb617fc2b85 100644 --- a/packages/react-dom/src/events/__tests__/DOMEventResponderSystem-test.internal.js +++ b/packages/react-dom/src/events/__tests__/DOMEventResponderSystem-test.internal.js @@ -219,4 +219,40 @@ describe('DOMEventResponderSystem', () => { expect(eventLog).toEqual(['B', 'A']); }); + + it('custom event dispatching for click -> magicClick works', () => { + let eventLog = []; + const buttonRef = React.createRef(); + + const ClickEventComponent = createReactEventComponent( + ['click'], + (context, props) => { + if (props.onMagicClick) { + context.dispatchEvent( + 'magicclick', + props.onMagicClick, + context.eventTarget, + ); + } + }, + ); + + function handleMagicEvent(e) { + eventLog.push('magic event fired', e.type); + } + + const Test = () => ( + + + + ); + + ReactDOM.render(, container); + + // Clicking the button should trigger the event responder handleEvent() + let buttonElement = buttonRef.current; + dispatchClickEvent(buttonElement); + + expect(eventLog).toEqual(['magic event fired', 'magicclick']); + }); }); From faab2023e97c1910169c616f6cc91e3813163973 Mon Sep 17 00:00:00 2001 From: Dominic Gannaway Date: Tue, 26 Mar 2019 14:58:11 -0700 Subject: [PATCH 11/12] Simplify logic --- .../src/events/DOMEventResponderSystem.js | 38 +++---------------- .../DOMEventResponderSystem-test.internal.js | 1 + 2 files changed, 7 insertions(+), 32 deletions(-) diff --git a/packages/react-dom/src/events/DOMEventResponderSystem.js b/packages/react-dom/src/events/DOMEventResponderSystem.js index 5f6656d4bc456..d4cb288eaff31 100644 --- a/packages/react-dom/src/events/DOMEventResponderSystem.js +++ b/packages/react-dom/src/events/DOMEventResponderSystem.js @@ -36,13 +36,6 @@ export const eventResponderValidEventTypes: Map< type EventListener = (event: SyntheticEvent) => void; -function createEventQueue() { - return { - bubble: [], - capture: [], - }; -} - // TODO add context methods for dispatching events function DOMEventResponderContext( topLevelType: DOMTopLevelEventType, @@ -78,8 +71,7 @@ DOMEventResponderContext.prototype.dispatchEvent = function( eventName: string, eventListener: EventListener, eventTarget: AnyNativeEvent, - capture?: boolean, - discrete?: boolean, + discrete: boolean, extraProperties?: Object, ): void { const eventTargetFiber = getClosestInstanceFromNode(eventTarget); @@ -100,43 +92,25 @@ DOMEventResponderContext.prototype.dispatchEvent = function( if (discrete) { events = this._discreteEvents; if (events === null) { - events = this._nonDiscreteEvents = createEventQueue(); - } - if (capture) { - events.capture.push(syntheticEvent); - } else { - events.bubble.push(syntheticEvent); + events = this._discreteEvents = []; } } else { events = this._nonDiscreteEvents; if (events === null) { - events = this._nonDiscreteEvents = createEventQueue(); - } - if (capture) { - events.capture.push(syntheticEvent); - } else { - events.bubble.push(syntheticEvent); + events = this._nonDiscreteEvents = []; } } + events.push(syntheticEvent); }; -function runTwoPhaseEvents({bubble, capture}) { - if (capture.length > 0) { - // We always fire capture events in LIFO order - capture.reverse(); - } - runEventsInBatch(capture); - runEventsInBatch(bubble); -} - DOMEventResponderContext.prototype._runEventsInBatch = function(): void { if (this._discreteEvents !== null) { interactiveUpdates(() => { - runTwoPhaseEvents(this._discreteEvents); + runEventsInBatch(this._discreteEvents); }); } if (this._nonDiscreteEvents !== null) { - runTwoPhaseEvents(this._nonDiscreteEvents); + runEventsInBatch(this._nonDiscreteEvents); } }; diff --git a/packages/react-dom/src/events/__tests__/DOMEventResponderSystem-test.internal.js b/packages/react-dom/src/events/__tests__/DOMEventResponderSystem-test.internal.js index 6ffb617fc2b85..4af4f210dffc1 100644 --- a/packages/react-dom/src/events/__tests__/DOMEventResponderSystem-test.internal.js +++ b/packages/react-dom/src/events/__tests__/DOMEventResponderSystem-test.internal.js @@ -232,6 +232,7 @@ describe('DOMEventResponderSystem', () => { 'magicclick', props.onMagicClick, context.eventTarget, + false, ); } }, From ea86b4b5fcf0b2eac08d12e8e92dd3fdbda7fc79 Mon Sep 17 00:00:00 2001 From: Dominic Gannaway Date: Tue, 26 Mar 2019 16:34:16 -0700 Subject: [PATCH 12/12] Move away from using a state Map --- packages/react-dom/src/events/DOMEventResponderSystem.js | 8 +++----- packages/react-reconciler/src/ReactFiber.js | 2 +- 2 files changed, 4 insertions(+), 6 deletions(-) diff --git a/packages/react-dom/src/events/DOMEventResponderSystem.js b/packages/react-dom/src/events/DOMEventResponderSystem.js index d4cb288eaff31..98969ac8a5b88 100644 --- a/packages/react-dom/src/events/DOMEventResponderSystem.js +++ b/packages/react-dom/src/events/DOMEventResponderSystem.js @@ -148,7 +148,7 @@ function handleTopLevelType( context: Object, ): void { const responder: ReactEventResponder = fiber.type.responder; - const {props, stateMap} = fiber.stateNode; + let {props, state} = fiber.stateNode; let validEventTypesForResponder = eventResponderValidEventTypes.get( responder, ); @@ -162,10 +162,8 @@ function handleTopLevelType( if (!validEventTypesForResponder.has(topLevelType)) { return; } - let state = stateMap.get(responder); - if (state === undefined && responder.createInitialState !== undefined) { - state = responder.createInitialState(props); - stateMap.set(responder, state); + if (state === null && responder.createInitialState !== undefined) { + state = fiber.stateNode.state = responder.createInitialState(props); } context._fiber = fiber; context._responder = responder; diff --git a/packages/react-reconciler/src/ReactFiber.js b/packages/react-reconciler/src/ReactFiber.js index fe4891e35c0b0..35b62021d574f 100644 --- a/packages/react-reconciler/src/ReactFiber.js +++ b/packages/react-reconciler/src/ReactFiber.js @@ -625,7 +625,7 @@ export function createFiberFromEventComponent( fiber.type = eventComponent; fiber.stateNode = { props: pendingProps, - stateMap: new Map(), + state: null, }; fiber.expirationTime = expirationTime; return fiber;