From 91e6ae2d8fa100ed347ef2ec4185a01e95edf93c Mon Sep 17 00:00:00 2001 From: Marco Salazar Date: Wed, 17 Nov 2021 15:52:23 -0500 Subject: [PATCH 1/4] Add flag for new client-render fallback behavior on hydration mismatch --- .../ReactDOMServerPartialHydration-test.internal.js | 12 +++++++++++- .../src/ReactFiberHydrationContext.new.js | 12 +++++++++--- .../src/ReactFiberHydrationContext.old.js | 12 +++++++++--- packages/shared/ReactFeatureFlags.js | 2 ++ packages/shared/forks/ReactFeatureFlags.native-fb.js | 1 + .../shared/forks/ReactFeatureFlags.native-oss.js | 1 + .../shared/forks/ReactFeatureFlags.test-renderer.js | 1 + .../forks/ReactFeatureFlags.test-renderer.native.js | 1 + .../forks/ReactFeatureFlags.test-renderer.www.js | 1 + packages/shared/forks/ReactFeatureFlags.testing.js | 1 + .../shared/forks/ReactFeatureFlags.testing.www.js | 1 + .../shared/forks/ReactFeatureFlags.www-dynamic.js | 1 + packages/shared/forks/ReactFeatureFlags.www.js | 1 + 13 files changed, 40 insertions(+), 7 deletions(-) diff --git a/packages/react-dom/src/__tests__/ReactDOMServerPartialHydration-test.internal.js b/packages/react-dom/src/__tests__/ReactDOMServerPartialHydration-test.internal.js index 25ec3c590ecb5..8833bcb737da7 100644 --- a/packages/react-dom/src/__tests__/ReactDOMServerPartialHydration-test.internal.js +++ b/packages/react-dom/src/__tests__/ReactDOMServerPartialHydration-test.internal.js @@ -209,12 +209,22 @@ describe('ReactDOMServerPartialHydration', () => { suspend = false; resolve(); await promise; - Scheduler.unstable_flushAll(); + if (gate(flags => flags.enableClientRenderFallbackOnHydrationMismatch)) { + Scheduler.unstable_flushAll(); + } else { + 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
.', + ); + } jest.runAllTimers(); // Hydration should not change anything. expect(container.textContent).toBe('HelloHello'); }); + // @gate enableClientRenderFallbackOnHydrationMismatch it('falls back to client rendering boundary on mismatch', async () => { let client = false; let suspend = false; diff --git a/packages/react-reconciler/src/ReactFiberHydrationContext.new.js b/packages/react-reconciler/src/ReactFiberHydrationContext.new.js index 5e915579a7c0d..614c6c9d946ca 100644 --- a/packages/react-reconciler/src/ReactFiberHydrationContext.new.js +++ b/packages/react-reconciler/src/ReactFiberHydrationContext.new.js @@ -62,7 +62,10 @@ import { didNotFindHydratableTextInstance, didNotFindHydratableSuspenseInstance, } from './ReactFiberHostConfig'; -import {enableSuspenseServerRenderer} from 'shared/ReactFeatureFlags'; +import { + enableClientRenderFallbackOnHydrationMismatch, + enableSuspenseServerRenderer, +} from 'shared/ReactFeatureFlags'; import {OffscreenLane} from './ReactFiberLane.new'; import { getSuspendedTreeContext, @@ -324,8 +327,11 @@ function tryHydrate(fiber, nextInstance) { } } -function throwOnHydrationMismatchIfConcurrentMode(fiber) { - if ((fiber.mode & ConcurrentMode) !== NoMode) { +function throwOnHydrationMismatchIfConcurrentMode(fiber: Fiber) { + if ( + enableClientRenderFallbackOnHydrationMismatch && + (fiber.mode & ConcurrentMode) !== NoMode + ) { throw new Error( 'An error occurred during hydration. The server HTML was replaced with client content', ); diff --git a/packages/react-reconciler/src/ReactFiberHydrationContext.old.js b/packages/react-reconciler/src/ReactFiberHydrationContext.old.js index e7e08d5f3b8bb..b7bca8217d979 100644 --- a/packages/react-reconciler/src/ReactFiberHydrationContext.old.js +++ b/packages/react-reconciler/src/ReactFiberHydrationContext.old.js @@ -62,7 +62,10 @@ import { didNotFindHydratableTextInstance, didNotFindHydratableSuspenseInstance, } from './ReactFiberHostConfig'; -import {enableSuspenseServerRenderer} from 'shared/ReactFeatureFlags'; +import { + enableClientRenderFallbackOnHydrationMismatch, + enableSuspenseServerRenderer, +} from 'shared/ReactFeatureFlags'; import {OffscreenLane} from './ReactFiberLane.old'; import { getSuspendedTreeContext, @@ -324,8 +327,11 @@ function tryHydrate(fiber, nextInstance) { } } -function throwOnHydrationMismatchIfConcurrentMode(fiber) { - if ((fiber.mode & ConcurrentMode) !== NoMode) { +function throwOnHydrationMismatchIfConcurrentMode(fiber: Fiber) { + if ( + enableClientRenderFallbackOnHydrationMismatch && + (fiber.mode & ConcurrentMode) !== NoMode + ) { throw new Error( 'An error occurred during hydration. The server HTML was replaced with client content', ); diff --git a/packages/shared/ReactFeatureFlags.js b/packages/shared/ReactFeatureFlags.js index ede13f2c2e8d4..a5d8f214bfff9 100644 --- a/packages/shared/ReactFeatureFlags.js +++ b/packages/shared/ReactFeatureFlags.js @@ -105,6 +105,8 @@ export const enableSuspenseAvoidThisFallback = false; export const enableCapturePhaseSelectiveHydrationWithoutDiscreteEventReplay = false; +export const enableClientRenderFallbackOnHydrationMismatch = true; + export const enableComponentStackLocations = true; export const enableNewReconciler = false; diff --git a/packages/shared/forks/ReactFeatureFlags.native-fb.js b/packages/shared/forks/ReactFeatureFlags.native-fb.js index 221e94fb855bb..5994f0eefcd65 100644 --- a/packages/shared/forks/ReactFeatureFlags.native-fb.js +++ b/packages/shared/forks/ReactFeatureFlags.native-fb.js @@ -51,6 +51,7 @@ export const warnAboutSpreadingKeyToJSX = false; export const warnOnSubscriptionInsideStartTransition = false; export const enableSuspenseAvoidThisFallback = false; export const enableCapturePhaseSelectiveHydrationWithoutDiscreteEventReplay = false; +export const enableClientRenderFallbackOnHydrationMismatch = true; export const enableComponentStackLocations = false; export const enableLegacyFBSupport = false; export const enableFilterEmptyStringAttributesDOM = false; diff --git a/packages/shared/forks/ReactFeatureFlags.native-oss.js b/packages/shared/forks/ReactFeatureFlags.native-oss.js index 7e24f2a0a2710..aac8579f0ed6b 100644 --- a/packages/shared/forks/ReactFeatureFlags.native-oss.js +++ b/packages/shared/forks/ReactFeatureFlags.native-oss.js @@ -42,6 +42,7 @@ export const warnAboutSpreadingKeyToJSX = false; export const warnOnSubscriptionInsideStartTransition = false; export const enableSuspenseAvoidThisFallback = false; export const enableCapturePhaseSelectiveHydrationWithoutDiscreteEventReplay = false; +export const enableClientRenderFallbackOnHydrationMismatch = true; export const enableComponentStackLocations = false; export const enableLegacyFBSupport = false; export const enableFilterEmptyStringAttributesDOM = false; diff --git a/packages/shared/forks/ReactFeatureFlags.test-renderer.js b/packages/shared/forks/ReactFeatureFlags.test-renderer.js index bce24128a7911..853907907e91d 100644 --- a/packages/shared/forks/ReactFeatureFlags.test-renderer.js +++ b/packages/shared/forks/ReactFeatureFlags.test-renderer.js @@ -42,6 +42,7 @@ export const warnAboutSpreadingKeyToJSX = false; export const warnOnSubscriptionInsideStartTransition = false; export const enableSuspenseAvoidThisFallback = false; export const enableCapturePhaseSelectiveHydrationWithoutDiscreteEventReplay = false; +export const enableClientRenderFallbackOnHydrationMismatch = true; export const enableComponentStackLocations = true; export const enableLegacyFBSupport = false; export const enableFilterEmptyStringAttributesDOM = false; diff --git a/packages/shared/forks/ReactFeatureFlags.test-renderer.native.js b/packages/shared/forks/ReactFeatureFlags.test-renderer.native.js index 02f063e58a822..e2431deb9e21a 100644 --- a/packages/shared/forks/ReactFeatureFlags.test-renderer.native.js +++ b/packages/shared/forks/ReactFeatureFlags.test-renderer.native.js @@ -52,6 +52,7 @@ export const deferRenderPhaseUpdateToNextBatch = false; export const warnOnSubscriptionInsideStartTransition = false; export const enableSuspenseAvoidThisFallback = false; export const enableCapturePhaseSelectiveHydrationWithoutDiscreteEventReplay = false; +export const enableClientRenderFallbackOnHydrationMismatch = true; export const enableStrictEffects = false; export const createRootStrictEffectsByDefault = false; export const enableUseRefAccessWarning = false; diff --git a/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js b/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js index 7227c254dccbe..63d6afaeb416b 100644 --- a/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js +++ b/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js @@ -42,6 +42,7 @@ export const warnAboutSpreadingKeyToJSX = false; export const warnOnSubscriptionInsideStartTransition = false; export const enableSuspenseAvoidThisFallback = false; export const enableCapturePhaseSelectiveHydrationWithoutDiscreteEventReplay = false; +export const enableClientRenderFallbackOnHydrationMismatch = true; export const enableComponentStackLocations = true; export const enableLegacyFBSupport = false; export const enableFilterEmptyStringAttributesDOM = false; diff --git a/packages/shared/forks/ReactFeatureFlags.testing.js b/packages/shared/forks/ReactFeatureFlags.testing.js index 264127fc197ef..aaee18ee6ba39 100644 --- a/packages/shared/forks/ReactFeatureFlags.testing.js +++ b/packages/shared/forks/ReactFeatureFlags.testing.js @@ -42,6 +42,7 @@ export const warnAboutSpreadingKeyToJSX = false; export const warnOnSubscriptionInsideStartTransition = false; export const enableSuspenseAvoidThisFallback = false; export const enableCapturePhaseSelectiveHydrationWithoutDiscreteEventReplay = false; +export const enableClientRenderFallbackOnHydrationMismatch = true; export const enableComponentStackLocations = true; export const enableLegacyFBSupport = false; export const enableFilterEmptyStringAttributesDOM = false; diff --git a/packages/shared/forks/ReactFeatureFlags.testing.www.js b/packages/shared/forks/ReactFeatureFlags.testing.www.js index 87d7247d6eebe..b1c0b679d5441 100644 --- a/packages/shared/forks/ReactFeatureFlags.testing.www.js +++ b/packages/shared/forks/ReactFeatureFlags.testing.www.js @@ -42,6 +42,7 @@ export const warnAboutSpreadingKeyToJSX = false; export const warnOnSubscriptionInsideStartTransition = false; export const enableSuspenseAvoidThisFallback = false; export const enableCapturePhaseSelectiveHydrationWithoutDiscreteEventReplay = false; +export const enableClientRenderFallbackOnHydrationMismatch = true; export const enableComponentStackLocations = true; export const enableLegacyFBSupport = !__EXPERIMENTAL__; export const enableFilterEmptyStringAttributesDOM = false; diff --git a/packages/shared/forks/ReactFeatureFlags.www-dynamic.js b/packages/shared/forks/ReactFeatureFlags.www-dynamic.js index ef980529f3f21..288e9631c75e1 100644 --- a/packages/shared/forks/ReactFeatureFlags.www-dynamic.js +++ b/packages/shared/forks/ReactFeatureFlags.www-dynamic.js @@ -28,6 +28,7 @@ export const enableSyncDefaultUpdates = __VARIANT__; export const consoleManagedByDevToolsDuringStrictMode = __VARIANT__; export const warnOnSubscriptionInsideStartTransition = __VARIANT__; export const enableCapturePhaseSelectiveHydrationWithoutDiscreteEventReplay = __VARIANT__; +export const enableClientRenderFallbackOnHydrationMismatch = __VARIANT__; // Enable this flag to help with concurrent mode debugging. // It logs information to the console about React scheduling, rendering, and commit phases. diff --git a/packages/shared/forks/ReactFeatureFlags.www.js b/packages/shared/forks/ReactFeatureFlags.www.js index e280e1ff9a6d0..7c17ae8b5cfb5 100644 --- a/packages/shared/forks/ReactFeatureFlags.www.js +++ b/packages/shared/forks/ReactFeatureFlags.www.js @@ -33,6 +33,7 @@ export const { enableSyncDefaultUpdates, warnOnSubscriptionInsideStartTransition, enableCapturePhaseSelectiveHydrationWithoutDiscreteEventReplay, + enableClientRenderFallbackOnHydrationMismatch, } = dynamicFeatureFlags; // On WWW, __EXPERIMENTAL__ is used for a new modern build. From a88213096d2e702a79586acdb920a6def8b0d5b7 Mon Sep 17 00:00:00 2001 From: Marco Salazar Date: Wed, 17 Nov 2021 16:20:54 -0500 Subject: [PATCH 2/4] gate test --- .../src/__tests__/ReactDOMFizzServer-test.js | 32 ++++++++++++++----- 1 file changed, 24 insertions(+), 8 deletions(-) diff --git a/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js b/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js index 6d169f9fbbb17..c4e7147a71345 100644 --- a/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js @@ -1790,15 +1790,31 @@ describe('ReactDOMFizzServer', () => { ReactDOM.hydrateRoot(container, ); - // The first paint uses the client due to mismatch forcing client render - expect(() => { - // The first paint switches to client rendering due to mismatch + if (gate(flags => flags.enableClientRenderFallbackOnHydrationMismatch)) { + // 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(
client
); + } else { + const serverRenderedDiv = container.getElementsByTagName('div')[0]; + + // The first paint uses the server snapshot + expect(Scheduler).toFlushUntilNextPaint(['server']); + expect(getVisibleChildren(container)).toEqual(
server
); + // 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']); - }).toErrorDev( - 'Warning: An error occurred during hydration. The server HTML was replaced with client content', - {withoutStack: true}, - ); - expect(getVisibleChildren(container)).toEqual(
client
); + expect(getVisibleChildren(container)).toEqual(
client
); + expect(ref.current).toEqual(serverRenderedDiv); + } }); // @gate experimental From 3693f5f492f5c43ebcd4d119e2f1c7032f48d581 Mon Sep 17 00:00:00 2001 From: Marco Salazar Date: Wed, 17 Nov 2021 16:39:20 -0500 Subject: [PATCH 3/4] gate tests too --- .../src/__tests__/ReactDOMFizzServer-test.js | 38 +++++++++++++++---- 1 file changed, 30 insertions(+), 8 deletions(-) diff --git a/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js b/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js index c4e7147a71345..ca592002ad25e 100644 --- a/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js @@ -1680,19 +1680,25 @@ 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, @@ -1700,11 +1706,12 @@ describe('ReactDOMFizzServer', () => { getServerSnapshot, ); return ( -
+
); } + const loggedErrors = []; await act(async () => { const {pipe} = ReactDOMFizzServer.renderToPipeableStream( @@ -1723,14 +1730,29 @@ describe('ReactDOMFizzServer', () => { ReactDOM.hydrateRoot(container, ); - expect(() => { - // The first paint switches to client rendering due to mismatch + if (gate(flags => flags.enableClientRenderFallbackOnHydrationMismatch)) { + 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(
client
); + } else { + const serverRenderedDiv = container.getElementsByTagName('div')[0]; + // The first paint uses the server snapshot + expect(Scheduler).toFlushUntilNextPaint(['server']); + expect(getVisibleChildren(container)).toEqual(
server
); + // 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']); - }).toErrorDev( - 'Warning: An error occurred during hydration. The server HTML was replaced with client content', - {withoutStack: true}, - ); - expect(getVisibleChildren(container)).toEqual(
client
); + expect(getVisibleChildren(container)).toEqual(
client
); + expect(ref.current).toEqual(serverRenderedDiv); + } }); // The selector implementation uses the lazy ref initialization pattern From d5924444fdcb1cb6577ab0cf67e7f49bd40946ee Mon Sep 17 00:00:00 2001 From: Marco Salazar Date: Wed, 17 Nov 2021 16:51:01 -0500 Subject: [PATCH 4/4] fix test gating --- ...DOMServerPartialHydration-test.internal.js | 20 +++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/packages/react-dom/src/__tests__/ReactDOMServerPartialHydration-test.internal.js b/packages/react-dom/src/__tests__/ReactDOMServerPartialHydration-test.internal.js index 8833bcb737da7..f6bee806f3ac2 100644 --- a/packages/react-dom/src/__tests__/ReactDOMServerPartialHydration-test.internal.js +++ b/packages/react-dom/src/__tests__/ReactDOMServerPartialHydration-test.internal.js @@ -199,16 +199,6 @@ describe('ReactDOMServerPartialHydration', () => { // hydrating anyway. suspend = true; ReactDOM.hydrateRoot(container, ); - Scheduler.unstable_flushAll(); - jest.runAllTimers(); - - // Expect the server-generated HTML to stay intact. - expect(container.textContent).toBe('HelloHello'); - - // Resolving the promise should continue hydration - suspend = false; - resolve(); - await promise; if (gate(flags => flags.enableClientRenderFallbackOnHydrationMismatch)) { Scheduler.unstable_flushAll(); } else { @@ -220,6 +210,16 @@ describe('ReactDOMServerPartialHydration', () => { ); } jest.runAllTimers(); + + // Expect the server-generated HTML to stay intact. + expect(container.textContent).toBe('HelloHello'); + + // Resolving the promise should continue hydration + suspend = false; + resolve(); + await promise; + Scheduler.unstable_flushAll(); + jest.runAllTimers(); // Hydration should not change anything. expect(container.textContent).toBe('HelloHello'); });