Change retry priority to "Never" for dehydrated boundaries (#17105)
This changes the "default" retryTime to NoWork which schedules at Normal pri. Dehydrated bouundaries normally hydrate at Never priority except when they retry where we accidentally increased them to Normal because Never was used as the default value. This changes it so NoWork is the default. Dehydrated boundaries however get initialized to Never as the default. Therefore they now hydrate as Never pri unless their priority gets increased by a forced rerender or selective hydration. This revealed that erroring at this Never priority can cause an infinite rerender. So I fixed that too.
Sebastian Markbåge committed
Oct 15, 2019 at 19:53 UTC
6ff23f2a5da0e60fa008cef97529469945618d06
4 files changed
+102
-16
packages/react-dom/src/__tests__/ReactDOMServerPartialHydration-test.internal.js
+82
@@ -77,6 +77,7 @@ describe('ReactDOMServerPartialHydration', () => {
77
ReactFeatureFlags = require('shared/ReactFeatureFlags');
78
ReactFeatureFlags.enableSuspenseCallback = true;
79
ReactFeatureFlags.enableFlareAPI = true;
80
+ ReactFeatureFlags.debugRenderPhaseSideEffectsForStrictMode = false;
81
82
React = require('react');
83
ReactDOM = require('react-dom');
@@ -2475,4 +2476,85 @@ describe('ReactDOMServerPartialHydration', () => {
2476
2477
document.body.removeChild(container);
2478
});
2479
+
2480
+ it('finishes normal pri work before continuing to hydrate a retry', async () => {
2481
+ let suspend = false;
2482
+ let resolve;
2483
+ let promise = new Promise(resolvePromise => (resolve = resolvePromise));
2484
+ let ref = React.createRef();
2485
+
2486
+ function Child() {
2487
+ if (suspend) {
2488
+ throw promise;
2489
+ } else {
2490
+ Scheduler.unstable_yieldValue('Child');
2491
+ return 'Hello';
2492
+ }
2493
+ }
2494
+
2495
+ function Sibling() {
2496
+ Scheduler.unstable_yieldValue('Sibling');
2497
+ React.useLayoutEffect(() => {
2498
+ Scheduler.unstable_yieldValue('Commit Sibling');
2499
+ });
2500
+ return 'World';
2501
+ }
2502
+
2503
+ // Avoid rerendering the tree by hoisting it.
2504
+ const tree = (
2505
+ <Suspense fallback="Loading...">
2506
+ <span ref={ref}>
2507
+ <Child />
2508
+ </span>
2509
+ </Suspense>
2510
+ );
2511
+
2512
+ function App({showSibling}) {
2513
+ return (
2514
+ <div>
2515
+ {tree}
2516
+ {showSibling ? <Sibling /> : null}
2517
+ </div>
2518
+ );
2519
+ }
2520
+
2521
+ suspend = false;
2522
+ let finalHTML = ReactDOMServer.renderToString(<App />);
2523
+ expect(Scheduler).toHaveYielded(['Child']);
2524
+
2525
+ let container = document.createElement('div');
2526
+ container.innerHTML = finalHTML;
2527
+
2528
+ suspend = true;
2529
+ let root = ReactDOM.unstable_createRoot(container, {hydrate: true});
2530
+ root.render(<App showSibling={false} />);
2531
+ expect(Scheduler).toFlushAndYield([]);
2532
+
2533
+ expect(ref.current).toBe(null);
2534
+ expect(container.textContent).toBe('Hello');
2535
+
2536
+ // Resolving the promise should continue hydration
2537
+ suspend = false;
2538
+ resolve();
2539
+ await promise;
2540
+
2541
+ Scheduler.unstable_advanceTime(100);
2542
+
2543
+ // Before we have a chance to flush it, we'll also render an update.
2544
+ root.render(<App showSibling={true} />);
2545
+
2546
+ // When we flush we expect the Normal pri render to take priority
2547
+ // over hydration.
2548
+ expect(Scheduler).toFlushAndYieldThrough(['Sibling', 'Commit Sibling']);
2549
+
2550
+ // We shouldn't have hydrated the child yet.
2551
+ expect(ref.current).toBe(null);
2552
+ // But we did have a chance to update the content.
2553
+ expect(container.textContent).toBe('HelloWorld');
2554
+
2555
+ expect(Scheduler).toFlushAndYield(['Child']);
2556
+
2557
+ // Now we're hydrated.
2558
+ expect(ref.current).not.toBe(null);
2559
+ });
2560
});
packages/react-reconciler/src/ReactFiberBeginWork.js
+1
-1
@@ -1482,7 +1482,7 @@ function validateFunctionComponentInDev(workInProgress: Fiber, Component: any) {
1482
1483
const SUSPENDED_MARKER: SuspenseState = {
1484
dehydrated: null,
1485
- retryTime: Never,
1485
+ retryTime: NoWork,
1486
};
1487
1488
function shouldRemainOnFallback(
packages/react-reconciler/src/ReactFiberSuspenseComponent.js
+3
-1
@@ -35,7 +35,9 @@ export type SuspenseState = {|
35
// to check things like isSuspenseInstancePending.
36
dehydrated: null | SuspenseInstance,
37
// Represents the earliest expiration time we should attempt to hydrate
38
- // a dehydrated boundary at. Never is the default.
38
+ // a dehydrated boundary at.
39
+ // Never is the default for dehydrated boundaries.
40
+ // NoWork is the default for normal boundaries, which turns into "normal" pri.
41
retryTime: ExpirationTime,
42
|};
43
packages/react-reconciler/src/ReactFiberWorkLoop.js
+16
-14
@@ -741,17 +741,19 @@ function finishConcurrentRender(
741
// statement, but eslint doesn't know about invariant, so it complains
742
// if I do. eslint-disable-next-line no-fallthrough
743
case RootErrored: {
744
- if (expirationTime !== Idle) {
745
- // If this was an async render, the error may have happened due to
746
- // a mutation in a concurrent event. Try rendering one more time,
747
- // synchronously, to see if the error goes away. If there are
748
- // lower priority updates, let's include those, too, in case they
749
- // fix the inconsistency. Render at Idle to include all updates.
750
- markRootExpiredAtTime(root, Idle);
751
- break;
752
- }
753
- // Commit the root in its errored state.
754
- commitRoot(root);
744
+ // If this was an async render, the error may have happened due to
745
+ // a mutation in a concurrent event. Try rendering one more time,
746
+ // synchronously, to see if the error goes away. If there are
747
+ // lower priority updates, let's include those, too, in case they
748
+ // fix the inconsistency. Render at Idle to include all updates.
749
+ // If it was Idle or Never or some not-yet-invented time, render
750
+ // at that time.
751
+ markRootExpiredAtTime(
752
+ root,
753
+ expirationTime > Idle ? Idle : expirationTime,
754
+ );
755
+ // We assume that this second render pass will be synchronous
756
+ // and therefore not hit this path again.
757
break;
758
}
759
case RootSuspended: {
@@ -2376,7 +2378,7 @@ function retryTimedOutBoundary(
2378
// previously was rendered in its fallback state. One of the promises that
2379
// suspended it has resolved, which means at least part of the tree was
2380
// likely unblocked. Try rendering again, at a new expiration time.
2379
- if (retryTime === Never) {
2381
+ if (retryTime === NoWork) {
2382
const suspenseConfig = null; // Retries don't carry over the already committed update.
2383
const currentTime = requestCurrentTime();
2384
retryTime = computeExpirationForFiber(
@@ -2395,7 +2397,7 @@ function retryTimedOutBoundary(
2397
2398
export function retryDehydratedSuspenseBoundary(boundaryFiber: Fiber) {
2399
const suspenseState: null | SuspenseState = boundaryFiber.memoizedState;
2398
- let retryTime = Never;
2400
+ let retryTime = NoWork;
2401
if (suspenseState !== null) {
2402
retryTime = suspenseState.retryTime;
2403
}
@@ -2403,7 +2405,7 @@ export function retryDehydratedSuspenseBoundary(boundaryFiber: Fiber) {
2405
}
2406
2407
export function resolveRetryThenable(boundaryFiber: Fiber, thenable: Thenable) {
2406
- let retryTime = Never; // Default
2408
+ let retryTime = NoWork; // Default
2409
let retryCache: WeakSet<Thenable> | Set<Thenable> | null;
2410
if (enableSuspenseServerRenderer) {
2411
switch (boundaryFiber.tag) {