@samitouri / QOS-React-2 / commits / 9ae80d6a2b

Suppress hydration warnings when a preceding sibling suspends (#24404)

* Add failing test case for #24384 If a components suspends during hydration we expect there to be mismatches with server rendered HTML but we were not always supressing warning messages related to these expected mismatches * Mark hydration as suspending on every thrownException previously hydration would only be marked as supsending when a genuine error was thrown. This created an opportunity for a hydration mismatch that would warn after which later hydration mismatches would not lead to warnings. By moving the marker check earlier in the thrownException function we get the hydration context to enter the didSuspend state on both error and thrown promise cases which eliminates this gap. * Fix failing test related to client render fallbacks This test was actually subject to the project identified in the issue fixed in this branch. After fixing the underlying issue the assertion logic needed to change to pick the right warning which now emits after hydration successfully completes on promise resolution. I changed the container type to 'section' to make the error message slightly easier to read/understand (for me) * Only mark didSuspend on suspense path For unknown reasons the didSuspend was being set only on the error path and nto the suspense path. The original change hoisted this to happen on both paths. This change moves the didSuspend call to the suspense path only. This appears to be a noop because if the error path marked didSuspend it would suppress later warnings but if it does not the warning functions themsevles do that suppression (after the first one which necessarily already happened) * gate test on hydration fallback flags * refactor didSuspend to didSuspendOrError the orignial behavior applied the hydration warning bailout to error paths only. originally I moved it to Suspense paths only but this commit restores it to both paths and renames the marker function as didThrow rather than didSuspend The logic here is that for either case if we get a mismatch in hydration we want to warm up components but effectively consider the hydration for this boundary halted * factor tests to assert different behavior between prod and dev * add DEV suffix to didSuspendOrError to better indicate this feature should only affect dev behavior * move tests back to ReactDOMFizzServer-test * fix comment casing * drop extra flag gates in tests * add test for user error case * remove unnecessary gate * Make test better it now has an intentional client mismatch that would error if there wasn't suppression brought about by the earlier error. when it client renders it has the updated value not found in the server response but we do not see a hydration warning because it was superseded by the thrown error in that render

Josh Story committed Apr 20, 2022 at 14:52 UTC 9ae80d6a2bf8f48f20e3d62b9672f21c1ff77bd8
6 files changed +290 -29
packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js
+243
@@ -2841,4 +2841,247 @@ describe('ReactDOMFizzServer', () => {
2841 expect(window.__test_outlet).toBe(1);
2842 });
2843 });
2844 +
2845 + // @gate experimental && enableClientRenderFallbackOnTextMismatch
2846 + it('#24384: Suspending should halt hydration warnings while still allowing siblings to warm up', async () => {
2847 + const makeApp = () => {
2848 + let resolve, resolved;
2849 + const promise = new Promise(r => {
2850 + resolve = () => {
2851 + resolved = true;
2852 + return r();
2853 + };
2854 + });
2855 + function ComponentThatSuspends() {
2856 + if (!resolved) {
2857 + throw promise;
2858 + }
2859 + return <p>A</p>;
2860 + }
2861 +
2862 + const App = ({text}) => {
2863 + return (
2864 + <div>
2865 + <Suspense fallback={<h1>Loading...</h1>}>
2866 + <ComponentThatSuspends />
2867 + <h2 name={text}>{text}</h2>
2868 + </Suspense>
2869 + </div>
2870 + );
2871 + };
2872 +
2873 + return [App, resolve];
2874 + };
2875 +
2876 + const [ServerApp, serverResolve] = makeApp();
2877 + await act(async () => {
2878 + const {pipe} = ReactDOMFizzServer.renderToPipeableStream(
2879 + <ServerApp text="initial" />,
2880 + );
2881 + pipe(writable);
2882 + });
2883 + await act(() => {
2884 + serverResolve();
2885 + });
2886 +
2887 + expect(getVisibleChildren(container)).toEqual(
2888 + <div>
2889 + <p>A</p>
2890 + <h2 name="initial">initial</h2>
2891 + </div>,
2892 + );
2893 +
2894 + // The client app is rendered with an intentionally incorrect text. The still Suspended component causes
2895 + // hydration to fail silently (allowing for cache warming but otherwise skipping this boundary) until it
2896 + // resolves.
2897 + const [ClientApp, clientResolve] = makeApp();
2898 + ReactDOMClient.hydrateRoot(container, <ClientApp text="replaced" />, {
2899 + onRecoverableError(error) {
2900 + Scheduler.unstable_yieldValue(
2901 + 'Logged recoverable error: ' + error.message,
2902 + );
2903 + },
2904 + });
2905 + Scheduler.unstable_flushAll();
2906 +
2907 + expect(getVisibleChildren(container)).toEqual(
2908 + <div>
2909 + <p>A</p>
2910 + <h2 name="initial">initial</h2>
2911 + </div>,
2912 + );
2913 +
2914 + // Now that the boundary resolves to it's children the hydration completes and discovers that there is a mismatch requiring
2915 + // client-side rendering.
2916 + await clientResolve();
2917 + expect(() => {
2918 + expect(Scheduler).toFlushAndYield([
2919 + 'Logged recoverable error: Text content does not match server-rendered HTML.',
2920 + 'Logged recoverable error: There was an error while hydrating this Suspense boundary. Switched to client rendering.',
2921 + ]);
2922 + }).toErrorDev(
2923 + 'Warning: Prop `name` did not match. Server: "initial" Client: "replaced"',
2924 + );
2925 + expect(getVisibleChildren(container)).toEqual(
2926 + <div>
2927 + <p>A</p>
2928 + <h2 name="replaced">replaced</h2>
2929 + </div>,
2930 + );
2931 +
2932 + expect(Scheduler).toFlushAndYield([]);
2933 + });
2934 +
2935 + // @gate experimental && enableClientRenderFallbackOnTextMismatch
2936 + it('only warns once on hydration mismatch while within a suspense boundary', async () => {
2937 + const originalConsoleError = console.error;
2938 + const mockError = jest.fn();
2939 + console.error = (...args) => {
2940 + mockError(...args.map(normalizeCodeLocInfo));
2941 + };
2942 +
2943 + const App = ({text}) => {
2944 + return (
2945 + <div>
2946 + <Suspense fallback={<h1>Loading...</h1>}>
2947 + <h2>{text}</h2>
2948 + <h2>{text}</h2>
2949 + <h2>{text}</h2>
2950 + </Suspense>
2951 + </div>
2952 + );
2953 + };
2954 +
2955 + try {
2956 + await act(async () => {
2957 + const {pipe} = ReactDOMFizzServer.renderToPipeableStream(
2958 + <App text="initial" />,
2959 + );
2960 + pipe(writable);
2961 + });
2962 +
2963 + expect(getVisibleChildren(container)).toEqual(
2964 + <div>
2965 + <h2>initial</h2>
2966 + <h2>initial</h2>
2967 + <h2>initial</h2>
2968 + </div>,
2969 + );
2970 +
2971 + ReactDOMClient.hydrateRoot(container, <App text="replaced" />, {
2972 + onRecoverableError(error) {
2973 + Scheduler.unstable_yieldValue(
2974 + 'Logged recoverable error: ' + error.message,
2975 + );
2976 + },
2977 + });
2978 + expect(Scheduler).toFlushAndYield([
2979 + 'Logged recoverable error: Text content does not match server-rendered HTML.',
2980 + 'Logged recoverable error: Text content does not match server-rendered HTML.',
2981 + 'Logged recoverable error: Text content does not match server-rendered HTML.',
2982 + 'Logged recoverable error: There was an error while hydrating this Suspense boundary. Switched to client rendering.',
2983 + ]);
2984 +
2985 + expect(getVisibleChildren(container)).toEqual(
2986 + <div>
2987 + <h2>replaced</h2>
2988 + <h2>replaced</h2>
2989 + <h2>replaced</h2>
2990 + </div>,
2991 + );
2992 +
2993 + expect(Scheduler).toFlushAndYield([]);
2994 + if (__DEV__) {
2995 + expect(mockError.mock.calls.length).toBe(1);
2996 + expect(mockError.mock.calls[0]).toEqual([
2997 + 'Warning: Text content did not match. Server: "%s" Client: "%s"%s',
2998 + 'initial',
2999 + 'replaced',
3000 + '\n' +
3001 + ' in h2 (at **)\n' +
3002 + ' in Suspense (at **)\n' +
3003 + ' in div (at **)\n' +
3004 + ' in App (at **)',
3005 + ]);
3006 + } else {
3007 + expect(mockError.mock.calls.length).toBe(0);
3008 + }
3009 + } finally {
3010 + console.error = originalConsoleError;
3011 + }
3012 + });
3013 +
3014 + // @gate experimental
3015 + it('supresses hydration warnings when an error occurs within a Suspense boundary', async () => {
3016 + let isClient = false;
3017 + let shouldThrow = true;
3018 +
3019 + function ThrowUntilOnClient({children}) {
3020 + if (isClient && shouldThrow) {
3021 + throw new Error('uh oh');
3022 + }
3023 + return children;
3024 + }
3025 +
3026 + function StopThrowingOnClient() {
3027 + if (isClient) {
3028 + shouldThrow = false;
3029 + }
3030 + return null;
3031 + }
3032 +
3033 + const App = () => {
3034 + return (
3035 + <div>
3036 + <Suspense fallback={<h1>Loading...</h1>}>
3037 + <ThrowUntilOnClient>
3038 + <h1>one</h1>
3039 + </ThrowUntilOnClient>
3040 + <h2>two</h2>
3041 + <h3>{isClient ? 'five' : 'three'}</h3>
3042 + <StopThrowingOnClient />
3043 + </Suspense>
3044 + </div>
3045 + );
3046 + };
3047 +
3048 + await act(async () => {
3049 + const {pipe} = ReactDOMFizzServer.renderToPipeableStream(<App />);
3050 + pipe(writable);
3051 + });
3052 +
3053 + expect(getVisibleChildren(container)).toEqual(
3054 + <div>
3055 + <h1>one</h1>
3056 + <h2>two</h2>
3057 + <h3>three</h3>
3058 + </div>,
3059 + );
3060 +
3061 + isClient = true;
3062 +
3063 + ReactDOMClient.hydrateRoot(container, <App />, {
3064 + onRecoverableError(error) {
3065 + Scheduler.unstable_yieldValue(
3066 + 'Logged recoverable error: ' + error.message,
3067 + );
3068 + },
3069 + });
3070 + expect(Scheduler).toFlushAndYield([
3071 + 'Logged recoverable error: uh oh',
3072 + 'Logged recoverable error: Hydration failed because the initial UI does not match what was rendered on the server.',
3073 + 'Logged recoverable error: Hydration failed because the initial UI does not match what was rendered on the server.',
3074 + 'Logged recoverable error: There was an error while hydrating this Suspense boundary. Switched to client rendering.',
3075 + ]);
3076 +
3077 + expect(getVisibleChildren(container)).toEqual(
3078 + <div>
3079 + <h1>one</h1>
3080 + <h2>two</h2>
3081 + <h3>five</h3>
3082 + </div>,
3083 + );
3084 +
3085 + expect(Scheduler).toFlushAndYield([]);
3086 + });
3087 });
packages/react-dom/src/__tests__/ReactDOMServerPartialHydration-test.internal.js
+7 -5
@@ -285,7 +285,7 @@ describe('ReactDOMServerPartialHydration', () => {
285 }
286 try {
287 const finalHTML = ReactDOMServer.renderToString(<App />);
288 - const container = document.createElement('div');
288 + const container = document.createElement('section');
289 container.innerHTML = finalHTML;
290 expect(Scheduler).toHaveYielded([
291 'Hello',
@@ -350,12 +350,14 @@ describe('ReactDOMServerPartialHydration', () => {
350 );
351
352 if (__DEV__) {
353 - expect(mockError.mock.calls[0]).toEqual([
353 + const secondToLastCall =
354 + mockError.mock.calls[mockError.mock.calls.length - 2];
355 + expect(secondToLastCall).toEqual([
356 'Warning: Expected server HTML to contain a matching <%s> in <%s>.%s',
355 - 'div',
356 - 'div',
357 + 'article',
358 + 'section',
359 '\n' +
358 - ' in div (at **)\n' +
360 + ' in article (at **)\n' +
361 ' in Component (at **)\n' +
362 ' in Suspense (at **)\n' +
363 ' in App (at **)',
packages/react-reconciler/src/ReactFiberHydrationContext.new.js
+12 -9
@@ -80,7 +80,10 @@ import {queueRecoverableErrors} from './ReactFiberWorkLoop.new';
80 let hydrationParentFiber: null | Fiber = null;
81 let nextHydratableInstance: null | HydratableInstance = null;
82 let isHydrating: boolean = false;
83 -let didSuspend: boolean = false;
83 +
84 +// This flag allows for warning supression when we expect there to be mismatches
85 +// due to earlier mismatches or a suspended fiber.
86 +let didSuspendOrErrorDEV: boolean = false;
87
88 // Hydration errors that were thrown inside this boundary
89 let hydrationErrors: Array<mixed> | null = null;
@@ -95,9 +98,9 @@ function warnIfHydrating() {
98 }
99 }
100
98 -export function markDidSuspendWhileHydratingDEV() {
101 +export function markDidThrowWhileHydratingDEV() {
102 if (__DEV__) {
100 - didSuspend = true;
103 + didSuspendOrErrorDEV = true;
104 }
105 }
106
@@ -113,7 +116,7 @@ function enterHydrationState(fiber: Fiber): boolean {
116 hydrationParentFiber = fiber;
117 isHydrating = true;
118 hydrationErrors = null;
116 - didSuspend = false;
119 + didSuspendOrErrorDEV = false;
120 return true;
121 }
122
@@ -131,7 +134,7 @@ function reenterHydrationStateFromDehydratedSuspenseInstance(
134 hydrationParentFiber = fiber;
135 isHydrating = true;
136 hydrationErrors = null;
134 - didSuspend = false;
137 + didSuspendOrErrorDEV = false;
138 if (treeContext !== null) {
139 restoreSuspendedTreeContext(fiber, treeContext);
140 }
@@ -196,7 +199,7 @@ function deleteHydratableInstance(
199
200 function warnNonhydratedInstance(returnFiber: Fiber, fiber: Fiber) {
201 if (__DEV__) {
199 - if (didSuspend) {
202 + if (didSuspendOrErrorDEV) {
203 // Inside a boundary that already suspended. We're currently rendering the
204 // siblings of a suspended node. The mismatch may be due to the missing
205 // data, so it's probably a false positive.
@@ -444,7 +447,7 @@ function prepareToHydrateHostInstance(
447 }
448
449 const instance: Instance = fiber.stateNode;
447 - const shouldWarnIfMismatchDev = !didSuspend;
450 + const shouldWarnIfMismatchDev = !didSuspendOrErrorDEV;
451 const updatePayload = hydrateInstance(
452 instance,
453 fiber.type,
@@ -474,7 +477,7 @@ function prepareToHydrateHostTextInstance(fiber: Fiber): boolean {
477
478 const textInstance: TextInstance = fiber.stateNode;
479 const textContent: string = fiber.memoizedProps;
477 - const shouldWarnIfMismatchDev = !didSuspend;
480 + const shouldWarnIfMismatchDev = !didSuspendOrErrorDEV;
481 const shouldUpdate = hydrateTextInstance(
482 textInstance,
483 textContent,
@@ -653,7 +656,7 @@ function resetHydrationState(): void {
656 hydrationParentFiber = null;
657 nextHydratableInstance = null;
658 isHydrating = false;
656 - didSuspend = false;
659 + didSuspendOrErrorDEV = false;
660 }
661
662 export function upgradeHydrationErrorsToRecoverable(): void {
packages/react-reconciler/src/ReactFiberHydrationContext.old.js
+12 -9
@@ -80,7 +80,10 @@ import {queueRecoverableErrors} from './ReactFiberWorkLoop.old';
80 let hydrationParentFiber: null | Fiber = null;
81 let nextHydratableInstance: null | HydratableInstance = null;
82 let isHydrating: boolean = false;
83 -let didSuspend: boolean = false;
83 +
84 +// This flag allows for warning supression when we expect there to be mismatches
85 +// due to earlier mismatches or a suspended fiber.
86 +let didSuspendOrErrorDEV: boolean = false;
87
88 // Hydration errors that were thrown inside this boundary
89 let hydrationErrors: Array<mixed> | null = null;
@@ -95,9 +98,9 @@ function warnIfHydrating() {
98 }
99 }
100
98 -export function markDidSuspendWhileHydratingDEV() {
101 +export function markDidThrowWhileHydratingDEV() {
102 if (__DEV__) {
100 - didSuspend = true;
103 + didSuspendOrErrorDEV = true;
104 }
105 }
106
@@ -113,7 +116,7 @@ function enterHydrationState(fiber: Fiber): boolean {
116 hydrationParentFiber = fiber;
117 isHydrating = true;
118 hydrationErrors = null;
116 - didSuspend = false;
119 + didSuspendOrErrorDEV = false;
120 return true;
121 }
122
@@ -131,7 +134,7 @@ function reenterHydrationStateFromDehydratedSuspenseInstance(
134 hydrationParentFiber = fiber;
135 isHydrating = true;
136 hydrationErrors = null;
134 - didSuspend = false;
137 + didSuspendOrErrorDEV = false;
138 if (treeContext !== null) {
139 restoreSuspendedTreeContext(fiber, treeContext);
140 }
@@ -196,7 +199,7 @@ function deleteHydratableInstance(
199
200 function warnNonhydratedInstance(returnFiber: Fiber, fiber: Fiber) {
201 if (__DEV__) {
199 - if (didSuspend) {
202 + if (didSuspendOrErrorDEV) {
203 // Inside a boundary that already suspended. We're currently rendering the
204 // siblings of a suspended node. The mismatch may be due to the missing
205 // data, so it's probably a false positive.
@@ -444,7 +447,7 @@ function prepareToHydrateHostInstance(
447 }
448
449 const instance: Instance = fiber.stateNode;
447 - const shouldWarnIfMismatchDev = !didSuspend;
450 + const shouldWarnIfMismatchDev = !didSuspendOrErrorDEV;
451 const updatePayload = hydrateInstance(
452 instance,
453 fiber.type,
@@ -474,7 +477,7 @@ function prepareToHydrateHostTextInstance(fiber: Fiber): boolean {
477
478 const textInstance: TextInstance = fiber.stateNode;
479 const textContent: string = fiber.memoizedProps;
477 - const shouldWarnIfMismatchDev = !didSuspend;
480 + const shouldWarnIfMismatchDev = !didSuspendOrErrorDEV;
481 const shouldUpdate = hydrateTextInstance(
482 textInstance,
483 textContent,
@@ -653,7 +656,7 @@ function resetHydrationState(): void {
656 hydrationParentFiber = null;
657 nextHydratableInstance = null;
658 isHydrating = false;
656 - didSuspend = false;
659 + didSuspendOrErrorDEV = false;
660 }
661
662 export function upgradeHydrationErrorsToRecoverable(): void {
packages/react-reconciler/src/ReactFiberThrow.new.js
+8 -3
@@ -83,7 +83,7 @@ import {
83 } from './ReactFiberLane.new';
84 import {
85 getIsHydrating,
86 - markDidSuspendWhileHydratingDEV,
86 + markDidThrowWhileHydratingDEV,
87 queueHydrationError,
88 } from './ReactFiberHydrationContext.new';
89
@@ -453,6 +453,12 @@ function throwException(
453 const wakeable: Wakeable = (value: any);
454 resetSuspendedComponent(sourceFiber, rootRenderLanes);
455
456 + if (__DEV__) {
457 + if (getIsHydrating() && sourceFiber.mode & ConcurrentMode) {
458 + markDidThrowWhileHydratingDEV();
459 + }
460 + }
461 +
462 if (__DEV__) {
463 if (enableDebugTracing) {
464 if (sourceFiber.mode & DebugTracingMode) {
@@ -514,8 +520,7 @@ function throwException(
520 } else {
521 // This is a regular error, not a Suspense wakeable.
522 if (getIsHydrating() && sourceFiber.mode & ConcurrentMode) {
517 - markDidSuspendWhileHydratingDEV();
518 -
523 + markDidThrowWhileHydratingDEV();
524 const suspenseBoundary = getNearestSuspenseBoundaryToCapture(returnFiber);
525 // If the error was thrown during hydration, we may be able to recover by
526 // discarding the dehydrated content and switching to a client render.
packages/react-reconciler/src/ReactFiberThrow.old.js
+8 -3
@@ -83,7 +83,7 @@ import {
83 } from './ReactFiberLane.old';
84 import {
85 getIsHydrating,
86 - markDidSuspendWhileHydratingDEV,
86 + markDidThrowWhileHydratingDEV,
87 queueHydrationError,
88 } from './ReactFiberHydrationContext.old';
89
@@ -453,6 +453,12 @@ function throwException(
453 const wakeable: Wakeable = (value: any);
454 resetSuspendedComponent(sourceFiber, rootRenderLanes);
455
456 + if (__DEV__) {
457 + if (getIsHydrating() && sourceFiber.mode & ConcurrentMode) {
458 + markDidThrowWhileHydratingDEV();
459 + }
460 + }
461 +
462 if (__DEV__) {
463 if (enableDebugTracing) {
464 if (sourceFiber.mode & DebugTracingMode) {
@@ -514,8 +520,7 @@ function throwException(
520 } else {
521 // This is a regular error, not a Suspense wakeable.
522 if (getIsHydrating() && sourceFiber.mode & ConcurrentMode) {
517 - markDidSuspendWhileHydratingDEV();
518 -
523 + markDidThrowWhileHydratingDEV();
524 const suspenseBoundary = getNearestSuspenseBoundaryToCapture(returnFiber);
525 // If the error was thrown during hydration, we may be able to recover by
526 // discarding the dehydrated content and switching to a client render.