From 1b1ad81a5d0e300575619f6cd1bef9a4ffe254cf Mon Sep 17 00:00:00 2001 From: Mofei Zhang Date: Fri, 11 Oct 2024 16:22:05 -0400 Subject: [PATCH] Temporarily disable suspending during work loop (#30762) ### Based on - #30761 - #30759 --- `use` has an optimization where in some cases it can suspend the work loop during the render phase until the data has resolved, rather than unwind the stack and lose context. However, the current implementation is not compatible with sibling prerendering. So I've temporarily disabled it until the sibling prerendering has been refactored. We will add it back in a later step. [ghstack-poisoned] --- packages/react-reconciler/src/ReactFiberWorkLoop.js | 5 +++++ .../react-reconciler/src/__tests__/ReactUse-test.js | 12 ++++++++++++ scripts/jest/TestFlags.js | 4 ++++ 3 files changed, 21 insertions(+) diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.js b/packages/react-reconciler/src/ReactFiberWorkLoop.js index 40eed7a71325a..3190cd0035b33 100644 --- a/packages/react-reconciler/src/ReactFiberWorkLoop.js +++ b/packages/react-reconciler/src/ReactFiberWorkLoop.js @@ -43,6 +43,7 @@ import { disableLegacyMode, disableDefaultPropsExceptForClasses, disableStringRefs, + enableSiblingPrerendering, } from 'shared/ReactFeatureFlags'; import ReactSharedInternals from 'shared/ReactSharedInternals'; import is from 'shared/objectIs'; @@ -1700,6 +1701,10 @@ function handleThrow(root: FiberRoot, thrownValue: any): void { // deprecate the old API in favor of `use`. thrownValue = getSuspendedThenable(); workInProgressSuspendedReason = + // TODO: Suspending the work loop during the render phase is + // currently not compatible with sibling prerendering. We will add + // this optimization back in a later step. + !enableSiblingPrerendering && shouldRemainOnPreviousScreen() && // Check if there are other pending updates that might possibly unblock this // component from suspending. This mirrors the check in diff --git a/packages/react-reconciler/src/__tests__/ReactUse-test.js b/packages/react-reconciler/src/__tests__/ReactUse-test.js index 1dae8a56dd25f..0abb47d516a3d 100644 --- a/packages/react-reconciler/src/__tests__/ReactUse-test.js +++ b/packages/react-reconciler/src/__tests__/ReactUse-test.js @@ -558,6 +558,7 @@ describe('ReactUse', () => { } }); + // @gate enableSuspendingDuringWorkLoop it('during a transition, can unwrap async operations even if nothing is cached', async () => { function App() { return ; @@ -593,6 +594,7 @@ describe('ReactUse', () => { expect(root).toMatchRenderedOutput('Async'); }); + // @gate enableSuspendingDuringWorkLoop it("does not prevent a Suspense fallback from showing if it's a new boundary, even during a transition", async () => { function App() { return ; @@ -635,6 +637,7 @@ describe('ReactUse', () => { expect(root).toMatchRenderedOutput('Async'); }); + // @gate enableSuspendingDuringWorkLoop it('when waiting for data to resolve, a fresh update will trigger a restart', async () => { function App() { return ; @@ -666,6 +669,7 @@ describe('ReactUse', () => { assertLog(['Something different']); }); + // @gate enableSuspendingDuringWorkLoop it('when waiting for data to resolve, an update on a different root does not cause work to be dropped', async () => { const promise = getAsyncText('Hi'); @@ -708,6 +712,7 @@ describe('ReactUse', () => { expect(root1).toMatchRenderedOutput('Hi'); }); + // @gate enableSuspendingDuringWorkLoop it('while suspended, hooks cannot be called (i.e. current dispatcher is unset correctly)', async () => { function App() { return ; @@ -845,6 +850,7 @@ describe('ReactUse', () => { expect(root).toMatchRenderedOutput('(empty)'); }); + // @gate enableSuspendingDuringWorkLoop it('when replaying a suspended component, reuses the hooks computed during the previous attempt (Memo)', async () => { function ExcitingText({text}) { // This computes the uppercased version of some text. Pretend it's an @@ -894,6 +900,7 @@ describe('ReactUse', () => { ]); }); + // @gate enableSuspendingDuringWorkLoop it('when replaying a suspended component, reuses the hooks computed during the previous attempt (State)', async () => { let _setFruit; let _setVegetable; @@ -950,6 +957,7 @@ describe('ReactUse', () => { expect(root).toMatchRenderedOutput('banana dill'); }); + // @gate enableSuspendingDuringWorkLoop it('when replaying a suspended component, reuses the hooks computed during the previous attempt (DebugValue+State)', async () => { // Make sure we don't get a Hook mismatch warning on updates if there were non-stateful Hooks before the use(). let _setLawyer; @@ -991,6 +999,7 @@ describe('ReactUse', () => { expect(root).toMatchRenderedOutput('aguacate avocat'); }); + // @gate enableSuspendingDuringWorkLoop it( 'wrap an async function with useMemo to skip running the function ' + 'twice when loading new data', @@ -1073,6 +1082,7 @@ describe('ReactUse', () => { expect(root).toMatchRenderedOutput('ABC'); }); + // @gate enableSuspendingDuringWorkLoop it('load multiple nested Suspense boundaries (uncached requests)', async () => { // This the same as the previous test, except the requests are not cached. // The tree should still eventually resolve, despite the @@ -1196,6 +1206,7 @@ describe('ReactUse', () => { expect(root).toMatchRenderedOutput('Hi'); }); + // @gate enableSuspendingDuringWorkLoop it('basic async component', async () => { async function App() { await getAsyncText('Hi'); @@ -1220,6 +1231,7 @@ describe('ReactUse', () => { expect(root).toMatchRenderedOutput('Hi'); }); + // @gate enableSuspendingDuringWorkLoop it('async child of a non-function component (e.g. a class)', async () => { class App extends React.Component { async render() { diff --git a/scripts/jest/TestFlags.js b/scripts/jest/TestFlags.js index 6ced58a32be3c..b970955491aae 100644 --- a/scripts/jest/TestFlags.js +++ b/scripts/jest/TestFlags.js @@ -83,6 +83,10 @@ function getTestFlags() { enableActivity: releaseChannel === 'experimental' || www || xplat, enableSuspenseList: releaseChannel === 'experimental' || www || xplat, enableLegacyHidden: www, + // TODO: Suspending the work loop during the render phase is currently + // not compatible with sibling prerendering. We will add this optimization + // back in a later step. + enableSuspendingDuringWorkLoop: !featureFlags.enableSiblingPrerendering, // This flag is used to determine whether we should run Fizz tests using // the external runtime or the inline script runtime.