From 1d093ad1da96363bae8daeaa8b690f00f97b91a6 Mon Sep 17 00:00:00 2001 From: Andrew Clark Date: Tue, 20 Aug 2024 15:05:24 -0400 Subject: [PATCH] Temporarily disable suspending during work loop `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. --- 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..da55fd612625d 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 !enableSiblingPrerendering 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 !enableSiblingPrerendering 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 !enableSiblingPrerendering 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 !enableSiblingPrerendering 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 !enableSiblingPrerendering 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 !enableSiblingPrerendering 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 !enableSiblingPrerendering 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 !enableSiblingPrerendering 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 !enableSiblingPrerendering 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 !enableSiblingPrerendering 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 !enableSiblingPrerendering it('basic async component', async () => { async function App() { await getAsyncText('Hi'); @@ -1220,6 +1231,7 @@ describe('ReactUse', () => { expect(root).toMatchRenderedOutput('Hi'); }); + // @gate !enableSiblingPrerendering 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.