Skip to content

Commit

Permalink
[bugfix] Fix false positive render phase update (#16907)
Browse files Browse the repository at this point in the history
Need to reset the current "debug phase" inside the catch block.
Otherwise React thinks we're still in the render phase during the
subsequent event.
  • Loading branch information
acdlite committed Sep 26, 2019
1 parent a9cd9a7 commit fad5102
Show file tree
Hide file tree
Showing 2 changed files with 57 additions and 5 deletions.
13 changes: 8 additions & 5 deletions packages/react-reconciler/src/ReactFiberWorkLoop.js
Original file line number Diff line number Diff line change
Expand Up @@ -1329,6 +1329,7 @@ function handleError(root, thrownValue) {
// Reset module-level state that was set during the render phase.
resetContextDependencies();
resetHooks();
resetCurrentDebugFiberInDEV();

if (workInProgress === null || workInProgress.return === null) {
// Expected to be working on a non-root fiber. This is a fatal error
Expand Down Expand Up @@ -2672,10 +2673,12 @@ if (__DEV__ && replayFailedUnitOfWorkWithInvokeGuardedCallback) {
throw originalError;
}

// Keep this code in sync with renderRoot; any changes here must have
// Keep this code in sync with handleError; any changes here must have
// corresponding changes there.
resetContextDependencies();
resetHooks();
// Don't reset current debug fiber, since we're about to work on the
// same fiber again.

// Unwind the failed stack frame
unwindInterruptedWork(unitOfWork);
Expand Down Expand Up @@ -3072,10 +3075,10 @@ function startWorkOnPendingInteractions(root, expirationTime) {
);

// Store the current set of interactions on the FiberRoot for a few reasons:
// We can re-use it in hot functions like renderRoot() without having to
// recalculate it. We will also use it in commitWork() to pass to any Profiler
// onRender() hooks. This also provides DevTools with a way to access it when
// the onCommitRoot() hook is called.
// We can re-use it in hot functions like performConcurrentWorkOnRoot()
// without having to recalculate it. We will also use it in commitWork() to
// pass to any Profiler onRender() hooks. This also provides DevTools with a
// way to access it when the onCommitRoot() hook is called.
root.memoizedInteractions = interactions;

if (interactions.size > 0) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2576,4 +2576,53 @@ describe('ReactSuspenseWithNoopRenderer', () => {
expect(root).toMatchRenderedOutput(<span prop="Initial" />);
});
});

it('regression test: resets current "debug phase" after suspending', async () => {
function App() {
return (
<Suspense fallback="Loading...">
<Foo suspend={false} />
</Suspense>
);
}

const thenable = {then() {}};

let foo;
class Foo extends React.Component {
state = {suspend: false};
render() {
foo = this;

if (this.state.suspend) {
Scheduler.unstable_yieldValue('Suspend!');
throw thenable;
}

return <Text text="Foo" />;
}
}

const root = ReactNoop.createRoot();
await ReactNoop.act(async () => {
root.render(<App />);
});

expect(Scheduler).toHaveYielded(['Foo']);

await ReactNoop.act(async () => {
foo.setState({suspend: true});

// In the regression that this covers, we would neglect to reset the
// current debug phase after suspending (in the catch block), so React
// thinks we're still inside the render phase.
expect(Scheduler).toFlushAndYieldThrough(['Suspend!']);

// Then when this setState happens, React would incorrectly fire a warning
// about updates that happen the render phase (only fired by classes).
foo.setState({suspend: false});
});

expect(root).toMatchRenderedOutput(<span prop="Foo" />);
});
});

0 comments on commit fad5102

Please sign in to comment.