[Bugfix] Pending state is always user-blocking (#17382)

Fixes a bug where `isPending` is only set to `true` if `startTransition`
is called from inside an input event. That's usually the case, but
not always.

Now it works regardless of where you call it.

Author: acdlite

packages/react-reconciler/src/ReactFiberHooks.js

@@ -19,7 +19,6 @@ import type {HookEffectTag} from './ReactHookEffectTags';
 import type {SuspenseConfig} from './ReactFiberSuspenseConfig';
 import type {ReactPriorityLevel} from './SchedulerWithReactIntegration';
 
-import * as Scheduler from 'scheduler';
 import ReactSharedInternals from 'shared/ReactSharedInternals';
 
 import {NoWork} from './ReactFiberExpirationTime';
@@ -53,7 +52,12 @@ import getComponentName from 'shared/getComponentName';
 import is from 'shared/objectIs';
 import {markWorkInProgressReceivedUpdate} from './ReactFiberBeginWork';
 import {requestCurrentSuspenseConfig} from './ReactFiberSuspenseConfig';
-import {getCurrentPriorityLevel} from './SchedulerWithReactIntegration';
+import {
+  UserBlockingPriority,
+  NormalPriority,
+  runWithPriority,
+  getCurrentPriorityLevel,
+} from './SchedulerWithReactIntegration';
 
 const {ReactCurrentDispatcher, ReactCurrentBatchConfig} = ReactSharedInternals;
 
@@ -1135,15 +1139,13 @@ function mountDeferredValue<T>(
   const [prevValue, setValue] = mountState(value);
   mountEffect(
     () => {
-      Scheduler.unstable_next(() => {
-        const previousConfig = ReactCurrentBatchConfig.suspense;
-        ReactCurrentBatchConfig.suspense = config === undefined ? null : config;
-        try {
-          setValue(value);
-        } finally {
-          ReactCurrentBatchConfig.suspense = previousConfig;
-        }
-      });
+      const previousConfig = ReactCurrentBatchConfig.suspense;
+      ReactCurrentBatchConfig.suspense = config === undefined ? null : config;
+      try {
+        setValue(value);
+      } finally {
+        ReactCurrentBatchConfig.suspense = previousConfig;
+      }
     },
     [value, config],
   );
@@ -1157,65 +1159,62 @@ function updateDeferredValue<T>(
   const [prevValue, setValue] = updateState(value);
   updateEffect(
     () => {
-      Scheduler.unstable_next(() => {
-        const previousConfig = ReactCurrentBatchConfig.suspense;
-        ReactCurrentBatchConfig.suspense = config === undefined ? null : config;
-        try {
-          setValue(value);
-        } finally {
-          ReactCurrentBatchConfig.suspense = previousConfig;
-        }
-      });
+      const previousConfig = ReactCurrentBatchConfig.suspense;
+      ReactCurrentBatchConfig.suspense = config === undefined ? null : config;
+      try {
+        setValue(value);
+      } finally {
+        ReactCurrentBatchConfig.suspense = previousConfig;
+      }
     },
     [value, config],
   );
   return prevValue;
 }
 
+function startTransition(setPending, config, callback) {
+  const priorityLevel = getCurrentPriorityLevel();
+  runWithPriority(
+    priorityLevel < UserBlockingPriority ? UserBlockingPriority : priorityLevel,
+    () => {
+      setPending(true);
+    },
+  );
+  runWithPriority(
+    priorityLevel > NormalPriority ? NormalPriority : priorityLevel,
+    () => {
+      const previousConfig = ReactCurrentBatchConfig.suspense;
+      ReactCurrentBatchConfig.suspense = config === undefined ? null : config;
+      try {
+        setPending(false);
+        callback();
+      } finally {
+        ReactCurrentBatchConfig.suspense = previousConfig;
+      }
+    },
+  );
+}
+
 function mountTransition(
   config: SuspenseConfig | void | null,
 ): [(() => void) => void, boolean] {
   const [isPending, setPending] = mountState(false);
-  const startTransition = mountCallback(
-    callback => {
-      setPending(true);
-      Scheduler.unstable_next(() => {
-        const previousConfig = ReactCurrentBatchConfig.suspense;
-        ReactCurrentBatchConfig.suspense = config === undefined ? null : config;
-        try {
-          setPending(false);
-          callback();
-        } finally {
-          ReactCurrentBatchConfig.suspense = previousConfig;
-        }
-      });
-    },
-    [config, isPending],
-  );
-  return [startTransition, isPending];
+  const start = mountCallback(startTransition.bind(null, setPending, config), [
+    setPending,
+    config,
+  ]);
+  return [start, isPending];
 }
 
 function updateTransition(
   config: SuspenseConfig | void | null,
 ): [(() => void) => void, boolean] {
   const [isPending, setPending] = updateState(false);
-  const startTransition = updateCallback(
-    callback => {
-      setPending(true);
-      Scheduler.unstable_next(() => {
-        const previousConfig = ReactCurrentBatchConfig.suspense;
-        ReactCurrentBatchConfig.suspense = config === undefined ? null : config;
-        try {
-          setPending(false);
-          callback();
-        } finally {
-          ReactCurrentBatchConfig.suspense = previousConfig;
-        }
-      });
-    },
-    [config, isPending],
-  );
-  return [startTransition, isPending];
+  const start = updateCallback(startTransition.bind(null, setPending, config), [
+    setPending,
+    config,
+  ]);
+  return [start, isPending];
 }
 
 function dispatchAction<S, A>(

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

@@ -0,0 +1,102 @@
+/**
+ * Copyright (c) Facebook, Inc. and its affiliates.
+ *
+ * This source code is licensed under the MIT license found in the
+ * LICENSE file in the root directory of this source tree.
+ *
+ * @emails react-core
+ * @jest-environment node
+ */
+
+'use strict';
+
+let ReactFeatureFlags;
+let React;
+let ReactNoop;
+let Scheduler;
+let Suspense;
+let useState;
+let useTransition;
+let act;
+
+describe('ReactHooksWithNoopRenderer', () => {
+  beforeEach(() => {
+    jest.resetModules();
+
+    ReactFeatureFlags = require('shared/ReactFeatureFlags');
+    ReactFeatureFlags.debugRenderPhaseSideEffectsForStrictMode = false;
+    ReactFeatureFlags.enableSchedulerTracing = true;
+    ReactFeatureFlags.flushSuspenseFallbacksInTests = false;
+    React = require('react');
+    ReactNoop = require('react-noop-renderer');
+    Scheduler = require('scheduler');
+    useState = React.useState;
+    useTransition = React.useTransition;
+    Suspense = React.Suspense;
+    act = ReactNoop.act;
+  });
+
+  function Text(props) {
+    Scheduler.unstable_yieldValue(props.text);
+    return props.text;
+  }
+
+  function createAsyncText(text) {
+    let resolved = false;
+    let Component = function() {
+      if (!resolved) {
+        Scheduler.unstable_yieldValue('Suspend! [' + text + ']');
+        throw promise;
+      }
+      return <Text text={text} />;
+    };
+    let promise = new Promise(resolve => {
+      Component.resolve = function() {
+        resolved = true;
+        return resolve();
+      };
+    });
+    return Component;
+  }
+
+  it('isPending works even if called from outside an input event', async () => {
+    const Async = createAsyncText('Async');
+    let start;
+    function App() {
+      const [show, setShow] = useState(false);
+      const [startTransition, isPending] = useTransition();
+      start = () => startTransition(() => setShow(true));
+      return (
+        <Suspense fallback={<Text text="Loading..." />}>
+          {isPending ? <Text text="Pending..." /> : null}
+          {show ? <Async /> : <Text text="(empty)" />}
+        </Suspense>
+      );
+    }
+
+    const root = ReactNoop.createRoot();
+
+    await act(async () => {
+      root.render(<App />);
+    });
+    expect(Scheduler).toHaveYielded(['(empty)']);
+    expect(root).toMatchRenderedOutput('(empty)');
+
+    await act(async () => {
+      start();
+
+      expect(Scheduler).toFlushAndYield([
+        'Pending...',
+        '(empty)',
+        'Suspend! [Async]',
+        'Loading...',
+      ]);
+
+      expect(root).toMatchRenderedOutput('Pending...(empty)');
+
+      await Async.resolve();
+    });
+    expect(Scheduler).toHaveYielded(['Async']);
+    expect(root).toMatchRenderedOutput('Async');
+  });
+});