From 06cc9969947e614636e9260849fe5b50514eadfe Mon Sep 17 00:00:00 2001 From: lunaruan Date: Tue, 23 Jul 2019 19:08:41 -0700 Subject: [PATCH] Edit Suspense Priority Warning Message (#16186) * move 'component that triggered the update' in suspense priority warning message to the beginning of the message * renamed warnings --- .../src/ReactFiberWorkLoop.js | 45 +++++++++++------- .../__tests__/ReactSuspense-test.internal.js | 5 +- ...tSuspenseWithNoopRenderer-test.internal.js | 47 ++++++++++++++----- .../__tests__/ReactProfiler-test.internal.js | 5 +- 4 files changed, 69 insertions(+), 33 deletions(-) diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.js b/packages/react-reconciler/src/ReactFiberWorkLoop.js index 966573139c6e3..d4d4872450f12 100644 --- a/packages/react-reconciler/src/ReactFiberWorkLoop.js +++ b/packages/react-reconciler/src/ReactFiberWorkLoop.js @@ -2639,29 +2639,38 @@ function flushSuspensePriorityWarningInDEV() { componentsThatTriggeredHighPriSuspend = null; - const componentThatTriggeredSuspenseError = - componentsThatTriggeredSuspendNames.length > 0 - ? '\n' + - 'The components that triggered the update: ' + - componentsThatTriggeredSuspendNames.sort().join(', ') - : ''; + const componentNamesString = componentNames.sort().join(', '); + let componentThatTriggeredSuspenseError = ''; + if (componentsThatTriggeredSuspendNames.length > 0) { + componentThatTriggeredSuspenseError = + 'The following components triggered a user-blocking update:' + + '\n\n' + + ' ' + + componentsThatTriggeredSuspendNames.sort().join(', ') + + '\n\n' + + 'that was then suspended by:' + + '\n\n' + + ' ' + + componentNamesString; + } else { + componentThatTriggeredSuspenseError = + 'A user-blocking update was suspended by:' + + '\n\n' + + ' ' + + componentNamesString; + } + warningWithoutStack( false, - 'The following components suspended during a user-blocking update: %s' + - '%s' + + '%s' + '\n\n' + - 'Updates triggered by user interactions (e.g. click events) are ' + - 'considered user-blocking by default. They should not suspend. ' + - 'Updates that can afford to take a bit longer should be wrapped ' + - 'with `Scheduler.next` (or an equivalent abstraction). This ' + - 'typically includes any update that shows new content, like ' + - 'a navigation.' + + 'The fix is to split the update into multiple parts: a user-blocking ' + + 'update to provide immediate feedback, and another update that ' + + 'triggers the bulk of the changes.' + '\n\n' + - 'Generally, you should split user interactions into at least two ' + - 'seprate updates: a user-blocking update to provide immediate ' + - 'feedback, and another update to perform the actual change.', + 'Refer to the documentation for useSuspenseTransition to learn how ' + + 'to implement this pattern.', // TODO: Add link to React docs with more information, once it exists - componentNames.sort().join(', '), componentThatTriggeredSuspenseError, ); } diff --git a/packages/react-reconciler/src/__tests__/ReactSuspense-test.internal.js b/packages/react-reconciler/src/__tests__/ReactSuspense-test.internal.js index d313de6b5949d..5050c2936ffe1 100644 --- a/packages/react-reconciler/src/__tests__/ReactSuspense-test.internal.js +++ b/packages/react-reconciler/src/__tests__/ReactSuspense-test.internal.js @@ -344,7 +344,10 @@ describe('ReactSuspense', () => { if (__DEV__) { expect(console.error).toHaveBeenCalledTimes(2); expect(console.error.calls.argsFor(0)[0]).toContain( - 'Warning: The following components suspended during a user-blocking update: ', + 'Warning: %s\n\nThe fix is to split the update', + ); + expect(console.error.calls.argsFor(0)[1]).toContain( + 'A user-blocking update was suspended by:', ); expect(console.error.calls.argsFor(0)[1]).toContain('AsyncText'); } diff --git a/packages/react-reconciler/src/__tests__/ReactSuspenseWithNoopRenderer-test.internal.js b/packages/react-reconciler/src/__tests__/ReactSuspenseWithNoopRenderer-test.internal.js index 00dc20493cc49..8c9773009b2b2 100644 --- a/packages/react-reconciler/src/__tests__/ReactSuspenseWithNoopRenderer-test.internal.js +++ b/packages/react-reconciler/src/__tests__/ReactSuspenseWithNoopRenderer-test.internal.js @@ -519,7 +519,10 @@ describe('ReactSuspenseWithNoopRenderer', () => { if (__DEV__) { expect(console.error).toHaveBeenCalledTimes(1); expect(console.error.calls.argsFor(0)[0]).toContain( - 'Warning: The following components suspended during a user-blocking update: ', + 'Warning: %s\n\nThe fix is to split the update', + ); + expect(console.error.calls.argsFor(0)[1]).toContain( + 'A user-blocking update was suspended by:', ); expect(console.error.calls.argsFor(0)[1]).toContain('AsyncText'); } @@ -671,7 +674,10 @@ describe('ReactSuspenseWithNoopRenderer', () => { if (__DEV__) { expect(console.error).toHaveBeenCalledTimes(1); expect(console.error.calls.argsFor(0)[0]).toContain( - 'Warning: The following components suspended during a user-blocking update: ', + 'Warning: %s\n\nThe fix is to split the update', + ); + expect(console.error.calls.argsFor(0)[1]).toContain( + 'A user-blocking update was suspended by:', ); expect(console.error.calls.argsFor(0)[1]).toContain('AsyncText'); } @@ -704,7 +710,10 @@ describe('ReactSuspenseWithNoopRenderer', () => { if (__DEV__) { expect(console.error).toHaveBeenCalledTimes(1); expect(console.error.calls.argsFor(0)[0]).toContain( - 'Warning: The following components suspended during a user-blocking update: ', + 'Warning: %s\n\nThe fix is to split the update', + ); + expect(console.error.calls.argsFor(0)[1]).toContain( + 'A user-blocking update was suspended by:', ); expect(console.error.calls.argsFor(0)[1]).toContain('AsyncText'); } @@ -793,7 +802,10 @@ describe('ReactSuspenseWithNoopRenderer', () => { if (__DEV__) { expect(console.error).toHaveBeenCalledTimes(2); expect(console.error.calls.argsFor(0)[0]).toContain( - 'Warning: The following components suspended during a user-blocking update: ', + 'Warning: %s\n\nThe fix is to split the update', + ); + expect(console.error.calls.argsFor(0)[1]).toContain( + 'A user-blocking update was suspended by:', ); expect(console.error.calls.argsFor(0)[1]).toContain('AsyncText'); } @@ -1631,8 +1643,9 @@ describe('ReactSuspenseWithNoopRenderer', () => { 'Loading...', ]); }).toWarnDev( - 'The following components suspended during a user-blocking ' + - 'update: AsyncText', + 'Warning: A user-blocking update was suspended by:' + + '\n\n' + + ' AsyncText', {withoutStack: true}, ); @@ -1675,9 +1688,13 @@ describe('ReactSuspenseWithNoopRenderer', () => { ); }); }).toWarnDev( - 'The following components suspended during a user-blocking update: AsyncText' + - '\n' + - 'The components that triggered the update: App', + 'Warning: The following components triggered a user-blocking update:' + + '\n\n' + + ' App' + + '\n\n' + + 'that was then suspended by:' + + '\n\n' + + ' AsyncText', {withoutStack: true}, ); }); @@ -1709,9 +1726,13 @@ describe('ReactSuspenseWithNoopRenderer', () => { ); }); }).toWarnDev( - 'The following components suspended during a user-blocking update: AsyncText' + - '\n' + - 'The components that triggered the update: App', + 'Warning: The following components triggered a user-blocking update:' + + '\n\n' + + ' App' + + '\n\n' + + 'that was then suspended by:' + + '\n\n' + + ' AsyncText', {withoutStack: true}, ); }); @@ -1754,7 +1775,7 @@ describe('ReactSuspenseWithNoopRenderer', () => { expect(() => { Scheduler.unstable_flushAll(); }).toWarnDev( - 'Warning: The following components suspended during a user-blocking update: A, C', + 'Warning: A user-blocking update was suspended by:' + '\n\n' + ' A, C', {withoutStack: true}, ); }); diff --git a/packages/react/src/__tests__/ReactProfiler-test.internal.js b/packages/react/src/__tests__/ReactProfiler-test.internal.js index 6323f02049e9b..e7e5396103972 100644 --- a/packages/react/src/__tests__/ReactProfiler-test.internal.js +++ b/packages/react/src/__tests__/ReactProfiler-test.internal.js @@ -2735,7 +2735,10 @@ describe('Profiler', () => { if (__DEV__) { expect(console.error).toHaveBeenCalledTimes(1); expect(console.error.calls.argsFor(0)[0]).toContain( - 'Warning: The following components suspended during a user-blocking update: ', + 'Warning: %s\n\nThe fix is to split the update', + ); + expect(console.error.calls.argsFor(0)[1]).toContain( + 'A user-blocking update was suspended by:', ); expect(console.error.calls.argsFor(0)[1]).toContain('AsyncText'); }