Skip to content

Commit 938a27b

Browse files
committed
Bug: Extra render pass when reverting to client render
I noticed while working on a PR that when an error happens during hydration, and we revert to client rendering, React actually does _two_ additional render passes instead of just one. We didn't notice it earlier because none of our tests happened to assert on how many renders it took to recover, only on the final output. It's possible this extra render pass had other consequences that I'm not aware of, like messing with some assumption in the recoverable errors logic. This adds a test to demonstrate the issue. (One problem is that we don't have much test coverage of this scenario in the first place, which likely would have caught this earlier.)
1 parent 0131d0c commit 938a27b

File tree

1 file changed

+26
-2
lines changed

1 file changed

+26
-2
lines changed

packages/react-dom/src/__tests__/ReactDOMServerPartialHydration-test.internal.js

Lines changed: 26 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ let ReactFeatureFlags;
1919
let Suspense;
2020
let SuspenseList;
2121
let Offscreen;
22+
let useSyncExternalStore;
2223
let act;
2324
let IdleEventPriority;
2425
let waitForAll;
@@ -113,6 +114,7 @@ describe('ReactDOMServerPartialHydration', () => {
113114
Scheduler = require('scheduler');
114115
Suspense = React.Suspense;
115116
Offscreen = React.unstable_Offscreen;
117+
useSyncExternalStore = React.useSyncExternalStore;
116118
if (gate(flags => flags.enableSuspenseList)) {
117119
SuspenseList = React.SuspenseList;
118120
}
@@ -480,20 +482,42 @@ describe('ReactDOMServerPartialHydration', () => {
480482
});
481483

482484
it('recovers with client render when server rendered additional nodes at suspense root', async () => {
485+
function CheckIfHydrating({children}) {
486+
// This is a trick to check whether we're hydrating or not, since React
487+
// doesn't expose that information currently except
488+
// via useSyncExternalStore.
489+
let serverOrClient = '(unknown)';
490+
useSyncExternalStore(
491+
() => {},
492+
() => {
493+
serverOrClient = 'Client rendered';
494+
return null;
495+
},
496+
() => {
497+
serverOrClient = 'Server rendered';
498+
return null;
499+
},
500+
);
501+
Scheduler.log(serverOrClient);
502+
return null;
503+
}
504+
483505
const ref = React.createRef();
484506
function App({hasB}) {
485507
return (
486508
<div>
487509
<Suspense fallback="Loading...">
488510
<span ref={ref}>A</span>
489511
{hasB ? <span>B</span> : null}
512+
<CheckIfHydrating />
490513
</Suspense>
491514
<div>Sibling</div>
492515
</div>
493516
);
494517
}
495518

496519
const finalHTML = ReactDOMServer.renderToString(<App hasB={true} />);
520+
assertLog(['Server rendered']);
497521

498522
const container = document.createElement('div');
499523
container.innerHTML = finalHTML;
@@ -514,12 +538,12 @@ describe('ReactDOMServerPartialHydration', () => {
514538
});
515539
}).toErrorDev('Did not expect server HTML to contain a <span> in <div>');
516540

517-
jest.runAllTimers();
518-
519541
expect(container.innerHTML).toContain('<span>A</span>');
520542
expect(container.innerHTML).not.toContain('<span>B</span>');
521543

522544
assertLog([
545+
'Server rendered',
546+
'Client rendered',
523547
'There was an error while hydrating this Suspense boundary. ' +
524548
'Switched to client rendering.',
525549
]);

0 commit comments

Comments
 (0)