Skip to content

Commit

Permalink
Temporarily disable suspending during work loop (#30762)
Browse files Browse the repository at this point in the history
### 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]
  • Loading branch information
mofeiZ committed Oct 11, 2024
1 parent e2c0670 commit 1b1ad81
Show file tree
Hide file tree
Showing 3 changed files with 21 additions and 0 deletions.
5 changes: 5 additions & 0 deletions packages/react-reconciler/src/ReactFiberWorkLoop.js
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ import {
disableLegacyMode,
disableDefaultPropsExceptForClasses,
disableStringRefs,
enableSiblingPrerendering,
} from 'shared/ReactFeatureFlags';
import ReactSharedInternals from 'shared/ReactSharedInternals';
import is from 'shared/objectIs';
Expand Down Expand Up @@ -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
Expand Down
12 changes: 12 additions & 0 deletions packages/react-reconciler/src/__tests__/ReactUse-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 <Text text={use(getAsyncText('Async'))} />;
Expand Down Expand Up @@ -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 <Text text={use(getAsyncText('Async'))} />;
Expand Down Expand Up @@ -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 <Text text={use(getAsyncText('Will never resolve'))} />;
Expand Down Expand Up @@ -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');

Expand Down Expand Up @@ -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 <Text text={use(getAsyncText('Will never resolve'))} />;
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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',
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -1196,6 +1206,7 @@ describe('ReactUse', () => {
expect(root).toMatchRenderedOutput('Hi');
});

// @gate enableSuspendingDuringWorkLoop
it('basic async component', async () => {
async function App() {
await getAsyncText('Hi');
Expand All @@ -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() {
Expand Down
4 changes: 4 additions & 0 deletions scripts/jest/TestFlags.js
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down

0 comments on commit 1b1ad81

Please sign in to comment.