From b48b38af68c27fd401fe4b923a8fa0b229693cd4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sebastian=20Markb=C3=A5ge?= Date: Wed, 31 Mar 2021 11:22:49 -0400 Subject: [PATCH] Support nesting of startTransition and flushSync (alt) (#21149) * Support nesting of startTransition and flushSync * Unset transition before entering any special execution contexts Co-authored-by: Andrew Clark --- .../src/events/ReactDOMEventListener.js | 6 +++ .../src/ReactFiberWorkLoop.new.js | 22 ++++++++++ .../src/ReactFiberWorkLoop.old.js | 22 ++++++++++ .../src/__tests__/ReactFlushSync-test.js | 43 +++++++++++++++++++ 4 files changed, 93 insertions(+) diff --git a/packages/react-dom/src/events/ReactDOMEventListener.js b/packages/react-dom/src/events/ReactDOMEventListener.js index 3f00dcf305b3..f09d6ee86448 100644 --- a/packages/react-dom/src/events/ReactDOMEventListener.js +++ b/packages/react-dom/src/events/ReactDOMEventListener.js @@ -57,6 +57,9 @@ import { getCurrentUpdatePriority, setCurrentUpdatePriority, } from 'react-reconciler/src/ReactEventPriorities'; +import ReactSharedInternals from 'shared/ReactSharedInternals'; + +const {ReactCurrentBatchConfig} = ReactSharedInternals; // TODO: can we stop exporting these? export let _enabled = true; @@ -141,11 +144,14 @@ function dispatchContinuousEvent( nativeEvent, ) { const previousPriority = getCurrentUpdatePriority(); + const prevTransition = ReactCurrentBatchConfig.transition; + ReactCurrentBatchConfig.transition = 0; try { setCurrentUpdatePriority(ContinuousEventPriority); dispatchEvent(domEventName, eventSystemFlags, container, nativeEvent); } finally { setCurrentUpdatePriority(previousPriority); + ReactCurrentBatchConfig.transition = prevTransition; } } diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.new.js b/packages/react-reconciler/src/ReactFiberWorkLoop.new.js index 1cff8db1fe65..37fa324f5a1a 100644 --- a/packages/react-reconciler/src/ReactFiberWorkLoop.new.js +++ b/packages/react-reconciler/src/ReactFiberWorkLoop.new.js @@ -240,6 +240,7 @@ const ceil = Math.ceil; const { ReactCurrentDispatcher, ReactCurrentOwner, + ReactCurrentBatchConfig, IsSomeRendererActing, } = ReactSharedInternals; @@ -1072,11 +1073,14 @@ export function flushDiscreteUpdates() { export function deferredUpdates(fn: () => A): A { const previousPriority = getCurrentUpdatePriority(); + const prevTransition = ReactCurrentBatchConfig.transition; try { + ReactCurrentBatchConfig.transition = 0; setCurrentUpdatePriority(DefaultEventPriority); return fn(); } finally { setCurrentUpdatePriority(previousPriority); + ReactCurrentBatchConfig.transition = prevTransition; } } @@ -1118,11 +1122,14 @@ export function discreteUpdates( d: D, ): R { const previousPriority = getCurrentUpdatePriority(); + const prevTransition = ReactCurrentBatchConfig.transition; try { + ReactCurrentBatchConfig.transition = 0; setCurrentUpdatePriority(DiscreteEventPriority); return fn(a, b, c, d); } finally { setCurrentUpdatePriority(previousPriority); + ReactCurrentBatchConfig.transition = prevTransition; if (executionContext === NoContext) { // Flush the immediate callbacks that were scheduled during this batch resetRenderTimer(); @@ -1151,8 +1158,10 @@ export function flushSync(fn: A => R, a: A): R { const prevExecutionContext = executionContext; executionContext |= BatchedContext; + const prevTransition = ReactCurrentBatchConfig.transition; const previousPriority = getCurrentUpdatePriority(); try { + ReactCurrentBatchConfig.transition = 0; setCurrentUpdatePriority(DiscreteEventPriority); if (fn) { return fn(a); @@ -1161,6 +1170,7 @@ export function flushSync(fn: A => R, a: A): R { } } finally { setCurrentUpdatePriority(previousPriority); + ReactCurrentBatchConfig.transition = prevTransition; executionContext = prevExecutionContext; // Flush the immediate callbacks that were scheduled during this batch. // Note that this will happen even if batchedUpdates is higher up @@ -1182,12 +1192,15 @@ export function flushSync(fn: A => R, a: A): R { export function flushControlled(fn: () => mixed): void { const prevExecutionContext = executionContext; executionContext |= BatchedContext; + const prevTransition = ReactCurrentBatchConfig.transition; const previousPriority = getCurrentUpdatePriority(); try { + ReactCurrentBatchConfig.transition = 0; setCurrentUpdatePriority(DiscreteEventPriority); fn(); } finally { setCurrentUpdatePriority(previousPriority); + ReactCurrentBatchConfig.transition = prevTransition; executionContext = prevExecutionContext; if (executionContext === NoContext) { @@ -1681,10 +1694,13 @@ function commitRoot(root) { // TODO: This no longer makes any sense. We already wrap the mutation and // layout phases. Should be able to remove. const previousUpdateLanePriority = getCurrentUpdatePriority(); + const prevTransition = ReactCurrentBatchConfig.transition; try { + ReactCurrentBatchConfig.transition = 0; setCurrentUpdatePriority(DiscreteEventPriority); commitRootImpl(root, previousUpdateLanePriority); } finally { + ReactCurrentBatchConfig.transition = prevTransition; setCurrentUpdatePriority(previousUpdateLanePriority); } @@ -1797,6 +1813,8 @@ function commitRootImpl(root, renderPriorityLevel) { NoFlags; if (subtreeHasEffects || rootHasEffect) { + const prevTransition = ReactCurrentBatchConfig.transition; + ReactCurrentBatchConfig.transition = 0; const previousPriority = getCurrentUpdatePriority(); setCurrentUpdatePriority(DiscreteEventPriority); @@ -1882,6 +1900,7 @@ function commitRootImpl(root, renderPriorityLevel) { // Reset the priority to the previous non-sync value. setCurrentUpdatePriority(previousPriority); + ReactCurrentBatchConfig.transition = prevTransition; } else { // No effects. root.current = finishedWork; @@ -2019,12 +2038,15 @@ export function flushPassiveEffects(): boolean { DefaultEventPriority, lanesToEventPriority(pendingPassiveEffectsLanes), ); + const prevTransition = ReactCurrentBatchConfig.transition; const previousPriority = getCurrentUpdatePriority(); try { + ReactCurrentBatchConfig.transition = 0; setCurrentUpdatePriority(priority); return flushPassiveEffectsImpl(); } finally { setCurrentUpdatePriority(previousPriority); + ReactCurrentBatchConfig.transition = prevTransition; } } return false; diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.old.js b/packages/react-reconciler/src/ReactFiberWorkLoop.old.js index 98f4d119e408..f5102b96fe8b 100644 --- a/packages/react-reconciler/src/ReactFiberWorkLoop.old.js +++ b/packages/react-reconciler/src/ReactFiberWorkLoop.old.js @@ -240,6 +240,7 @@ const ceil = Math.ceil; const { ReactCurrentDispatcher, ReactCurrentOwner, + ReactCurrentBatchConfig, IsSomeRendererActing, } = ReactSharedInternals; @@ -1072,11 +1073,14 @@ export function flushDiscreteUpdates() { export function deferredUpdates(fn: () => A): A { const previousPriority = getCurrentUpdatePriority(); + const prevTransition = ReactCurrentBatchConfig.transition; try { + ReactCurrentBatchConfig.transition = 0; setCurrentUpdatePriority(DefaultEventPriority); return fn(); } finally { setCurrentUpdatePriority(previousPriority); + ReactCurrentBatchConfig.transition = prevTransition; } } @@ -1118,11 +1122,14 @@ export function discreteUpdates( d: D, ): R { const previousPriority = getCurrentUpdatePriority(); + const prevTransition = ReactCurrentBatchConfig.transition; try { + ReactCurrentBatchConfig.transition = 0; setCurrentUpdatePriority(DiscreteEventPriority); return fn(a, b, c, d); } finally { setCurrentUpdatePriority(previousPriority); + ReactCurrentBatchConfig.transition = prevTransition; if (executionContext === NoContext) { // Flush the immediate callbacks that were scheduled during this batch resetRenderTimer(); @@ -1151,8 +1158,10 @@ export function flushSync(fn: A => R, a: A): R { const prevExecutionContext = executionContext; executionContext |= BatchedContext; + const prevTransition = ReactCurrentBatchConfig.transition; const previousPriority = getCurrentUpdatePriority(); try { + ReactCurrentBatchConfig.transition = 0; setCurrentUpdatePriority(DiscreteEventPriority); if (fn) { return fn(a); @@ -1161,6 +1170,7 @@ export function flushSync(fn: A => R, a: A): R { } } finally { setCurrentUpdatePriority(previousPriority); + ReactCurrentBatchConfig.transition = prevTransition; executionContext = prevExecutionContext; // Flush the immediate callbacks that were scheduled during this batch. // Note that this will happen even if batchedUpdates is higher up @@ -1182,12 +1192,15 @@ export function flushSync(fn: A => R, a: A): R { export function flushControlled(fn: () => mixed): void { const prevExecutionContext = executionContext; executionContext |= BatchedContext; + const prevTransition = ReactCurrentBatchConfig.transition; const previousPriority = getCurrentUpdatePriority(); try { + ReactCurrentBatchConfig.transition = 0; setCurrentUpdatePriority(DiscreteEventPriority); fn(); } finally { setCurrentUpdatePriority(previousPriority); + ReactCurrentBatchConfig.transition = prevTransition; executionContext = prevExecutionContext; if (executionContext === NoContext) { @@ -1681,10 +1694,13 @@ function commitRoot(root) { // TODO: This no longer makes any sense. We already wrap the mutation and // layout phases. Should be able to remove. const previousUpdateLanePriority = getCurrentUpdatePriority(); + const prevTransition = ReactCurrentBatchConfig.transition; try { + ReactCurrentBatchConfig.transition = 0; setCurrentUpdatePriority(DiscreteEventPriority); commitRootImpl(root, previousUpdateLanePriority); } finally { + ReactCurrentBatchConfig.transition = prevTransition; setCurrentUpdatePriority(previousUpdateLanePriority); } @@ -1797,6 +1813,8 @@ function commitRootImpl(root, renderPriorityLevel) { NoFlags; if (subtreeHasEffects || rootHasEffect) { + const prevTransition = ReactCurrentBatchConfig.transition; + ReactCurrentBatchConfig.transition = 0; const previousPriority = getCurrentUpdatePriority(); setCurrentUpdatePriority(DiscreteEventPriority); @@ -1882,6 +1900,7 @@ function commitRootImpl(root, renderPriorityLevel) { // Reset the priority to the previous non-sync value. setCurrentUpdatePriority(previousPriority); + ReactCurrentBatchConfig.transition = prevTransition; } else { // No effects. root.current = finishedWork; @@ -2019,12 +2038,15 @@ export function flushPassiveEffects(): boolean { DefaultEventPriority, lanesToEventPriority(pendingPassiveEffectsLanes), ); + const prevTransition = ReactCurrentBatchConfig.transition; const previousPriority = getCurrentUpdatePriority(); try { + ReactCurrentBatchConfig.transition = 0; setCurrentUpdatePriority(priority); return flushPassiveEffectsImpl(); } finally { setCurrentUpdatePriority(previousPriority); + ReactCurrentBatchConfig.transition = prevTransition; } } return false; diff --git a/packages/react-reconciler/src/__tests__/ReactFlushSync-test.js b/packages/react-reconciler/src/__tests__/ReactFlushSync-test.js index 3c96f5ee6591..bc7499a3d43a 100644 --- a/packages/react-reconciler/src/__tests__/ReactFlushSync-test.js +++ b/packages/react-reconciler/src/__tests__/ReactFlushSync-test.js @@ -3,6 +3,7 @@ let ReactNoop; let Scheduler; let useState; let useEffect; +let startTransition; describe('ReactFlushSync', () => { beforeEach(() => { @@ -13,6 +14,7 @@ describe('ReactFlushSync', () => { Scheduler = require('scheduler'); useState = React.useState; useEffect = React.useEffect; + startTransition = React.unstable_startTransition; }); function Text({text}) { @@ -54,4 +56,45 @@ describe('ReactFlushSync', () => { }); expect(root).toMatchRenderedOutput('1, 1'); }); + + // @gate experimental + test('nested with startTransition', async () => { + let setSyncState; + let setState; + function App() { + const [syncState, _setSyncState] = useState(0); + const [state, _setState] = useState(0); + setSyncState = _setSyncState; + setState = _setState; + return ; + } + + const root = ReactNoop.createRoot(); + await ReactNoop.act(async () => { + root.render(); + }); + expect(Scheduler).toHaveYielded(['0, 0']); + expect(root).toMatchRenderedOutput('0, 0'); + + await ReactNoop.act(async () => { + ReactNoop.flushSync(() => { + startTransition(() => { + // This should be async even though flushSync is on the stack, because + // startTransition is closer. + setState(1); + ReactNoop.flushSync(() => { + // This should be async even though startTransition is on the stack, + // because flushSync is closer. + setSyncState(1); + }); + }); + }); + // Only the sync update should have flushed + expect(Scheduler).toHaveYielded(['1, 0']); + expect(root).toMatchRenderedOutput('1, 0'); + }); + // Now the async update has flushed, too. + expect(Scheduler).toHaveYielded(['1, 1']); + expect(root).toMatchRenderedOutput('1, 1'); + }); });