Warn when suspending at wrong priority

packages/react-reconciler/src/ReactFiberScheduler.js

@@ -698,6 +698,7 @@ function prepareFreshStack(root, expirationTime) {
 
   if (__DEV__) {
     ReactStrictModeWarnings.discardPendingWarnings();
+    componentsWithSuspendedDiscreteUpdates = null;
   }
 }
 
@@ -1225,6 +1226,7 @@ function commitRoot(root, expirationTime) {
 function commitRootImpl(root, expirationTime) {
   flushPassiveEffects();
   flushRenderPhaseStrictModeWarningsInDEV();
+  flushSuspensePriorityWarningInDEV();
 
   invariant(
     workPhase !== RenderPhase && workPhase !== CommitPhase,
@@ -2114,6 +2116,76 @@ function warnIfNotCurrentlyActingUpdatesInDEV(fiber: Fiber): void {
 
 export const warnIfNotCurrentlyActingUpdatesInDev = warnIfNotCurrentlyActingUpdatesInDEV;
 
+let componentsWithSuspendedDiscreteUpdates = null;
+export function checkForWrongSuspensePriorityInDEV(sourceFiber: Fiber) {
+  if (__DEV__) {
+    if (
+      (sourceFiber.mode & ConcurrentMode) !== NoEffect &&
+      // Check if we're currently rendering a discrete update. Ideally, all we
+      // would need to do is check the current priority level. But we currently
+      // have no rigorous way to distinguish work that was scheduled at user-
+      // blocking priority from work that expired a bit and was "upgraded" to
+      // a higher priority. That's because we don't schedule separate callbacks
+      // for every level, only the highest priority level per root. The priority
+      // of subsequent levels is inferred from the expiration time, but this is
+      // an imprecise heuristic.
+      //
+      // However, we do store the last discrete pending update per root. So we
+      // can reliably compare to that one. (If we broaden this warning to include
+      // high pri updates that aren't discrete, then this won't be sufficient.)
+      //
+      // My rationale is that it's better for this warning to have false
+      // negatives than false positives.
+      rootsWithPendingDiscreteUpdates !== null &&
+      workInProgressRoot !== null &&
+      renderExpirationTime ===
+        rootsWithPendingDiscreteUpdates.get(workInProgressRoot)
+    ) {
+      // Add the component name to a set.
+      const componentName = getComponentName(sourceFiber.type);
+      if (componentsWithSuspendedDiscreteUpdates === null) {
+        componentsWithSuspendedDiscreteUpdates = new Set([componentName]);
+      } else {
+        componentsWithSuspendedDiscreteUpdates.add(componentName);
+      }
+    }
+  }
+}
+
+function flushSuspensePriorityWarningInDEV() {
+  if (__DEV__) {
+    if (componentsWithSuspendedDiscreteUpdates !== null) {
+      const componentNames = [];
+      componentsWithSuspendedDiscreteUpdates.forEach(name => {
+        componentNames.push(name);
+      });
+      componentsWithSuspendedDiscreteUpdates = null;
+
+      // TODO: A more helpful version of this message could include the names of
+      // the component that were updated, not the ones that suspended. To do
+      // that we'd need to track all the components that updated during this
+      // render, perhaps using the same mechanism as `markRenderEventTime`.
+      warningWithoutStack(
+        false,
+        'The following components suspended during a user-blocking update: %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.' +
+          '\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.',
+        // 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/ReactFiberUnwindWork.js

@@ -68,6 +68,7 @@ import {
   isAlreadyFailedLegacyErrorBoundary,
   pingSuspendedRoot,
   resolveRetryThenable,
+  checkForWrongSuspensePriorityInDEV,
 } from './ReactFiberScheduler';
 
 import invariant from 'shared/invariant';
@@ -203,6 +204,8 @@ function throwException(
     // This is a thenable.
     const thenable: Thenable = (value: any);
 
+    checkForWrongSuspensePriorityInDEV(sourceFiber);
+
     // Schedule the nearest Suspense to re-render the timed out view.
     let workInProgress = returnFiber;
     do {

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

@@ -1715,12 +1715,61 @@ describe('ReactSuspenseWithNoopRenderer', () => {
 
     // Flush some of the time
     Scheduler.advanceTime(500);
-    await advanceTimers(500);
+    expect(() => {
+      jest.advanceTimersByTime(500);
+    }).toWarnDev(
+      'The following components suspended during a user-blocking ' +
+        'update: AsyncText',
+      {withoutStack: true},
+    );
+
     // We should have already shown the fallback.
     // When we wrote this test, we inferred the start time of high priority
     // updates as way earlier in the past. This test ensures that we don't
     // use this assumption to add a very long JND.
     expect(Scheduler).toFlushWithoutYielding();
     expect(ReactNoop.getChildren()).toEqual([span('Loading...')]);
   });
+
+  it('warns when suspending inside discrete update', async () => {
+    function A() {
+      TextResource.read(['A', 1000]);
+      return 'A';
+    }
+
+    function B() {
+      return 'B';
+    }
+
+    function C() {
+      TextResource.read(['C', 1000]);
+      return 'C';
+    }
+
+    function App() {
+      return (
+        <Suspense fallback="Loading...">
+          <A />
+          <B />
+          <C />
+          <C />
+          <C />
+          <C />
+        </Suspense>
+      );
+    }
+
+    ReactNoop.interactiveUpdates(() => ReactNoop.render(<App />));
+    Scheduler.flushAll();
+
+    // Warning is not flushed until the commit phase
+
+    // Timeout and commit the fallback
+    expect(() => {
+      jest.advanceTimersByTime(1000);
+    }).toWarnDev(
+      'The following components suspended during a user-blocking update: A, C',
+      {withoutStack: true},
+    );
+  });
 });