Remove Suspense priority warning (#17971)

* Remove Suspense priority warning

* Fix tests

Author: gaearon

packages/react-reconciler/src/ReactFiberThrow.js

@@ -54,7 +54,6 @@ import {
   markLegacyErrorBoundaryAsFailed,
   isAlreadyFailedLegacyErrorBoundary,
   pingSuspendedRoot,
-  checkForWrongSuspensePriorityInDEV,
 } from './ReactFiberWorkLoop';
 
 import {Sync} from './ReactFiberExpirationTime';
@@ -207,8 +206,6 @@ function throwException(
       }
     }
 
-    checkForWrongSuspensePriorityInDEV(sourceFiber);
-
     let hasInvisibleParentBoundary = hasSuspenseContext(
       suspenseStackCursor.current,
       (InvisibleParentSuspenseContext: SuspenseContext),

packages/react-reconciler/src/ReactFiberWorkLoop.js

@@ -14,7 +14,6 @@ import type {ReactPriorityLevel} from './SchedulerWithReactIntegration';
 import type {Interaction} from 'scheduler/src/Tracing';
 import type {SuspenseConfig} from './ReactFiberSuspenseConfig';
 import type {SuspenseState} from './ReactFiberSuspenseComponent';
-import type {Hook} from './ReactFiberHooks';
 
 import {
   warnAboutDeprecatedLifecycles,
@@ -779,7 +778,6 @@ function finishConcurrentRender(
       if (expirationTime === lastSuspendedTime) {
         root.nextKnownPendingLevel = getRemainingExpirationTime(finishedWork);
       }
-      flushSuspensePriorityWarningInDEV();
 
       // We have an acceptable loading state. We need to figure out if we
       // should immediately commit it or wait a bit.
@@ -855,7 +853,6 @@ function finishConcurrentRender(
       if (expirationTime === lastSuspendedTime) {
         root.nextKnownPendingLevel = getRemainingExpirationTime(finishedWork);
       }
-      flushSuspensePriorityWarningInDEV();
 
       if (
         // do not delay if we're inside an act() scope
@@ -1051,7 +1048,7 @@ function performSyncWorkOnRoot(root) {
       stopFinishedWorkLoopTimer();
       root.finishedWork = (root.current.alternate: any);
       root.finishedExpirationTime = expirationTime;
-      finishSyncRender(root, workInProgressRootExitStatus, expirationTime);
+      finishSyncRender(root);
     }
 
     // Before exiting, make sure there's a callback scheduled for the next
@@ -1062,15 +1059,9 @@ function performSyncWorkOnRoot(root) {
   return null;
 }
 
-function finishSyncRender(root, exitStatus, expirationTime) {
+function finishSyncRender(root) {
   // Set this to null to indicate there's no in-progress render.
   workInProgressRoot = null;
-
-  if (__DEV__) {
-    if (exitStatus === RootSuspended || exitStatus === RootSuspendedWithDelay) {
-      flushSuspensePriorityWarningInDEV();
-    }
-  }
   commitRoot(root);
 }
 
@@ -1274,7 +1265,6 @@ function prepareFreshStack(root, expirationTime) {
 
   if (__DEV__) {
     ReactStrictModeWarnings.discardPendingWarnings();
-    componentsThatTriggeredHighPriSuspend = null;
   }
 }
 
@@ -2859,121 +2849,6 @@ export function warnIfUnmockedScheduler(fiber: Fiber) {
   }
 }
 
-let componentsThatTriggeredHighPriSuspend = null;
-export function checkForWrongSuspensePriorityInDEV(sourceFiber: Fiber) {
-  if (__DEV__) {
-    const currentPriorityLevel = getCurrentPriorityLevel();
-    if (
-      (sourceFiber.mode & ConcurrentMode) !== NoEffect &&
-      (currentPriorityLevel === UserBlockingPriority ||
-        currentPriorityLevel === ImmediatePriority)
-    ) {
-      let workInProgressNode = sourceFiber;
-      while (workInProgressNode !== null) {
-        // Add the component that triggered the suspense
-        const current = workInProgressNode.alternate;
-        if (current !== null) {
-          // TODO: warn component that triggers the high priority
-          // suspend is the HostRoot
-          switch (workInProgressNode.tag) {
-            case ClassComponent:
-              // Loop through the component's update queue and see whether the component
-              // has triggered any high priority updates
-              const updateQueue = current.updateQueue;
-              if (updateQueue !== null) {
-                let update = updateQueue.baseQueue;
-                while (update !== null) {
-                  const priorityLevel = update.priority;
-                  if (
-                    priorityLevel === UserBlockingPriority ||
-                    priorityLevel === ImmediatePriority
-                  ) {
-                    if (componentsThatTriggeredHighPriSuspend === null) {
-                      componentsThatTriggeredHighPriSuspend = new Set([
-                        getComponentName(workInProgressNode.type),
-                      ]);
-                    } else {
-                      componentsThatTriggeredHighPriSuspend.add(
-                        getComponentName(workInProgressNode.type),
-                      );
-                    }
-                    break;
-                  }
-                  update = update.next;
-                }
-              }
-              break;
-            case FunctionComponent:
-            case ForwardRef:
-            case SimpleMemoComponent:
-            case Chunk: {
-              let firstHook: null | Hook = current.memoizedState;
-              // TODO: This just checks the first Hook. Isn't it suppose to check all Hooks?
-              if (firstHook !== null && firstHook.baseQueue !== null) {
-                let update = firstHook.baseQueue;
-                // Loop through the functional component's memoized state to see whether
-                // the component has triggered any high pri updates
-                while (update !== null) {
-                  const priority = update.priority;
-                  if (
-                    priority === UserBlockingPriority ||
-                    priority === ImmediatePriority
-                  ) {
-                    if (componentsThatTriggeredHighPriSuspend === null) {
-                      componentsThatTriggeredHighPriSuspend = new Set([
-                        getComponentName(workInProgressNode.type),
-                      ]);
-                    } else {
-                      componentsThatTriggeredHighPriSuspend.add(
-                        getComponentName(workInProgressNode.type),
-                      );
-                    }
-                    break;
-                  }
-                  if (update.next === firstHook.baseQueue) {
-                    break;
-                  }
-                  update = update.next;
-                }
-              }
-              break;
-            }
-            default:
-              break;
-          }
-        }
-        workInProgressNode = workInProgressNode.return;
-      }
-    }
-  }
-}
-
-function flushSuspensePriorityWarningInDEV() {
-  if (__DEV__) {
-    if (componentsThatTriggeredHighPriSuspend !== null) {
-      const componentNames = [];
-      componentsThatTriggeredHighPriSuspend.forEach(name =>
-        componentNames.push(name),
-      );
-      componentsThatTriggeredHighPriSuspend = null;
-
-      if (componentNames.length > 0) {
-        console.error(
-          '%s triggered a user-blocking update that suspended.' +
-            '\n\n' +
-            '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' +
-            'Refer to the documentation for useTransition to learn how ' +
-            'to implement this pattern.', // TODO: Add link to React docs with more information, once it exists
-          componentNames.sort().join(', '),
-        );
-      }
-    }
-  }
-}
-
 function computeThreadID(root, expirationTime) {
   // Interaction threads are unique per root and expiration time.
   return expirationTime * 1000 + root.interactionThreadID;

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

@@ -1867,7 +1867,8 @@ describe('ReactSuspenseWithNoopRenderer', () => {
     expect(ReactNoop.getChildren()).toEqual([span('Loading...')]);
   });
 
-  it('warns when a low priority update suspends inside a high priority update for functional components', async () => {
+  // TODO: flip to "warns" when this is implemented again.
+  it('does not warn when a low priority update suspends inside a high priority update for functional components', async () => {
     let _setShow;
     function App() {
       let [show, setShow] = React.useState(false);
@@ -1883,20 +1884,17 @@ describe('ReactSuspenseWithNoopRenderer', () => {
       ReactNoop.render(<App />);
     });
 
-    expect(() => {
-      ReactNoop.act(() => {
-        Scheduler.unstable_runWithPriority(
-          Scheduler.unstable_UserBlockingPriority,
-          () => _setShow(true),
-        );
-      });
-    }).toErrorDev(
-      'Warning: App triggered a user-blocking update that suspended.' + '\n\n',
-      {withoutStack: true},
-    );
+    // TODO: assert toErrorDev() when the warning is implemented again.
+    ReactNoop.act(() => {
+      Scheduler.unstable_runWithPriority(
+        Scheduler.unstable_UserBlockingPriority,
+        () => _setShow(true),
+      );
+    });
   });
 
-  it('warns when a low priority update suspends inside a high priority update for class components', async () => {
+  // TODO: flip to "warns" when this is implemented again.
+  it('does not warn when a low priority update suspends inside a high priority update for class components', async () => {
     let show;
     class App extends React.Component {
       state = {show: false};
@@ -1915,17 +1913,13 @@ describe('ReactSuspenseWithNoopRenderer', () => {
       ReactNoop.render(<App />);
     });
 
-    expect(() => {
-      ReactNoop.act(() => {
-        Scheduler.unstable_runWithPriority(
-          Scheduler.unstable_UserBlockingPriority,
-          () => show(),
-        );
-      });
-    }).toErrorDev(
-      'Warning: App triggered a user-blocking update that suspended.' + '\n\n',
-      {withoutStack: true},
-    );
+    // TODO: assert toErrorDev() when the warning is implemented again.
+    ReactNoop.act(() => {
+      Scheduler.unstable_runWithPriority(
+        Scheduler.unstable_UserBlockingPriority,
+        () => show(),
+      );
+    });
   });
 
   it('does not warn about wrong Suspense priority if no new fallbacks are shown', async () => {
@@ -1961,8 +1955,9 @@ describe('ReactSuspenseWithNoopRenderer', () => {
     expect(Scheduler).toHaveYielded(['Suspend! [A]', 'Suspend! [B]']);
   });
 
+  // TODO: flip to "warns" when this is implemented again.
   it(
-    'warns when component that triggered user-blocking update is between Suspense boundary ' +
+    'does not warn when component that triggered user-blocking update is between Suspense boundary ' +
       'and component that suspended',
     async () => {
       let _setShow;
@@ -1982,17 +1977,13 @@ describe('ReactSuspenseWithNoopRenderer', () => {
         ReactNoop.render(<App />);
       });
 
-      expect(() => {
-        ReactNoop.act(() => {
-          Scheduler.unstable_runWithPriority(
-            Scheduler.unstable_UserBlockingPriority,
-            () => _setShow(true),
-          );
-        });
-      }).toErrorDev(
-        'Warning: A triggered a user-blocking update that suspended.' + '\n\n',
-        {withoutStack: true},
-      );
+      // TODO: assert toErrorDev() when the warning is implemented again.
+      ReactNoop.act(() => {
+        Scheduler.unstable_runWithPriority(
+          Scheduler.unstable_UserBlockingPriority,
+          () => _setShow(true),
+        );
+      });
     },
   );