From 93487a9ad771c0aa51ba86c8854ec6920bb9ceca Mon Sep 17 00:00:00 2001 From: Andrew Clark Date: Wed, 24 Apr 2019 20:25:41 -0700 Subject: [PATCH 1/4] Add Batched Mode MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit React has an unfortunate quirk where updates are sometimes synchronous -- where React starts rendering immediately within the call stack of `setState` — and sometimes batched, where updates are flushed at the end of the current event. Any update that originates within the call stack of the React event system is batched. This encompasses most updates, since most updates originate from an event handler like `onClick` or `onChange`. It also includes updates triggered by lifecycle methods or effects. But there are also updates that originate outside React's event system, like timer events, network events, and microtasks (promise resolution handlers). These are not batched, which results in both worse performance (multiple render passes instead of single one) and confusing semantics. Ideally all updates would be batched by default. Unfortunately, it's easy for components to accidentally rely on this behavior, so changing it could break existing apps in subtle ways. One way to move to a batched-by-default model is to opt into Concurrent Mode (still experimental). But Concurrent Mode introduces additional semantic changes that apps may not be ready to adopt. This commit introduces an additional mode called Batched Mode. Batched Mode enables a batched-by-default model that defers all updates to the next React event. Once it begins rendering, React will not yield to the browser until the entire render is finished. Batched Mode is superset of Strict Mode. It fires all the same warnings. It also drops the forked Suspense behavior used by Legacy Mode, in favor of the proper semantics used by Concurrent Mode. I have not added any public APIs that expose the new mode yet. I'll do that in subsequent commits. --- packages/react-dom/src/client/ReactDOM.js | 3 +- packages/react-dom/src/fire/ReactFire.js | 3 +- .../react-native-renderer/src/ReactFabric.js | 2 +- .../src/ReactNativeRenderer.js | 2 +- .../src/createReactNoop.js | 175 +++++++++++++----- packages/react-reconciler/src/ReactFiber.js | 59 +++--- .../src/ReactFiberBeginWork.js | 8 +- .../src/ReactFiberCompleteWork.js | 4 +- .../src/ReactFiberExpirationTime.js | 3 +- .../src/ReactFiberReconciler.js | 3 +- .../react-reconciler/src/ReactFiberRoot.js | 3 +- .../src/ReactFiberScheduler.js | 25 ++- .../src/ReactFiberUnwindWork.js | 4 +- .../react-reconciler/src/ReactTypeOfMode.js | 9 +- .../ReactBatchedMode-test.internal.js | 58 ++++++ .../src/ReactTestRenderer.js | 2 + 16 files changed, 256 insertions(+), 107 deletions(-) create mode 100644 packages/react-reconciler/src/__tests__/ReactBatchedMode-test.internal.js diff --git a/packages/react-dom/src/client/ReactDOM.js b/packages/react-dom/src/client/ReactDOM.js index 728f775adf5e2..af726812f5326 100644 --- a/packages/react-dom/src/client/ReactDOM.js +++ b/packages/react-dom/src/client/ReactDOM.js @@ -366,7 +366,8 @@ function ReactRoot( isConcurrent: boolean, hydrate: boolean, ) { - const root = createContainer(container, isConcurrent, hydrate); + const isBatched = false; + const root = createContainer(container, isBatched, isConcurrent, hydrate); this._internalRoot = root; } ReactRoot.prototype.render = function( diff --git a/packages/react-dom/src/fire/ReactFire.js b/packages/react-dom/src/fire/ReactFire.js index e4f555ebd627d..514fa5b44470b 100644 --- a/packages/react-dom/src/fire/ReactFire.js +++ b/packages/react-dom/src/fire/ReactFire.js @@ -372,7 +372,8 @@ function ReactRoot( isConcurrent: boolean, hydrate: boolean, ) { - const root = createContainer(container, isConcurrent, hydrate); + const isBatched = false; + const root = createContainer(container, isBatched, isConcurrent, hydrate); this._internalRoot = root; } ReactRoot.prototype.render = function( diff --git a/packages/react-native-renderer/src/ReactFabric.js b/packages/react-native-renderer/src/ReactFabric.js index a22c78aa5793a..fc883e079ac4a 100644 --- a/packages/react-native-renderer/src/ReactFabric.js +++ b/packages/react-native-renderer/src/ReactFabric.js @@ -119,7 +119,7 @@ const ReactFabric: ReactFabricType = { if (!root) { // TODO (bvaughn): If we decide to keep the wrapper component, // We could create a wrapper for containerTag as well to reduce special casing. - root = createContainer(containerTag, false, false); + root = createContainer(containerTag, false, false, false); roots.set(containerTag, root); } updateContainer(element, root, null, callback); diff --git a/packages/react-native-renderer/src/ReactNativeRenderer.js b/packages/react-native-renderer/src/ReactNativeRenderer.js index 6cfe575dcb3a9..9189a50e3e18d 100644 --- a/packages/react-native-renderer/src/ReactNativeRenderer.js +++ b/packages/react-native-renderer/src/ReactNativeRenderer.js @@ -125,7 +125,7 @@ const ReactNativeRenderer: ReactNativeType = { if (!root) { // TODO (bvaughn): If we decide to keep the wrapper component, // We could create a wrapper for containerTag as well to reduce special casing. - root = createContainer(containerTag, false, false); + root = createContainer(containerTag, false, false, false); roots.set(containerTag, root); } updateContainer(element, root, null, callback); diff --git a/packages/react-noop-renderer/src/createReactNoop.js b/packages/react-noop-renderer/src/createReactNoop.js index 3ac1c7021de5d..93310773bc05d 100644 --- a/packages/react-noop-renderer/src/createReactNoop.js +++ b/packages/react-noop-renderer/src/createReactNoop.js @@ -832,77 +832,160 @@ function createReactNoop(reconciler: Function, useMutation: boolean) { return textInstance.text; } + function getChildren(root) { + if (root) { + return root.children; + } else { + return null; + } + } + + function getPendingChildren(root) { + if (root) { + return root.pendingChildren; + } else { + return null; + } + } + + function getChildrenAsJSX(root) { + const children = childToJSX(getChildren(root), null); + if (children === null) { + return null; + } + if (Array.isArray(children)) { + return { + $$typeof: REACT_ELEMENT_TYPE, + type: REACT_FRAGMENT_TYPE, + key: null, + ref: null, + props: {children}, + _owner: null, + _store: __DEV__ ? {} : undefined, + }; + } + return children; + } + + function getPendingChildrenAsJSX(root) { + const children = childToJSX(getChildren(root), null); + if (children === null) { + return null; + } + if (Array.isArray(children)) { + return { + $$typeof: REACT_ELEMENT_TYPE, + type: REACT_FRAGMENT_TYPE, + key: null, + ref: null, + props: {children}, + _owner: null, + _store: __DEV__ ? {} : undefined, + }; + } + return children; + } + + let idCounter = 0; + const ReactNoop = { _Scheduler: Scheduler, getChildren(rootID: string = DEFAULT_ROOT_ID) { const container = rootContainers.get(rootID); - if (container) { - return container.children; - } else { - return null; - } + return getChildren(container); }, getPendingChildren(rootID: string = DEFAULT_ROOT_ID) { const container = rootContainers.get(rootID); - if (container) { - return container.pendingChildren; - } else { - return null; - } + return getPendingChildren(container); }, getOrCreateRootContainer( rootID: string = DEFAULT_ROOT_ID, - isConcurrent: boolean = false, + isBatched: boolean, + isConcurrent: boolean, ) { let root = roots.get(rootID); if (!root) { const container = {rootID: rootID, pendingChildren: [], children: []}; rootContainers.set(rootID, container); - root = NoopRenderer.createContainer(container, isConcurrent, false); + root = NoopRenderer.createContainer( + container, + isBatched, + isConcurrent, + false, + ); roots.set(rootID, root); } return root.current.stateNode.containerInfo; }, + // TODO: Replace ReactNoop.render with createRoot + root.render + createRoot() { + const isBatched = true; + const isConcurrent = true; + const container = { + rootID: '' + idCounter++, + pendingChildren: [], + children: [], + }; + const fiberRoot = NoopRenderer.createContainer( + container, + isBatched, + isConcurrent, + false, + ); + return { + _Scheduler: Scheduler, + render(children: ReactNodeList) { + NoopRenderer.updateContainer(children, fiberRoot, null, null); + }, + getChildren() { + return getChildren(fiberRoot); + }, + getChildrenAsJSX() { + return getChildrenAsJSX(fiberRoot); + }, + }; + }, + + createSyncRoot() { + const isBatched = true; + const isConcurrent = false; + const container = { + rootID: '' + idCounter++, + pendingChildren: [], + children: [], + }; + const fiberRoot = NoopRenderer.createContainer( + container, + isBatched, + isConcurrent, + false, + ); + return { + _Scheduler: Scheduler, + render(children: ReactNodeList) { + NoopRenderer.updateContainer(children, fiberRoot, null, null); + }, + getChildren() { + return getChildren(container); + }, + getChildrenAsJSX() { + return getChildrenAsJSX(container); + }, + }; + }, + getChildrenAsJSX(rootID: string = DEFAULT_ROOT_ID) { - const children = childToJSX(ReactNoop.getChildren(rootID), null); - if (children === null) { - return null; - } - if (Array.isArray(children)) { - return { - $$typeof: REACT_ELEMENT_TYPE, - type: REACT_FRAGMENT_TYPE, - key: null, - ref: null, - props: {children}, - _owner: null, - _store: __DEV__ ? {} : undefined, - }; - } - return children; + const container = rootContainers.get(rootID); + return getChildrenAsJSX(container); }, getPendingChildrenAsJSX(rootID: string = DEFAULT_ROOT_ID) { - const children = childToJSX(ReactNoop.getPendingChildren(rootID), null); - if (children === null) { - return null; - } - if (Array.isArray(children)) { - return { - $$typeof: REACT_ELEMENT_TYPE, - type: REACT_FRAGMENT_TYPE, - key: null, - ref: null, - props: {children}, - _owner: null, - _store: __DEV__ ? {} : undefined, - }; - } - return children; + const container = rootContainers.get(rootID); + return getPendingChildrenAsJSX(container); }, createPortal( @@ -920,9 +1003,11 @@ function createReactNoop(reconciler: Function, useMutation: boolean) { renderLegacySyncRoot(element: React$Element, callback: ?Function) { const rootID = DEFAULT_ROOT_ID; + const isBatched = false; const isConcurrent = false; const container = ReactNoop.getOrCreateRootContainer( rootID, + isBatched, isConcurrent, ); const root = roots.get(container.rootID); @@ -934,9 +1019,11 @@ function createReactNoop(reconciler: Function, useMutation: boolean) { rootID: string, callback: ?Function, ) { + const isBatched = true; const isConcurrent = true; const container = ReactNoop.getOrCreateRootContainer( rootID, + isBatched, isConcurrent, ); const root = roots.get(container.rootID); diff --git a/packages/react-reconciler/src/ReactFiber.js b/packages/react-reconciler/src/ReactFiber.js index 8ca24a16a670c..38fafa1cd1ddd 100644 --- a/packages/react-reconciler/src/ReactFiber.js +++ b/packages/react-reconciler/src/ReactFiber.js @@ -52,10 +52,11 @@ import getComponentName from 'shared/getComponentName'; import {isDevToolsPresent} from './ReactFiberDevToolsHook'; import {NoWork} from './ReactFiberExpirationTime'; import { - NoContext, + NoMode, ConcurrentMode, ProfileMode, StrictMode, + BatchedMode, } from './ReactTypeOfMode'; import { REACT_FORWARD_REF_TYPE, @@ -434,8 +435,18 @@ export function createWorkInProgress( return workInProgress; } -export function createHostRootFiber(isConcurrent: boolean): Fiber { - let mode = isConcurrent ? ConcurrentMode | StrictMode : NoContext; +export function createHostRootFiber( + isBatched: boolean, + isConcurrent: boolean, +): Fiber { + let mode; + if (isConcurrent) { + mode = ConcurrentMode | BatchedMode | StrictMode; + } else if (isBatched) { + mode = BatchedMode | StrictMode; + } else { + mode = NoMode; + } if (enableProfilerTimer && isDevToolsPresent) { // Always collect profile timings when DevTools are present. @@ -476,19 +487,13 @@ export function createFiberFromTypeAndProps( key, ); case REACT_CONCURRENT_MODE_TYPE: - return createFiberFromMode( - pendingProps, - mode | ConcurrentMode | StrictMode, - expirationTime, - key, - ); + fiberTag = Mode; + mode |= ConcurrentMode | BatchedMode | StrictMode; + break; case REACT_STRICT_MODE_TYPE: - return createFiberFromMode( - pendingProps, - mode | StrictMode, - expirationTime, - key, - ); + fiberTag = Mode; + mode |= StrictMode; + break; case REACT_PROFILER_TYPE: return createFiberFromProfiler(pendingProps, mode, expirationTime, key); case REACT_SUSPENSE_TYPE: @@ -672,26 +677,6 @@ function createFiberFromProfiler( return fiber; } -function createFiberFromMode( - pendingProps: any, - mode: TypeOfMode, - expirationTime: ExpirationTime, - key: null | string, -): Fiber { - const fiber = createFiber(Mode, pendingProps, key, mode); - - // TODO: The Mode fiber shouldn't have a type. It has a tag. - const type = - (mode & ConcurrentMode) === NoContext - ? REACT_STRICT_MODE_TYPE - : REACT_CONCURRENT_MODE_TYPE; - fiber.elementType = type; - fiber.type = type; - - fiber.expirationTime = expirationTime; - return fiber; -} - export function createFiberFromSuspense( pendingProps: any, mode: TypeOfMode, @@ -720,7 +705,7 @@ export function createFiberFromText( } export function createFiberFromHostInstanceForDeletion(): Fiber { - const fiber = createFiber(HostComponent, null, null, NoContext); + const fiber = createFiber(HostComponent, null, null, NoMode); // TODO: These should not need a type. fiber.elementType = 'DELETED'; fiber.type = 'DELETED'; @@ -751,7 +736,7 @@ export function assignFiberPropertiesInDEV( if (target === null) { // This Fiber's initial properties will always be overwritten. // We only use a Fiber to ensure the same hidden class so DEV isn't slow. - target = createFiber(IndeterminateComponent, null, null, NoContext); + target = createFiber(IndeterminateComponent, null, null, NoMode); } // This is intentionally written as a list of all properties. diff --git a/packages/react-reconciler/src/ReactFiberBeginWork.js b/packages/react-reconciler/src/ReactFiberBeginWork.js index 8659e010a9847..94f5b0cbb0262 100644 --- a/packages/react-reconciler/src/ReactFiberBeginWork.js +++ b/packages/react-reconciler/src/ReactFiberBeginWork.js @@ -84,7 +84,7 @@ import { } from './ReactFiberExpirationTime'; import { ConcurrentMode, - NoContext, + NoMode, ProfileMode, StrictMode, } from './ReactTypeOfMode'; @@ -1493,7 +1493,7 @@ function updateSuspenseComponent( null, ); - if ((workInProgress.mode & ConcurrentMode) === NoContext) { + if ((workInProgress.mode & ConcurrentMode) === NoMode) { // Outside of concurrent mode, we commit the effects from the // partially completed, timed-out tree, too. const progressedState: SuspenseState = workInProgress.memoizedState; @@ -1546,7 +1546,7 @@ function updateSuspenseComponent( NoWork, ); - if ((workInProgress.mode & ConcurrentMode) === NoContext) { + if ((workInProgress.mode & ConcurrentMode) === NoMode) { // Outside of concurrent mode, we commit the effects from the // partially completed, timed-out tree, too. const progressedState: SuspenseState = workInProgress.memoizedState; @@ -1629,7 +1629,7 @@ function updateSuspenseComponent( // schedule a placement. // primaryChildFragment.effectTag |= Placement; - if ((workInProgress.mode & ConcurrentMode) === NoContext) { + if ((workInProgress.mode & ConcurrentMode) === NoMode) { // Outside of concurrent mode, we commit the effects from the // partially completed, timed-out tree, too. const progressedState: SuspenseState = workInProgress.memoizedState; diff --git a/packages/react-reconciler/src/ReactFiberCompleteWork.js b/packages/react-reconciler/src/ReactFiberCompleteWork.js index 9bf288fc63ee2..dbd0c7e1743f4 100644 --- a/packages/react-reconciler/src/ReactFiberCompleteWork.js +++ b/packages/react-reconciler/src/ReactFiberCompleteWork.js @@ -43,7 +43,7 @@ import { EventComponent, EventTarget, } from 'shared/ReactWorkTags'; -import {ConcurrentMode, NoContext} from './ReactTypeOfMode'; +import {ConcurrentMode, NoMode} from './ReactTypeOfMode'; import { Placement, Ref, @@ -721,7 +721,7 @@ function completeWork( // TODO: This will still suspend a synchronous tree if anything // in the concurrent tree already suspended during this render. // This is a known bug. - if ((workInProgress.mode & ConcurrentMode) !== NoContext) { + if ((workInProgress.mode & ConcurrentMode) !== NoMode) { renderDidSuspend(); } } diff --git a/packages/react-reconciler/src/ReactFiberExpirationTime.js b/packages/react-reconciler/src/ReactFiberExpirationTime.js index a8594a57a3fe6..9efc530f38cc3 100644 --- a/packages/react-reconciler/src/ReactFiberExpirationTime.js +++ b/packages/react-reconciler/src/ReactFiberExpirationTime.js @@ -23,9 +23,10 @@ export type ExpirationTime = number; export const NoWork = 0; export const Never = 1; export const Sync = MAX_SIGNED_31_BIT_INT; +export const Batched = Sync - 1; const UNIT_SIZE = 10; -const MAGIC_NUMBER_OFFSET = MAX_SIGNED_31_BIT_INT - 1; +const MAGIC_NUMBER_OFFSET = Batched - 1; // 1 unit of expiration time represents 10ms. export function msToExpirationTime(ms: number): ExpirationTime { diff --git a/packages/react-reconciler/src/ReactFiberReconciler.js b/packages/react-reconciler/src/ReactFiberReconciler.js index 77fadd31e9b2e..0ac8890390635 100644 --- a/packages/react-reconciler/src/ReactFiberReconciler.js +++ b/packages/react-reconciler/src/ReactFiberReconciler.js @@ -273,10 +273,11 @@ function findHostInstanceWithWarning( export function createContainer( containerInfo: Container, + isBatched: boolean, isConcurrent: boolean, hydrate: boolean, ): OpaqueRoot { - return createFiberRoot(containerInfo, isConcurrent, hydrate); + return createFiberRoot(containerInfo, isBatched, isConcurrent, hydrate); } export function updateContainer( diff --git a/packages/react-reconciler/src/ReactFiberRoot.js b/packages/react-reconciler/src/ReactFiberRoot.js index 38563ae2533bd..9329194459559 100644 --- a/packages/react-reconciler/src/ReactFiberRoot.js +++ b/packages/react-reconciler/src/ReactFiberRoot.js @@ -116,6 +116,7 @@ function FiberRootNode(containerInfo, hydrate) { export function createFiberRoot( containerInfo: any, + isBatched: boolean, isConcurrent: boolean, hydrate: boolean, ): FiberRoot { @@ -123,7 +124,7 @@ export function createFiberRoot( // Cyclic construction. This cheats the type system right now because // stateNode is any. - const uninitializedFiber = createHostRootFiber(isConcurrent); + const uninitializedFiber = createHostRootFiber(isBatched, isConcurrent); root.current = uninitializedFiber; uninitializedFiber.stateNode = root; diff --git a/packages/react-reconciler/src/ReactFiberScheduler.js b/packages/react-reconciler/src/ReactFiberScheduler.js index 4ba3436ea93de..a5b807cde9f3c 100644 --- a/packages/react-reconciler/src/ReactFiberScheduler.js +++ b/packages/react-reconciler/src/ReactFiberScheduler.js @@ -55,7 +55,12 @@ import { } from './ReactFiberHostConfig'; import {createWorkInProgress, assignFiberPropertiesInDEV} from './ReactFiber'; -import {NoContext, ConcurrentMode, ProfileMode} from './ReactTypeOfMode'; +import { + NoMode, + ProfileMode, + BatchedMode, + ConcurrentMode, +} from './ReactTypeOfMode'; import { HostRoot, ClassComponent, @@ -91,6 +96,7 @@ import { computeAsyncExpiration, inferPriorityFromExpirationTime, LOW_PRIORITY_EXPIRATION, + Batched, } from './ReactFiberExpirationTime'; import {beginWork as originalBeginWork} from './ReactFiberBeginWork'; import {completeWork} from './ReactFiberCompleteWork'; @@ -255,10 +261,16 @@ export function computeExpirationForFiber( currentTime: ExpirationTime, fiber: Fiber, ): ExpirationTime { - if ((fiber.mode & ConcurrentMode) === NoContext) { + const mode = fiber.mode; + if ((mode & BatchedMode) === NoMode) { return Sync; } + const priorityLevel = getCurrentPriorityLevel(); + if ((mode & ConcurrentMode) === NoMode) { + return priorityLevel === ImmediatePriority ? Sync : Batched; + } + if (workPhase === RenderPhase) { // Use whatever time we're already rendering return renderExpirationTime; @@ -266,7 +278,6 @@ export function computeExpirationForFiber( // Compute an expiration time based on the Scheduler priority. let expirationTime; - const priorityLevel = getCurrentPriorityLevel(); switch (priorityLevel) { case ImmediatePriority: expirationTime = Sync; @@ -983,7 +994,7 @@ function performUnitOfWork(unitOfWork: Fiber): Fiber | null { setCurrentDebugFiberInDEV(unitOfWork); let next; - if (enableProfilerTimer && (unitOfWork.mode & ProfileMode) !== NoContext) { + if (enableProfilerTimer && (unitOfWork.mode & ProfileMode) !== NoMode) { startProfilerTimer(unitOfWork); next = beginWork(current, unitOfWork, renderExpirationTime); stopProfilerTimerIfRunningAndRecordDelta(unitOfWork, true); @@ -1019,7 +1030,7 @@ function completeUnitOfWork(unitOfWork: Fiber): Fiber | null { let next; if ( !enableProfilerTimer || - (workInProgress.mode & ProfileMode) === NoContext + (workInProgress.mode & ProfileMode) === NoMode ) { next = completeWork(current, workInProgress, renderExpirationTime); } else { @@ -1085,7 +1096,7 @@ function completeUnitOfWork(unitOfWork: Fiber): Fiber | null { if ( enableProfilerTimer && - (workInProgress.mode & ProfileMode) !== NoContext + (workInProgress.mode & ProfileMode) !== NoMode ) { // Record the render duration for the fiber that errored. stopProfilerTimerIfRunningAndRecordDelta(workInProgress, false); @@ -1149,7 +1160,7 @@ function resetChildExpirationTime(completedWork: Fiber) { let newChildExpirationTime = NoWork; // Bubble up the earliest expiration time. - if (enableProfilerTimer && (completedWork.mode & ProfileMode) !== NoContext) { + if (enableProfilerTimer && (completedWork.mode & ProfileMode) !== NoMode) { // In profiling mode, resetChildExpirationTime is also used to reset // profiler durations. let actualDuration = completedWork.actualDuration; diff --git a/packages/react-reconciler/src/ReactFiberUnwindWork.js b/packages/react-reconciler/src/ReactFiberUnwindWork.js index c6c8276733037..a39632acf08cb 100644 --- a/packages/react-reconciler/src/ReactFiberUnwindWork.js +++ b/packages/react-reconciler/src/ReactFiberUnwindWork.js @@ -41,7 +41,7 @@ import { enableSuspenseServerRenderer, enableEventAPI, } from 'shared/ReactFeatureFlags'; -import {ConcurrentMode, NoContext} from './ReactTypeOfMode'; +import {ConcurrentMode, NoMode} from './ReactTypeOfMode'; import {shouldCaptureSuspense} from './ReactFiberSuspenseComponent'; import {createCapturedValue} from './ReactCapturedValue'; @@ -231,7 +231,7 @@ function throwException( // Note: It doesn't matter whether the component that suspended was // inside a concurrent mode tree. If the Suspense is outside of it, we // should *not* suspend the commit. - if ((workInProgress.mode & ConcurrentMode) === NoContext) { + if ((workInProgress.mode & ConcurrentMode) === NoMode) { workInProgress.effectTag |= DidCapture; // We're going to commit this fiber even though it didn't complete. diff --git a/packages/react-reconciler/src/ReactTypeOfMode.js b/packages/react-reconciler/src/ReactTypeOfMode.js index a727bec843425..a097830ae89e6 100644 --- a/packages/react-reconciler/src/ReactTypeOfMode.js +++ b/packages/react-reconciler/src/ReactTypeOfMode.js @@ -9,7 +9,8 @@ export type TypeOfMode = number; -export const NoContext = 0b000; -export const ConcurrentMode = 0b001; -export const StrictMode = 0b010; -export const ProfileMode = 0b100; +export const NoMode = 0b0000; +export const StrictMode = 0b0001; +export const BatchedMode = 0b0010; +export const ConcurrentMode = 0b0100; +export const ProfileMode = 0b1000; diff --git a/packages/react-reconciler/src/__tests__/ReactBatchedMode-test.internal.js b/packages/react-reconciler/src/__tests__/ReactBatchedMode-test.internal.js new file mode 100644 index 0000000000000..ad85a7a4bc980 --- /dev/null +++ b/packages/react-reconciler/src/__tests__/ReactBatchedMode-test.internal.js @@ -0,0 +1,58 @@ +let React; +let ReactFeatureFlags; +let ReactNoop; +let Scheduler; + +describe('ReactBatchedMode', () => { + beforeEach(() => { + jest.resetModules(); + ReactFeatureFlags = require('shared/ReactFeatureFlags'); + ReactFeatureFlags.debugRenderPhaseSideEffectsForStrictMode = false; + ReactFeatureFlags.replayFailedUnitOfWorkWithInvokeGuardedCallback = false; + React = require('react'); + ReactNoop = require('react-noop-renderer'); + Scheduler = require('scheduler'); + }); + + function Text(props) { + Scheduler.yieldValue(props.text); + return props.text; + } + + it('updates flush without yielding in the next event', () => { + const root = ReactNoop.createSyncRoot(); + + root.render( + + + + + , + ); + + // Nothing should have rendered yet + expect(root).toMatchRenderedOutput(null); + + // Everything should render immediately in the next event + expect(Scheduler).toFlushExpired(['A', 'B', 'C']); + expect(root).toMatchRenderedOutput('ABC'); + }); + + it('layout updates flush synchronously in same event', () => { + const {useLayoutEffect} = React; + + function App() { + useLayoutEffect(() => { + Scheduler.yieldValue('Layout effect'); + }); + return ; + } + + const root = ReactNoop.createSyncRoot(); + root.render(); + expect(root).toMatchRenderedOutput(null); + + expect(Scheduler).toFlushExpired(['Hi', 'Layout effect']); + expect(root).toMatchRenderedOutput('Hi'); + }); +}); diff --git a/packages/react-test-renderer/src/ReactTestRenderer.js b/packages/react-test-renderer/src/ReactTestRenderer.js index 20553ba821912..caa7670169490 100644 --- a/packages/react-test-renderer/src/ReactTestRenderer.js +++ b/packages/react-test-renderer/src/ReactTestRenderer.js @@ -437,8 +437,10 @@ const ReactTestRendererFiber = { createNodeMock, tag: 'CONTAINER', }; + const isBatched = false; let root: FiberRoot | null = createContainer( container, + isBatched, isConcurrent, false, ); From 944dd85f721660c951eb4327ea9726f0382c1411 Mon Sep 17 00:00:00 2001 From: Andrew Clark Date: Thu, 25 Apr 2019 14:23:18 -0700 Subject: [PATCH 2/4] Suspense in Batched Mode Should have same semantics as Concurrent Mode. --- .../src/ReactFiberBeginWork.js | 13 ++-- .../src/ReactFiberCompleteWork.js | 6 +- .../src/ReactFiberUnwindWork.js | 8 +-- .../ReactBatchedMode-test.internal.js | 64 +++++++++++++++++++ ...eactHooksWithNoopRenderer-test.internal.js | 2 +- .../ReactSuspenseFuzz-test.internal.js | 4 +- .../ReactSuspensePlaceholder-test.internal.js | 4 +- ...tSuspenseWithNoopRenderer-test.internal.js | 6 +- 8 files changed, 86 insertions(+), 21 deletions(-) diff --git a/packages/react-reconciler/src/ReactFiberBeginWork.js b/packages/react-reconciler/src/ReactFiberBeginWork.js index 94f5b0cbb0262..247bf7ce8739e 100644 --- a/packages/react-reconciler/src/ReactFiberBeginWork.js +++ b/packages/react-reconciler/src/ReactFiberBeginWork.js @@ -87,6 +87,7 @@ import { NoMode, ProfileMode, StrictMode, + BatchedMode, } from './ReactTypeOfMode'; import { shouldSetTextContent, @@ -1493,8 +1494,8 @@ function updateSuspenseComponent( null, ); - if ((workInProgress.mode & ConcurrentMode) === NoMode) { - // Outside of concurrent mode, we commit the effects from the + if ((workInProgress.mode & BatchedMode) === NoMode) { + // Outside of batched mode, we commit the effects from the // partially completed, timed-out tree, too. const progressedState: SuspenseState = workInProgress.memoizedState; const progressedPrimaryChild: Fiber | null = @@ -1546,8 +1547,8 @@ function updateSuspenseComponent( NoWork, ); - if ((workInProgress.mode & ConcurrentMode) === NoMode) { - // Outside of concurrent mode, we commit the effects from the + if ((workInProgress.mode & BatchedMode) === NoMode) { + // Outside of batched mode, we commit the effects from the // partially completed, timed-out tree, too. const progressedState: SuspenseState = workInProgress.memoizedState; const progressedPrimaryChild: Fiber | null = @@ -1629,8 +1630,8 @@ function updateSuspenseComponent( // schedule a placement. // primaryChildFragment.effectTag |= Placement; - if ((workInProgress.mode & ConcurrentMode) === NoMode) { - // Outside of concurrent mode, we commit the effects from the + if ((workInProgress.mode & BatchedMode) === NoMode) { + // Outside of batched mode, we commit the effects from the // partially completed, timed-out tree, too. const progressedState: SuspenseState = workInProgress.memoizedState; const progressedPrimaryChild: Fiber | null = diff --git a/packages/react-reconciler/src/ReactFiberCompleteWork.js b/packages/react-reconciler/src/ReactFiberCompleteWork.js index dbd0c7e1743f4..95ed3b9de78bd 100644 --- a/packages/react-reconciler/src/ReactFiberCompleteWork.js +++ b/packages/react-reconciler/src/ReactFiberCompleteWork.js @@ -43,7 +43,7 @@ import { EventComponent, EventTarget, } from 'shared/ReactWorkTags'; -import {ConcurrentMode, NoMode} from './ReactTypeOfMode'; +import {NoMode, BatchedMode} from './ReactTypeOfMode'; import { Placement, Ref, @@ -716,12 +716,12 @@ function completeWork( } if (nextDidTimeout && !prevDidTimeout) { - // If this subtreee is running in concurrent mode we can suspend, + // If this subtreee is running in batched mode we can suspend, // otherwise we won't suspend. // TODO: This will still suspend a synchronous tree if anything // in the concurrent tree already suspended during this render. // This is a known bug. - if ((workInProgress.mode & ConcurrentMode) !== NoMode) { + if ((workInProgress.mode & BatchedMode) !== NoMode) { renderDidSuspend(); } } diff --git a/packages/react-reconciler/src/ReactFiberUnwindWork.js b/packages/react-reconciler/src/ReactFiberUnwindWork.js index a39632acf08cb..1a84ba2af2350 100644 --- a/packages/react-reconciler/src/ReactFiberUnwindWork.js +++ b/packages/react-reconciler/src/ReactFiberUnwindWork.js @@ -41,7 +41,7 @@ import { enableSuspenseServerRenderer, enableEventAPI, } from 'shared/ReactFeatureFlags'; -import {ConcurrentMode, NoMode} from './ReactTypeOfMode'; +import {NoMode, BatchedMode} from './ReactTypeOfMode'; import {shouldCaptureSuspense} from './ReactFiberSuspenseComponent'; import {createCapturedValue} from './ReactCapturedValue'; @@ -223,15 +223,15 @@ function throwException( thenables.add(thenable); } - // If the boundary is outside of concurrent mode, we should *not* + // If the boundary is outside of batched mode, we should *not* // suspend the commit. Pretend as if the suspended component rendered // null and keep rendering. In the commit phase, we'll schedule a // subsequent synchronous update to re-render the Suspense. // // Note: It doesn't matter whether the component that suspended was - // inside a concurrent mode tree. If the Suspense is outside of it, we + // inside a batched mode tree. If the Suspense is outside of it, we // should *not* suspend the commit. - if ((workInProgress.mode & ConcurrentMode) === NoMode) { + if ((workInProgress.mode & BatchedMode) === NoMode) { workInProgress.effectTag |= DidCapture; // We're going to commit this fiber even though it didn't complete. diff --git a/packages/react-reconciler/src/__tests__/ReactBatchedMode-test.internal.js b/packages/react-reconciler/src/__tests__/ReactBatchedMode-test.internal.js index ad85a7a4bc980..b270033071228 100644 --- a/packages/react-reconciler/src/__tests__/ReactBatchedMode-test.internal.js +++ b/packages/react-reconciler/src/__tests__/ReactBatchedMode-test.internal.js @@ -2,6 +2,9 @@ let React; let ReactFeatureFlags; let ReactNoop; let Scheduler; +let ReactCache; +let Suspense; +let TextResource; describe('ReactBatchedMode', () => { beforeEach(() => { @@ -12,6 +15,17 @@ describe('ReactBatchedMode', () => { React = require('react'); ReactNoop = require('react-noop-renderer'); Scheduler = require('scheduler'); + ReactCache = require('react-cache'); + Suspense = React.Suspense; + + TextResource = ReactCache.unstable_createResource(([text, ms = 0]) => { + return new Promise((resolve, reject) => + setTimeout(() => { + Scheduler.yieldValue(`Promise resolved [${text}]`); + resolve(text); + }, ms), + ); + }, ([text, ms]) => text); }); function Text(props) { @@ -19,6 +33,22 @@ describe('ReactBatchedMode', () => { return props.text; } + function AsyncText(props) { + const text = props.text; + try { + TextResource.read([props.text, props.ms]); + Scheduler.yieldValue(text); + return props.text; + } catch (promise) { + if (typeof promise.then === 'function') { + Scheduler.yieldValue(`Suspend! [${text}]`); + } else { + Scheduler.yieldValue(`Error! [${text}]`); + } + throw promise; + } + } + it('updates flush without yielding in the next event', () => { const root = ReactNoop.createSyncRoot(); @@ -55,4 +85,38 @@ describe('ReactBatchedMode', () => { expect(Scheduler).toFlushExpired(['Hi', 'Layout effect']); expect(root).toMatchRenderedOutput('Hi'); }); + + it('uses proper Suspense semantics, not legacy ones', async () => { + const root = ReactNoop.createSyncRoot(); + root.render( + }> + + + + + + + + + + , + ); + + expect(Scheduler).toFlushExpired(['A', 'Suspend! [B]', 'C', 'Loading...']); + // In Legacy Mode, A and B would mount in a hidden primary tree. In Batched + // and Concurrent Mode, nothing in the primary tree should mount. But the + // fallback should mount immediately. + expect(root).toMatchRenderedOutput('Loading...'); + + await jest.advanceTimersByTime(1000); + expect(Scheduler).toHaveYielded(['Promise resolved [B]']); + expect(Scheduler).toFlushExpired(['A', 'B', 'C']); + expect(root).toMatchRenderedOutput( + + A + B + C + , + ); + }); }); diff --git a/packages/react-reconciler/src/__tests__/ReactHooksWithNoopRenderer-test.internal.js b/packages/react-reconciler/src/__tests__/ReactHooksWithNoopRenderer-test.internal.js index a1e12ca463750..83cfd07dc91a1 100644 --- a/packages/react-reconciler/src/__tests__/ReactHooksWithNoopRenderer-test.internal.js +++ b/packages/react-reconciler/src/__tests__/ReactHooksWithNoopRenderer-test.internal.js @@ -941,7 +941,7 @@ describe('ReactHooksWithNoopRenderer', () => { }); it( - 'in sync mode, useEffect is deferred and updates finish synchronously ' + + 'in legacy mode, useEffect is deferred and updates finish synchronously ' + '(in a single batch)', () => { function Counter(props) { diff --git a/packages/react-reconciler/src/__tests__/ReactSuspenseFuzz-test.internal.js b/packages/react-reconciler/src/__tests__/ReactSuspenseFuzz-test.internal.js index 5c0323a657494..4bbc8cc370528 100644 --- a/packages/react-reconciler/src/__tests__/ReactSuspenseFuzz-test.internal.js +++ b/packages/react-reconciler/src/__tests__/ReactSuspenseFuzz-test.internal.js @@ -175,8 +175,8 @@ describe('ReactSuspenseFuzz', () => { resetCache(); ReactNoop.renderLegacySyncRoot(children); resolveAllTasks(); - const syncOutput = ReactNoop.getChildrenAsJSX(); - expect(syncOutput).toEqual(expectedOutput); + const legacyOutput = ReactNoop.getChildrenAsJSX(); + expect(legacyOutput).toEqual(expectedOutput); ReactNoop.renderLegacySyncRoot(null); resetCache(); diff --git a/packages/react-reconciler/src/__tests__/ReactSuspensePlaceholder-test.internal.js b/packages/react-reconciler/src/__tests__/ReactSuspensePlaceholder-test.internal.js index 7060f427303a1..619ad456732af 100644 --- a/packages/react-reconciler/src/__tests__/ReactSuspensePlaceholder-test.internal.js +++ b/packages/react-reconciler/src/__tests__/ReactSuspensePlaceholder-test.internal.js @@ -303,7 +303,7 @@ describe('ReactSuspensePlaceholder', () => { }); describe('when suspending during mount', () => { - it('properly accounts for base durations when a suspended times out in a sync tree', () => { + it('properly accounts for base durations when a suspended times out in a legacy tree', () => { ReactNoop.renderLegacySyncRoot(); expect(Scheduler).toHaveYielded([ 'App', @@ -373,7 +373,7 @@ describe('ReactSuspensePlaceholder', () => { }); describe('when suspending during update', () => { - it('properly accounts for base durations when a suspended times out in a sync tree', () => { + it('properly accounts for base durations when a suspended times out in a legacy tree', () => { ReactNoop.renderLegacySyncRoot( , ); diff --git a/packages/react-reconciler/src/__tests__/ReactSuspenseWithNoopRenderer-test.internal.js b/packages/react-reconciler/src/__tests__/ReactSuspenseWithNoopRenderer-test.internal.js index 10764e607cc1d..390b43c7a1331 100644 --- a/packages/react-reconciler/src/__tests__/ReactSuspenseWithNoopRenderer-test.internal.js +++ b/packages/react-reconciler/src/__tests__/ReactSuspenseWithNoopRenderer-test.internal.js @@ -869,7 +869,7 @@ describe('ReactSuspenseWithNoopRenderer', () => { ); }); - describe('sync mode', () => { + describe('legacy mode mode', () => { it('times out immediately', async () => { function App() { return ( @@ -977,7 +977,7 @@ describe('ReactSuspenseWithNoopRenderer', () => { it( 'continues rendering asynchronously even if a promise is captured by ' + - 'a sync boundary (default mode)', + 'a sync boundary (legacy mode)', async () => { class UpdatingText extends React.Component { state = {text: this.props.initialText}; @@ -1109,7 +1109,7 @@ describe('ReactSuspenseWithNoopRenderer', () => { it( 'continues rendering asynchronously even if a promise is captured by ' + - 'a sync boundary (strict, non-concurrent)', + 'a sync boundary (strict, legacy)', async () => { class UpdatingText extends React.Component { state = {text: this.props.initialText}; From 4f5545b40c5acfdb9cd485e3dbf4625dbfa6b199 Mon Sep 17 00:00:00 2001 From: Andrew Clark Date: Mon, 29 Apr 2019 11:12:55 -0700 Subject: [PATCH 3/4] Use RootTag field to configure type of root There are three types of roots: Legacy, Batched, and Concurrent. --- packages/react-art/src/ReactART.js | 3 +- packages/react-dom/src/client/ReactDOM.js | 17 +++------ packages/react-dom/src/fire/ReactFire.js | 17 +++------ .../react-native-renderer/src/ReactFabric.js | 3 +- .../src/ReactNativeRenderer.js | 3 +- .../src/createReactNoop.js | 38 ++++--------------- packages/react-reconciler/src/ReactFiber.js | 11 +++--- .../src/ReactFiberReconciler.js | 6 +-- .../react-reconciler/src/ReactFiberRoot.js | 14 ++++--- .../react-reconciler/src/ReactTypeOfMode.js | 2 + ...=> ReactFiberHostContext-test.internal.js} | 14 ++++++- .../src/ReactTestRenderer.js | 5 +-- packages/shared/ReactRootTags.js | 14 +++++++ 13 files changed, 73 insertions(+), 74 deletions(-) rename packages/react-reconciler/src/__tests__/{ReactFiberHostContext-test.js => ReactFiberHostContext-test.internal.js} (90%) create mode 100644 packages/shared/ReactRootTags.js diff --git a/packages/react-art/src/ReactART.js b/packages/react-art/src/ReactART.js index 640a0448204bf..bf97e65031ffa 100644 --- a/packages/react-art/src/ReactART.js +++ b/packages/react-art/src/ReactART.js @@ -7,6 +7,7 @@ import React from 'react'; import ReactVersion from 'shared/ReactVersion'; +import {LegacyRoot} from 'shared/ReactRootTags'; import { createContainer, updateContainer, @@ -65,7 +66,7 @@ class Surface extends React.Component { this._surface = Mode.Surface(+width, +height, this._tagRef); - this._mountNode = createContainer(this._surface); + this._mountNode = createContainer(this._surface, LegacyRoot, false); updateContainer(this.props.children, this._mountNode, this); } diff --git a/packages/react-dom/src/client/ReactDOM.js b/packages/react-dom/src/client/ReactDOM.js index af726812f5326..bc74df6d8abb2 100644 --- a/packages/react-dom/src/client/ReactDOM.js +++ b/packages/react-dom/src/client/ReactDOM.js @@ -8,6 +8,7 @@ */ import type {ReactNodeList} from 'shared/ReactTypes'; +import type {RootTag} from 'shared/ReactRootTags'; // TODO: This type is shared between the reconciler and ReactDOM, but will // eventually be lifted out to the renderer. import type { @@ -52,6 +53,7 @@ import { accumulateTwoPhaseDispatches, accumulateDirectDispatches, } from 'events/EventPropagators'; +import {LegacyRoot, ConcurrentRoot} from 'shared/ReactRootTags'; import {has as hasInstance} from 'shared/ReactInstanceMap'; import ReactVersion from 'shared/ReactVersion'; import ReactSharedInternals from 'shared/ReactSharedInternals'; @@ -361,13 +363,8 @@ ReactWork.prototype._onCommit = function(): void { } }; -function ReactRoot( - container: DOMContainer, - isConcurrent: boolean, - hydrate: boolean, -) { - const isBatched = false; - const root = createContainer(container, isBatched, isConcurrent, hydrate); +function ReactRoot(container: DOMContainer, tag: RootTag, hydrate: boolean) { + const root = createContainer(container, tag, hydrate); this._internalRoot = root; } ReactRoot.prototype.render = function( @@ -532,9 +529,7 @@ function legacyCreateRootFromDOMContainer( ); } } - // Legacy roots are not async by default. - const isConcurrent = false; - return new ReactRoot(container, isConcurrent, shouldHydrate); + return new ReactRoot(container, LegacyRoot, shouldHydrate); } function legacyRenderSubtreeIntoContainer( @@ -850,7 +845,7 @@ function createRoot(container: DOMContainer, options?: RootOptions): ReactRoot { container._reactHasBeenPassedToCreateRootDEV = true; } const hydrate = options != null && options.hydrate === true; - return new ReactRoot(container, true, hydrate); + return new ReactRoot(container, ConcurrentRoot, hydrate); } if (enableStableConcurrentModeAPIs) { diff --git a/packages/react-dom/src/fire/ReactFire.js b/packages/react-dom/src/fire/ReactFire.js index 514fa5b44470b..e3af4444deee3 100644 --- a/packages/react-dom/src/fire/ReactFire.js +++ b/packages/react-dom/src/fire/ReactFire.js @@ -13,6 +13,7 @@ // console.log('Hello from Fire entry point.'); import type {ReactNodeList} from 'shared/ReactTypes'; +import type {RootTag} from 'shared/ReactRootTags'; // TODO: This type is shared between the reconciler and ReactDOM, but will // eventually be lifted out to the renderer. import type { @@ -58,6 +59,7 @@ import { accumulateTwoPhaseDispatches, accumulateDirectDispatches, } from 'events/EventPropagators'; +import {LegacyRoot, ConcurrentRoot} from 'shared/ReactRootTags'; import {has as hasInstance} from 'shared/ReactInstanceMap'; import ReactVersion from 'shared/ReactVersion'; import ReactSharedInternals from 'shared/ReactSharedInternals'; @@ -367,13 +369,8 @@ ReactWork.prototype._onCommit = function(): void { } }; -function ReactRoot( - container: DOMContainer, - isConcurrent: boolean, - hydrate: boolean, -) { - const isBatched = false; - const root = createContainer(container, isBatched, isConcurrent, hydrate); +function ReactRoot(container: DOMContainer, tag: RootTag, hydrate: boolean) { + const root = createContainer(container, tag, hydrate); this._internalRoot = root; } ReactRoot.prototype.render = function( @@ -538,9 +535,7 @@ function legacyCreateRootFromDOMContainer( ); } } - // Legacy roots are not async by default. - const isConcurrent = false; - return new ReactRoot(container, isConcurrent, shouldHydrate); + return new ReactRoot(container, LegacyRoot, shouldHydrate); } function legacyRenderSubtreeIntoContainer( @@ -856,7 +851,7 @@ function createRoot(container: DOMContainer, options?: RootOptions): ReactRoot { container._reactHasBeenPassedToCreateRootDEV = true; } const hydrate = options != null && options.hydrate === true; - return new ReactRoot(container, true, hydrate); + return new ReactRoot(container, ConcurrentRoot, hydrate); } if (enableStableConcurrentModeAPIs) { diff --git a/packages/react-native-renderer/src/ReactFabric.js b/packages/react-native-renderer/src/ReactFabric.js index fc883e079ac4a..0e4bbc51d9bab 100644 --- a/packages/react-native-renderer/src/ReactFabric.js +++ b/packages/react-native-renderer/src/ReactFabric.js @@ -33,6 +33,7 @@ import ReactNativeComponent from './ReactNativeComponent'; import {getClosestInstanceFromNode} from './ReactFabricComponentTree'; import {getInspectorDataForViewTag} from './ReactNativeFiberInspector'; +import {LegacyRoot} from 'shared/ReactRootTags'; import ReactSharedInternals from 'shared/ReactSharedInternals'; import getComponentName from 'shared/getComponentName'; import warningWithoutStack from 'shared/warningWithoutStack'; @@ -119,7 +120,7 @@ const ReactFabric: ReactFabricType = { if (!root) { // TODO (bvaughn): If we decide to keep the wrapper component, // We could create a wrapper for containerTag as well to reduce special casing. - root = createContainer(containerTag, false, false, false); + root = createContainer(containerTag, LegacyRoot, false); roots.set(containerTag, root); } updateContainer(element, root, null, callback); diff --git a/packages/react-native-renderer/src/ReactNativeRenderer.js b/packages/react-native-renderer/src/ReactNativeRenderer.js index 9189a50e3e18d..1c5bd87707427 100644 --- a/packages/react-native-renderer/src/ReactNativeRenderer.js +++ b/packages/react-native-renderer/src/ReactNativeRenderer.js @@ -40,6 +40,7 @@ import {getClosestInstanceFromNode} from './ReactNativeComponentTree'; import {getInspectorDataForViewTag} from './ReactNativeFiberInspector'; import {setNativeProps} from './ReactNativeRendererSharedExports'; +import {LegacyRoot} from 'shared/ReactRootTags'; import ReactSharedInternals from 'shared/ReactSharedInternals'; import getComponentName from 'shared/getComponentName'; import warningWithoutStack from 'shared/warningWithoutStack'; @@ -125,7 +126,7 @@ const ReactNativeRenderer: ReactNativeType = { if (!root) { // TODO (bvaughn): If we decide to keep the wrapper component, // We could create a wrapper for containerTag as well to reduce special casing. - root = createContainer(containerTag, false, false, false); + root = createContainer(containerTag, LegacyRoot, false); roots.set(containerTag, root); } updateContainer(element, root, null, callback); diff --git a/packages/react-noop-renderer/src/createReactNoop.js b/packages/react-noop-renderer/src/createReactNoop.js index 93310773bc05d..4cdb0fe4beb99 100644 --- a/packages/react-noop-renderer/src/createReactNoop.js +++ b/packages/react-noop-renderer/src/createReactNoop.js @@ -18,6 +18,7 @@ import type {Thenable} from 'react-reconciler/src/ReactFiberScheduler'; import type {Fiber} from 'react-reconciler/src/ReactFiber'; import type {UpdateQueue} from 'react-reconciler/src/ReactUpdateQueue'; import type {ReactNodeList} from 'shared/ReactTypes'; +import type {RootTag} from 'shared/ReactRootTags'; import * as Scheduler from 'scheduler/unstable_mock'; import {createPortal} from 'shared/ReactPortal'; @@ -32,6 +33,7 @@ import enqueueTask from 'shared/enqueueTask'; import ReactSharedInternals from 'shared/ReactSharedInternals'; import warningWithoutStack from 'shared/warningWithoutStack'; import {enableEventAPI} from 'shared/ReactFeatureFlags'; +import {ConcurrentRoot, BatchedRoot, LegacyRoot} from 'shared/ReactRootTags'; type EventTargetChildElement = { type: string, @@ -901,21 +903,12 @@ function createReactNoop(reconciler: Function, useMutation: boolean) { return getPendingChildren(container); }, - getOrCreateRootContainer( - rootID: string = DEFAULT_ROOT_ID, - isBatched: boolean, - isConcurrent: boolean, - ) { + getOrCreateRootContainer(rootID: string = DEFAULT_ROOT_ID, tag: RootTag) { let root = roots.get(rootID); if (!root) { const container = {rootID: rootID, pendingChildren: [], children: []}; rootContainers.set(rootID, container); - root = NoopRenderer.createContainer( - container, - isBatched, - isConcurrent, - false, - ); + root = NoopRenderer.createContainer(container, tag, false); roots.set(rootID, root); } return root.current.stateNode.containerInfo; @@ -923,8 +916,6 @@ function createReactNoop(reconciler: Function, useMutation: boolean) { // TODO: Replace ReactNoop.render with createRoot + root.render createRoot() { - const isBatched = true; - const isConcurrent = true; const container = { rootID: '' + idCounter++, pendingChildren: [], @@ -932,8 +923,7 @@ function createReactNoop(reconciler: Function, useMutation: boolean) { }; const fiberRoot = NoopRenderer.createContainer( container, - isBatched, - isConcurrent, + ConcurrentRoot, false, ); return { @@ -951,8 +941,6 @@ function createReactNoop(reconciler: Function, useMutation: boolean) { }, createSyncRoot() { - const isBatched = true; - const isConcurrent = false; const container = { rootID: '' + idCounter++, pendingChildren: [], @@ -960,8 +948,7 @@ function createReactNoop(reconciler: Function, useMutation: boolean) { }; const fiberRoot = NoopRenderer.createContainer( container, - isBatched, - isConcurrent, + BatchedRoot, false, ); return { @@ -1003,13 +990,7 @@ function createReactNoop(reconciler: Function, useMutation: boolean) { renderLegacySyncRoot(element: React$Element, callback: ?Function) { const rootID = DEFAULT_ROOT_ID; - const isBatched = false; - const isConcurrent = false; - const container = ReactNoop.getOrCreateRootContainer( - rootID, - isBatched, - isConcurrent, - ); + const container = ReactNoop.getOrCreateRootContainer(rootID, LegacyRoot); const root = roots.get(container.rootID); NoopRenderer.updateContainer(element, root, null, callback); }, @@ -1019,12 +1000,9 @@ function createReactNoop(reconciler: Function, useMutation: boolean) { rootID: string, callback: ?Function, ) { - const isBatched = true; - const isConcurrent = true; const container = ReactNoop.getOrCreateRootContainer( rootID, - isBatched, - isConcurrent, + ConcurrentRoot, ); const root = roots.get(container.rootID); NoopRenderer.updateContainer(element, root, null, callback); diff --git a/packages/react-reconciler/src/ReactFiber.js b/packages/react-reconciler/src/ReactFiber.js index 38fafa1cd1ddd..1330887c14c5c 100644 --- a/packages/react-reconciler/src/ReactFiber.js +++ b/packages/react-reconciler/src/ReactFiber.js @@ -15,6 +15,7 @@ import type { ReactEventComponent, ReactEventTarget, } from 'shared/ReactTypes'; +import type {RootTag} from 'shared/ReactRootTags'; import type {WorkTag} from 'shared/ReactWorkTags'; import type {TypeOfMode} from './ReactTypeOfMode'; import type {SideEffectTag} from 'shared/ReactSideEffectTags'; @@ -27,6 +28,7 @@ import invariant from 'shared/invariant'; import warningWithoutStack from 'shared/warningWithoutStack'; import {enableProfilerTimer, enableEventAPI} from 'shared/ReactFeatureFlags'; import {NoEffect} from 'shared/ReactSideEffectTags'; +import {ConcurrentRoot, BatchedRoot} from 'shared/ReactRootTags'; import { IndeterminateComponent, ClassComponent, @@ -435,14 +437,11 @@ export function createWorkInProgress( return workInProgress; } -export function createHostRootFiber( - isBatched: boolean, - isConcurrent: boolean, -): Fiber { +export function createHostRootFiber(tag: RootTag): Fiber { let mode; - if (isConcurrent) { + if (tag === ConcurrentRoot) { mode = ConcurrentMode | BatchedMode | StrictMode; - } else if (isBatched) { + } else if (tag === BatchedRoot) { mode = BatchedMode | StrictMode; } else { mode = NoMode; diff --git a/packages/react-reconciler/src/ReactFiberReconciler.js b/packages/react-reconciler/src/ReactFiberReconciler.js index 0ac8890390635..3e1be7241e9d4 100644 --- a/packages/react-reconciler/src/ReactFiberReconciler.js +++ b/packages/react-reconciler/src/ReactFiberReconciler.js @@ -9,6 +9,7 @@ import type {Fiber} from './ReactFiber'; import type {FiberRoot} from './ReactFiberRoot'; +import type {RootTag} from 'shared/ReactRootTags'; import type { Instance, TextInstance, @@ -273,11 +274,10 @@ function findHostInstanceWithWarning( export function createContainer( containerInfo: Container, - isBatched: boolean, - isConcurrent: boolean, + tag: RootTag, hydrate: boolean, ): OpaqueRoot { - return createFiberRoot(containerInfo, isBatched, isConcurrent, hydrate); + return createFiberRoot(containerInfo, tag, hydrate); } export function updateContainer( diff --git a/packages/react-reconciler/src/ReactFiberRoot.js b/packages/react-reconciler/src/ReactFiberRoot.js index 9329194459559..426a55c81f01f 100644 --- a/packages/react-reconciler/src/ReactFiberRoot.js +++ b/packages/react-reconciler/src/ReactFiberRoot.js @@ -9,6 +9,7 @@ import type {Fiber} from './ReactFiber'; import type {ExpirationTime} from './ReactFiberExpirationTime'; +import type {RootTag} from 'shared/ReactRootTags'; import type {TimeoutHandle, NoTimeout} from './ReactFiberHostConfig'; import type {Thenable} from './ReactFiberScheduler'; import type {Interaction} from 'scheduler/src/Tracing'; @@ -30,6 +31,9 @@ export type Batch = { export type PendingInteractionMap = Map>; type BaseFiberRootProperties = {| + // The type of root (legacy, batched, concurrent, etc.) + tag: RootTag, + // Any additional information from the host associated with this root. containerInfo: any, // Used only by persistent updates. @@ -89,7 +93,8 @@ export type FiberRoot = { ...ProfilingOnlyFiberRootProperties, }; -function FiberRootNode(containerInfo, hydrate) { +function FiberRootNode(containerInfo, tag, hydrate) { + this.tag = tag; this.current = null; this.containerInfo = containerInfo; this.pendingChildren = null; @@ -116,15 +121,14 @@ function FiberRootNode(containerInfo, hydrate) { export function createFiberRoot( containerInfo: any, - isBatched: boolean, - isConcurrent: boolean, + tag: RootTag, hydrate: boolean, ): FiberRoot { - const root: FiberRoot = (new FiberRootNode(containerInfo, hydrate): any); + const root: FiberRoot = (new FiberRootNode(containerInfo, tag, hydrate): any); // Cyclic construction. This cheats the type system right now because // stateNode is any. - const uninitializedFiber = createHostRootFiber(isBatched, isConcurrent); + const uninitializedFiber = createHostRootFiber(tag); root.current = uninitializedFiber; uninitializedFiber.stateNode = root; diff --git a/packages/react-reconciler/src/ReactTypeOfMode.js b/packages/react-reconciler/src/ReactTypeOfMode.js index a097830ae89e6..2aba0d9d23cb1 100644 --- a/packages/react-reconciler/src/ReactTypeOfMode.js +++ b/packages/react-reconciler/src/ReactTypeOfMode.js @@ -11,6 +11,8 @@ export type TypeOfMode = number; export const NoMode = 0b0000; export const StrictMode = 0b0001; +// TODO: Remove BatchedMode and ConcurrentMode by reading from the root +// tag instead export const BatchedMode = 0b0010; export const ConcurrentMode = 0b0100; export const ProfileMode = 0b1000; diff --git a/packages/react-reconciler/src/__tests__/ReactFiberHostContext-test.js b/packages/react-reconciler/src/__tests__/ReactFiberHostContext-test.internal.js similarity index 90% rename from packages/react-reconciler/src/__tests__/ReactFiberHostContext-test.js rename to packages/react-reconciler/src/__tests__/ReactFiberHostContext-test.internal.js index 8ffa524bd94d2..1315f9b1b38a0 100644 --- a/packages/react-reconciler/src/__tests__/ReactFiberHostContext-test.js +++ b/packages/react-reconciler/src/__tests__/ReactFiberHostContext-test.internal.js @@ -12,12 +12,14 @@ let React; let ReactFiberReconciler; +let ConcurrentRoot; describe('ReactFiberHostContext', () => { beforeEach(() => { jest.resetModules(); React = require('react'); ReactFiberReconciler = require('react-reconciler'); + ConcurrentRoot = require('shared/ReactRootTags'); }); it('works with null host context', () => { @@ -58,7 +60,11 @@ describe('ReactFiberHostContext', () => { supportsMutation: true, }); - const container = Renderer.createContainer(/* root: */ null); + const container = Renderer.createContainer( + /* root: */ null, + ConcurrentRoot, + false, + ); Renderer.updateContainer( @@ -106,7 +112,11 @@ describe('ReactFiberHostContext', () => { supportsMutation: true, }); - const container = Renderer.createContainer(rootContext); + const container = Renderer.createContainer( + rootContext, + ConcurrentRoot, + false, + ); Renderer.updateContainer( diff --git a/packages/react-test-renderer/src/ReactTestRenderer.js b/packages/react-test-renderer/src/ReactTestRenderer.js index caa7670169490..11b6fed109697 100644 --- a/packages/react-test-renderer/src/ReactTestRenderer.js +++ b/packages/react-test-renderer/src/ReactTestRenderer.js @@ -43,6 +43,7 @@ import ReactVersion from 'shared/ReactVersion'; import act from './ReactTestRendererAct'; import {getPublicInstance} from './ReactTestHostConfig'; +import {ConcurrentRoot, LegacyRoot} from 'shared/ReactRootTags'; type TestRendererOptions = { createNodeMock: (element: React$Element) => any, @@ -437,11 +438,9 @@ const ReactTestRendererFiber = { createNodeMock, tag: 'CONTAINER', }; - const isBatched = false; let root: FiberRoot | null = createContainer( container, - isBatched, - isConcurrent, + isConcurrent ? ConcurrentRoot : LegacyRoot, false, ); invariant(root != null, 'something went wrong'); diff --git a/packages/shared/ReactRootTags.js b/packages/shared/ReactRootTags.js new file mode 100644 index 0000000000000..44784b928ddd3 --- /dev/null +++ b/packages/shared/ReactRootTags.js @@ -0,0 +1,14 @@ +/** + * 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 + */ + +export type RootTag = 0 | 1 | 2; + +export const LegacyRoot = 0; +export const BatchedRoot = 1; +export const ConcurrentRoot = 2; From e4fdd20791baf256a0cd87f1d6e820508dd3b99a Mon Sep 17 00:00:00 2001 From: Andrew Clark Date: Mon, 29 Apr 2019 12:59:12 -0700 Subject: [PATCH 4/4] flushSync should not flush batched work Treat Sync and Batched expiration times separately. Only Sync updates are pushed to our internal queue of synchronous callbacks. Renamed `flushImmediateQueue` to `flushSyncCallbackQueue` for clarity. --- .../src/ReactFiberScheduler.js | 101 +++++++++--------- .../src/SchedulerWithReactIntegration.js | 64 +++++------ .../ReactBatchedMode-test.internal.js | 44 ++++++++ 3 files changed, 128 insertions(+), 81 deletions(-) diff --git a/packages/react-reconciler/src/ReactFiberScheduler.js b/packages/react-reconciler/src/ReactFiberScheduler.js index a5b807cde9f3c..cf44fc549101d 100644 --- a/packages/react-reconciler/src/ReactFiberScheduler.js +++ b/packages/react-reconciler/src/ReactFiberScheduler.js @@ -41,7 +41,8 @@ import { NormalPriority, LowPriority, IdlePriority, - flushImmediateQueue, + flushSyncCallbackQueue, + scheduleSyncCallback, } from './SchedulerWithReactIntegration'; import {__interactionsRef, __subscriberRef} from 'scheduler/tracing'; @@ -357,7 +358,7 @@ export function scheduleUpdateOnFiber( // scheduleCallbackForFiber to preserve the ability to schedule a callback // without immediately flushing it. We only do this for user-initated // updates, to preserve historical behavior of sync mode. - flushImmediateQueue(); + flushSyncCallbackQueue(); } } } else { @@ -464,36 +465,47 @@ function scheduleCallbackForRoot( } root.callbackExpirationTime = expirationTime; - let options = null; - if (expirationTime !== Sync && expirationTime !== Never) { - let timeout = expirationTimeToMs(expirationTime) - now(); - if (timeout > 5000) { - // Sanity check. Should never take longer than 5 seconds. - // TODO: Add internal warning? - timeout = 5000; + if (expirationTime === Sync) { + // Sync React callbacks are scheduled on a special internal queue + root.callbackNode = scheduleSyncCallback( + runRootCallback.bind( + null, + root, + renderRoot.bind(null, root, expirationTime), + ), + ); + } else { + let options = null; + if (expirationTime !== Sync && expirationTime !== Never) { + let timeout = expirationTimeToMs(expirationTime) - now(); + if (timeout > 5000) { + // Sanity check. Should never take longer than 5 seconds. + // TODO: Add internal warning? + timeout = 5000; + } + options = {timeout}; } - options = {timeout}; - } - root.callbackNode = scheduleCallback( - priorityLevel, - runRootCallback.bind( - null, - root, - renderRoot.bind(null, root, expirationTime), - ), - options, - ); - if ( - enableUserTimingAPI && - expirationTime !== Sync && - workPhase !== RenderPhase && - workPhase !== CommitPhase - ) { - // Scheduled an async callback, and we're not already working. Add an - // entry to the flamegraph that shows we're waiting for a callback - // to fire. - startRequestCallbackTimer(); + root.callbackNode = scheduleCallback( + priorityLevel, + runRootCallback.bind( + null, + root, + renderRoot.bind(null, root, expirationTime), + ), + options, + ); + if ( + enableUserTimingAPI && + expirationTime !== Sync && + workPhase !== RenderPhase && + workPhase !== CommitPhase + ) { + // Scheduled an async callback, and we're not already working. Add an + // entry to the flamegraph that shows we're waiting for a callback + // to fire. + startRequestCallbackTimer(); + } } } @@ -532,11 +544,8 @@ export function flushRoot(root: FiberRoot, expirationTime: ExpirationTime) { 'means you attempted to commit from inside a lifecycle method.', ); } - scheduleCallback( - ImmediatePriority, - renderRoot.bind(null, root, expirationTime), - ); - flushImmediateQueue(); + scheduleSyncCallback(renderRoot.bind(null, root, expirationTime)); + flushSyncCallbackQueue(); } export function flushInteractiveUpdates() { @@ -604,13 +613,10 @@ function flushPendingDiscreteUpdates() { const roots = rootsWithPendingDiscreteUpdates; rootsWithPendingDiscreteUpdates = null; roots.forEach((expirationTime, root) => { - scheduleCallback( - ImmediatePriority, - renderRoot.bind(null, root, expirationTime), - ); + scheduleSyncCallback(renderRoot.bind(null, root, expirationTime)); }); // Now flush the immediate queue. - flushImmediateQueue(); + flushSyncCallbackQueue(); } } @@ -625,7 +631,7 @@ export function batchedUpdates(fn: A => R, a: A): R { } finally { workPhase = NotWorking; // Flush the immediate callbacks that were scheduled during this batch - flushImmediateQueue(); + flushSyncCallbackQueue(); } } @@ -661,7 +667,7 @@ export function flushSync(fn: A => R, a: A): R { // Flush the immediate callbacks that were scheduled during this batch. // Note that this will happen even if batchedUpdates is higher up // the stack. - flushImmediateQueue(); + flushSyncCallbackQueue(); } } @@ -674,7 +680,7 @@ export function flushControlled(fn: () => mixed): void { workPhase = prevWorkPhase; if (workPhase === NotWorking) { // Flush the immediate callbacks that were scheduled during this batch - flushImmediateQueue(); + flushSyncCallbackQueue(); } } } @@ -890,10 +896,7 @@ function renderRoot( // caused by tearing due to a mutation during an event. Try rendering // one more time without yiedling to events. prepareFreshStack(root, expirationTime); - scheduleCallback( - ImmediatePriority, - renderRoot.bind(null, root, expirationTime), - ); + scheduleSyncCallback(renderRoot.bind(null, root, expirationTime)); return null; } // If we're already rendering synchronously, commit the root in its @@ -1490,7 +1493,7 @@ function commitRootImpl(root, expirationTime) { } // If layout work was scheduled, flush it now. - flushImmediateQueue(); + flushSyncCallbackQueue(); return null; } @@ -1661,7 +1664,7 @@ export function flushPassiveEffects() { } workPhase = prevWorkPhase; - flushImmediateQueue(); + flushSyncCallbackQueue(); // If additional passive effects were scheduled, increment a counter. If this // exceeds the limit, we'll fire a warning. diff --git a/packages/react-reconciler/src/SchedulerWithReactIntegration.js b/packages/react-reconciler/src/SchedulerWithReactIntegration.js index 0463ab03b8ec3..7482b7fa9a602 100644 --- a/packages/react-reconciler/src/SchedulerWithReactIntegration.js +++ b/packages/react-reconciler/src/SchedulerWithReactIntegration.js @@ -69,9 +69,9 @@ export const shouldYield = disableYielding ? () => false // Never yield when `disableYielding` is on : Scheduler_shouldYield; -let immediateQueue: Array | null = null; +let syncQueue: Array | null = null; let immediateQueueCallbackNode: mixed | null = null; -let isFlushingImmediate: boolean = false; +let isFlushingSyncQueue: boolean = false; let initialTimeMs: number = Scheduler_now(); // If the initial timestamp is reasonably small, use Scheduler's `now` directly. @@ -131,68 +131,68 @@ export function scheduleCallback( callback: SchedulerCallback, options: SchedulerCallbackOptions | void | null, ) { - if (reactPriorityLevel === ImmediatePriority) { - // Push this callback into an internal queue. We'll flush these either in - // the next tick, or earlier if something calls `flushImmediateQueue`. - if (immediateQueue === null) { - immediateQueue = [callback]; - // Flush the queue in the next tick, at the earliest. - immediateQueueCallbackNode = Scheduler_scheduleCallback( - Scheduler_ImmediatePriority, - flushImmediateQueueImpl, - ); - } else { - // Push onto existing queue. Don't need to schedule a callback because - // we already scheduled one when we created the queue. - immediateQueue.push(callback); - } - return fakeCallbackNode; - } - // Otherwise pass through to Scheduler. const priorityLevel = reactPriorityToSchedulerPriority(reactPriorityLevel); return Scheduler_scheduleCallback(priorityLevel, callback, options); } +export function scheduleSyncCallback(callback: SchedulerCallback) { + // Push this callback into an internal queue. We'll flush these either in + // the next tick, or earlier if something calls `flushSyncCallbackQueue`. + if (syncQueue === null) { + syncQueue = [callback]; + // Flush the queue in the next tick, at the earliest. + immediateQueueCallbackNode = Scheduler_scheduleCallback( + Scheduler_ImmediatePriority, + flushSyncCallbackQueueImpl, + ); + } else { + // Push onto existing queue. Don't need to schedule a callback because + // we already scheduled one when we created the queue. + syncQueue.push(callback); + } + return fakeCallbackNode; +} + export function cancelCallback(callbackNode: mixed) { if (callbackNode !== fakeCallbackNode) { Scheduler_cancelCallback(callbackNode); } } -export function flushImmediateQueue() { +export function flushSyncCallbackQueue() { if (immediateQueueCallbackNode !== null) { Scheduler_cancelCallback(immediateQueueCallbackNode); } - flushImmediateQueueImpl(); + flushSyncCallbackQueueImpl(); } -function flushImmediateQueueImpl() { - if (!isFlushingImmediate && immediateQueue !== null) { +function flushSyncCallbackQueueImpl() { + if (!isFlushingSyncQueue && syncQueue !== null) { // Prevent re-entrancy. - isFlushingImmediate = true; + isFlushingSyncQueue = true; let i = 0; try { const isSync = true; - for (; i < immediateQueue.length; i++) { - let callback = immediateQueue[i]; + for (; i < syncQueue.length; i++) { + let callback = syncQueue[i]; do { callback = callback(isSync); } while (callback !== null); } - immediateQueue = null; + syncQueue = null; } catch (error) { // If something throws, leave the remaining callbacks on the queue. - if (immediateQueue !== null) { - immediateQueue = immediateQueue.slice(i + 1); + if (syncQueue !== null) { + syncQueue = syncQueue.slice(i + 1); } // Resume flushing in the next tick Scheduler_scheduleCallback( Scheduler_ImmediatePriority, - flushImmediateQueue, + flushSyncCallbackQueue, ); throw error; } finally { - isFlushingImmediate = false; + isFlushingSyncQueue = false; } } } diff --git a/packages/react-reconciler/src/__tests__/ReactBatchedMode-test.internal.js b/packages/react-reconciler/src/__tests__/ReactBatchedMode-test.internal.js index b270033071228..670b0e49fb1bc 100644 --- a/packages/react-reconciler/src/__tests__/ReactBatchedMode-test.internal.js +++ b/packages/react-reconciler/src/__tests__/ReactBatchedMode-test.internal.js @@ -1,6 +1,7 @@ let React; let ReactFeatureFlags; let ReactNoop; +let act; let Scheduler; let ReactCache; let Suspense; @@ -14,6 +15,7 @@ describe('ReactBatchedMode', () => { ReactFeatureFlags.replayFailedUnitOfWorkWithInvokeGuardedCallback = false; React = require('react'); ReactNoop = require('react-noop-renderer'); + act = ReactNoop.act; Scheduler = require('scheduler'); ReactCache = require('react-cache'); Suspense = React.Suspense; @@ -119,4 +121,46 @@ describe('ReactBatchedMode', () => { , ); }); + + it('flushSync does not flush batched work', () => { + const {useState, forwardRef, useImperativeHandle} = React; + const root = ReactNoop.createSyncRoot(); + + const Foo = forwardRef(({label}, ref) => { + const [step, setStep] = useState(0); + useImperativeHandle(ref, () => ({setStep})); + return ; + }); + + const foo1 = React.createRef(null); + const foo2 = React.createRef(null); + root.render( + + + + , + ); + + // Mount + expect(Scheduler).toFlushExpired(['A0', 'B0']); + expect(root).toMatchRenderedOutput('A0B0'); + + // Schedule a batched update to the first sibling + act(() => foo1.current.setStep(1)); + + // Before it flushes, update the second sibling inside flushSync + act(() => + ReactNoop.flushSync(() => { + foo2.current.setStep(1); + }), + ); + + // Only the second update should have flushed synchronously + expect(Scheduler).toHaveYielded(['B1']); + expect(root).toMatchRenderedOutput('A0B1'); + + // Now flush the first update + expect(Scheduler).toFlushExpired(['A1']); + expect(root).toMatchRenderedOutput('A1B1'); + }); });