diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.js b/packages/react-reconciler/src/ReactFiberWorkLoop.js index edcf20fc4379..7d5a38ad393d 100644 --- a/packages/react-reconciler/src/ReactFiberWorkLoop.js +++ b/packages/react-reconciler/src/ReactFiberWorkLoop.js @@ -3401,8 +3401,16 @@ function pingSuspendedRoot( includesOnlyRetries(workInProgressRootRenderLanes) && now() - globalMostRecentFallbackTime < FALLBACK_THROTTLE_MS) ) { - // Restart from the root. - prepareFreshStack(root, NoLanes); + // Force a restart from the root by unwinding the stack. Unless this is + // being called from the render phase, because that would cause a crash. + if ((executionContext & RenderContext) === NoContext) { + prepareFreshStack(root, NoLanes); + } else { + // TODO: If this does happen during the render phase, we should throw + // the special internal exception that we use to interrupt the stack for + // selective hydration. That was temporarily reverted but we once we add + // it back we can use it here. + } } else { // Even though we can't restart right now, we might get an // opportunity later. So we mark this render as having a ping. diff --git a/packages/react-reconciler/src/__tests__/ReactSuspenseWithNoopRenderer-test.js b/packages/react-reconciler/src/__tests__/ReactSuspenseWithNoopRenderer-test.js index 5b4c9345443b..bb4ea103ff12 100644 --- a/packages/react-reconciler/src/__tests__/ReactSuspenseWithNoopRenderer-test.js +++ b/packages/react-reconciler/src/__tests__/ReactSuspenseWithNoopRenderer-test.js @@ -4218,4 +4218,80 @@ describe('ReactSuspenseWithNoopRenderer', () => { }); expect(Scheduler).toHaveYielded(['Unmount Child']); }); + + // @gate enableLegacyCache + it( + 'regression test: pinging synchronously within the render phase ' + + 'does not unwind the stack', + async () => { + // This is a regression test that reproduces a very specific scenario that + // used to cause a crash. + const thenable = { + then(resolve) { + resolve('hi'); + }, + status: 'pending', + }; + + function ImmediatelyPings() { + if (thenable.status === 'pending') { + thenable.status = 'fulfilled'; + throw thenable; + } + return ; + } + + function App({showMore}) { + return ( +
+ }> + {showMore ? ( + <> + + + ) : null} + + {showMore ? ( + + + + ) : null} +
+ ); + } + + // Initial render. This mounts a Suspense boundary, so that in the next + // update we can trigger a "suspend with delay" scenario. + const root = ReactNoop.createRoot(); + await act(async () => { + root.render(); + }); + expect(Scheduler).toHaveYielded([]); + expect(root).toMatchRenderedOutput(
); + + // Update. This will cause two separate trees to suspend. The first tree + // will be inside an already mounted Suspense boundary, so it will trigger + // a "suspend with delay". The second tree will be a new Suspense + // boundary, but the thenable that is thrown will immediately call its + // ping listener. + // + // Before the bug was fixed, this would lead to a `prepareFreshStack` call + // that unwinds the work-in-progress stack. When that code was written, it + // was expected that pings always happen from an asynchronous task (or + // microtask). But this test shows an example where that's not the case. + // + // The fix was to check if we're in the render phase before calling + // `prepareFreshStack`. + await act(async () => { + root.render(); + }); + expect(Scheduler).toHaveYielded(['Suspend! [Async]', 'Loading...', 'Hi']); + expect(root).toMatchRenderedOutput( +
+ + +
, + ); + }, + ); });