@samitouri / QOS-React-2 / commits / b4eb0ad71f

Do not replay erroring beginWork with invokeGuardedCallback when suspended or previously errored (#24480)

When hydrating a suspense boundary an error or a suspending fiber can often lead to a cascade of hydration errors. While in many cases these errors are simply discarded (e.g. when teh root does not commit and we fall back to client rendering) the use of invokeGuardedCallback can lead to many of these errors appearing as uncaught in the browser console. This change avoids error replaying using invokeGuardedCallback when we are hydrating a suspense boundary and have either already suspended or we have one previous error which was replayed.

Josh Story committed May 3, 2022 at 11:07 UTC b4eb0ad71fb365cb760a5b9ab1a1e2dd6193fac7

5 files changed +468 -9

packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js

+440 -1

@@ -2850,8 +2850,84 @@ describe('ReactDOMFizzServer', () => {

2850 });

2851 });

2852

2853 + // @gate experimental

2854 + it('#24384: Suspending should halt hydration warnings and not emit any if hydration completes successfully after unsuspending', async () => {

2855 + const makeApp = () => {

2856 + let resolve, resolved;

2857 + const promise = new Promise(r => {

2858 + resolve = () => {

2859 + resolved = true;

2860 + return r();

2861 + };

2862 + });

2863 + function ComponentThatSuspends() {

2864 + if (!resolved) {

2865 + throw promise;

2866 + }

2867 + return <p>A</p>;

2868 + }

2869 +

2870 + const App = () => {

2871 + return (

2872 + <div>

2873 + <Suspense fallback={<h1>Loading...</h1>}>

2874 + <ComponentThatSuspends />

2875 + <h2 name="hello">world</h2>

2876 + </Suspense>

2877 + </div>

2878 + );

2879 + };

2880 +

2881 + return [App, resolve];

2882 + };

2883 +

2884 + const [ServerApp, serverResolve] = makeApp();

2885 + await act(async () => {

2886 + const {pipe} = ReactDOMFizzServer.renderToPipeableStream(<ServerApp />);

2887 + pipe(writable);

2888 + });

2889 + await act(() => {

2890 + serverResolve();

2891 + });

2892 +

2893 + expect(getVisibleChildren(container)).toEqual(

2894 + <div>

2895 + <p>A</p>

2896 + <h2 name="hello">world</h2>

2897 + </div>,

2898 + );

2899 +

2900 + const [ClientApp, clientResolve] = makeApp();

2901 + ReactDOMClient.hydrateRoot(container, <ClientApp />, {

2902 + onRecoverableError(error) {

2903 + Scheduler.unstable_yieldValue(

2904 + 'Logged recoverable error: ' + error.message,

2905 + );

2906 + },

2907 + });

2908 + Scheduler.unstable_flushAll();

2909 +

2910 + expect(getVisibleChildren(container)).toEqual(

2911 + <div>

2912 + <p>A</p>

2913 + <h2 name="hello">world</h2>

2914 + </div>,

2915 + );

2916 +

2917 + // Now that the boundary resolves to it's children the hydration completes and discovers that there is a mismatch requiring

2918 + // client-side rendering.

2919 + await clientResolve();

2920 + expect(Scheduler).toFlushWithoutYielding();

2921 + expect(getVisibleChildren(container)).toEqual(

2922 + <div>

2923 + <p>A</p>

2924 + <h2 name="hello">world</h2>

2925 + </div>,

2926 + );

2927 + });

2928 +

2929 // @gate experimental && enableClientRenderFallbackOnTextMismatch

2854 - it('#24384: Suspending should halt hydration warnings while still allowing siblings to warm up', async () => {

2930 + it('#24384: Suspending should halt hydration warnings but still emit hydration warnings after unsuspending if mismatches are genuine', async () => {

2931 const makeApp = () => {

2932 let resolve, resolved;

2933 const promise = new Promise(r => {

@@ -3092,4 +3168,367 @@ describe('ReactDOMFizzServer', () => {

3168

3169 expect(Scheduler).toFlushAndYield([]);

3170 });

3171 +

3172 + // @gate experimental && __DEV__

3173 + it('does not invokeGuardedCallback for errors after the first hydration error', async () => {

3174 + // We can't use the toErrorDev helper here because this is async.

3175 + const originalConsoleError = console.error;

3176 + const mockError = jest.fn();

3177 + console.error = (...args) => {

3178 + if (args.length > 1) {

3179 + if (typeof args[1] === 'object') {

3180 + mockError(args[0].split('\n')[0]);

3181 + return;

3182 + }

3183 + }

3184 + mockError(...args.map(normalizeCodeLocInfo));

3185 + };

3186 + let isClient = false;

3187 + let shouldThrow = true;

3188 +

3189 + function ThrowUntilOnClient({children, message}) {

3190 + if (isClient && shouldThrow) {

3191 + Scheduler.unstable_yieldValue('throwing: ' + message);

3192 + throw new Error(message);

3193 + }

3194 + return children;

3195 + }

3196 +

3197 + function StopThrowingOnClient() {

3198 + if (isClient) {

3199 + shouldThrow = false;

3200 + }

3201 + return null;

3202 + }

3203 +

3204 + const App = () => {

3205 + return (

3206 + <div>

3207 + <Suspense fallback={<h1>Loading...</h1>}>

3208 + <ThrowUntilOnClient message="first error">

3209 + <h1>one</h1>

3210 + </ThrowUntilOnClient>

3211 + <ThrowUntilOnClient message="second error">

3212 + <h2>two</h2>

3213 + </ThrowUntilOnClient>

3214 + <ThrowUntilOnClient message="third error">

3215 + <h3>three</h3>

3216 + </ThrowUntilOnClient>

3217 + <StopThrowingOnClient />

3218 + </Suspense>

3219 + </div>

3220 + );

3221 + };

3222 +

3223 + try {

3224 + await act(async () => {

3225 + const {pipe} = ReactDOMFizzServer.renderToPipeableStream(<App />);

3226 + pipe(writable);

3227 + });

3228 +

3229 + expect(getVisibleChildren(container)).toEqual(

3230 + <div>

3231 + <h1>one</h1>

3232 + <h2>two</h2>

3233 + <h3>three</h3>

3234 + </div>,

3235 + );

3236 +

3237 + isClient = true;

3238 +

3239 + ReactDOMClient.hydrateRoot(container, <App />, {

3240 + onRecoverableError(error) {

3241 + Scheduler.unstable_yieldValue(

3242 + 'Logged recoverable error: ' + error.message,

3243 + );

3244 + },

3245 + });

3246 + expect(Scheduler).toFlushAndYield([

3247 + 'throwing: first error',

3248 + // this repeated first error is the invokeGuardedCallback throw

3249 + 'throwing: first error',

3250 + // these are actually thrown during render but no iGC repeat and no queueing as hydration errors

3251 + 'throwing: second error',

3252 + 'throwing: third error',

3253 + // all hydration errors are still queued

3254 + 'Logged recoverable error: first error',

3255 + 'Logged recoverable error: second error',

3256 + 'Logged recoverable error: third error',

3257 + // other recoverable errors are queued as hydration errors

3258 + 'Logged recoverable error: There was an error while hydrating this Suspense boundary. Switched to client rendering.',

3259 + ]);

3260 + // These Uncaught error calls are the error reported by the runtime (jsdom here, browser in actual use)

3261 + // when invokeGuardedCallback is used to replay an error in dev using event dispatching in the document

3262 + expect(mockError.mock.calls).toEqual([

3263 + // we only get one because we suppress invokeGuardedCallback after the first one when hydrating in a

3264 + // suspense boundary

3265 + ['Error: Uncaught [Error: first error]'],

3266 + ]);

3267 + mockError.mockClear();

3268 +

3269 + expect(getVisibleChildren(container)).toEqual(

3270 + <div>

3271 + <h1>one</h1>

3272 + <h2>two</h2>

3273 + <h3>three</h3>

3274 + </div>,

3275 + );

3276 +

3277 + expect(Scheduler).toFlushAndYield([]);

3278 + expect(mockError.mock.calls).toEqual([]);

3279 + } finally {

3280 + console.error = originalConsoleError;

3281 + }

3282 + });

3283 +

3284 + // @gate experimental

3285 + it('does not invokeGuardedCallback for errors after a preceding fiber suspends', async () => {

3286 + // We can't use the toErrorDev helper here because this is async.

3287 + const originalConsoleError = console.error;

3288 + const mockError = jest.fn();

3289 + console.error = (...args) => {

3290 + if (args.length > 1) {

3291 + if (typeof args[1] === 'object') {

3292 + mockError(args[0].split('\n')[0]);

3293 + return;

3294 + }

3295 + }

3296 + mockError(...args.map(normalizeCodeLocInfo));

3297 + };

3298 + let isClient = false;

3299 + let shouldThrow = true;

3300 + let promise = null;

3301 + let unsuspend = null;

3302 + let isResolved = false;

3303 +

3304 + function ComponentThatSuspendsOnClient() {

3305 + if (isClient && !isResolved) {

3306 + if (promise === null) {

3307 + promise = new Promise(resolve => {

3308 + unsuspend = () => {

3309 + isResolved = true;

3310 + resolve();

3311 + };

3312 + });

3313 + }

3314 + Scheduler.unstable_yieldValue('suspending');

3315 + throw promise;

3316 + }

3317 + return null;

3318 + }

3319 +

3320 + function ThrowUntilOnClient({children, message}) {

3321 + if (isClient && shouldThrow) {

3322 + Scheduler.unstable_yieldValue('throwing: ' + message);

3323 + throw new Error(message);

3324 + }

3325 + return children;

3326 + }

3327 +

3328 + function StopThrowingOnClient() {

3329 + if (isClient) {

3330 + shouldThrow = false;

3331 + }

3332 + return null;

3333 + }

3334 +

3335 + const App = () => {

3336 + return (

3337 + <div>

3338 + <Suspense fallback={<h1>Loading...</h1>}>

3339 + <ComponentThatSuspendsOnClient />

3340 + <ThrowUntilOnClient message="first error">

3341 + <h1>one</h1>

3342 + </ThrowUntilOnClient>

3343 + <ThrowUntilOnClient message="second error">

3344 + <h2>two</h2>

3345 + </ThrowUntilOnClient>

3346 + <ThrowUntilOnClient message="third error">

3347 + <h3>three</h3>

3348 + </ThrowUntilOnClient>

3349 + <StopThrowingOnClient />

3350 + </Suspense>

3351 + </div>

3352 + );

3353 + };

3354 +

3355 + try {

3356 + await act(async () => {

3357 + const {pipe} = ReactDOMFizzServer.renderToPipeableStream(<App />);

3358 + pipe(writable);

3359 + });

3360 +

3361 + expect(getVisibleChildren(container)).toEqual(

3362 + <div>

3363 + <h1>one</h1>

3364 + <h2>two</h2>

3365 + <h3>three</h3>

3366 + </div>,

3367 + );

3368 +

3369 + isClient = true;

3370 +

3371 + ReactDOMClient.hydrateRoot(container, <App />, {

3372 + onRecoverableError(error) {

3373 + Scheduler.unstable_yieldValue(

3374 + 'Logged recoverable error: ' + error.message,

3375 + );

3376 + },

3377 + });

3378 + expect(Scheduler).toFlushAndYield([

3379 + 'suspending',

3380 + 'throwing: first error',

3381 + // There is no repeated first error because we already suspended and no

3382 + // invokeGuardedCallback is used if we are in dev

3383 + // or in prod there is just never an invokeGuardedCallback

3384 + 'throwing: second error',

3385 + 'throwing: third error',

3386 + ]);

3387 + expect(mockError.mock.calls).toEqual([]);

3388 +

3389 + expect(getVisibleChildren(container)).toEqual(

3390 + <div>

3391 + <h1>one</h1>

3392 + <h2>two</h2>

3393 + <h3>three</h3>

3394 + </div>,

3395 + );

3396 + await unsuspend();

3397 + // Since our client components only throw on the very first render there are no

3398 + // new throws in this pass

3399 + expect(Scheduler).toFlushAndYield([]);

3400 +

3401 + expect(mockError.mock.calls).toEqual([]);

3402 + } finally {

3403 + console.error = originalConsoleError;

3404 + }

3405 + });

3406 +

3407 + // @gate experimental && __DEV__

3408 + it('suspending after erroring will cause errors previously queued to be silenced until the boundary resolves', async () => {

3409 + // We can't use the toErrorDev helper here because this is async.

3410 + const originalConsoleError = console.error;

3411 + const mockError = jest.fn();

3412 + console.error = (...args) => {

3413 + if (args.length > 1) {

3414 + if (typeof args[1] === 'object') {

3415 + mockError(args[0].split('\n')[0]);

3416 + return;

3417 + }

3418 + }

3419 + mockError(...args.map(normalizeCodeLocInfo));

3420 + };

3421 + let isClient = false;

3422 + let shouldThrow = true;

3423 + let promise = null;

3424 + let unsuspend = null;

3425 + let isResolved = false;

3426 +

3427 + function ComponentThatSuspendsOnClient() {

3428 + if (isClient && !isResolved) {

3429 + if (promise === null) {

3430 + promise = new Promise(resolve => {

3431 + unsuspend = () => {

3432 + isResolved = true;

3433 + resolve();

3434 + };

3435 + });

3436 + }

3437 + Scheduler.unstable_yieldValue('suspending');

3438 + throw promise;

3439 + }

3440 + return null;

3441 + }

3442 +

3443 + function ThrowUntilOnClient({children, message}) {

3444 + if (isClient && shouldThrow) {

3445 + Scheduler.unstable_yieldValue('throwing: ' + message);

3446 + throw new Error(message);

3447 + }

3448 + return children;

3449 + }

3450 +

3451 + function StopThrowingOnClient() {

3452 + if (isClient) {

3453 + shouldThrow = false;

3454 + }

3455 + return null;

3456 + }

3457 +

3458 + const App = () => {

3459 + return (

3460 + <div>

3461 + <Suspense fallback={<h1>Loading...</h1>}>

3462 + <ThrowUntilOnClient message="first error">

3463 + <h1>one</h1>

3464 + </ThrowUntilOnClient>

3465 + <ThrowUntilOnClient message="second error">

3466 + <h2>two</h2>

3467 + </ThrowUntilOnClient>

3468 + <ComponentThatSuspendsOnClient />

3469 + <ThrowUntilOnClient message="third error">

3470 + <h3>three</h3>

3471 + </ThrowUntilOnClient>

3472 + <StopThrowingOnClient />

3473 + </Suspense>

3474 + </div>

3475 + );

3476 + };

3477 +

3478 + try {

3479 + await act(async () => {

3480 + const {pipe} = ReactDOMFizzServer.renderToPipeableStream(<App />);

3481 + pipe(writable);

3482 + });

3483 +

3484 + expect(getVisibleChildren(container)).toEqual(

3485 + <div>

3486 + <h1>one</h1>

3487 + <h2>two</h2>

3488 + <h3>three</h3>

3489 + </div>,

3490 + );

3491 +

3492 + isClient = true;

3493 +

3494 + ReactDOMClient.hydrateRoot(container, <App />, {

3495 + onRecoverableError(error) {

3496 + Scheduler.unstable_yieldValue(

3497 + 'Logged recoverable error: ' + error.message,

3498 + );

3499 + },

3500 + });

3501 + expect(Scheduler).toFlushAndYield([

3502 + 'throwing: first error',

3503 + // duplicate because first error is re-done in invokeGuardedCallback

3504 + 'throwing: first error',

3505 + 'throwing: second error',

3506 + 'suspending',

3507 + 'throwing: third error',

3508 + ]);

3509 + // These Uncaught error calls are the error reported by the runtime (jsdom here, browser in actual use)

3510 + // when invokeGuardedCallback is used to replay an error in dev using event dispatching in the document

3511 + expect(mockError.mock.calls).toEqual([

3512 + // we only get one because we suppress invokeGuardedCallback after the first one when hydrating in a

3513 + // suspense boundary

3514 + ['Error: Uncaught [Error: first error]'],

3515 + ]);

3516 + mockError.mockClear();

3517 +

3518 + expect(getVisibleChildren(container)).toEqual(

3519 + <div>

3520 + <h1>one</h1>

3521 + <h2>two</h2>

3522 + <h3>three</h3>

3523 + </div>,

3524 + );

3525 + await unsuspend();

3526 + // Since our client components only throw on the very first render there are no

3527 + // new throws in this pass

3528 + expect(Scheduler).toFlushAndYield([]);

3529 + expect(mockError.mock.calls).toEqual([]);

3530 + } finally {

3531 + console.error = originalConsoleError;

3532 + }

3533 + });

3534 });

packages/react-reconciler/src/ReactFiberHydrationContext.new.js

@@ -104,6 +104,13 @@ export function markDidThrowWhileHydratingDEV() {

104 }

105 }

106

107 +export function didSuspendOrErrorWhileHydratingDEV() {

108 + if (__DEV__) {

109 + return didSuspendOrErrorDEV;

110 + }

111 + return false;

112 +}

113 +

114 function enterHydrationState(fiber: Fiber): boolean {

115 if (!supportsHydration) {

116 return false;

packages/react-reconciler/src/ReactFiberHydrationContext.old.js

@@ -104,6 +104,13 @@ export function markDidThrowWhileHydratingDEV() {

104 }

105 }

106

107 +export function didSuspendOrErrorWhileHydratingDEV() {

108 + if (__DEV__) {

109 + return didSuspendOrErrorDEV;

110 + }

111 + return false;

112 +}

113 +

114 function enterHydrationState(fiber: Fiber): boolean {

115 if (!supportsHydration) {

116 return false;

packages/react-reconciler/src/ReactFiberWorkLoop.new.js

+7 -4

@@ -88,6 +88,7 @@ import {

88 assignFiberPropertiesInDEV,

89 } from './ReactFiber.new';

90 import {isRootDehydrated} from './ReactFiberShellHydration';

91 +import {didSuspendOrErrorWhileHydratingDEV} from './ReactFiberHydrationContext.new';

92 import {NoMode, ProfileMode, ConcurrentMode} from './ReactTypeOfMode';

93 import {

94 HostRoot,

@@ -3001,11 +3002,13 @@ if (__DEV__ && replayFailedUnitOfWorkWithInvokeGuardedCallback) {

3002 return originalBeginWork(current, unitOfWork, lanes);

3003 } catch (originalError) {

3004 if (

3004 - originalError !== null &&

3005 - typeof originalError === 'object' &&

3006 - typeof originalError.then === 'function'

3005 + didSuspendOrErrorWhileHydratingDEV() ||

3006 + (originalError !== null &&

3007 + typeof originalError === 'object' &&

3008 + typeof originalError.then === 'function')

3009 ) {

3008 - // Don't replay promises. Treat everything else like an error.

3010 + // Don't replay promises.

3011 + // Don't replay errors if we are hydrating and have already suspended or handled an error

3012 throw originalError;

3013 }

3014

packages/react-reconciler/src/ReactFiberWorkLoop.old.js