@samitouri / QOS-React / commits / 8b4c54c00f

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.

Andrew Clark committed Sep 4, 2024 at 12:38 UTC 8b4c54c00f5c047a72a4cecc2689196786c3e5ff
3 files changed +21
packages/react-reconciler/src/ReactFiberWorkLoop.js
+5
@@ -43,6 +43,7 @@ import {
43 disableLegacyMode,
44 disableDefaultPropsExceptForClasses,
45 disableStringRefs,
46 + enableSiblingPrerendering,
47 } from 'shared/ReactFeatureFlags';
48 import ReactSharedInternals from 'shared/ReactSharedInternals';
49 import is from 'shared/objectIs';
@@ -1700,6 +1701,10 @@ function handleThrow(root: FiberRoot, thrownValue: any): void {
1701 // deprecate the old API in favor of `use`.
1702 thrownValue = getSuspendedThenable();
1703 workInProgressSuspendedReason =
1704 + // TODO: Suspending the work loop during the render phase is
1705 + // currently not compatible with sibling prerendering. We will add
1706 + // this optimization back in a later step.
1707 + !enableSiblingPrerendering &&
1708 shouldRemainOnPreviousScreen() &&
1709 // Check if there are other pending updates that might possibly unblock this
1710 // component from suspending. This mirrors the check in
packages/react-reconciler/src/__tests__/ReactUse-test.js
+12
@@ -558,6 +558,7 @@ describe('ReactUse', () => {
558 }
559 });
560
561 + // @gate enableSuspendingDuringWorkLoop
562 it('during a transition, can unwrap async operations even if nothing is cached', async () => {
563 function App() {
564 return <Text text={use(getAsyncText('Async'))} />;
@@ -593,6 +594,7 @@ describe('ReactUse', () => {
594 expect(root).toMatchRenderedOutput('Async');
595 });
596
597 + // @gate enableSuspendingDuringWorkLoop
598 it("does not prevent a Suspense fallback from showing if it's a new boundary, even during a transition", async () => {
599 function App() {
600 return <Text text={use(getAsyncText('Async'))} />;
@@ -635,6 +637,7 @@ describe('ReactUse', () => {
637 expect(root).toMatchRenderedOutput('Async');
638 });
639
640 + // @gate enableSuspendingDuringWorkLoop
641 it('when waiting for data to resolve, a fresh update will trigger a restart', async () => {
642 function App() {
643 return <Text text={use(getAsyncText('Will never resolve'))} />;
@@ -666,6 +669,7 @@ describe('ReactUse', () => {
669 assertLog(['Something different']);
670 });
671
672 + // @gate enableSuspendingDuringWorkLoop
673 it('when waiting for data to resolve, an update on a different root does not cause work to be dropped', async () => {
674 const promise = getAsyncText('Hi');
675
@@ -708,6 +712,7 @@ describe('ReactUse', () => {
712 expect(root1).toMatchRenderedOutput('Hi');
713 });
714
715 + // @gate enableSuspendingDuringWorkLoop
716 it('while suspended, hooks cannot be called (i.e. current dispatcher is unset correctly)', async () => {
717 function App() {
718 return <Text text={use(getAsyncText('Will never resolve'))} />;
@@ -845,6 +850,7 @@ describe('ReactUse', () => {
850 expect(root).toMatchRenderedOutput('(empty)');
851 });
852
853 + // @gate enableSuspendingDuringWorkLoop
854 it('when replaying a suspended component, reuses the hooks computed during the previous attempt (Memo)', async () => {
855 function ExcitingText({text}) {
856 // This computes the uppercased version of some text. Pretend it's an
@@ -894,6 +900,7 @@ describe('ReactUse', () => {
900 ]);
901 });
902
903 + // @gate enableSuspendingDuringWorkLoop
904 it('when replaying a suspended component, reuses the hooks computed during the previous attempt (State)', async () => {
905 let _setFruit;
906 let _setVegetable;
@@ -950,6 +957,7 @@ describe('ReactUse', () => {
957 expect(root).toMatchRenderedOutput('banana dill');
958 });
959
960 + // @gate enableSuspendingDuringWorkLoop
961 it('when replaying a suspended component, reuses the hooks computed during the previous attempt (DebugValue+State)', async () => {
962 // Make sure we don't get a Hook mismatch warning on updates if there were non-stateful Hooks before the use().
963 let _setLawyer;
@@ -991,6 +999,7 @@ describe('ReactUse', () => {
999 expect(root).toMatchRenderedOutput('aguacate avocat');
1000 });
1001
1002 + // @gate enableSuspendingDuringWorkLoop
1003 it(
1004 'wrap an async function with useMemo to skip running the function ' +
1005 'twice when loading new data',
@@ -1073,6 +1082,7 @@ describe('ReactUse', () => {
1082 expect(root).toMatchRenderedOutput('ABC');
1083 });
1084
1085 + // @gate enableSuspendingDuringWorkLoop
1086 it('load multiple nested Suspense boundaries (uncached requests)', async () => {
1087 // This the same as the previous test, except the requests are not cached.
1088 // The tree should still eventually resolve, despite the
@@ -1196,6 +1206,7 @@ describe('ReactUse', () => {
1206 expect(root).toMatchRenderedOutput('Hi');
1207 });
1208
1209 + // @gate enableSuspendingDuringWorkLoop
1210 it('basic async component', async () => {
1211 async function App() {
1212 await getAsyncText('Hi');
@@ -1220,6 +1231,7 @@ describe('ReactUse', () => {
1231 expect(root).toMatchRenderedOutput('Hi');
1232 });
1233
1234 + // @gate enableSuspendingDuringWorkLoop
1235 it('async child of a non-function component (e.g. a class)', async () => {
1236 class App extends React.Component {
1237 async render() {
scripts/jest/TestFlags.js
+4
@@ -83,6 +83,10 @@ function getTestFlags() {
83 enableActivity: releaseChannel === 'experimental' || www || xplat,
84 enableSuspenseList: releaseChannel === 'experimental' || www || xplat,
85 enableLegacyHidden: www,
86 + // TODO: Suspending the work loop during the render phase is currently
87 + // not compatible with sibling prerendering. We will add this optimization
88 + // back in a later step.
89 + enableSuspendingDuringWorkLoop: !featureFlags.enableSiblingPrerendering,
90
91 // This flag is used to determine whether we should run Fizz tests using
92 // the external runtime or the inline script runtime.