[bugfix] Fix false positive render phase update (#16907)
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.
Andrew Clark committed
Sep 26, 2019 at 12:47 UTC
fad5102101e4b34bbd004a9044ae5e46581231ed
2 files changed
+57
-5
packages/react-reconciler/src/ReactFiberWorkLoop.js
+8
-5
@@ -1329,6 +1329,7 @@ function handleError(root, thrownValue) {
1329
// Reset module-level state that was set during the render phase.
1330
resetContextDependencies();
1331
resetHooks();
1332
+ resetCurrentDebugFiberInDEV();
1333
1334
if (workInProgress === null || workInProgress.return === null) {
1335
// Expected to be working on a non-root fiber. This is a fatal error
@@ -2672,10 +2673,12 @@ if (__DEV__ && replayFailedUnitOfWorkWithInvokeGuardedCallback) {
2673
throw originalError;
2674
}
2675
2675
- // Keep this code in sync with renderRoot; any changes here must have
2676
+ // Keep this code in sync with handleError; any changes here must have
2677
// corresponding changes there.
2678
resetContextDependencies();
2679
resetHooks();
2680
+ // Don't reset current debug fiber, since we're about to work on the
2681
+ // same fiber again.
2682
2683
// Unwind the failed stack frame
2684
unwindInterruptedWork(unitOfWork);
@@ -3072,10 +3075,10 @@ function startWorkOnPendingInteractions(root, expirationTime) {
3075
);
3076
3077
// Store the current set of interactions on the FiberRoot for a few reasons:
3075
- // We can re-use it in hot functions like renderRoot() without having to
3076
- // recalculate it. We will also use it in commitWork() to pass to any Profiler
3077
- // onRender() hooks. This also provides DevTools with a way to access it when
3078
- // the onCommitRoot() hook is called.
3078
+ // We can re-use it in hot functions like performConcurrentWorkOnRoot()
3079
+ // without having to recalculate it. We will also use it in commitWork() to
3080
+ // pass to any Profiler onRender() hooks. This also provides DevTools with a
3081
+ // way to access it when the onCommitRoot() hook is called.
3082
root.memoizedInteractions = interactions;
3083
3084
if (interactions.size > 0) {
packages/react-reconciler/src/__tests__/ReactSuspenseWithNoopRenderer-test.internal.js
+49
@@ -2576,4 +2576,53 @@ describe('ReactSuspenseWithNoopRenderer', () => {
2576
expect(root).toMatchRenderedOutput(<span prop="Initial" />);
2577
});
2578
});
2579
+
2580
+ it('regression test: resets current "debug phase" after suspending', async () => {
2581
+ function App() {
2582
+ return (
2583
+ <Suspense fallback="Loading...">
2584
+ <Foo suspend={false} />
2585
+ </Suspense>
2586
+ );
2587
+ }
2588
+
2589
+ const thenable = {then() {}};
2590
+
2591
+ let foo;
2592
+ class Foo extends React.Component {
2593
+ state = {suspend: false};
2594
+ render() {
2595
+ foo = this;
2596
+
2597
+ if (this.state.suspend) {
2598
+ Scheduler.unstable_yieldValue('Suspend!');
2599
+ throw thenable;
2600
+ }
2601
+
2602
+ return <Text text="Foo" />;
2603
+ }
2604
+ }
2605
+
2606
+ const root = ReactNoop.createRoot();
2607
+ await ReactNoop.act(async () => {
2608
+ root.render(<App />);
2609
+ });
2610
+
2611
+ expect(Scheduler).toHaveYielded(['Foo']);
2612
+
2613
+ await ReactNoop.act(async () => {
2614
+ foo.setState({suspend: true});
2615
+
2616
+ // In the regression that this covers, we would neglect to reset the
2617
+ // current debug phase after suspending (in the catch block), so React
2618
+ // thinks we're still inside the render phase.
2619
+ expect(Scheduler).toFlushAndYieldThrough(['Suspend!']);
2620
+
2621
+ // Then when this setState happens, React would incorrectly fire a warning
2622
+ // about updates that happen the render phase (only fired by classes).
2623
+ foo.setState({suspend: false});
2624
+ });
2625
+
2626
+ expect(root).toMatchRenderedOutput(<span prop="Foo" />);
2627
+ });
2628
});