From 2ada4bd0c2c8e2ccac0c3c952712d6e03c9a7362 Mon Sep 17 00:00:00 2001 From: Toru Kobayashi Date: Mon, 24 Aug 2020 22:30:17 +0900 Subject: [PATCH 1/9] Add a test for non-passive event handlers for events affected by the browsers' intervention (#19658) --- .../DOMPluginEventSystem-test.internal.js | 61 +++++++++++++++++++ 1 file changed, 61 insertions(+) diff --git a/packages/react-dom/src/events/__tests__/DOMPluginEventSystem-test.internal.js b/packages/react-dom/src/events/__tests__/DOMPluginEventSystem-test.internal.js index d0e5ba48457..abf1c14dc27 100644 --- a/packages/react-dom/src/events/__tests__/DOMPluginEventSystem-test.internal.js +++ b/packages/react-dom/src/events/__tests__/DOMPluginEventSystem-test.internal.js @@ -3137,6 +3137,67 @@ describe('DOMPluginEventSystem', () => { expect(onClick).toHaveBeenCalledTimes(1); }); + + // @gate experimental + it('should be able to register non-passive handlers for events affected by the intervention', () => { + const rootContainer = document.createElement('div'); + container.appendChild(rootContainer); + + const defaultPreventedEvents = []; + const handler = e => { + if (e.defaultPrevented) defaultPreventedEvents.push(e.type); + }; + + container.addEventListener('touchstart', handler); + container.addEventListener('touchmove', handler); + container.addEventListener('wheel', handler); + + const ref = React.createRef(); + const setTouchStart = ReactDOM.unstable_createEventHandle( + 'touchstart', + {passive: false}, + ); + const setTouchMove = ReactDOM.unstable_createEventHandle( + 'touchmove', + {passive: false}, + ); + const setWheel = ReactDOM.unstable_createEventHandle('wheel', { + passive: false, + }); + + function Component() { + React.useEffect(() => { + const clearTouchStart = setTouchStart(ref.current, e => + e.preventDefault(), + ); + const clearTouchMove = setTouchMove(ref.current, e => + e.preventDefault(), + ); + const clearWheel = setWheel(ref.current, e => + e.preventDefault(), + ); + return () => { + clearTouchStart(); + clearTouchMove(); + clearWheel(); + }; + }); + return
test
; + } + + ReactDOM.render(, rootContainer); + Scheduler.unstable_flushAll(); + + dispatchEvent(ref.current, 'touchstart'); + dispatchEvent(ref.current, 'touchmove'); + dispatchEvent(ref.current, 'wheel'); + + expect(defaultPreventedEvents).toEqual([ + 'touchstart', + 'touchmove', + 'wheel', + ]); + }); }); }); }, From 8c9fc4e90f58fab25c2991ac60ddb6a6afe9271c Mon Sep 17 00:00:00 2001 From: Dominic Gannaway Date: Mon, 24 Aug 2020 16:10:23 +0100 Subject: [PATCH 2/9] Remove usage of PossiblyWeakSet from createEventHandle (#19686) --- .../src/client/ReactDOMComponentTree.js | 28 ++++++++++++++++++- .../src/client/ReactDOMEventHandle.js | 14 +++++----- 2 files changed, 34 insertions(+), 8 deletions(-) diff --git a/packages/react-dom/src/client/ReactDOMComponentTree.js b/packages/react-dom/src/client/ReactDOMComponentTree.js index d5f09c1caf4..d0edca73bae 100644 --- a/packages/react-dom/src/client/ReactDOMComponentTree.js +++ b/packages/react-dom/src/client/ReactDOMComponentTree.js @@ -9,7 +9,10 @@ import type {Fiber} from 'react-reconciler/src/ReactInternalTypes'; import type {ReactScopeInstance} from 'shared/ReactTypes'; -import type {ReactDOMEventHandleListener} from '../shared/ReactDOMTypes'; +import type { + ReactDOMEventHandle, + ReactDOMEventHandleListener, +} from '../shared/ReactDOMTypes'; import type { Container, TextInstance, @@ -39,6 +42,7 @@ const internalPropsKey = '__reactProps$' + randomKey; const internalContainerInstanceKey = '__reactContainer$' + randomKey; const internalEventHandlersKey = '__reactEvents$' + randomKey; const internalEventHandlerListenersKey = '__reactListeners$' + randomKey; +const internalEventHandlesSetKey = '__reactHandles$' + randomKey; export type ElementListenerMap = Map< DOMEventName | string, @@ -232,3 +236,25 @@ export function getEventHandlerListeners( ): null | Set { return (scope: any)[internalEventHandlerListenersKey] || null; } + +export function addEventHandleToTarget( + target: EventTarget | ReactScopeInstance, + eventHandle: ReactDOMEventHandle, +): void { + let eventHandles = (target: any)[internalEventHandlesSetKey]; + if (eventHandles === undefined) { + eventHandles = (target: any)[internalEventHandlesSetKey] = new Set(); + } + eventHandles.add(eventHandle); +} + +export function doesTargetHaveEventHandle( + target: EventTarget | ReactScopeInstance, + eventHandle: ReactDOMEventHandle, +): boolean { + const eventHandles = (target: any)[internalEventHandlesSetKey]; + if (eventHandles === undefined) { + return false; + } + return eventHandles.has(eventHandle); +} diff --git a/packages/react-dom/src/client/ReactDOMEventHandle.js b/packages/react-dom/src/client/ReactDOMEventHandle.js index 19e69126aef..303b083313e 100644 --- a/packages/react-dom/src/client/ReactDOMEventHandle.js +++ b/packages/react-dom/src/client/ReactDOMEventHandle.js @@ -20,6 +20,8 @@ import { getEventHandlerListeners, setEventHandlerListeners, getFiberFromScopeInstance, + doesTargetHaveEventHandle, + addEventHandleToTarget, } from './ReactDOMComponentTree'; import {ELEMENT_NODE, COMMENT_NODE} from '../shared/HTMLNodeType'; import { @@ -42,8 +44,6 @@ type EventHandleOptions = {| priority?: EventPriority, |}; -const PossiblyWeakSet = typeof WeakSet === 'function' ? WeakSet : Set; - function getNearestRootOrPortalContainer(node: Fiber): null | Element { while (node !== null) { const tag = node.tag; @@ -201,9 +201,7 @@ export function createEventHandle( listenerPriority = getEventPriorityForListenerSystem(domEventName); } - const registeredReactDOMEvents = new PossiblyWeakSet(); - - return ( + const eventHandle = ( target: EventTarget | ReactScopeInstance, callback: (SyntheticEvent) => void, ) => { @@ -212,8 +210,8 @@ export function createEventHandle( 'ReactDOM.createEventHandle: setter called with an invalid ' + 'callback. The callback must be a function.', ); - if (!registeredReactDOMEvents.has(target)) { - registeredReactDOMEvents.add(target); + if (!doesTargetHaveEventHandle(target, eventHandle)) { + addEventHandleToTarget(target, eventHandle); registerReactDOMEvent( target, domEventName, @@ -241,6 +239,8 @@ export function createEventHandle( ); }; }; + + return eventHandle; } return (null: any); } From 848bb2426e44606e0a55dfe44c7b3ece33772485 Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Mon, 24 Aug 2020 16:50:20 +0100 Subject: [PATCH 3/9] Attach Listeners Eagerly to Roots and Portal Containers (#19659) * Failing test for #19608 * Attach Listeners Eagerly to Roots and Portal Containers * Forbid createEventHandle with custom events We can't support this without adding more complexity. It's not clear that this is even desirable, as none of our existing use cases need custom events. This API primarily exists as a deprecation strategy for Flare, so I don't think it is important to expand its support beyond what Flare replacement code currently needs. We can later revisit it with a better understanding of the eager/lazy tradeoff but for now let's remove the inconsistency. * Reduce risk by changing condition only under the flag Co-authored-by: koba04 --- .../__tests__/ReactDOMEventListener-test.js | 49 ++++- .../src/__tests__/ReactDOMFiber-test.js | 19 ++ .../react-dom/src/client/ReactDOMComponent.js | 115 ++++++---- .../src/client/ReactDOMEventHandle.js | 22 ++ .../src/client/ReactDOMHostConfig.js | 12 +- packages/react-dom/src/client/ReactDOMRoot.js | 36 +-- .../src/events/DOMEventProperties.js | 3 +- .../src/events/DOMPluginEventSystem.js | 125 +++++++---- .../react-dom/src/events/EventRegistry.js | 14 +- .../src/events/ReactDOMEventListener.js | 74 ++++--- .../src/events/ReactDOMEventReplaying.js | 29 ++- .../DOMPluginEventSystem-test.internal.js | 206 ++++++++++++------ .../src/events/plugins/SelectEventPlugin.js | 29 +-- .../__tests__/SimpleEventPlugin-test.js | 13 +- packages/shared/ReactFeatureFlags.js | 2 + .../forks/ReactFeatureFlags.native-fb.js | 1 + .../forks/ReactFeatureFlags.native-oss.js | 1 + .../forks/ReactFeatureFlags.test-renderer.js | 1 + .../ReactFeatureFlags.test-renderer.native.js | 1 + .../ReactFeatureFlags.test-renderer.www.js | 1 + .../shared/forks/ReactFeatureFlags.testing.js | 1 + .../forks/ReactFeatureFlags.testing.www.js | 1 + .../forks/ReactFeatureFlags.www-dynamic.js | 1 + .../shared/forks/ReactFeatureFlags.www.js | 1 + scripts/error-codes/codes.json | 3 +- 25 files changed, 527 insertions(+), 233 deletions(-) diff --git a/packages/react-dom/src/__tests__/ReactDOMEventListener-test.js b/packages/react-dom/src/__tests__/ReactDOMEventListener-test.js index 5cb207409da..bced36b0ff5 100644 --- a/packages/react-dom/src/__tests__/ReactDOMEventListener-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMEventListener-test.js @@ -398,18 +398,49 @@ describe('ReactDOMEventListener', () => { const originalDocAddEventListener = document.addEventListener; const originalRootAddEventListener = container.addEventListener; document.addEventListener = function(type) { - throw new Error( - `Did not expect to add a document-level listener for the "${type}" event.`, - ); + switch (type) { + case 'selectionchange': + break; + default: + throw new Error( + `Did not expect to add a document-level listener for the "${type}" event.`, + ); + } }; - container.addEventListener = function(type) { - if (type === 'mouseout' || type === 'mouseover') { - // We currently listen to it unconditionally. + container.addEventListener = function(type, fn, options) { + if (options && (options === true || options.capture)) { return; } - throw new Error( - `Did not expect to add a root-level listener for the "${type}" event.`, - ); + switch (type) { + case 'abort': + case 'canplay': + case 'canplaythrough': + case 'durationchange': + case 'emptied': + case 'encrypted': + case 'ended': + case 'error': + case 'loadeddata': + case 'loadedmetadata': + case 'loadstart': + case 'pause': + case 'play': + case 'playing': + case 'progress': + case 'ratechange': + case 'seeked': + case 'seeking': + case 'stalled': + case 'suspend': + case 'timeupdate': + case 'volumechange': + case 'waiting': + throw new Error( + `Did not expect to add a root-level listener for the "${type}" event.`, + ); + default: + break; + } }; try { diff --git a/packages/react-dom/src/__tests__/ReactDOMFiber-test.js b/packages/react-dom/src/__tests__/ReactDOMFiber-test.js index b05b4c1f961..a91c2fac41c 100644 --- a/packages/react-dom/src/__tests__/ReactDOMFiber-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMFiber-test.js @@ -1040,6 +1040,25 @@ describe('ReactDOMFiber', () => { expect(ops).toEqual([]); }); + // @gate enableEagerRootListeners + it('listens to events that do not exist in the Portal subtree', () => { + const onClick = jest.fn(); + + const ref = React.createRef(); + ReactDOM.render( +
+ {ReactDOM.createPortal(, document.body)} +
, + container, + ); + const event = new MouseEvent('click', { + bubbles: true, + }); + ref.current.dispatchEvent(event); + + expect(onClick).toHaveBeenCalledTimes(1); + }); + it('should throw on bad createPortal argument', () => { expect(() => { ReactDOM.createPortal(
portal
, null); diff --git a/packages/react-dom/src/client/ReactDOMComponent.js b/packages/react-dom/src/client/ReactDOMComponent.js index d3e72d2a2d2..48b979af64c 100644 --- a/packages/react-dom/src/client/ReactDOMComponent.js +++ b/packages/react-dom/src/client/ReactDOMComponent.js @@ -74,7 +74,10 @@ import {validateProperties as validateInputProperties} from '../shared/ReactDOMN import {validateProperties as validateUnknownProperties} from '../shared/ReactDOMUnknownPropertyHook'; import {REACT_OPAQUE_ID_TYPE} from 'shared/ReactSymbols'; -import {enableTrustedTypesIntegration} from 'shared/ReactFeatureFlags'; +import { + enableTrustedTypesIntegration, + enableEagerRootListeners, +} from 'shared/ReactFeatureFlags'; import { listenToReactEvent, mediaEventTypes, @@ -260,30 +263,32 @@ export function ensureListeningTo( reactPropEvent: string, targetElement: Element | null, ): void { - // If we have a comment node, then use the parent node, - // which should be an element. - const rootContainerElement = - rootContainerInstance.nodeType === COMMENT_NODE - ? rootContainerInstance.parentNode - : rootContainerInstance; - if (__DEV__) { - if ( - rootContainerElement == null || - (rootContainerElement.nodeType !== ELEMENT_NODE && - // This is to support rendering into a ShadowRoot: - rootContainerElement.nodeType !== DOCUMENT_FRAGMENT_NODE) - ) { - console.error( - 'ensureListeningTo(): received a container that was not an element node. ' + - 'This is likely a bug in React. Please file an issue.', - ); + if (!enableEagerRootListeners) { + // If we have a comment node, then use the parent node, + // which should be an element. + const rootContainerElement = + rootContainerInstance.nodeType === COMMENT_NODE + ? rootContainerInstance.parentNode + : rootContainerInstance; + if (__DEV__) { + if ( + rootContainerElement == null || + (rootContainerElement.nodeType !== ELEMENT_NODE && + // This is to support rendering into a ShadowRoot: + rootContainerElement.nodeType !== DOCUMENT_FRAGMENT_NODE) + ) { + console.error( + 'ensureListeningTo(): received a container that was not an element node. ' + + 'This is likely a bug in React. Please file an issue.', + ); + } } + listenToReactEvent( + reactPropEvent, + ((rootContainerElement: any): Element), + targetElement, + ); } - listenToReactEvent( - reactPropEvent, - ((rootContainerElement: any): Element), - targetElement, - ); } function getOwnerDocumentFromRootContainer( @@ -364,7 +369,11 @@ function setInitialDOMProperties( if (__DEV__ && typeof nextProp !== 'function') { warnForInvalidEventListener(propKey, nextProp); } - ensureListeningTo(rootContainerElement, propKey, domElement); + if (!enableEagerRootListeners) { + ensureListeningTo(rootContainerElement, propKey, domElement); + } else if (propKey === 'onScroll') { + listenToNonDelegatedEvent('scroll', domElement); + } } } else if (nextProp != null) { setValueForProperty(domElement, propKey, nextProp, isCustomComponentTag); @@ -573,9 +582,11 @@ export function setInitialProperties( // We listen to this event in case to ensure emulated bubble // listeners still fire for the invalid event. listenToNonDelegatedEvent('invalid', domElement); - // For controlled components we always need to ensure we're listening - // to onChange. Even if there is no listener. - ensureListeningTo(rootContainerElement, 'onChange', domElement); + if (!enableEagerRootListeners) { + // For controlled components we always need to ensure we're listening + // to onChange. Even if there is no listener. + ensureListeningTo(rootContainerElement, 'onChange', domElement); + } break; case 'option': ReactDOMOptionValidateProps(domElement, rawProps); @@ -587,9 +598,11 @@ export function setInitialProperties( // We listen to this event in case to ensure emulated bubble // listeners still fire for the invalid event. listenToNonDelegatedEvent('invalid', domElement); - // For controlled components we always need to ensure we're listening - // to onChange. Even if there is no listener. - ensureListeningTo(rootContainerElement, 'onChange', domElement); + if (!enableEagerRootListeners) { + // For controlled components we always need to ensure we're listening + // to onChange. Even if there is no listener. + ensureListeningTo(rootContainerElement, 'onChange', domElement); + } break; case 'textarea': ReactDOMTextareaInitWrapperState(domElement, rawProps); @@ -597,9 +610,11 @@ export function setInitialProperties( // We listen to this event in case to ensure emulated bubble // listeners still fire for the invalid event. listenToNonDelegatedEvent('invalid', domElement); - // For controlled components we always need to ensure we're listening - // to onChange. Even if there is no listener. - ensureListeningTo(rootContainerElement, 'onChange', domElement); + if (!enableEagerRootListeners) { + // For controlled components we always need to ensure we're listening + // to onChange. Even if there is no listener. + ensureListeningTo(rootContainerElement, 'onChange', domElement); + } break; default: props = rawProps; @@ -817,7 +832,11 @@ export function diffProperties( if (__DEV__ && typeof nextProp !== 'function') { warnForInvalidEventListener(propKey, nextProp); } - ensureListeningTo(rootContainerElement, propKey, domElement); + if (!enableEagerRootListeners) { + ensureListeningTo(rootContainerElement, propKey, domElement); + } else if (propKey === 'onScroll') { + listenToNonDelegatedEvent('scroll', domElement); + } } if (!updatePayload && lastProp !== nextProp) { // This is a special case. If any listener updates we need to ensure @@ -969,9 +988,11 @@ export function diffHydratedProperties( // We listen to this event in case to ensure emulated bubble // listeners still fire for the invalid event. listenToNonDelegatedEvent('invalid', domElement); - // For controlled components we always need to ensure we're listening - // to onChange. Even if there is no listener. - ensureListeningTo(rootContainerElement, 'onChange', domElement); + if (!enableEagerRootListeners) { + // For controlled components we always need to ensure we're listening + // to onChange. Even if there is no listener. + ensureListeningTo(rootContainerElement, 'onChange', domElement); + } break; case 'option': ReactDOMOptionValidateProps(domElement, rawProps); @@ -981,18 +1002,22 @@ export function diffHydratedProperties( // We listen to this event in case to ensure emulated bubble // listeners still fire for the invalid event. listenToNonDelegatedEvent('invalid', domElement); - // For controlled components we always need to ensure we're listening - // to onChange. Even if there is no listener. - ensureListeningTo(rootContainerElement, 'onChange', domElement); + if (!enableEagerRootListeners) { + // For controlled components we always need to ensure we're listening + // to onChange. Even if there is no listener. + ensureListeningTo(rootContainerElement, 'onChange', domElement); + } break; case 'textarea': ReactDOMTextareaInitWrapperState(domElement, rawProps); // We listen to this event in case to ensure emulated bubble // listeners still fire for the invalid event. listenToNonDelegatedEvent('invalid', domElement); - // For controlled components we always need to ensure we're listening - // to onChange. Even if there is no listener. - ensureListeningTo(rootContainerElement, 'onChange', domElement); + if (!enableEagerRootListeners) { + // For controlled components we always need to ensure we're listening + // to onChange. Even if there is no listener. + ensureListeningTo(rootContainerElement, 'onChange', domElement); + } break; } @@ -1059,7 +1084,9 @@ export function diffHydratedProperties( if (__DEV__ && typeof nextProp !== 'function') { warnForInvalidEventListener(propKey, nextProp); } - ensureListeningTo(rootContainerElement, propKey, domElement); + if (!enableEagerRootListeners) { + ensureListeningTo(rootContainerElement, propKey, domElement); + } } } else if ( __DEV__ && diff --git a/packages/react-dom/src/client/ReactDOMEventHandle.js b/packages/react-dom/src/client/ReactDOMEventHandle.js index 303b083313e..52edc8aaf7e 100644 --- a/packages/react-dom/src/client/ReactDOMEventHandle.js +++ b/packages/react-dom/src/client/ReactDOMEventHandle.js @@ -15,6 +15,7 @@ import type { } from '../shared/ReactDOMTypes'; import {getEventPriorityForListenerSystem} from '../events/DOMEventProperties'; +import {allNativeEvents} from '../events/EventRegistry'; import { getClosestInstanceFromNode, getEventHandlerListeners, @@ -35,6 +36,7 @@ import {IS_EVENT_HANDLE_NON_MANAGED_NODE} from '../events/EventSystemFlags'; import { enableScopeAPI, enableCreateEventHandleAPI, + enableEagerRootListeners, } from 'shared/ReactFeatureFlags'; import invariant from 'shared/invariant'; @@ -178,6 +180,26 @@ export function createEventHandle( ): ReactDOMEventHandle { if (enableCreateEventHandleAPI) { const domEventName = ((type: any): DOMEventName); + + if (enableEagerRootListeners) { + // We cannot support arbitrary native events with eager root listeners + // because the eager strategy relies on knowing the whole list ahead of time. + // If we wanted to support this, we'd have to add code to keep track + // (or search) for all portal and root containers, and lazily add listeners + // to them whenever we see a previously unknown event. This seems like a lot + // of complexity for something we don't even have a particular use case for. + // Unfortunately, the downside of this invariant is that *removing* a native + // event from the list of known events has now become a breaking change for + // any code relying on the createEventHandle API. + invariant( + allNativeEvents.has(domEventName) || + domEventName === 'beforeblur' || + domEventName === 'afterblur', + 'Cannot call unstable_createEventHandle with "%s", as it is not an event known to React.', + domEventName, + ); + } + let isCapturePhaseListener = false; let isPassiveListener = undefined; // Undefined means to use the browser default let listenerPriority; diff --git a/packages/react-dom/src/client/ReactDOMHostConfig.js b/packages/react-dom/src/client/ReactDOMHostConfig.js index 22d38dea043..e5ce50add75 100644 --- a/packages/react-dom/src/client/ReactDOMHostConfig.js +++ b/packages/react-dom/src/client/ReactDOMHostConfig.js @@ -67,9 +67,13 @@ import { enableFundamentalAPI, enableCreateEventHandleAPI, enableScopeAPI, + enableEagerRootListeners, } from 'shared/ReactFeatureFlags'; import {HostComponent, HostText} from 'react-reconciler/src/ReactWorkTags'; -import {listenToReactEvent} from '../events/DOMPluginEventSystem'; +import { + listenToReactEvent, + listenToAllSupportedEvents, +} from '../events/DOMPluginEventSystem'; export type Type = string; export type Props = { @@ -1069,7 +1073,11 @@ export function makeOpaqueHydratingObject( } export function preparePortalMount(portalInstance: Instance): void { - listenToReactEvent('onMouseEnter', portalInstance, null); + if (enableEagerRootListeners) { + listenToAllSupportedEvents(portalInstance); + } else { + listenToReactEvent('onMouseEnter', portalInstance, null); + } } export function prepareScopeUpdate( diff --git a/packages/react-dom/src/client/ReactDOMRoot.js b/packages/react-dom/src/client/ReactDOMRoot.js index d9e446749c1..11ec850a4ab 100644 --- a/packages/react-dom/src/client/ReactDOMRoot.js +++ b/packages/react-dom/src/client/ReactDOMRoot.js @@ -35,6 +35,7 @@ import { markContainerAsRoot, unmarkContainerAsRoot, } from './ReactDOMComponentTree'; +import {listenToAllSupportedEvents} from '../events/DOMPluginEventSystem'; import {eagerlyTrapReplayableEvents} from '../events/ReactDOMEventReplaying'; import { ELEMENT_NODE, @@ -51,6 +52,7 @@ import { registerMutableSourceForHydration, } from 'react-reconciler/src/ReactFiberReconciler'; import invariant from 'shared/invariant'; +import {enableEagerRootListeners} from 'shared/ReactFeatureFlags'; import { BlockingRoot, ConcurrentRoot, @@ -133,19 +135,27 @@ function createRootImpl( markContainerAsRoot(root.current, container); const containerNodeType = container.nodeType; - if (hydrate && tag !== LegacyRoot) { - const doc = - containerNodeType === DOCUMENT_NODE ? container : container.ownerDocument; - // We need to cast this because Flow doesn't work - // with the hoisted containerNodeType. If we inline - // it, then Flow doesn't complain. We intentionally - // hoist it to reduce code-size. - eagerlyTrapReplayableEvents(container, ((doc: any): Document)); - } else if ( - containerNodeType !== DOCUMENT_FRAGMENT_NODE && - containerNodeType !== DOCUMENT_NODE - ) { - ensureListeningTo(container, 'onMouseEnter', null); + if (enableEagerRootListeners) { + const rootContainerElement = + container.nodeType === COMMENT_NODE ? container.parentNode : container; + listenToAllSupportedEvents(rootContainerElement); + } else { + if (hydrate && tag !== LegacyRoot) { + const doc = + containerNodeType === DOCUMENT_NODE + ? container + : container.ownerDocument; + // We need to cast this because Flow doesn't work + // with the hoisted containerNodeType. If we inline + // it, then Flow doesn't complain. We intentionally + // hoist it to reduce code-size. + eagerlyTrapReplayableEvents(container, ((doc: any): Document)); + } else if ( + containerNodeType !== DOCUMENT_FRAGMENT_NODE && + containerNodeType !== DOCUMENT_NODE + ) { + ensureListeningTo(container, 'onMouseEnter', null); + } } if (mutableSources) { diff --git a/packages/react-dom/src/events/DOMEventProperties.js b/packages/react-dom/src/events/DOMEventProperties.js index 342e653bd48..8c5090ecaee 100644 --- a/packages/react-dom/src/events/DOMEventProperties.js +++ b/packages/react-dom/src/events/DOMEventProperties.js @@ -201,8 +201,9 @@ export function getEventPriorityForListenerSystem( } if (__DEV__) { console.warn( - 'The event "type" provided to createEventHandle() does not have a known priority type.' + + 'The event "%s" provided to createEventHandle() does not have a known priority type.' + ' It is recommended to provide a "priority" option to specify a priority.', + type, ); } return ContinuousEvent; diff --git a/packages/react-dom/src/events/DOMPluginEventSystem.js b/packages/react-dom/src/events/DOMPluginEventSystem.js index 6a216ace127..97a37ace2e3 100644 --- a/packages/react-dom/src/events/DOMPluginEventSystem.js +++ b/packages/react-dom/src/events/DOMPluginEventSystem.js @@ -23,7 +23,7 @@ import type {ElementListenerMapEntry} from '../client/ReactDOMComponentTree'; import type {EventPriority} from 'shared/ReactTypes'; import type {Fiber} from 'react-reconciler/src/ReactInternalTypes'; -import {registrationNameDependencies} from './EventRegistry'; +import {registrationNameDependencies, allNativeEvents} from './EventRegistry'; import { IS_CAPTURE_PHASE, IS_EVENT_HANDLE_NON_MANAGED_NODE, @@ -54,11 +54,13 @@ import { enableCreateEventHandleAPI, enableScopeAPI, enablePassiveEventIntervention, + enableEagerRootListeners, } from 'shared/ReactFeatureFlags'; import { invokeGuardedCallbackAndCatchFirstError, rethrowCaughtError, } from 'shared/ReactErrorUtils'; +import {DOCUMENT_NODE} from '../shared/HTMLNodeType'; import {createEventListenerWrapperWithPriority} from './ReactDOMEventListener'; import { removeEventListener, @@ -327,6 +329,41 @@ export function listenToNonDelegatedEvent( } } +const listeningMarker = + '_reactListening' + + Math.random() + .toString(36) + .slice(2); + +export function listenToAllSupportedEvents(rootContainerElement: EventTarget) { + if (enableEagerRootListeners) { + if ((rootContainerElement: any)[listeningMarker]) { + // Performance optimization: don't iterate through events + // for the same portal container or root node more than once. + // TODO: once we remove the flag, we may be able to also + // remove some of the bookkeeping maps used for laziness. + return; + } + (rootContainerElement: any)[listeningMarker] = true; + allNativeEvents.forEach(domEventName => { + if (!nonDelegatedEvents.has(domEventName)) { + listenToNativeEvent( + domEventName, + false, + ((rootContainerElement: any): Element), + null, + ); + } + listenToNativeEvent( + domEventName, + true, + ((rootContainerElement: any): Element), + null, + ); + }); + } +} + export function listenToNativeEvent( domEventName: DOMEventName, isCapturePhaseListener: boolean, @@ -337,10 +374,14 @@ export function listenToNativeEvent( eventSystemFlags?: EventSystemFlags = 0, ): void { let target = rootContainerElement; + // selectionchange needs to be attached to the document // otherwise it won't capture incoming events that are only // triggered on the document directly. - if (domEventName === 'selectionchange') { + if ( + domEventName === 'selectionchange' && + (rootContainerElement: any).nodeType !== DOCUMENT_NODE + ) { target = (rootContainerElement: any).ownerDocument; } if (enablePassiveEventIntervention && isPassiveListener === undefined) { @@ -426,48 +467,50 @@ export function listenToReactEvent( rootContainerElement: Element, targetElement: Element | null, ): void { - const dependencies = registrationNameDependencies[reactEvent]; - const dependenciesLength = dependencies.length; - // If the dependencies length is 1, that means we're not using a polyfill - // plugin like ChangeEventPlugin, BeforeInputPlugin, EnterLeavePlugin - // and SelectEventPlugin. We always use the native bubble event phase for - // these plugins and emulate two phase event dispatching. SimpleEventPlugin - // always only has a single dependency and SimpleEventPlugin events also - // use either the native capture event phase or bubble event phase, there - // is no emulation (except for focus/blur, but that will be removed soon). - const isPolyfillEventPlugin = dependenciesLength !== 1; + if (!enableEagerRootListeners) { + const dependencies = registrationNameDependencies[reactEvent]; + const dependenciesLength = dependencies.length; + // If the dependencies length is 1, that means we're not using a polyfill + // plugin like ChangeEventPlugin, BeforeInputPlugin, EnterLeavePlugin + // and SelectEventPlugin. We always use the native bubble event phase for + // these plugins and emulate two phase event dispatching. SimpleEventPlugin + // always only has a single dependency and SimpleEventPlugin events also + // use either the native capture event phase or bubble event phase, there + // is no emulation (except for focus/blur, but that will be removed soon). + const isPolyfillEventPlugin = dependenciesLength !== 1; - if (isPolyfillEventPlugin) { - const listenerMap = getEventListenerMap(rootContainerElement); - // For optimization, we register plugins on the listener map, so we - // don't need to check each of their dependencies each time. - if (!listenerMap.has(reactEvent)) { - listenerMap.set(reactEvent, null); - for (let i = 0; i < dependenciesLength; i++) { - listenToNativeEvent( - dependencies[i], - false, - rootContainerElement, - targetElement, - ); + if (isPolyfillEventPlugin) { + const listenerMap = getEventListenerMap(rootContainerElement); + // For optimization, we register plugins on the listener map, so we + // don't need to check each of their dependencies each time. + if (!listenerMap.has(reactEvent)) { + listenerMap.set(reactEvent, null); + for (let i = 0; i < dependenciesLength; i++) { + listenToNativeEvent( + dependencies[i], + false, + rootContainerElement, + targetElement, + ); + } } + } else { + const isCapturePhaseListener = + reactEvent.substr(-7) === 'Capture' && + // Edge case: onGotPointerCapture and onLostPointerCapture + // end with "Capture" but that's part of their event names. + // The Capture versions would end with CaptureCapture. + // So we have to check against that. + // This check works because none of the events we support + // end with "Pointer". + reactEvent.substr(-14, 7) !== 'Pointer'; + listenToNativeEvent( + dependencies[0], + isCapturePhaseListener, + rootContainerElement, + targetElement, + ); } - } else { - const isCapturePhaseListener = - reactEvent.substr(-7) === 'Capture' && - // Edge case: onGotPointerCapture and onLostPointerCapture - // end with "Capture" but that's part of their event names. - // The Capture versions would end with CaptureCapture. - // So we have to check against that. - // This check works because none of the events we support - // end with "Pointer". - reactEvent.substr(-14, 7) !== 'Pointer'; - listenToNativeEvent( - dependencies[0], - isCapturePhaseListener, - rootContainerElement, - targetElement, - ); } } diff --git a/packages/react-dom/src/events/EventRegistry.js b/packages/react-dom/src/events/EventRegistry.js index 3dd665623f4..dfe938d1130 100644 --- a/packages/react-dom/src/events/EventRegistry.js +++ b/packages/react-dom/src/events/EventRegistry.js @@ -9,6 +9,10 @@ import type {DOMEventName} from './DOMEventNames'; +import {enableEagerRootListeners} from 'shared/ReactFeatureFlags'; + +export const allNativeEvents: Set = new Set(); + /** * Mapping from registration name to event name */ @@ -25,7 +29,7 @@ export const possibleRegistrationNames = __DEV__ ? {} : (null: any); export function registerTwoPhaseEvent( registrationName: string, - dependencies: ?Array, + dependencies: Array, ): void { registerDirectEvent(registrationName, dependencies); registerDirectEvent(registrationName + 'Capture', dependencies); @@ -33,7 +37,7 @@ export function registerTwoPhaseEvent( export function registerDirectEvent( registrationName: string, - dependencies: ?Array, + dependencies: Array, ) { if (__DEV__) { if (registrationNameDependencies[registrationName]) { @@ -55,4 +59,10 @@ export function registerDirectEvent( possibleRegistrationNames.ondblclick = registrationName; } } + + if (enableEagerRootListeners) { + for (let i = 0; i < dependencies.length; i++) { + allNativeEvents.add(dependencies[i]); + } + } } diff --git a/packages/react-dom/src/events/ReactDOMEventListener.js b/packages/react-dom/src/events/ReactDOMEventListener.js index d92c6d5eb58..d8ff5300aaf 100644 --- a/packages/react-dom/src/events/ReactDOMEventListener.js +++ b/packages/react-dom/src/events/ReactDOMEventListener.js @@ -32,6 +32,7 @@ import { import {HostRoot, SuspenseComponent} from 'react-reconciler/src/ReactWorkTags'; import { type EventSystemFlags, + IS_CAPTURE_PHASE, IS_LEGACY_FB_SUPPORT_MODE, } from './EventSystemFlags'; @@ -40,6 +41,7 @@ import {getClosestInstanceFromNode} from '../client/ReactDOMComponentTree'; import { enableLegacyFBSupport, + enableEagerRootListeners, decoupleUpdatePriorityFromScheduler, } from 'shared/ReactFeatureFlags'; import { @@ -191,7 +193,21 @@ export function dispatchEvent( if (!_enabled) { return; } - if (hasQueuedDiscreteEvents() && isReplayableDiscreteEvent(domEventName)) { + let allowReplay = true; + if (enableEagerRootListeners) { + // TODO: replaying capture phase events is currently broken + // because we used to do it during top-level native bubble handlers + // but now we use different bubble and capture handlers. + // In eager mode, we attach capture listeners early, so we need + // to filter them out until we fix the logic to handle them correctly. + // This could've been outside the flag but I put it inside to reduce risk. + allowReplay = (eventSystemFlags & IS_CAPTURE_PHASE) === 0; + } + if ( + allowReplay && + hasQueuedDiscreteEvents() && + isReplayableDiscreteEvent(domEventName) + ) { // If we already have a queue of discrete events, and this is another discrete // event, then we can't dispatch it regardless of its target, since they // need to dispatch in order. @@ -214,38 +230,40 @@ export function dispatchEvent( if (blockedOn === null) { // We successfully dispatched this event. - clearIfContinuousEvent(domEventName, nativeEvent); - return; - } - - if (isReplayableDiscreteEvent(domEventName)) { - // This this to be replayed later once the target is available. - queueDiscreteEvent( - blockedOn, - domEventName, - eventSystemFlags, - targetContainer, - nativeEvent, - ); + if (allowReplay) { + clearIfContinuousEvent(domEventName, nativeEvent); + } return; } - if ( - queueIfContinuousEvent( - blockedOn, - domEventName, - eventSystemFlags, - targetContainer, - nativeEvent, - ) - ) { - return; + if (allowReplay) { + if (isReplayableDiscreteEvent(domEventName)) { + // This this to be replayed later once the target is available. + queueDiscreteEvent( + blockedOn, + domEventName, + eventSystemFlags, + targetContainer, + nativeEvent, + ); + return; + } + if ( + queueIfContinuousEvent( + blockedOn, + domEventName, + eventSystemFlags, + targetContainer, + nativeEvent, + ) + ) { + return; + } + // We need to clear only if we didn't queue because + // queueing is accummulative. + clearIfContinuousEvent(domEventName, nativeEvent); } - // We need to clear only if we didn't queue because - // queueing is accummulative. - clearIfContinuousEvent(domEventName, nativeEvent); - // This is not replayable so we'll invoke it but without a target, // in case the event system needs to trace it. dispatchEventForPluginEventSystem( diff --git a/packages/react-dom/src/events/ReactDOMEventReplaying.js b/packages/react-dom/src/events/ReactDOMEventReplaying.js index ded99d19618..8b8d08f5e06 100644 --- a/packages/react-dom/src/events/ReactDOMEventReplaying.js +++ b/packages/react-dom/src/events/ReactDOMEventReplaying.js @@ -14,7 +14,10 @@ import type {EventSystemFlags} from './EventSystemFlags'; import type {FiberRoot} from 'react-reconciler/src/ReactInternalTypes'; import type {LanePriority} from 'react-reconciler/src/ReactFiberLane'; -import {enableSelectiveHydration} from 'shared/ReactFeatureFlags'; +import { + enableSelectiveHydration, + enableEagerRootListeners, +} from 'shared/ReactFeatureFlags'; import { unstable_runWithPriority as runWithPriority, unstable_scheduleCallback as scheduleCallback, @@ -177,21 +180,27 @@ function trapReplayableEventForContainer( domEventName: DOMEventName, container: Container, ) { - listenToNativeEvent(domEventName, false, ((container: any): Element), null); + // When the flag is on, we do this in a unified codepath elsewhere. + if (!enableEagerRootListeners) { + listenToNativeEvent(domEventName, false, ((container: any): Element), null); + } } export function eagerlyTrapReplayableEvents( container: Container, document: Document, ) { - // Discrete - discreteReplayableEvents.forEach(domEventName => { - trapReplayableEventForContainer(domEventName, container); - }); - // Continuous - continuousReplayableEvents.forEach(domEventName => { - trapReplayableEventForContainer(domEventName, container); - }); + // When the flag is on, we do this in a unified codepath elsewhere. + if (!enableEagerRootListeners) { + // Discrete + discreteReplayableEvents.forEach(domEventName => { + trapReplayableEventForContainer(domEventName, container); + }); + // Continuous + continuousReplayableEvents.forEach(domEventName => { + trapReplayableEventForContainer(domEventName, container); + }); + } } function createQueuedReplayableEvent( diff --git a/packages/react-dom/src/events/__tests__/DOMPluginEventSystem-test.internal.js b/packages/react-dom/src/events/__tests__/DOMPluginEventSystem-test.internal.js index abf1c14dc27..96387223787 100644 --- a/packages/react-dom/src/events/__tests__/DOMPluginEventSystem-test.internal.js +++ b/packages/react-dom/src/events/__tests__/DOMPluginEventSystem-test.internal.js @@ -2401,85 +2401,97 @@ describe('DOMPluginEventSystem', () => { ); let setCustomEventHandle; + if (gate(flags => flags.enableEagerRootListeners)) { + // With eager listeners, supporting custom events via this API doesn't make sense + // because we can't know a full list of them ahead of time. Let's check we throw + // since otherwise we'd end up with inconsistent behavior, like no portal bubbling. + expect(() => { + setCustomEventHandle = ReactDOM.unstable_createEventHandle( + 'custom-event', + ); + }).toThrow( + 'Cannot call unstable_createEventHandle with "custom-event", as it is not an event known to React.', + ); + } else { + // Test that we get a warning when we don't provide an explicit priority + expect(() => { + setCustomEventHandle = ReactDOM.unstable_createEventHandle( + 'custom-event', + ); + }).toWarnDev( + 'Warning: The event "custom-event" provided to createEventHandle() does not have a known priority type. ' + + 'It is recommended to provide a "priority" option to specify a priority.', + {withoutStack: true}, + ); - // Test that we get a warning when we don't provide an explicit priority - expect(() => { setCustomEventHandle = ReactDOM.unstable_createEventHandle( 'custom-event', + { + priority: 0, // Discrete + }, ); - }).toWarnDev( - 'Warning: The event "type" provided to createEventHandle() does not have a known priority type. ' + - 'It is recommended to provide a "priority" option to specify a priority.', - {withoutStack: true}, - ); - setCustomEventHandle = ReactDOM.unstable_createEventHandle( - 'custom-event', - { - priority: 0, // Discrete - }, - ); - - const setCustomCaptureHandle = ReactDOM.unstable_createEventHandle( - 'custom-event', - { - capture: true, - priority: 0, // Discrete - }, - ); + const setCustomCaptureHandle = ReactDOM.unstable_createEventHandle( + 'custom-event', + { + capture: true, + priority: 0, // Discrete + }, + ); - function Test() { - React.useEffect(() => { - const clearCustom1 = setCustomEventHandle( - buttonRef.current, - onCustomEvent, - ); - const clearCustom2 = setCustomCaptureHandle( - buttonRef.current, - onCustomEventCapture, - ); - const clearCustom3 = setCustomEventHandle( - divRef.current, - onCustomEvent, - ); - const clearCustom4 = setCustomCaptureHandle( - divRef.current, - onCustomEventCapture, - ); + const Test = () => { + React.useEffect(() => { + const clearCustom1 = setCustomEventHandle( + buttonRef.current, + onCustomEvent, + ); + const clearCustom2 = setCustomCaptureHandle( + buttonRef.current, + onCustomEventCapture, + ); + const clearCustom3 = setCustomEventHandle( + divRef.current, + onCustomEvent, + ); + const clearCustom4 = setCustomCaptureHandle( + divRef.current, + onCustomEventCapture, + ); - return () => { - clearCustom1(); - clearCustom2(); - clearCustom3(); - clearCustom4(); - }; - }); + return () => { + clearCustom1(); + clearCustom2(); + clearCustom3(); + clearCustom4(); + }; + }); - return ( - - ); - } + return ( + + ); + }; - ReactDOM.render(, container); - Scheduler.unstable_flushAll(); + ReactDOM.render(, container); + Scheduler.unstable_flushAll(); - const buttonElement = buttonRef.current; - dispatchEvent(buttonElement, 'custom-event'); - expect(onCustomEvent).toHaveBeenCalledTimes(1); - expect(onCustomEventCapture).toHaveBeenCalledTimes(1); - expect(log[0]).toEqual(['capture', buttonElement]); - expect(log[1]).toEqual(['bubble', buttonElement]); + const buttonElement = buttonRef.current; + dispatchEvent(buttonElement, 'custom-event'); + expect(onCustomEvent).toHaveBeenCalledTimes(1); + expect(onCustomEventCapture).toHaveBeenCalledTimes(1); + expect(log[0]).toEqual(['capture', buttonElement]); + expect(log[1]).toEqual(['bubble', buttonElement]); - const divElement = divRef.current; - dispatchEvent(divElement, 'custom-event'); - expect(onCustomEvent).toHaveBeenCalledTimes(3); - expect(onCustomEventCapture).toHaveBeenCalledTimes(3); - expect(log[2]).toEqual(['capture', buttonElement]); - expect(log[3]).toEqual(['capture', divElement]); - expect(log[4]).toEqual(['bubble', divElement]); - expect(log[5]).toEqual(['bubble', buttonElement]); + const divElement = divRef.current; + dispatchEvent(divElement, 'custom-event'); + expect(onCustomEvent).toHaveBeenCalledTimes(3); + expect(onCustomEventCapture).toHaveBeenCalledTimes(3); + expect(log[2]).toEqual(['capture', buttonElement]); + expect(log[3]).toEqual(['capture', divElement]); + expect(log[4]).toEqual(['bubble', divElement]); + expect(log[5]).toEqual(['bubble', buttonElement]); + } }); // @gate experimental @@ -2823,6 +2835,64 @@ describe('DOMPluginEventSystem', () => { expect(log[5]).toEqual(['bubble', buttonElement]); }); + // @gate experimental && enableEagerRootListeners + it('propagates known createEventHandle events through portals without inner listeners', () => { + const buttonRef = React.createRef(); + const divRef = React.createRef(); + const log = []; + const onClick = jest.fn(e => log.push(['bubble', e.currentTarget])); + const onClickCapture = jest.fn(e => + log.push(['capture', e.currentTarget]), + ); + const setClick = ReactDOM.unstable_createEventHandle('click'); + const setClickCapture = ReactDOM.unstable_createEventHandle( + 'click', + { + capture: true, + }, + ); + + const portalElement = document.createElement('div'); + document.body.appendChild(portalElement); + + function Child() { + return
Click me!
; + } + + function Parent() { + React.useEffect(() => { + const clear1 = setClick(buttonRef.current, onClick); + const clear2 = setClickCapture( + buttonRef.current, + onClickCapture, + ); + return () => { + clear1(); + clear2(); + }; + }); + + return ( + + ); + } + + ReactDOM.render(, container); + Scheduler.unstable_flushAll(); + + const divElement = divRef.current; + const buttonElement = buttonRef.current; + dispatchClickEvent(divElement); + expect(onClick).toHaveBeenCalledTimes(1); + expect(onClickCapture).toHaveBeenCalledTimes(1); + expect(log[0]).toEqual(['capture', buttonElement]); + expect(log[1]).toEqual(['bubble', buttonElement]); + + document.body.removeChild(portalElement); + }); + describe('Compatibility with Scopes API', () => { beforeEach(() => { jest.resetModules(); diff --git a/packages/react-dom/src/events/plugins/SelectEventPlugin.js b/packages/react-dom/src/events/plugins/SelectEventPlugin.js index bc5d7df1a49..7f8b72408f9 100644 --- a/packages/react-dom/src/events/plugins/SelectEventPlugin.js +++ b/packages/react-dom/src/events/plugins/SelectEventPlugin.js @@ -16,6 +16,7 @@ import {canUseDOM} from 'shared/ExecutionEnvironment'; import {SyntheticEvent} from '../../events/SyntheticEvent'; import isTextInputElement from '../isTextInputElement'; import shallowEqual from 'shared/shallowEqual'; +import {enableEagerRootListeners} from 'shared/ReactFeatureFlags'; import {registerTwoPhaseEvent} from '../EventRegistry'; import getActiveElement from '../../client/getActiveElement'; @@ -152,19 +153,21 @@ function extractEvents( eventSystemFlags: EventSystemFlags, targetContainer: EventTarget, ) { - const eventListenerMap = getEventListenerMap(targetContainer); - // Track whether all listeners exists for this plugin. If none exist, we do - // not extract events. See #3639. - if ( - // If we are handling selectionchange, then we don't need to - // check for the other dependencies, as selectionchange is only - // event attached from the onChange plugin and we don't expose an - // onSelectionChange event from React. - domEventName !== 'selectionchange' && - !eventListenerMap.has('onSelect') && - !eventListenerMap.has('onSelectCapture') - ) { - return; + if (!enableEagerRootListeners) { + const eventListenerMap = getEventListenerMap(targetContainer); + // Track whether all listeners exists for this plugin. If none exist, we do + // not extract events. See #3639. + if ( + // If we are handling selectionchange, then we don't need to + // check for the other dependencies, as selectionchange is only + // event attached from the onChange plugin and we don't expose an + // onSelectionChange event from React. + domEventName !== 'selectionchange' && + !eventListenerMap.has('onSelect') && + !eventListenerMap.has('onSelectCapture') + ) { + return; + } } const targetNode = targetInst ? getNodeFromInstance(targetInst) : window; diff --git a/packages/react-dom/src/events/plugins/__tests__/SimpleEventPlugin-test.js b/packages/react-dom/src/events/plugins/__tests__/SimpleEventPlugin-test.js index d8eb2c9219e..4614052f50b 100644 --- a/packages/react-dom/src/events/plugins/__tests__/SimpleEventPlugin-test.js +++ b/packages/react-dom/src/events/plugins/__tests__/SimpleEventPlugin-test.js @@ -538,7 +538,18 @@ describe('SimpleEventPlugin', function() { ); if (gate(flags => flags.enablePassiveEventIntervention)) { - expect(passiveEvents).toEqual(['touchstart', 'touchmove', 'wheel']); + if (gate(flags => flags.enableEagerRootListeners)) { + expect(passiveEvents).toEqual([ + 'touchstart', + 'touchstart', + 'touchmove', + 'touchmove', + 'wheel', + 'wheel', + ]); + } else { + expect(passiveEvents).toEqual(['touchstart', 'touchmove', 'wheel']); + } } else { expect(passiveEvents).toEqual([]); } diff --git a/packages/shared/ReactFeatureFlags.js b/packages/shared/ReactFeatureFlags.js index 2129245913c..1ae1c94e618 100644 --- a/packages/shared/ReactFeatureFlags.js +++ b/packages/shared/ReactFeatureFlags.js @@ -136,3 +136,5 @@ export const enableDiscreteEventFlushingChange = false; // https://github.com/facebook/react/pull/19654 export const enablePassiveEventIntervention = true; export const disableOnScrollBubbling = true; + +export const enableEagerRootListeners = false; diff --git a/packages/shared/forks/ReactFeatureFlags.native-fb.js b/packages/shared/forks/ReactFeatureFlags.native-fb.js index 078d1e6b0f4..ed2b0952cb1 100644 --- a/packages/shared/forks/ReactFeatureFlags.native-fb.js +++ b/packages/shared/forks/ReactFeatureFlags.native-fb.js @@ -51,6 +51,7 @@ export const deferRenderPhaseUpdateToNextBatch = true; export const decoupleUpdatePriorityFromScheduler = false; export const enableDiscreteEventFlushingChange = false; export const enablePassiveEventIntervention = true; +export const enableEagerRootListeners = false; // Flow magic to verify the exports of this file match the original version. // eslint-disable-next-line no-unused-vars diff --git a/packages/shared/forks/ReactFeatureFlags.native-oss.js b/packages/shared/forks/ReactFeatureFlags.native-oss.js index 89512478b45..0137e890879 100644 --- a/packages/shared/forks/ReactFeatureFlags.native-oss.js +++ b/packages/shared/forks/ReactFeatureFlags.native-oss.js @@ -50,6 +50,7 @@ export const deferRenderPhaseUpdateToNextBatch = true; export const decoupleUpdatePriorityFromScheduler = false; export const enableDiscreteEventFlushingChange = false; export const enablePassiveEventIntervention = true; +export const enableEagerRootListeners = false; // Flow magic to verify the exports of this file match the original version. // eslint-disable-next-line no-unused-vars diff --git a/packages/shared/forks/ReactFeatureFlags.test-renderer.js b/packages/shared/forks/ReactFeatureFlags.test-renderer.js index 280fc358729..9481dfda8c7 100644 --- a/packages/shared/forks/ReactFeatureFlags.test-renderer.js +++ b/packages/shared/forks/ReactFeatureFlags.test-renderer.js @@ -50,6 +50,7 @@ export const deferRenderPhaseUpdateToNextBatch = true; export const decoupleUpdatePriorityFromScheduler = false; export const enableDiscreteEventFlushingChange = false; export const enablePassiveEventIntervention = true; +export const enableEagerRootListeners = false; // Flow magic to verify the exports of this file match the original version. // eslint-disable-next-line no-unused-vars diff --git a/packages/shared/forks/ReactFeatureFlags.test-renderer.native.js b/packages/shared/forks/ReactFeatureFlags.test-renderer.native.js index baee0a991e5..4a710543cb9 100644 --- a/packages/shared/forks/ReactFeatureFlags.test-renderer.native.js +++ b/packages/shared/forks/ReactFeatureFlags.test-renderer.native.js @@ -49,6 +49,7 @@ export const deferRenderPhaseUpdateToNextBatch = true; export const decoupleUpdatePriorityFromScheduler = false; export const enableDiscreteEventFlushingChange = false; export const enablePassiveEventIntervention = true; +export const enableEagerRootListeners = false; // Flow magic to verify the exports of this file match the original version. // eslint-disable-next-line no-unused-vars diff --git a/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js b/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js index aeb589d1b83..5efc2138d0e 100644 --- a/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js +++ b/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js @@ -50,6 +50,7 @@ export const deferRenderPhaseUpdateToNextBatch = true; export const decoupleUpdatePriorityFromScheduler = false; export const enableDiscreteEventFlushingChange = false; export const enablePassiveEventIntervention = true; +export const enableEagerRootListeners = false; // Flow magic to verify the exports of this file match the original version. // eslint-disable-next-line no-unused-vars diff --git a/packages/shared/forks/ReactFeatureFlags.testing.js b/packages/shared/forks/ReactFeatureFlags.testing.js index 1ba4439b28e..8a715c02f96 100644 --- a/packages/shared/forks/ReactFeatureFlags.testing.js +++ b/packages/shared/forks/ReactFeatureFlags.testing.js @@ -50,6 +50,7 @@ export const deferRenderPhaseUpdateToNextBatch = true; export const decoupleUpdatePriorityFromScheduler = false; export const enableDiscreteEventFlushingChange = false; export const enablePassiveEventIntervention = true; +export const enableEagerRootListeners = false; // Flow magic to verify the exports of this file match the original version. // eslint-disable-next-line no-unused-vars diff --git a/packages/shared/forks/ReactFeatureFlags.testing.www.js b/packages/shared/forks/ReactFeatureFlags.testing.www.js index b1b37b557f4..3dbbb378244 100644 --- a/packages/shared/forks/ReactFeatureFlags.testing.www.js +++ b/packages/shared/forks/ReactFeatureFlags.testing.www.js @@ -50,6 +50,7 @@ export const deferRenderPhaseUpdateToNextBatch = true; export const decoupleUpdatePriorityFromScheduler = false; export const enableDiscreteEventFlushingChange = true; export const enablePassiveEventIntervention = true; +export const enableEagerRootListeners = false; // Flow magic to verify the exports of this file match the original version. // eslint-disable-next-line no-unused-vars diff --git a/packages/shared/forks/ReactFeatureFlags.www-dynamic.js b/packages/shared/forks/ReactFeatureFlags.www-dynamic.js index f58f9f8aae2..3cf35c86073 100644 --- a/packages/shared/forks/ReactFeatureFlags.www-dynamic.js +++ b/packages/shared/forks/ReactFeatureFlags.www-dynamic.js @@ -21,6 +21,7 @@ export const decoupleUpdatePriorityFromScheduler = __VARIANT__; export const skipUnmountedBoundaries = __VARIANT__; export const enablePassiveEventIntervention = __VARIANT__; export const disableOnScrollBubbling = __VARIANT__; +export const enableEagerRootListeners = __VARIANT__; // Enable this flag to help with concurrent mode debugging. // It logs information to the console about React scheduling, rendering, and commit phases. diff --git a/packages/shared/forks/ReactFeatureFlags.www.js b/packages/shared/forks/ReactFeatureFlags.www.js index d9c237b048a..dbbaf0c70da 100644 --- a/packages/shared/forks/ReactFeatureFlags.www.js +++ b/packages/shared/forks/ReactFeatureFlags.www.js @@ -29,6 +29,7 @@ export const { skipUnmountedBoundaries, enablePassiveEventIntervention, disableOnScrollBubbling, + enableEagerRootListeners, } = dynamicFeatureFlags; // On WWW, __EXPERIMENTAL__ is used for a new modern build. diff --git a/scripts/error-codes/codes.json b/scripts/error-codes/codes.json index 9498d6e5ed3..06174f33af5 100644 --- a/scripts/error-codes/codes.json +++ b/scripts/error-codes/codes.json @@ -360,5 +360,6 @@ "368": "ReactDOM.createEventHandle: setListener called on an invalid target. Provide a valid EventTarget or an element managed by React.", "369": "ReactDOM.createEventHandle: setter called on an invalid target. Provide a valid EventTarget or an element managed by React.", "370": "ReactDOM.createEventHandle: setter called with an invalid callback. The callback must be a function.", - "371": "Text string must be rendered within a component.\n\nText: %s" + "371": "Text string must be rendered within a component.\n\nText: %s", + "372": "Cannot call unstable_createEventHandle with \"%s\", as it is not an event known to React." } From af219cc6e6c514099a667ffab4e2d80c5c0c1bcc Mon Sep 17 00:00:00 2001 From: Andrew Clark Date: Mon, 24 Aug 2020 12:07:11 -0500 Subject: [PATCH 4/9] Lint rule to forbid access of cross-fork fields (#19679) * Lint rule to forbid access of cross-fork fields We use a shared Fiber type for both reconciler forks (old and new). It is a superset of all the fields used by both forks. However, there are some fields that should only be used in the new fork, and others that should only be used in the old fork. Ideally we would enforce this with separate Flow types for each fork. The problem is that the Fiber type is accessed by some packages outside the reconciler (like React DOM), and get passed into the reconciler as arguments. So there's no way to fork the Fiber type without also forking the packages where they are used. FiberRoot has the same issue. Instead, I've added a lint rule that forbids cross-fork access of fork-specific fields. Fields that end in `_old` or `_new` are forbidden from being used inside the new or old fork respectively. Or you can specific custom fields using the ESLint plugin options. I used this plugin to find and remove references to the effect list in d2e914a. * Mark effect list fields as old And `subtreeTag` as new. I didn't mark `lastEffect` because that name is also used by the Hook type. Not super important; could rename to `lastEffect_old` but idk if it's worth the effort. --- .eslintrc.js | 12 ++ .../no-cross-fork-types-test.internal.js | 89 +++++++++++++ scripts/eslint-rules/index.js | 1 + scripts/eslint-rules/no-cross-fork-types.js | 126 ++++++++++++++++++ 4 files changed, 228 insertions(+) create mode 100644 scripts/eslint-rules/__tests__/no-cross-fork-types-test.internal.js create mode 100644 scripts/eslint-rules/no-cross-fork-types.js diff --git a/.eslintrc.js b/.eslintrc.js index 6998bca453f..b6d07a79e66 100644 --- a/.eslintrc.js +++ b/.eslintrc.js @@ -99,6 +99,18 @@ module.exports = { 'react-internal/warning-args': ERROR, 'react-internal/no-production-logging': ERROR, 'react-internal/no-cross-fork-imports': ERROR, + 'react-internal/no-cross-fork-types': [ + ERROR, + { + old: [ + 'firstEffect', + 'nextEffect', + // Disabled because it's also used by the Hook type. + // 'lastEffect', + ], + new: ['subtreeTag'], + }, + ], }, overrides: [ diff --git a/scripts/eslint-rules/__tests__/no-cross-fork-types-test.internal.js b/scripts/eslint-rules/__tests__/no-cross-fork-types-test.internal.js new file mode 100644 index 00000000000..57c50a55b31 --- /dev/null +++ b/scripts/eslint-rules/__tests__/no-cross-fork-types-test.internal.js @@ -0,0 +1,89 @@ +/** + * 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'; + +const rule = require('../no-cross-fork-types'); +const RuleTester = require('eslint').RuleTester; +const ruleTester = new RuleTester({ + parserOptions: { + ecmaVersion: 8, + sourceType: 'module', + }, +}); + +const newAccessWarning = + 'Field cannot be accessed inside the old reconciler fork, only the ' + + 'new fork.'; + +const oldAccessWarning = + 'Field cannot be accessed inside the new reconciler fork, only the ' + + 'old fork.'; + +ruleTester.run('eslint-rules/no-cross-fork-types', rule, { + valid: [ + { + code: ` +const a = obj.key_old; +const b = obj.key_new; +const {key_old, key_new} = obj; +`, + filename: 'ReactFiberWorkLoop.js', + }, + { + code: ` +const a = obj.key_old; +const {key_old} = obj; +`, + filename: 'ReactFiberWorkLoop.old.js', + }, + { + code: ` +const a = obj.key_new; +const {key_new} = obj; +`, + filename: 'ReactFiberWorkLoop.new.js', + }, + ], + invalid: [ + { + code: 'const a = obj.key_new;', + filename: 'ReactFiberWorkLoop.old.js', + errors: [{message: newAccessWarning}], + }, + { + code: 'const a = obj.key_old;', + filename: 'ReactFiberWorkLoop.new.js', + errors: [{message: oldAccessWarning}], + }, + + { + code: 'const {key_new} = obj;', + filename: 'ReactFiberWorkLoop.old.js', + errors: [{message: newAccessWarning}], + }, + { + code: 'const {key_old} = obj;', + filename: 'ReactFiberWorkLoop.new.js', + errors: [{message: oldAccessWarning}], + }, + { + code: 'const subtreeTag = obj.subtreeTag;', + filename: 'ReactFiberWorkLoop.old.js', + options: [{new: ['subtreeTag']}], + errors: [{message: newAccessWarning}], + }, + { + code: 'const firstEffect = obj.firstEffect;', + filename: 'ReactFiberWorkLoop.new.js', + options: [{old: ['firstEffect']}], + errors: [{message: oldAccessWarning}], + }, + ], +}); diff --git a/scripts/eslint-rules/index.js b/scripts/eslint-rules/index.js index 1451eb906d2..0a08b0d4cfc 100644 --- a/scripts/eslint-rules/index.js +++ b/scripts/eslint-rules/index.js @@ -8,5 +8,6 @@ module.exports = { 'invariant-args': require('./invariant-args'), 'no-production-logging': require('./no-production-logging'), 'no-cross-fork-imports': require('./no-cross-fork-imports'), + 'no-cross-fork-types': require('./no-cross-fork-types'), }, }; diff --git a/scripts/eslint-rules/no-cross-fork-types.js b/scripts/eslint-rules/no-cross-fork-types.js new file mode 100644 index 00000000000..eaa9f07d74e --- /dev/null +++ b/scripts/eslint-rules/no-cross-fork-types.js @@ -0,0 +1,126 @@ +/** + * 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 + */ + +/* eslint-disable no-for-of-loops/no-for-of-loops */ + +'use strict'; + +function isOldFork(filename) { + return filename.endsWith('.old.js') || filename.endsWith('.old'); +} + +function isNewFork(filename) { + return filename.endsWith('.new.js') || filename.endsWith('.new'); +} + +function warnIfNewField(context, newFields, identifier) { + const name = identifier.name; + if (name.endsWith('_new') || (newFields !== null && newFields.has(name))) { + context.report({ + node: identifier, + message: + 'Field cannot be accessed inside the old reconciler fork, only the ' + + 'new fork.', + }); + } +} + +function warnIfOldField(context, oldFields, identifier) { + const name = identifier.name; + if (name.endsWith('_old') || (oldFields !== null && oldFields.has(name))) { + context.report({ + node: identifier, + message: + 'Field cannot be accessed inside the new reconciler fork, only the ' + + 'old fork.', + }); + } +} + +module.exports = { + meta: { + type: 'problem', + fixable: 'code', + }, + create(context) { + const sourceFilename = context.getFilename(); + + if (isOldFork(sourceFilename)) { + const options = context.options; + let newFields = null; + if (options !== null) { + for (const option of options) { + if (option.new !== undefined) { + if (newFields === null) { + newFields = new Set(option.new); + } else { + for (const field of option.new) { + newFields.add(field); + } + } + } + } + } + return { + MemberExpression(node) { + const property = node.property; + if (property.type === 'Identifier') { + warnIfNewField(context, newFields, property); + } + }, + + ObjectPattern(node) { + for (const property of node.properties) { + const key = property.key; + if (key.type === 'Identifier') { + warnIfNewField(context, newFields, key); + } + } + }, + }; + } + + if (isNewFork(sourceFilename)) { + const options = context.options; + let oldFields = null; + if (options !== null) { + for (const option of options) { + if (option.old !== undefined) { + if (oldFields === null) { + oldFields = new Set(option.old); + } else { + for (const field of option.new) { + oldFields.add(field); + } + } + } + } + } + return { + MemberExpression(node) { + const property = node.property; + if (property.type === 'Identifier') { + warnIfOldField(context, oldFields, property); + } + }, + + ObjectPattern(node) { + for (const property of node.properties) { + const key = property.key; + if (key.type === 'Identifier') { + warnIfOldField(context, oldFields, key); + } + } + }, + }; + } + + return {}; + }, +}; From c4e0768d7487a9359b74986e3b07841d2520f593 Mon Sep 17 00:00:00 2001 From: inottn Date: Tue, 25 Aug 2020 20:49:57 +0800 Subject: [PATCH 5/9] Remove unused argument from `finishConcurrentRender` (#19689) --- packages/react-reconciler/src/ReactFiberWorkLoop.new.js | 4 ++-- packages/react-reconciler/src/ReactFiberWorkLoop.old.js | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.new.js b/packages/react-reconciler/src/ReactFiberWorkLoop.new.js index 5a3d2fe59b6..ecd4611c340 100644 --- a/packages/react-reconciler/src/ReactFiberWorkLoop.new.js +++ b/packages/react-reconciler/src/ReactFiberWorkLoop.new.js @@ -859,7 +859,7 @@ function performConcurrentWorkOnRoot(root) { const finishedWork: Fiber = (root.current.alternate: any); root.finishedWork = finishedWork; root.finishedLanes = lanes; - finishConcurrentRender(root, finishedWork, exitStatus, lanes); + finishConcurrentRender(root, exitStatus, lanes); } ensureRootIsScheduled(root, now()); @@ -871,7 +871,7 @@ function performConcurrentWorkOnRoot(root) { return null; } -function finishConcurrentRender(root, finishedWork, exitStatus, lanes) { +function finishConcurrentRender(root, exitStatus, lanes) { switch (exitStatus) { case RootIncomplete: case RootFatalErrored: { diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.old.js b/packages/react-reconciler/src/ReactFiberWorkLoop.old.js index 59cdb989055..21ac3eeb65c 100644 --- a/packages/react-reconciler/src/ReactFiberWorkLoop.old.js +++ b/packages/react-reconciler/src/ReactFiberWorkLoop.old.js @@ -847,7 +847,7 @@ function performConcurrentWorkOnRoot(root) { const finishedWork: Fiber = (root.current.alternate: any); root.finishedWork = finishedWork; root.finishedLanes = lanes; - finishConcurrentRender(root, finishedWork, exitStatus, lanes); + finishConcurrentRender(root, exitStatus, lanes); } ensureRootIsScheduled(root, now()); @@ -859,7 +859,7 @@ function performConcurrentWorkOnRoot(root) { return null; } -function finishConcurrentRender(root, finishedWork, exitStatus, lanes) { +function finishConcurrentRender(root, exitStatus, lanes) { switch (exitStatus) { case RootIncomplete: case RootFatalErrored: { From 5564f2c95bb61b446f93dc5c519740bdb39e1989 Mon Sep 17 00:00:00 2001 From: Ricky Date: Wed, 26 Aug 2020 11:34:17 -0400 Subject: [PATCH 6/9] Add React.startTransition (#19696) * Add React.startTransition * Export startTransition from index.js as well --- .../ReactSuspenseWithNoopRenderer-test.js | 209 ++++++++++++++++++ packages/react/index.classic.fb.js | 2 + packages/react/index.experimental.js | 1 + packages/react/index.js | 2 + packages/react/index.modern.fb.js | 2 + packages/react/src/React.js | 2 + packages/react/src/ReactStartTransition.js | 24 ++ 7 files changed, 242 insertions(+) create mode 100644 packages/react/src/ReactStartTransition.js diff --git a/packages/react-reconciler/src/__tests__/ReactSuspenseWithNoopRenderer-test.js b/packages/react-reconciler/src/__tests__/ReactSuspenseWithNoopRenderer-test.js index b76bae504dd..3ba37693491 100644 --- a/packages/react-reconciler/src/__tests__/ReactSuspenseWithNoopRenderer-test.js +++ b/packages/react-reconciler/src/__tests__/ReactSuspenseWithNoopRenderer-test.js @@ -2514,6 +2514,215 @@ describe('ReactSuspenseWithNoopRenderer', () => { }); }); + describe('delays transitions when using React.startTranistion', () => { + // @gate experimental + it('top level render', async () => { + function App({page}) { + return ( + }> + + + ); + } + + // Initial render. + React.unstable_startTransition(() => ReactNoop.render()); + + expect(Scheduler).toFlushAndYield(['Suspend! [A]', 'Loading...']); + // Only a short time is needed to unsuspend the initial loading state. + Scheduler.unstable_advanceTime(400); + await advanceTimers(400); + expect(ReactNoop.getChildren()).toEqual([span('Loading...')]); + + // Later we load the data. + Scheduler.unstable_advanceTime(5000); + await advanceTimers(5000); + expect(Scheduler).toHaveYielded(['Promise resolved [A]']); + expect(Scheduler).toFlushAndYield(['A']); + expect(ReactNoop.getChildren()).toEqual([span('A')]); + + // Start transition. + React.unstable_startTransition(() => ReactNoop.render()); + + expect(Scheduler).toFlushAndYield(['Suspend! [B]', 'Loading...']); + Scheduler.unstable_advanceTime(2999); + await advanceTimers(2999); + // Since the timeout is infinite (or effectively infinite), + // we have still not yet flushed the loading state. + expect(ReactNoop.getChildren()).toEqual([span('A')]); + + // Later we load the data. + Scheduler.unstable_advanceTime(3000); + await advanceTimers(3000); + expect(Scheduler).toHaveYielded(['Promise resolved [B]']); + expect(Scheduler).toFlushAndYield(['B']); + expect(ReactNoop.getChildren()).toEqual([span('B')]); + + // Start a long (infinite) transition. + React.unstable_startTransition(() => ReactNoop.render()); + expect(Scheduler).toFlushAndYield(['Suspend! [C]', 'Loading...']); + + // Advance past the current (effectively) infinite timeout. + // This is enforcing temporary behavior until it's truly infinite. + Scheduler.unstable_advanceTime(100000); + await advanceTimers(100000); + expect(ReactNoop.getChildren()).toEqual([ + hiddenSpan('B'), + span('Loading...'), + ]); + }); + + // @gate experimental + it('hooks', async () => { + let transitionToPage; + function App() { + const [page, setPage] = React.useState('none'); + transitionToPage = setPage; + if (page === 'none') { + return null; + } + return ( + }> + + + ); + } + + ReactNoop.render(); + expect(Scheduler).toFlushAndYield([]); + + // Initial render. + await ReactNoop.act(async () => { + React.unstable_startTransition(() => transitionToPage('A')); + + expect(Scheduler).toFlushAndYield(['Suspend! [A]', 'Loading...']); + // Only a short time is needed to unsuspend the initial loading state. + Scheduler.unstable_advanceTime(400); + await advanceTimers(400); + expect(ReactNoop.getChildren()).toEqual([span('Loading...')]); + }); + + // Later we load the data. + Scheduler.unstable_advanceTime(5000); + await advanceTimers(5000); + expect(Scheduler).toHaveYielded(['Promise resolved [A]']); + expect(Scheduler).toFlushAndYield(['A']); + expect(ReactNoop.getChildren()).toEqual([span('A')]); + + // Start transition. + await ReactNoop.act(async () => { + React.unstable_startTransition(() => transitionToPage('B')); + + expect(Scheduler).toFlushAndYield(['Suspend! [B]', 'Loading...']); + + Scheduler.unstable_advanceTime(2999); + await advanceTimers(2999); + // Since the timeout is infinite (or effectively infinite), + // we have still not yet flushed the loading state. + expect(ReactNoop.getChildren()).toEqual([span('A')]); + }); + + // Later we load the data. + Scheduler.unstable_advanceTime(3000); + await advanceTimers(3000); + expect(Scheduler).toHaveYielded(['Promise resolved [B]']); + expect(Scheduler).toFlushAndYield(['B']); + expect(ReactNoop.getChildren()).toEqual([span('B')]); + + // Start a long (infinite) transition. + await ReactNoop.act(async () => { + React.unstable_startTransition(() => transitionToPage('C')); + + expect(Scheduler).toFlushAndYield(['Suspend! [C]', 'Loading...']); + + // Advance past the current effectively infinite timeout. + // This is enforcing temporary behavior until it's truly infinite. + Scheduler.unstable_advanceTime(100000); + await advanceTimers(100000); + expect(ReactNoop.getChildren()).toEqual([ + hiddenSpan('B'), + span('Loading...'), + ]); + }); + }); + + // @gate experimental + it('classes', async () => { + let transitionToPage; + class App extends React.Component { + state = {page: 'none'}; + render() { + transitionToPage = page => this.setState({page}); + const page = this.state.page; + if (page === 'none') { + return null; + } + return ( + }> + + + ); + } + } + + ReactNoop.render(); + expect(Scheduler).toFlushAndYield([]); + + // Initial render. + await ReactNoop.act(async () => { + React.unstable_startTransition(() => transitionToPage('A')); + + expect(Scheduler).toFlushAndYield(['Suspend! [A]', 'Loading...']); + // Only a short time is needed to unsuspend the initial loading state. + Scheduler.unstable_advanceTime(400); + await advanceTimers(400); + expect(ReactNoop.getChildren()).toEqual([span('Loading...')]); + }); + + // Later we load the data. + Scheduler.unstable_advanceTime(5000); + await advanceTimers(5000); + expect(Scheduler).toHaveYielded(['Promise resolved [A]']); + expect(Scheduler).toFlushAndYield(['A']); + expect(ReactNoop.getChildren()).toEqual([span('A')]); + + // Start transition. + await ReactNoop.act(async () => { + React.unstable_startTransition(() => transitionToPage('B')); + + expect(Scheduler).toFlushAndYield(['Suspend! [B]', 'Loading...']); + Scheduler.unstable_advanceTime(2999); + await advanceTimers(2999); + // Since the timeout is infinite (or effectively infinite), + // we have still not yet flushed the loading state. + expect(ReactNoop.getChildren()).toEqual([span('A')]); + }); + + // Later we load the data. + Scheduler.unstable_advanceTime(3000); + await advanceTimers(3000); + expect(Scheduler).toHaveYielded(['Promise resolved [B]']); + expect(Scheduler).toFlushAndYield(['B']); + expect(ReactNoop.getChildren()).toEqual([span('B')]); + + // Start a long (infinite) transition. + await ReactNoop.act(async () => { + React.unstable_startTransition(() => transitionToPage('C')); + + expect(Scheduler).toFlushAndYield(['Suspend! [C]', 'Loading...']); + + // Advance past the current effectively infinite timeout. + // This is enforcing temporary behavior until it's truly infinite. + Scheduler.unstable_advanceTime(100000); + await advanceTimers(100000); + expect(ReactNoop.getChildren()).toEqual([ + hiddenSpan('B'), + span('Loading...'), + ]); + }); + }); + }); + // @gate experimental it('disables suspense config when nothing is passed to withSuspenseConfig', async () => { function App({page}) { diff --git a/packages/react/index.classic.fb.js b/packages/react/index.classic.fb.js index b06b59a10ab..50285328818 100644 --- a/packages/react/index.classic.fb.js +++ b/packages/react/index.classic.fb.js @@ -46,6 +46,8 @@ export { useTransition as unstable_useTransition, useDeferredValue, useDeferredValue as unstable_useDeferredValue, + startTransition, + startTransition as unstable_startTransition, SuspenseList, SuspenseList as unstable_SuspenseList, unstable_withSuspenseConfig, diff --git a/packages/react/index.experimental.js b/packages/react/index.experimental.js index f5332574a88..ff00b13333d 100644 --- a/packages/react/index.experimental.js +++ b/packages/react/index.experimental.js @@ -42,6 +42,7 @@ export { // exposeConcurrentModeAPIs useTransition as unstable_useTransition, useDeferredValue as unstable_useDeferredValue, + startTransition as unstable_startTransition, SuspenseList as unstable_SuspenseList, unstable_withSuspenseConfig, // enableBlocksAPI diff --git a/packages/react/index.js b/packages/react/index.js index a02725917e8..ebbb6cf42e6 100644 --- a/packages/react/index.js +++ b/packages/react/index.js @@ -72,6 +72,8 @@ export { createFactory, useTransition, useTransition as unstable_useTransition, + startTransition, + startTransition as unstable_startTransition, useDeferredValue, useDeferredValue as unstable_useDeferredValue, SuspenseList, diff --git a/packages/react/index.modern.fb.js b/packages/react/index.modern.fb.js index 916e6dcebe1..fb2e1dcbef4 100644 --- a/packages/react/index.modern.fb.js +++ b/packages/react/index.modern.fb.js @@ -45,6 +45,8 @@ export { useTransition as unstable_useTransition, useDeferredValue, useDeferredValue as unstable_useDeferredValue, + startTransition, + startTransition as unstable_startTransition, SuspenseList, SuspenseList as unstable_SuspenseList, unstable_withSuspenseConfig, diff --git a/packages/react/src/React.js b/packages/react/src/React.js index db7cf977ff9..bd737472a5e 100644 --- a/packages/react/src/React.js +++ b/packages/react/src/React.js @@ -58,6 +58,7 @@ import { import {createMutableSource} from './ReactMutableSource'; import ReactSharedInternals from './ReactSharedInternals'; import {createFundamental} from './ReactFundamental'; +import {startTransition} from './ReactStartTransition'; // TODO: Move this branching into the other module instead and just re-export. const createElement = __DEV__ ? createElementWithValidation : createElementProd; @@ -107,6 +108,7 @@ export { createFactory, // Concurrent Mode useTransition, + startTransition, useDeferredValue, REACT_SUSPENSE_LIST_TYPE as SuspenseList, REACT_LEGACY_HIDDEN_TYPE as unstable_LegacyHidden, diff --git a/packages/react/src/ReactStartTransition.js b/packages/react/src/ReactStartTransition.js new file mode 100644 index 00000000000..d17f083c473 --- /dev/null +++ b/packages/react/src/ReactStartTransition.js @@ -0,0 +1,24 @@ +/** + * 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 ReactCurrentBatchConfig from './ReactCurrentBatchConfig'; + +// Default to an arbitrarily large timeout. Effectively, this is infinite. The +// eventual goal is to never timeout when refreshing already visible content. +const IndefiniteTimeoutConfig = {timeoutMs: 100000}; + +export function startTransition(scope: () => void) { + const previousConfig = ReactCurrentBatchConfig.suspense; + ReactCurrentBatchConfig.suspense = IndefiniteTimeoutConfig; + try { + scope(); + } finally { + ReactCurrentBatchConfig.suspense = previousConfig; + } +} From 60ba723bf78b9a28f60dce854e88e206fab52301 Mon Sep 17 00:00:00 2001 From: Ben Pernick <57197898+bpernick@users.noreply.github.com> Date: Wed, 26 Aug 2020 12:04:43 -0500 Subject: [PATCH 7/9] Add SuspenseList to devTools (#19684) * ensure getDisplayName is only called on functions * add SuspenseList to Dev tools element names * Add SuspenseList and pass tests * Import SuspenseList directly * run prettier * Refactor tests to use real components * run linter --- .../src/__tests__/utils-test.js | 44 ++++++++++++++++++- packages/react-devtools-shared/src/utils.js | 8 +++- packages/react-is/src/ReactIs.js | 2 + 3 files changed, 51 insertions(+), 3 deletions(-) diff --git a/packages/react-devtools-shared/src/__tests__/utils-test.js b/packages/react-devtools-shared/src/__tests__/utils-test.js index 37e445974a2..f2d559485b2 100644 --- a/packages/react-devtools-shared/src/__tests__/utils-test.js +++ b/packages/react-devtools-shared/src/__tests__/utils-test.js @@ -7,7 +7,15 @@ * @flow */ -import {getDisplayName} from 'react-devtools-shared/src/utils'; +import { + getDisplayName, + getDisplayNameForReactElement, +} from 'react-devtools-shared/src/utils'; +import { + REACT_SUSPENSE_LIST_TYPE as SuspenseList, + REACT_STRICT_MODE_TYPE as StrictMode, +} from 'shared/ReactSymbols'; +import {createElement} from 'react/src/ReactElement'; describe('utils', () => { describe('getDisplayName', () => { @@ -37,4 +45,38 @@ describe('utils', () => { expect(getDisplayName(FauxComponent, 'Fallback')).toEqual('Fallback'); }); }); + describe('getDisplayNameForReactElement', () => { + it('should return correct display name for an element with function type', () => { + function FauxComponent() {} + FauxComponent.displayName = 'OverrideDisplayName'; + const element = createElement(FauxComponent); + expect(getDisplayNameForReactElement(element)).toEqual( + 'OverrideDisplayName', + ); + }); + it('should return correct display name for an element with a type of StrictMode', () => { + const element = createElement(StrictMode); + expect(getDisplayNameForReactElement(element)).toEqual('StrictMode'); + }); + it('should return correct display name for an element with a type of SuspenseList', () => { + const element = createElement(SuspenseList); + expect(getDisplayNameForReactElement(element)).toEqual('SuspenseList'); + }); + it('should return NotImplementedInDevtools for an element with invalid symbol type', () => { + const element = createElement(Symbol('foo')); + expect(getDisplayNameForReactElement(element)).toEqual( + 'NotImplementedInDevtools', + ); + }); + it('should return NotImplementedInDevtools for an element with invalid type', () => { + const element = createElement(true); + expect(getDisplayNameForReactElement(element)).toEqual( + 'NotImplementedInDevtools', + ); + }); + it('should return Element for null type', () => { + const element = createElement(); + expect(getDisplayNameForReactElement(element)).toEqual('Element'); + }); + }); }); diff --git a/packages/react-devtools-shared/src/utils.js b/packages/react-devtools-shared/src/utils.js index 87f845105b5..b0b873cca16 100644 --- a/packages/react-devtools-shared/src/utils.js +++ b/packages/react-devtools-shared/src/utils.js @@ -22,6 +22,7 @@ import { StrictMode, Suspense, } from 'react-is'; +import {REACT_SUSPENSE_LIST_TYPE as SuspenseList} from 'shared/ReactSymbols'; import { TREE_OPERATION_ADD, TREE_OPERATION_REMOVE, @@ -43,7 +44,6 @@ import { } from 'react-devtools-shared/src/types'; import {localStorageGetItem, localStorageSetItem} from './storage'; import {meta} from './hydration'; - import type {ComponentFilter, ElementType} from './types'; const cachedDisplayNames: WeakMap = new WeakMap(); @@ -489,12 +489,16 @@ export function getDisplayNameForReactElement( return 'StrictMode'; case Suspense: return 'Suspense'; + case SuspenseList: + return 'SuspenseList'; default: const {type} = element; if (typeof type === 'string') { return type; - } else if (type != null) { + } else if (typeof type === 'function') { return getDisplayName(type, 'Anonymous'); + } else if (type != null) { + return 'NotImplementedInDevtools'; } else { return 'Element'; } diff --git a/packages/react-is/src/ReactIs.js b/packages/react-is/src/ReactIs.js index bac0db1a880..2f132ba5de9 100644 --- a/packages/react-is/src/ReactIs.js +++ b/packages/react-is/src/ReactIs.js @@ -21,6 +21,7 @@ import { REACT_PROVIDER_TYPE, REACT_STRICT_MODE_TYPE, REACT_SUSPENSE_TYPE, + REACT_SUSPENSE_LIST_TYPE, } from 'shared/ReactSymbols'; import isValidElementType from 'shared/isValidElementType'; @@ -36,6 +37,7 @@ export function typeOf(object: any) { case REACT_PROFILER_TYPE: case REACT_STRICT_MODE_TYPE: case REACT_SUSPENSE_TYPE: + case REACT_SUSPENSE_LIST_TYPE: return type; default: const $$typeofType = type && type.$$typeof; From de71e1438e1428614b9163035b8947b2913c0fa8 Mon Sep 17 00:00:00 2001 From: Pascal Fong Kye Date: Thu, 27 Aug 2020 00:16:37 +0200 Subject: [PATCH 8/9] Add babel parser which supports ChainExpression --- package.json | 1 + yarn.lock | 22 ++++++++++++++++++++++ 2 files changed, 23 insertions(+) diff --git a/package.json b/package.json index b49bd2cc18b..6f9bc8d9c9b 100644 --- a/package.json +++ b/package.json @@ -7,6 +7,7 @@ "@babel/cli": "^7.10.5", "@babel/code-frame": "^7.10.4", "@babel/core": "^7.11.1", + "@babel/eslint-parser": "^7.11.4", "@babel/helper-module-imports": "^7.10.4", "@babel/parser": "^7.11.3", "@babel/plugin-external-helpers": "^7.10.4", diff --git a/yarn.lock b/yarn.lock index 90b4219bda2..ac526d6d4e5 100644 --- a/yarn.lock +++ b/yarn.lock @@ -106,6 +106,15 @@ semver "^5.4.1" source-map "^0.5.0" +"@babel/eslint-parser@^7.11.4": + version "7.11.4" + resolved "https://registry.yarnpkg.com/@babel/eslint-parser/-/eslint-parser-7.11.4.tgz#f79bac69088097a8418f5c67fc462c89a72c2f48" + integrity sha512-syIzsqEUvmc6WEYbLqrvBODCM1wMo3SQ4h4G9gtCcQctv1VUlA5davRAWHFm7ncQlxcJs4I7kaflsnAP8iA8Aw== + dependencies: + eslint-scope "5.1.0" + eslint-visitor-keys "^1.3.0" + semver "^6.3.0" + "@babel/generator@^7.11.0": version "7.11.0" resolved "https://registry.yarnpkg.com/@babel/generator/-/generator-7.11.0.tgz#4b90c78d8c12825024568cbe83ee6c9af193585c" @@ -5436,6 +5445,14 @@ eslint-scope@3.7.1: esrecurse "^4.1.0" estraverse "^4.1.1" +eslint-scope@5.1.0: + version "5.1.0" + resolved "https://registry.yarnpkg.com/eslint-scope/-/eslint-scope-5.1.0.tgz#d0f971dfe59c69e0cada684b23d49dbf82600ce5" + integrity sha512-iiGRvtxWqgtx5m8EyQUJihBloE4EnYeGE/bz1wSPwJE6tZuJUtHlhqDM4Xj2ukE8Dyy1+HCZ4hE0fzIVMzb58w== + dependencies: + esrecurse "^4.1.0" + estraverse "^4.1.1" + eslint-scope@^4.0.0, eslint-scope@^4.0.3: version "4.0.3" resolved "https://registry.yarnpkg.com/eslint-scope/-/eslint-scope-4.0.3.tgz#ca03833310f6889a3264781aa82e63eb9cfe7848" @@ -5476,6 +5493,11 @@ eslint-visitor-keys@^1.0.0, eslint-visitor-keys@^1.1.0: resolved "https://registry.yarnpkg.com/eslint-visitor-keys/-/eslint-visitor-keys-1.1.0.tgz#e2a82cea84ff246ad6fb57f9bde5b46621459ec2" integrity sha512-8y9YjtM1JBJU/A9Kc+SbaOV4y29sSWckBwMHa+FGtVj5gN/sbnKDf6xJUl+8g7FAij9LVaP8C24DUiH/f/2Z9A== +eslint-visitor-keys@^1.3.0: + version "1.3.0" + resolved "https://registry.yarnpkg.com/eslint-visitor-keys/-/eslint-visitor-keys-1.3.0.tgz#30ebd1ef7c2fdff01c3a4f151044af25fab0523e" + integrity sha512-6J72N8UNa462wa/KFODt/PJ3IU60SDpC3QXC1Hjc1BXXpfL2C9R5+AU7jhe0F6GREqVMh4Juu+NY7xn+6dipUQ== + eslint@5.16.0: version "5.16.0" resolved "https://registry.yarnpkg.com/eslint/-/eslint-5.16.0.tgz#a1e3ac1aae4a3fbd8296fcf8f7ab7314cbb6abea" From a4dd52dae57b28e44c10ec08cb8596a7b4757bf2 Mon Sep 17 00:00:00 2001 From: Pascal Fong Kye Date: Thu, 27 Aug 2020 01:16:16 +0200 Subject: [PATCH 9/9] Add and fix tests for new babel eslint parser --- .../ESLintRuleExhaustiveDeps-test.js | 5 +++ .../src/ExhaustiveDeps.js | 41 +++++++++++++++++-- 2 files changed, 42 insertions(+), 4 deletions(-) diff --git a/packages/eslint-plugin-react-hooks/__tests__/ESLintRuleExhaustiveDeps-test.js b/packages/eslint-plugin-react-hooks/__tests__/ESLintRuleExhaustiveDeps-test.js index 4de51e9c404..21016042f26 100644 --- a/packages/eslint-plugin-react-hooks/__tests__/ESLintRuleExhaustiveDeps-test.js +++ b/packages/eslint-plugin-react-hooks/__tests__/ESLintRuleExhaustiveDeps-test.js @@ -7961,6 +7961,11 @@ new ESLintTester({ parserOptions, }).run('react-hooks', ReactHooksESLintRule, tests); +new ESLintTester({ + parser: require.resolve('@babel/eslint-parser'), + parserOptions, +}).run('react-hooks', ReactHooksESLintRule, tests); + new ESLintTester({ parser: require.resolve('@typescript-eslint/parser'), parserOptions, diff --git a/packages/eslint-plugin-react-hooks/src/ExhaustiveDeps.js b/packages/eslint-plugin-react-hooks/src/ExhaustiveDeps.js index ed48afa7869..2c7edb000de 100644 --- a/packages/eslint-plugin-react-hooks/src/ExhaustiveDeps.js +++ b/packages/eslint-plugin-react-hooks/src/ExhaustiveDeps.js @@ -803,9 +803,10 @@ export default { let maybeID = declaredDependencyNode; while ( maybeID.type === 'MemberExpression' || - maybeID.type === 'OptionalMemberExpression' + maybeID.type === 'OptionalMemberExpression' || + maybeID.type === 'ChainExpression' ) { - maybeID = maybeID.object; + maybeID = maybeID.object || maybeID.expression.object; } const isDeclaredInComponent = !componentScope.through.some( ref => ref.identifier === maybeID, @@ -1599,8 +1600,19 @@ function analyzePropertyChain(node, optionalChains) { const property = analyzePropertyChain(node.property, null); const result = `${object}.${property}`; if (optionalChains) { - // Mark as required. - optionalChains.set(result, false); + // Note: OptionalMemberExpression doesn't necessarily mean this node is optional. + // It just means there is an optional member somewhere inside. + // This particular node might still represent a required member, so check .optional field. + if (node.optional) { + // We only want to consider it optional if *all* usages were optional. + if (!optionalChains.has(result)) { + // Mark as (maybe) optional. If there's a required usage, this will be overridden. + optionalChains.set(result, true); + } + } else { + // Mark as required. + optionalChains.set(result, false); + } } return result; } else if (node.type === 'OptionalMemberExpression' && !node.computed) { @@ -1623,6 +1635,27 @@ function analyzePropertyChain(node, optionalChains) { } } return result; + } else if (node.type === 'ChainExpression' && !node.computed) { + const expression = node.expression; + const object = analyzePropertyChain(expression.object, optionalChains); + const property = analyzePropertyChain(expression.property, null); + const result = `${object}.${property}`; + if (optionalChains) { + // Note: OptionalMemberExpression doesn't necessarily mean this node is optional. + // It just means there is an optional member somewhere inside. + // This particular node might still represent a required member, so check .optional field. + if (expression.optional) { + // We only want to consider it optional if *all* usages were optional. + if (!optionalChains.has(result)) { + // Mark as (maybe) optional. If there's a required usage, this will be overridden. + optionalChains.set(result, true); + } + } else { + // Mark as required. + optionalChains.set(result, false); + } + } + return result; } else { throw new Error(`Unsupported node type: ${node.type}`); }