Skip to content

Commit c4c9f08

Browse files
authored
BugFix: Suspense priority warning firing when not supposed to (#16256)
Previously, the suspense priority warning was fired even if the Root wasn't suspended. Changed the warning to fire only when the root is suspended. Also refactored the suspense priority warning so it's easier to read.
1 parent 05dce75 commit c4c9f08

File tree

4 files changed

+88
-187
lines changed

4 files changed

+88
-187
lines changed

packages/react-reconciler/src/ReactFiberWorkLoop.js

Lines changed: 22 additions & 58 deletions
Original file line numberDiff line numberDiff line change
@@ -802,7 +802,6 @@ function prepareFreshStack(root, expirationTime) {
802802

803803
if (__DEV__) {
804804
ReactStrictModeWarnings.discardPendingWarnings();
805-
componentsThatSuspendedAtHighPri = null;
806805
componentsThatTriggeredHighPriSuspend = null;
807806
}
808807
}
@@ -990,8 +989,6 @@ function renderRoot(
990989
// Set this to null to indicate there's no in-progress render.
991990
workInProgressRoot = null;
992991

993-
flushSuspensePriorityWarningInDEV();
994-
995992
switch (workInProgressRootExitStatus) {
996993
case RootIncomplete: {
997994
invariant(false, 'Should have a work-in-progress.');
@@ -1022,6 +1019,8 @@ function renderRoot(
10221019
return commitRoot.bind(null, root);
10231020
}
10241021
case RootSuspended: {
1022+
flushSuspensePriorityWarningInDEV();
1023+
10251024
// We have an acceptable loading state. We need to figure out if we should
10261025
// immediately commit it or wait a bit.
10271026

@@ -1076,6 +1075,8 @@ function renderRoot(
10761075
return commitRoot.bind(null, root);
10771076
}
10781077
case RootSuspendedWithDelay: {
1078+
flushSuspensePriorityWarningInDEV();
1079+
10791080
if (
10801081
!isSync &&
10811082
// do not delay if we're inside an act() scope
@@ -2610,7 +2611,6 @@ export function warnIfUnmockedScheduler(fiber: Fiber) {
26102611
}
26112612
}
26122613

2613-
let componentsThatSuspendedAtHighPri = null;
26142614
let componentsThatTriggeredHighPriSuspend = null;
26152615
export function checkForWrongSuspensePriorityInDEV(sourceFiber: Fiber) {
26162616
if (__DEV__) {
@@ -2697,70 +2697,34 @@ export function checkForWrongSuspensePriorityInDEV(sourceFiber: Fiber) {
26972697
}
26982698
workInProgressNode = workInProgressNode.return;
26992699
}
2700-
2701-
// Add the component name to a set.
2702-
const componentName = getComponentName(sourceFiber.type);
2703-
if (componentsThatSuspendedAtHighPri === null) {
2704-
componentsThatSuspendedAtHighPri = new Set([componentName]);
2705-
} else {
2706-
componentsThatSuspendedAtHighPri.add(componentName);
2707-
}
27082700
}
27092701
}
27102702
}
27112703

27122704
function flushSuspensePriorityWarningInDEV() {
27132705
if (__DEV__) {
2714-
if (componentsThatSuspendedAtHighPri !== null) {
2706+
if (componentsThatTriggeredHighPriSuspend !== null) {
27152707
const componentNames = [];
2716-
componentsThatSuspendedAtHighPri.forEach(name => {
2717-
componentNames.push(name);
2718-
});
2719-
componentsThatSuspendedAtHighPri = null;
2720-
2721-
const componentsThatTriggeredSuspendNames = [];
2722-
if (componentsThatTriggeredHighPriSuspend !== null) {
2723-
componentsThatTriggeredHighPriSuspend.forEach(name =>
2724-
componentsThatTriggeredSuspendNames.push(name),
2725-
);
2726-
}
2727-
2708+
componentsThatTriggeredHighPriSuspend.forEach(name =>
2709+
componentNames.push(name),
2710+
);
27282711
componentsThatTriggeredHighPriSuspend = null;
27292712

2730-
const componentNamesString = componentNames.sort().join(', ');
2731-
let componentThatTriggeredSuspenseError = '';
2732-
if (componentsThatTriggeredSuspendNames.length > 0) {
2733-
componentThatTriggeredSuspenseError =
2734-
'The following components triggered a user-blocking update:' +
2735-
'\n\n' +
2736-
' ' +
2737-
componentsThatTriggeredSuspendNames.sort().join(', ') +
2738-
'\n\n' +
2739-
'that was then suspended by:' +
2740-
'\n\n' +
2741-
' ' +
2742-
componentNamesString;
2743-
} else {
2744-
componentThatTriggeredSuspenseError =
2745-
'A user-blocking update was suspended by:' +
2746-
'\n\n' +
2747-
' ' +
2748-
componentNamesString;
2713+
if (componentNames.length > 0) {
2714+
warningWithoutStack(
2715+
false,
2716+
'%s triggered a user-blocking update that suspended.' +
2717+
'\n\n' +
2718+
'The fix is to split the update into multiple parts: a user-blocking ' +
2719+
'update to provide immediate feedback, and another update that ' +
2720+
'triggers the bulk of the changes.' +
2721+
'\n\n' +
2722+
'Refer to the documentation for useSuspenseTransition to learn how ' +
2723+
'to implement this pattern.',
2724+
// TODO: Add link to React docs with more information, once it exists
2725+
componentNames.sort().join(', '),
2726+
);
27492727
}
2750-
2751-
warningWithoutStack(
2752-
false,
2753-
'%s' +
2754-
'\n\n' +
2755-
'The fix is to split the update into multiple parts: a user-blocking ' +
2756-
'update to provide immediate feedback, and another update that ' +
2757-
'triggers the bulk of the changes.' +
2758-
'\n\n' +
2759-
'Refer to the documentation for useSuspenseTransition to learn how ' +
2760-
'to implement this pattern.',
2761-
// TODO: Add link to React docs with more information, once it exists
2762-
componentThatTriggeredSuspenseError,
2763-
);
27642728
}
27652729
}
27662730
}

packages/react-reconciler/src/__tests__/ReactSuspense-test.internal.js

Lines changed: 0 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -327,7 +327,6 @@ describe('ReactSuspense', () => {
327327
});
328328

329329
it('throws if tree suspends and none of the Suspense ancestors have a fallback', () => {
330-
spyOnDev(console, 'error');
331330
ReactTestRenderer.create(
332331
<Suspense>
333332
<AsyncText text="Hi" ms={1000} />
@@ -341,16 +340,6 @@ describe('ReactSuspense', () => {
341340
'AsyncText suspended while rendering, but no fallback UI was specified.',
342341
);
343342
expect(Scheduler).toHaveYielded(['Suspend! [Hi]', 'Suspend! [Hi]']);
344-
if (__DEV__) {
345-
expect(console.error).toHaveBeenCalledTimes(2);
346-
expect(console.error.calls.argsFor(0)[0]).toContain(
347-
'Warning: %s\n\nThe fix is to split the update',
348-
);
349-
expect(console.error.calls.argsFor(0)[1]).toContain(
350-
'A user-blocking update was suspended by:',
351-
);
352-
expect(console.error.calls.argsFor(0)[1]).toContain('AsyncText');
353-
}
354343
});
355344

356345
describe('outside concurrent mode', () => {

0 commit comments

Comments
 (0)