Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Edit Suspense Priority Warning Message #16186

Merged
merged 2 commits into from
Jul 24, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
45 changes: 27 additions & 18 deletions packages/react-reconciler/src/ReactFiberWorkLoop.js
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
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
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
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