Skip to content

Commit

Permalink
Edit Suspense Priority Warning Message (#16186)
Browse files Browse the repository at this point in the history 
* move 'component that triggered the update' in suspense priority warning message to the beginning of the message

* renamed warnings
  • Loading branch information
@lunaruan
lunaruan authored Jul 24, 2019
1 parent 42b75ab commit 06cc996
Show file tree
Hide file tree
Showing 4 changed files with 69 additions and 33 deletions.
45 changes: 27 additions & 18 deletions packages/react-reconciler/src/ReactFiberWorkLoop.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
);
}
Expand Down
5 changes: 4 additions & 1 deletion packages/react-reconciler/src/__tests__/ReactSuspense-test.internal.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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');
}
Expand Down
47 changes: 34 additions & 13 deletions packages/react-reconciler/src/__tests__/ReactSuspenseWithNoopRenderer-test.internal.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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');
}
Expand Down Expand Up @@ -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');
}
Expand Down Expand Up @@ -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');
}
Expand Down Expand Up @@ -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');
}
Expand Down Expand Up @@ -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},
);

Expand Down Expand Up @@ -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},
);
});
Expand Down Expand Up @@ -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},
);
});
Expand Down Expand Up @@ -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},
);
});
Expand Down
5 changes: 4 additions & 1 deletion packages/react/src/__tests__/ReactProfiler-test.internal.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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');
}
Expand Down

0 comments on commit 06cc996

Please sign in to comment.