Remove Suspense priority warning (#17971)
* Remove Suspense priority warning * Fix tests
Dan Abramov committed
Feb 4, 2020 at 18:05 UTC
d6e08fe0a806703241311b5331c21f29fdf4a139
3 files changed
+29
-166
packages/react-reconciler/src/ReactFiberThrow.js
-3
@@ -54,7 +54,6 @@ import {
54
markLegacyErrorBoundaryAsFailed,
55
isAlreadyFailedLegacyErrorBoundary,
56
pingSuspendedRoot,
57
- checkForWrongSuspensePriorityInDEV,
57
} from './ReactFiberWorkLoop';
58
59
import {Sync} from './ReactFiberExpirationTime';
@@ -207,8 +206,6 @@ function throwException(
206
}
207
}
208
210
- checkForWrongSuspensePriorityInDEV(sourceFiber);
211
-
209
let hasInvisibleParentBoundary = hasSuspenseContext(
210
suspenseStackCursor.current,
211
(InvisibleParentSuspenseContext: SuspenseContext),
packages/react-reconciler/src/ReactFiberWorkLoop.js
+2
-127
@@ -14,7 +14,6 @@ import type {ReactPriorityLevel} from './SchedulerWithReactIntegration';
14
import type {Interaction} from 'scheduler/src/Tracing';
15
import type {SuspenseConfig} from './ReactFiberSuspenseConfig';
16
import type {SuspenseState} from './ReactFiberSuspenseComponent';
17
-import type {Hook} from './ReactFiberHooks';
17
18
import {
19
warnAboutDeprecatedLifecycles,
@@ -779,7 +778,6 @@ function finishConcurrentRender(
778
if (expirationTime === lastSuspendedTime) {
779
root.nextKnownPendingLevel = getRemainingExpirationTime(finishedWork);
780
}
782
- flushSuspensePriorityWarningInDEV();
781
782
// We have an acceptable loading state. We need to figure out if we
783
// should immediately commit it or wait a bit.
@@ -855,7 +853,6 @@ function finishConcurrentRender(
853
if (expirationTime === lastSuspendedTime) {
854
root.nextKnownPendingLevel = getRemainingExpirationTime(finishedWork);
855
}
858
- flushSuspensePriorityWarningInDEV();
856
857
if (
858
// do not delay if we're inside an act() scope
@@ -1051,7 +1048,7 @@ function performSyncWorkOnRoot(root) {
1048
stopFinishedWorkLoopTimer();
1049
root.finishedWork = (root.current.alternate: any);
1050
root.finishedExpirationTime = expirationTime;
1054
- finishSyncRender(root, workInProgressRootExitStatus, expirationTime);
1051
+ finishSyncRender(root);
1052
}
1053
1054
// Before exiting, make sure there's a callback scheduled for the next
@@ -1062,15 +1059,9 @@ function performSyncWorkOnRoot(root) {
1059
return null;
1060
}
1061
1065
-function finishSyncRender(root, exitStatus, expirationTime) {
1062
+function finishSyncRender(root) {
1063
// Set this to null to indicate there's no in-progress render.
1064
workInProgressRoot = null;
1068
-
1069
- if (__DEV__) {
1070
- if (exitStatus === RootSuspended || exitStatus === RootSuspendedWithDelay) {
1071
- flushSuspensePriorityWarningInDEV();
1072
- }
1073
- }
1065
commitRoot(root);
1066
}
1067
@@ -1274,7 +1265,6 @@ function prepareFreshStack(root, expirationTime) {
1265
1266
if (__DEV__) {
1267
ReactStrictModeWarnings.discardPendingWarnings();
1277
- componentsThatTriggeredHighPriSuspend = null;
1268
}
1269
}
1270
@@ -2859,121 +2849,6 @@ export function warnIfUnmockedScheduler(fiber: Fiber) {
2849
}
2850
}
2851
2862
-let componentsThatTriggeredHighPriSuspend = null;
2863
-export function checkForWrongSuspensePriorityInDEV(sourceFiber: Fiber) {
2864
- if (__DEV__) {
2865
- const currentPriorityLevel = getCurrentPriorityLevel();
2866
- if (
2867
- (sourceFiber.mode & ConcurrentMode) !== NoEffect &&
2868
- (currentPriorityLevel === UserBlockingPriority ||
2869
- currentPriorityLevel === ImmediatePriority)
2870
- ) {
2871
- let workInProgressNode = sourceFiber;
2872
- while (workInProgressNode !== null) {
2873
- // Add the component that triggered the suspense
2874
- const current = workInProgressNode.alternate;
2875
- if (current !== null) {
2876
- // TODO: warn component that triggers the high priority
2877
- // suspend is the HostRoot
2878
- switch (workInProgressNode.tag) {
2879
- case ClassComponent:
2880
- // Loop through the component's update queue and see whether the component
2881
- // has triggered any high priority updates
2882
- const updateQueue = current.updateQueue;
2883
- if (updateQueue !== null) {
2884
- let update = updateQueue.baseQueue;
2885
- while (update !== null) {
2886
- const priorityLevel = update.priority;
2887
- if (
2888
- priorityLevel === UserBlockingPriority ||
2889
- priorityLevel === ImmediatePriority
2890
- ) {
2891
- if (componentsThatTriggeredHighPriSuspend === null) {
2892
- componentsThatTriggeredHighPriSuspend = new Set([
2893
- getComponentName(workInProgressNode.type),
2894
- ]);
2895
- } else {
2896
- componentsThatTriggeredHighPriSuspend.add(
2897
- getComponentName(workInProgressNode.type),
2898
- );
2899
- }
2900
- break;
2901
- }
2902
- update = update.next;
2903
- }
2904
- }
2905
- break;
2906
- case FunctionComponent:
2907
- case ForwardRef:
2908
- case SimpleMemoComponent:
2909
- case Chunk: {
2910
- let firstHook: null | Hook = current.memoizedState;
2911
- // TODO: This just checks the first Hook. Isn't it suppose to check all Hooks?
2912
- if (firstHook !== null && firstHook.baseQueue !== null) {
2913
- let update = firstHook.baseQueue;
2914
- // Loop through the functional component's memoized state to see whether
2915
- // the component has triggered any high pri updates
2916
- while (update !== null) {
2917
- const priority = update.priority;
2918
- if (
2919
- priority === UserBlockingPriority ||
2920
- priority === ImmediatePriority
2921
- ) {
2922
- if (componentsThatTriggeredHighPriSuspend === null) {
2923
- componentsThatTriggeredHighPriSuspend = new Set([
2924
- getComponentName(workInProgressNode.type),
2925
- ]);
2926
- } else {
2927
- componentsThatTriggeredHighPriSuspend.add(
2928
- getComponentName(workInProgressNode.type),
2929
- );
2930
- }
2931
- break;
2932
- }
2933
- if (update.next === firstHook.baseQueue) {
2934
- break;
2935
- }
2936
- update = update.next;
2937
- }
2938
- }
2939
- break;
2940
- }
2941
- default:
2942
- break;
2943
- }
2944
- }
2945
- workInProgressNode = workInProgressNode.return;
2946
- }
2947
- }
2948
- }
2949
-}
2950
-
2951
-function flushSuspensePriorityWarningInDEV() {
2952
- if (__DEV__) {
2953
- if (componentsThatTriggeredHighPriSuspend !== null) {
2954
- const componentNames = [];
2955
- componentsThatTriggeredHighPriSuspend.forEach(name =>
2956
- componentNames.push(name),
2957
- );
2958
- componentsThatTriggeredHighPriSuspend = null;
2959
-
2960
- if (componentNames.length > 0) {
2961
- console.error(
2962
- '%s triggered a user-blocking update that suspended.' +
2963
- '\n\n' +
2964
- 'The fix is to split the update into multiple parts: a user-blocking ' +
2965
- 'update to provide immediate feedback, and another update that ' +
2966
- 'triggers the bulk of the changes.' +
2967
- '\n\n' +
2968
- 'Refer to the documentation for useTransition to learn how ' +
2969
- 'to implement this pattern.', // TODO: Add link to React docs with more information, once it exists
2970
- componentNames.sort().join(', '),
2971
- );
2972
- }
2973
- }
2974
- }
2975
-}
2976
-
2852
function computeThreadID(root, expirationTime) {
2853
// Interaction threads are unique per root and expiration time.
2854
return expirationTime * 1000 + root.interactionThreadID;
packages/react-reconciler/src/__tests__/ReactSuspenseWithNoopRenderer-test.internal.js
+27
-36
@@ -1867,7 +1867,8 @@ describe('ReactSuspenseWithNoopRenderer', () => {
1867
expect(ReactNoop.getChildren()).toEqual([span('Loading...')]);
1868
});
1869
1870
- it('warns when a low priority update suspends inside a high priority update for functional components', async () => {
1870
+ // TODO: flip to "warns" when this is implemented again.
1871
+ it('does not warn when a low priority update suspends inside a high priority update for functional components', async () => {
1872
let _setShow;
1873
function App() {
1874
let [show, setShow] = React.useState(false);
@@ -1883,20 +1884,17 @@ describe('ReactSuspenseWithNoopRenderer', () => {
1884
ReactNoop.render(<App />);
1885
});
1886
1886
- expect(() => {
1887
- ReactNoop.act(() => {
1888
- Scheduler.unstable_runWithPriority(
1889
- Scheduler.unstable_UserBlockingPriority,
1890
- () => _setShow(true),
1891
- );
1892
- });
1893
- }).toErrorDev(
1894
- 'Warning: App triggered a user-blocking update that suspended.' + '\n\n',
1895
- {withoutStack: true},
1896
- );
1887
+ // TODO: assert toErrorDev() when the warning is implemented again.
1888
+ ReactNoop.act(() => {
1889
+ Scheduler.unstable_runWithPriority(
1890
+ Scheduler.unstable_UserBlockingPriority,
1891
+ () => _setShow(true),
1892
+ );
1893
+ });
1894
});
1895
1899
- it('warns when a low priority update suspends inside a high priority update for class components', async () => {
1896
+ // TODO: flip to "warns" when this is implemented again.
1897
+ it('does not warn when a low priority update suspends inside a high priority update for class components', async () => {
1898
let show;
1899
class App extends React.Component {
1900
state = {show: false};
@@ -1915,17 +1913,13 @@ describe('ReactSuspenseWithNoopRenderer', () => {
1913
ReactNoop.render(<App />);
1914
});
1915
1918
- expect(() => {
1919
- ReactNoop.act(() => {
1920
- Scheduler.unstable_runWithPriority(
1921
- Scheduler.unstable_UserBlockingPriority,
1922
- () => show(),
1923
- );
1924
- });
1925
- }).toErrorDev(
1926
- 'Warning: App triggered a user-blocking update that suspended.' + '\n\n',
1927
- {withoutStack: true},
1928
- );
1916
+ // TODO: assert toErrorDev() when the warning is implemented again.
1917
+ ReactNoop.act(() => {
1918
+ Scheduler.unstable_runWithPriority(
1919
+ Scheduler.unstable_UserBlockingPriority,
1920
+ () => show(),
1921
+ );
1922
+ });
1923
});
1924
1925
it('does not warn about wrong Suspense priority if no new fallbacks are shown', async () => {
@@ -1961,8 +1955,9 @@ describe('ReactSuspenseWithNoopRenderer', () => {
1955
expect(Scheduler).toHaveYielded(['Suspend! [A]', 'Suspend! [B]']);
1956
});
1957
1958
+ // TODO: flip to "warns" when this is implemented again.
1959
it(
1965
- 'warns when component that triggered user-blocking update is between Suspense boundary ' +
1960
+ 'does not warn when component that triggered user-blocking update is between Suspense boundary ' +
1961
'and component that suspended',
1962
async () => {
1963
let _setShow;
@@ -1982,17 +1977,13 @@ describe('ReactSuspenseWithNoopRenderer', () => {
1977
ReactNoop.render(<App />);
1978
});
1979
1985
- expect(() => {
1986
- ReactNoop.act(() => {
1987
- Scheduler.unstable_runWithPriority(
1988
- Scheduler.unstable_UserBlockingPriority,
1989
- () => _setShow(true),
1990
- );
1991
- });
1992
- }).toErrorDev(
1993
- 'Warning: A triggered a user-blocking update that suspended.' + '\n\n',
1994
- {withoutStack: true},
1995
- );
1980
+ // TODO: assert toErrorDev() when the warning is implemented again.
1981
+ ReactNoop.act(() => {
1982
+ Scheduler.unstable_runWithPriority(
1983
+ Scheduler.unstable_UserBlockingPriority,
1984
+ () => _setShow(true),
1985
+ );
1986
+ });
1987
},
1988
);
1989