Skip to content

Commit

Permalink
Throw on hydration mismatch and force client rendering if boundary ha…
Browse files Browse the repository at this point in the history 
…sn't suspended within concurrent root (facebook#22629)

* Throw on hydration mismatch

* remove debugger

* update error message

* update error message part2...

* fix test?

* test? :(

* tests 4real

* remove useRefAccessWarning gating

* split markSuspenseBoundary and getNearestBoundary

* also assert html is correct

* replace-fork

* also remove client render flag on suspend

* replace-fork

* fix mismerge????
  • Loading branch information
@salazarm @zhengjitf
salazarm authored and zhengjitf committed Apr 15, 2022
1 parent 2172959 commit 0464e6b
Show file tree
Hide file tree
Showing 7 changed files with 444 additions and 346 deletions.
59 changes: 16 additions & 43 deletions packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1677,45 +1677,37 @@ describe('ReactDOMFizzServer', () => {

// @gate experimental
it('calls getServerSnapshot instead of getSnapshot', async () => {
const ref = React.createRef();

function getServerSnapshot() {
return 'server';
}

function getClientSnapshot() {
return 'client';
}

function subscribe() {
return () => {};
}

function Child({text}) {
Scheduler.unstable_yieldValue(text);
return text;
}

function App() {
const value = useSyncExternalStore(
subscribe,
getClientSnapshot,
getServerSnapshot,
);
return (
<div ref={ref}>
<div>
<Child text={value} />
</div>
);
}

const loggedErrors = [];
await act(async () => {
const {pipe} = ReactDOMFizzServer.renderToPipeableStream(
<Suspense fallback="Loading...">
<App />
</Suspense>,

{
onError(x) {
loggedErrors.push(x);
Expand All @@ -1726,56 +1718,43 @@ describe('ReactDOMFizzServer', () => {
});
expect(Scheduler).toHaveYielded(['server']);

const serverRenderedDiv = container.getElementsByTagName('div')[0];

ReactDOM.hydrateRoot(container, <App />);

// The first paint uses the server snapshot
expect(Scheduler).toFlushUntilNextPaint(['server']);
expect(getVisibleChildren(container)).toEqual(<div>server</div>);
// Hydration succeeded
expect(ref.current).toEqual(serverRenderedDiv);

// Asynchronously we detect that the store has changed on the client,
// and patch up the inconsistency
expect(Scheduler).toFlushUntilNextPaint(['client']);
expect(() => {
// The first paint switches to client rendering due to mismatch
expect(Scheduler).toFlushUntilNextPaint(['client']);
}).toErrorDev(
'Warning: An error occurred during hydration. The server HTML was replaced with client content',
{withoutStack: true},
);
expect(getVisibleChildren(container)).toEqual(<div>client</div>);
expect(ref.current).toEqual(serverRenderedDiv);
});

// The selector implementation uses the lazy ref initialization pattern
// @gate !(enableUseRefAccessWarning && __DEV__)
// @gate experimental
it('calls getServerSnapshot instead of getSnapshot (with selector and isEqual)', async () => {
// Same as previous test, but with a selector that returns a complex object
// that is memoized with a custom `isEqual` function.
const ref = React.createRef();

function getServerSnapshot() {
return {env: 'server', other: 'unrelated'};
}

function getClientSnapshot() {
return {env: 'client', other: 'unrelated'};
}

function selector({env}) {
return {env};
}

function isEqual(a, b) {
return a.env === b.env;
}

function subscribe() {
return () => {};
}

function Child({text}) {
Scheduler.unstable_yieldValue(text);
return text;
}

function App() {
const {env} = useSyncExternalStoreWithSelector(
subscribe,
Expand All @@ -1790,14 +1769,12 @@ describe('ReactDOMFizzServer', () => {
</div>
);
}

const loggedErrors = [];
await act(async () => {
const {pipe} = ReactDOMFizzServer.renderToPipeableStream(
<Suspense fallback="Loading...">
<App />
</Suspense>,

{
onError(x) {
loggedErrors.push(x);
Expand All @@ -1808,21 +1785,17 @@ describe('ReactDOMFizzServer', () => {
});
expect(Scheduler).toHaveYielded(['server']);

const serverRenderedDiv = container.getElementsByTagName('div')[0];

ReactDOM.hydrateRoot(container, <App />);

// The first paint uses the server snapshot
expect(Scheduler).toFlushUntilNextPaint(['server']);
expect(getVisibleChildren(container)).toEqual(<div>server</div>);
// Hydration succeeded
expect(ref.current).toEqual(serverRenderedDiv);

// Asynchronously we detect that the store has changed on the client,
// and patch up the inconsistency
expect(Scheduler).toFlushUntilNextPaint(['client']);
// The first paint uses the client due to mismatch forcing client render
expect(() => {
// The first paint switches to client rendering due to mismatch
expect(Scheduler).toFlushUntilNextPaint(['client']);
}).toErrorDev(
'Warning: An error occurred during hydration. The server HTML was replaced with client content',
{withoutStack: true},
);
expect(getVisibleChildren(container)).toEqual(<div>client</div>);
expect(ref.current).toEqual(serverRenderedDiv);
});

// @gate experimental
Expand Down
102 changes: 96 additions & 6 deletions packages/react-dom/src/__tests__/ReactDOMServerPartialHydration-test.internal.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -197,12 +197,7 @@ describe('ReactDOMServerPartialHydration', () => {
// hydrating anyway.
suspend = true;
ReactDOM.hydrateRoot(container, <App />);
expect(() => {
Scheduler.unstable_flushAll();
}).toErrorDev(
// TODO: This error should not be logged in this case. It's a false positive.
'Did not expect server HTML to contain the text node "Hello" in <div>.',
);
Scheduler.unstable_flushAll();
jest.runAllTimers();

// Expect the server-generated HTML to stay intact.
Expand All @@ -218,6 +213,101 @@ describe('ReactDOMServerPartialHydration', () => {
expect(container.textContent).toBe('HelloHello');
});

it('falls back to client rendering boundary on mismatch', async () => {
let client = false;
let suspend = false;
let resolve;
const promise = new Promise(resolvePromise => {
resolve = () => {
suspend = false;
resolvePromise();
};
});
function Child() {
if (suspend) {
Scheduler.unstable_yieldValue('Suspend');
throw promise;
} else {
Scheduler.unstable_yieldValue('Hello');
return 'Hello';
}
}
function Component({shouldMismatch}) {
Scheduler.unstable_yieldValue('Component');
if (shouldMismatch && client) {
return <article>Mismatch</article>;
}
return <div>Component</div>;
}
function App() {
return (
<Suspense fallback="Loading...">
<Child />
<Component />
<Component />
<Component />
<Component shouldMismatch={true} />
</Suspense>
);
}
const finalHTML = ReactDOMServer.renderToString(<App />);
const container = document.createElement('div');
container.innerHTML = finalHTML;
expect(Scheduler).toHaveYielded([
'Hello',
'Component',
'Component',
'Component',
'Component',
]);

expect(container.innerHTML).toBe(
'<!--$-->Hello<div>Component</div><div>Component</div><div>Component</div><div>Component</div><!--/$-->',
);

suspend = true;
client = true;

ReactDOM.hydrateRoot(container, <App />);
expect(Scheduler).toFlushAndYield([
'Suspend',
'Component',
'Component',
'Component',
'Component',
]);
jest.runAllTimers();

// Unchanged
expect(container.innerHTML).toBe(
'<!--$-->Hello<div>Component</div><div>Component</div><div>Component</div><div>Component</div><!--/$-->',
);

suspend = false;
resolve();
await promise;

expect(Scheduler).toFlushAndYield([
// first pass, mismatches at end
'Hello',
'Component',
'Component',
'Component',
'Component',
// second pass as client render
'Hello',
'Component',
'Component',
'Component',
'Component',
]);

// Client rendered - suspense comment nodes removed
expect(container.innerHTML).toBe(
'Hello<div>Component</div><div>Component</div><div>Component</div><article>Mismatch</article>',
);
});

it('calls the hydration callbacks after hydration or deletion', async () => {
let suspend = false;
let resolve;
Expand Down
11 changes: 11 additions & 0 deletions packages/react-reconciler/src/ReactFiberHydrationContext.new.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
*/

import type {Fiber} from './ReactInternalTypes';
import {NoMode, ConcurrentMode} from './ReactTypeOfMode';
import type {
Instance,
TextInstance,
Expand Down Expand Up @@ -323,12 +324,21 @@ function tryHydrate(fiber, nextInstance) {
}
}

function throwOnHydrationMismatchIfConcurrentMode(fiber) {
if ((fiber.mode & ConcurrentMode) !== NoMode) {
throw new Error(
'An error occurred during hydration. The server HTML was replaced with client content',
);
}
}

function tryToClaimNextHydratableInstance(fiber: Fiber): void {
if (!isHydrating) {
return;
}
let nextInstance = nextHydratableInstance;
if (!nextInstance) {
throwOnHydrationMismatchIfConcurrentMode(fiber);
// Nothing to hydrate. Make it an insertion.
insertNonHydratedInstance((hydrationParentFiber: any), fiber);
isHydrating = false;
Expand All @@ -337,6 +347,7 @@ function tryToClaimNextHydratableInstance(fiber: Fiber): void {
}
const firstAttemptedInstance = nextInstance;
if (!tryHydrate(fiber, nextInstance)) {
throwOnHydrationMismatchIfConcurrentMode(fiber);
// If we can't hydrate this instance let's try the next one.
// We use this as a heuristic. It's based on intuition and not data so it
// might be flawed or unnecessary.
Expand Down
11 changes: 11 additions & 0 deletions packages/react-reconciler/src/ReactFiberHydrationContext.old.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
*/

import type {Fiber} from './ReactInternalTypes';
import {NoMode, ConcurrentMode} from './ReactTypeOfMode';
import type {
Instance,
TextInstance,
Expand Down Expand Up @@ -323,12 +324,21 @@ function tryHydrate(fiber, nextInstance) {
}
}

function throwOnHydrationMismatchIfConcurrentMode(fiber) {
if ((fiber.mode & ConcurrentMode) !== NoMode) {
throw new Error(
'An error occurred during hydration. The server HTML was replaced with client content',
);
}
}

function tryToClaimNextHydratableInstance(fiber: Fiber): void {
if (!isHydrating) {
return;
}
let nextInstance = nextHydratableInstance;
if (!nextInstance) {
throwOnHydrationMismatchIfConcurrentMode(fiber);
// Nothing to hydrate. Make it an insertion.
insertNonHydratedInstance((hydrationParentFiber: any), fiber);
isHydrating = false;
Expand All @@ -337,6 +347,7 @@ function tryToClaimNextHydratableInstance(fiber: Fiber): void {
}
const firstAttemptedInstance = nextInstance;
if (!tryHydrate(fiber, nextInstance)) {
throwOnHydrationMismatchIfConcurrentMode(fiber);
// If we can't hydrate this instance let's try the next one.
// We use this as a heuristic. It's based on intuition and not data so it
// might be flawed or unnecessary.
Expand Down
Loading

0 comments on commit 0464e6b

Please sign in to comment.