Avoid importing Scheduler directly (#14757)

* Avoid importing Scheduler directly

The reconciler should not depend directly on Scheduler. This adds it to
the host config for the renderer instead.

(Except for `scheduler/tracing` imports, which are used only by the
profiling build. I've left those imports as-is, though I'm open to
directing those through the host config, too.)

* Make throwaway root id longer to appease Brian

packages/react-art/src/ReactARTHostConfig.js

@@ -5,6 +5,10 @@
  * LICENSE file in the root directory of this source tree.
  */
 
+import {
+  unstable_scheduleCallback as scheduleDeferredCallback,
+  unstable_cancelCallback as cancelDeferredCallback,
+} from 'scheduler';
 export {
   unstable_now as now,
   unstable_scheduleCallback as scheduleDeferredCallback,
@@ -337,6 +341,8 @@ export function getChildHostContext() {
 export const scheduleTimeout = setTimeout;
 export const cancelTimeout = clearTimeout;
 export const noTimeout = -1;
+export const schedulePassiveEffects = scheduleDeferredCallback;
+export const cancelPassiveEffects = cancelDeferredCallback;
 
 export function shouldSetTextContent(type, props) {
   return (

packages/react-dom/src/client/ReactDOMHostConfig.js

@@ -69,6 +69,10 @@ export type ChildSet = void; // Unused
 export type TimeoutHandle = TimeoutID;
 export type NoTimeout = -1;
 
+import {
+  unstable_scheduleCallback as scheduleDeferredCallback,
+  unstable_cancelCallback as cancelDeferredCallback,
+} from 'scheduler';
 export {
   unstable_now as now,
   unstable_scheduleCallback as scheduleDeferredCallback,
@@ -296,6 +300,8 @@ export const scheduleTimeout =
 export const cancelTimeout =
   typeof clearTimeout === 'function' ? clearTimeout : (undefined: any);
 export const noTimeout = -1;
+export const schedulePassiveEffects = scheduleDeferredCallback;
+export const cancelPassiveEffects = cancelDeferredCallback;
 
 // -------------------
 //     Mutation

packages/react-native-renderer/src/ReactFabricHostConfig.js

@@ -330,6 +330,8 @@ export const shouldYield = ReactNativeFrameSchedulingShouldYield;
 export const scheduleTimeout = setTimeout;
 export const cancelTimeout = clearTimeout;
 export const noTimeout = -1;
+export const schedulePassiveEffects = scheduleDeferredCallback;
+export const cancelPassiveEffects = cancelDeferredCallback;
 
 // -------------------
 //     Persistence

packages/react-native-renderer/src/ReactNativeHostConfig.js

@@ -243,6 +243,8 @@ export const shouldYield = ReactNativeFrameSchedulingShouldYield;
 export const scheduleTimeout = setTimeout;
 export const cancelTimeout = clearTimeout;
 export const noTimeout = -1;
+export const schedulePassiveEffects = scheduleDeferredCallback;
+export const cancelPassiveEffects = cancelDeferredCallback;
 
 export function shouldDeprioritizeSubtree(type: string, props: Props): boolean {
   return false;

packages/react-noop-renderer/src/createReactNoop.js

@@ -47,6 +47,7 @@ if (__DEV__) {
 function createReactNoop(reconciler: Function, useMutation: boolean) {
   let scheduledCallback = null;
   let scheduledCallbackTimeout = -1;
+  let scheduledPassiveCallback = null;
   let instanceCounter = 0;
   let hostDiffCounter = 0;
   let hostUpdateCounter = 0;
@@ -338,6 +339,21 @@ function createReactNoop(reconciler: Function, useMutation: boolean) {
     scheduleTimeout: setTimeout,
     cancelTimeout: clearTimeout,
     noTimeout: -1,
+    schedulePassiveEffects(callback) {
+      if (scheduledCallback) {
+        throw new Error(
+          'Scheduling a callback twice is excessive. Instead, keep track of ' +
+            'whether the callback has already been scheduled.',
+        );
+      }
+      scheduledPassiveCallback = callback;
+    },
+    cancelPassiveEffects() {
+      if (scheduledPassiveCallback === null) {
+        throw new Error('No passive effects callback is scheduled.');
+      }
+      scheduledPassiveCallback = null;
+    },
 
     prepareForCommit(): void {},
 
@@ -854,6 +870,16 @@ function createReactNoop(reconciler: Function, useMutation: boolean) {
       return yieldedValues;
     },
 
+    flushPassiveEffects() {
+      // Trick to flush passive effects without exposing an internal API:
+      // Create a throwaway root and schedule a dummy update on it.
+      const rootID = 'bloopandthenmoreletterstoavoidaconflict';
+      const container = {rootID: rootID, children: []};
+      rootContainers.set(rootID, container);
+      const root = NoopRenderer.createContainer(container, true, false);
+      NoopRenderer.updateContainer(null, root, null, null);
+    },
+
     // Logs the current state of the tree.
     dumpTree(rootID: string = DEFAULT_ROOT_ID) {
       const root = roots.get(rootID);

packages/react-reconciler/src/ReactFiberScheduler.js

@@ -17,10 +17,6 @@ import {
   __subscriberRef,
   unstable_wrap as Schedule_tracing_wrap,
 } from 'scheduler/tracing';
-import {
-  unstable_scheduleCallback as Schedule_scheduleCallback,
-  unstable_cancelCallback as Schedule_cancelCallback,
-} from 'scheduler';
 import {
   invokeGuardedCallback,
   hasCaughtError,
@@ -83,6 +79,8 @@ import {
   scheduleTimeout,
   cancelTimeout,
   noTimeout,
+  schedulePassiveEffects,
+  cancelPassiveEffects,
 } from './ReactFiberHostConfig';
 import {
   markPendingPriorityLevel,
@@ -587,8 +585,10 @@ function markLegacyErrorBoundaryAsFailed(instance: mixed) {
 }
 
 function flushPassiveEffects() {
+  if (passiveEffectCallbackHandle !== null) {
+    cancelPassiveEffects(passiveEffectCallbackHandle);
+  }
   if (passiveEffectCallback !== null) {
-    Schedule_cancelCallback(passiveEffectCallbackHandle);
     // We call the scheduled callback instead of commitPassiveEffects directly
     // to ensure tracing works correctly.
     passiveEffectCallback();
@@ -795,7 +795,7 @@ function commitRoot(root: FiberRoot, finishedWork: Fiber): void {
       // here because that code is still in flux.
       callback = Schedule_tracing_wrap(callback);
     }
-    passiveEffectCallbackHandle = Schedule_scheduleCallback(callback);
+    passiveEffectCallbackHandle = schedulePassiveEffects(callback);
     passiveEffectCallback = callback;
   }
 

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

@@ -25,7 +25,6 @@ let useMemo;
 let useRef;
 let useImperativeHandle;
 let forwardRef;
-let flushPassiveEffects;
 let memo;
 
 // These tests use React Noop Renderer. All new tests should use React Test
@@ -35,28 +34,6 @@ describe('ReactHooksWithNoopRenderer', () => {
   beforeEach(() => {
     jest.resetModules();
 
-    jest.mock('scheduler', () => {
-      let scheduledCallbacks = new Map();
-
-      flushPassiveEffects = () => {
-        scheduledCallbacks.forEach(cb => {
-          cb();
-        });
-        scheduledCallbacks = new Map();
-      };
-
-      return {
-        unstable_scheduleCallback(callback) {
-          const handle = {};
-          scheduledCallbacks.set(handle, callback);
-          return handle;
-        },
-        unstable_cancelCallback(handle) {
-          scheduledCallbacks.delete(handle);
-        },
-      };
-    });
-
     ReactFeatureFlags = require('shared/ReactFeatureFlags');
     ReactFeatureFlags.debugRenderPhaseSideEffectsForStrictMode = false;
     ReactFeatureFlags.enableSchedulerTracing = true;
@@ -609,14 +586,14 @@ describe('ReactHooksWithNoopRenderer', () => {
       ReactNoop.render(<Counter count={0} />);
       expect(ReactNoop.flush()).toEqual(['Count: 0']);
       expect(ReactNoop.getChildren()).toEqual([span('Count: 0')]);
-      flushPassiveEffects();
+      ReactNoop.flushPassiveEffects();
       expect(ReactNoop.clearYields()).toEqual(['Did commit [0]']);
 
       ReactNoop.render(<Counter count={1} />);
       expect(ReactNoop.flush()).toEqual(['Count: 1']);
       expect(ReactNoop.getChildren()).toEqual([span('Count: 1')]);
       // Effects are deferred until after the commit
-      flushPassiveEffects();
+      ReactNoop.flushPassiveEffects();
       expect(ReactNoop.clearYields()).toEqual(['Did commit [1]']);
     });
 
@@ -648,7 +625,7 @@ describe('ReactHooksWithNoopRenderer', () => {
       expect(ReactNoop.getChildren()).toEqual([span('Passive')]);
 
       // (No effects are left to flush.)
-      flushPassiveEffects();
+      ReactNoop.flushPassiveEffects();
       expect(ReactNoop.clearYields()).toEqual(null);
     });
 
@@ -747,7 +724,7 @@ describe('ReactHooksWithNoopRenderer', () => {
         ]);
         expect(ReactNoop.getChildren()).toEqual([span(1)]);
 
-        flushPassiveEffects();
+        ReactNoop.flushPassiveEffects();
         expect(ReactNoop.clearYields()).toEqual([
           'Committed state when effect was fired: 1',
         ]);
@@ -769,14 +746,14 @@ describe('ReactHooksWithNoopRenderer', () => {
       ReactNoop.render(<Counter count={0} />);
       expect(ReactNoop.flush()).toEqual(['Count: (empty)']);
       expect(ReactNoop.getChildren()).toEqual([span('Count: (empty)')]);
-      flushPassiveEffects();
+      ReactNoop.flushPassiveEffects();
       expect(ReactNoop.clearYields()).toEqual(['Schedule update [0]']);
       expect(ReactNoop.flush()).toEqual(['Count: 0']);
 
       ReactNoop.render(<Counter count={1} />);
       expect(ReactNoop.flush()).toEqual(['Count: 0']);
       expect(ReactNoop.getChildren()).toEqual([span('Count: 0')]);
-      flushPassiveEffects();
+      ReactNoop.flushPassiveEffects();
       expect(ReactNoop.clearYields()).toEqual(['Schedule update [1]']);
       expect(ReactNoop.flush()).toEqual(['Count: 1']);
     });
@@ -805,7 +782,7 @@ describe('ReactHooksWithNoopRenderer', () => {
       expect(ReactNoop.flush()).toEqual([]);
       expect(ReactNoop.getChildren()).toEqual([span('Count: 0')]);
 
-      flushPassiveEffects();
+      ReactNoop.flushPassiveEffects();
       expect(ReactNoop.flush()).toEqual(['Schedule update [1]', 'Count: 1']);
       expect(ReactNoop.getChildren()).toEqual([span('Count: 1')]);
     });
@@ -908,7 +885,7 @@ describe('ReactHooksWithNoopRenderer', () => {
         expect(ReactNoop.flush()).toEqual(['Count: (empty)']);
         expect(ReactNoop.getChildren()).toEqual([span('Count: (empty)')]);
         // Now fire the effects
-        flushPassiveEffects();
+        ReactNoop.flushPassiveEffects();
         // There were multiple updates, but there should only be a
         // single render
         expect(ReactNoop.clearYields()).toEqual(['Count: 0']);
@@ -935,7 +912,7 @@ describe('ReactHooksWithNoopRenderer', () => {
       expect(ReactNoop.getChildren()).toEqual([span('Count: (empty)')]);
 
       expect(() => {
-        flushPassiveEffects();
+        ReactNoop.flushPassiveEffects();
       }).toThrow('flushSync was called from inside a lifecycle method');
     });
 
@@ -952,13 +929,13 @@ describe('ReactHooksWithNoopRenderer', () => {
       ReactNoop.render(<Counter count={0} />);
       expect(ReactNoop.flush()).toEqual(['Count: 0']);
       expect(ReactNoop.getChildren()).toEqual([span('Count: 0')]);
-      flushPassiveEffects();
+      ReactNoop.flushPassiveEffects();
       expect(ReactNoop.clearYields()).toEqual(['Did create [0]']);
 
       ReactNoop.render(<Counter count={1} />);
       expect(ReactNoop.flush()).toEqual(['Count: 1']);
       expect(ReactNoop.getChildren()).toEqual([span('Count: 1')]);
-      flushPassiveEffects();
+      ReactNoop.flushPassiveEffects();
       expect(ReactNoop.clearYields()).toEqual([
         'Did destroy [0]',
         'Did create [1]',
@@ -978,7 +955,7 @@ describe('ReactHooksWithNoopRenderer', () => {
       ReactNoop.render(<Counter count={0} />);
       expect(ReactNoop.flush()).toEqual(['Count: 0']);
       expect(ReactNoop.getChildren()).toEqual([span('Count: 0')]);
-      flushPassiveEffects();
+      ReactNoop.flushPassiveEffects();
       expect(ReactNoop.clearYields()).toEqual(['Did create [0]']);
 
       ReactNoop.render(null);
@@ -999,13 +976,13 @@ describe('ReactHooksWithNoopRenderer', () => {
       ReactNoop.render(<Counter count={0} />);
       expect(ReactNoop.flush()).toEqual(['Count: 0']);
       expect(ReactNoop.getChildren()).toEqual([span('Count: 0')]);
-      flushPassiveEffects();
+      ReactNoop.flushPassiveEffects();
       expect(ReactNoop.clearYields()).toEqual(['Did create [0]']);
 
       ReactNoop.render(<Counter count={1} />);
       expect(ReactNoop.flush()).toEqual(['Count: 1']);
       expect(ReactNoop.getChildren()).toEqual([span('Count: 1')]);
-      flushPassiveEffects();
+      ReactNoop.flushPassiveEffects();
       expect(ReactNoop.clearYields()).toEqual(null);
 
       ReactNoop.render(null);
@@ -1027,13 +1004,13 @@ describe('ReactHooksWithNoopRenderer', () => {
       ReactNoop.render(<Counter count={0} />);
       expect(ReactNoop.flush()).toEqual(['Count: 0']);
       expect(ReactNoop.getChildren()).toEqual([span('Count: 0')]);
-      flushPassiveEffects();
+      ReactNoop.flushPassiveEffects();
       expect(ReactNoop.clearYields()).toEqual(['Did create']);
 
       ReactNoop.render(<Counter count={1} />);
       expect(ReactNoop.flush()).toEqual(['Count: 1']);
       expect(ReactNoop.getChildren()).toEqual([span('Count: 1')]);
-      flushPassiveEffects();
+      ReactNoop.flushPassiveEffects();
       expect(ReactNoop.clearYields()).toEqual(['Did destroy', 'Did create']);
 
       ReactNoop.render(null);
@@ -1057,7 +1034,7 @@ describe('ReactHooksWithNoopRenderer', () => {
       }
       ReactNoop.render(<Counter label="Count" count={0} />);
       expect(ReactNoop.flush()).toEqual(['Count: 0']);
-      flushPassiveEffects();
+      ReactNoop.flushPassiveEffects();
       expect(ReactNoop.clearYields()).toEqual(['Did create [Count: 0]']);
       expect(ReactNoop.getChildren()).toEqual([span('Count: 0')]);
 
@@ -1065,7 +1042,7 @@ describe('ReactHooksWithNoopRenderer', () => {
       // Count changed
       expect(ReactNoop.flush()).toEqual(['Count: 1']);
       expect(ReactNoop.getChildren()).toEqual([span('Count: 1')]);
-      flushPassiveEffects();
+      ReactNoop.flushPassiveEffects();
       expect(ReactNoop.clearYields()).toEqual([
         'Did destroy [Count: 0]',
         'Did create [Count: 1]',
@@ -1074,7 +1051,7 @@ describe('ReactHooksWithNoopRenderer', () => {
       ReactNoop.render(<Counter label="Count" count={1} />);
       // Nothing changed, so no effect should have fired
       expect(ReactNoop.flush()).toEqual(['Count: 1']);
-      flushPassiveEffects();
+      ReactNoop.flushPassiveEffects();
       expect(ReactNoop.clearYields()).toEqual(null);
       expect(ReactNoop.getChildren()).toEqual([span('Count: 1')]);
 
@@ -1082,7 +1059,7 @@ describe('ReactHooksWithNoopRenderer', () => {
       // Label changed
       expect(ReactNoop.flush()).toEqual(['Total: 1']);
       expect(ReactNoop.getChildren()).toEqual([span('Total: 1')]);
-      flushPassiveEffects();
+      ReactNoop.flushPassiveEffects();
       expect(ReactNoop.clearYields()).toEqual([
         'Did destroy [Count: 1]',
         'Did create [Total: 1]',
@@ -1102,7 +1079,7 @@ describe('ReactHooksWithNoopRenderer', () => {
       ReactNoop.render(<Counter count={0} />);
       expect(ReactNoop.flush()).toEqual(['Count: 0']);
       expect(ReactNoop.getChildren()).toEqual([span('Count: 0')]);
-      flushPassiveEffects();
+      ReactNoop.flushPassiveEffects();
       expect(ReactNoop.clearYields()).toEqual([
         'Did commit 1 [0]',
         'Did commit 2 [0]',
@@ -1111,7 +1088,7 @@ describe('ReactHooksWithNoopRenderer', () => {
       ReactNoop.render(<Counter count={1} />);
       expect(ReactNoop.flush()).toEqual(['Count: 1']);
       expect(ReactNoop.getChildren()).toEqual([span('Count: 1')]);
-      flushPassiveEffects();
+      ReactNoop.flushPassiveEffects();
       expect(ReactNoop.clearYields()).toEqual([
         'Did commit 1 [1]',
         'Did commit 2 [1]',
@@ -1137,13 +1114,13 @@ describe('ReactHooksWithNoopRenderer', () => {
       ReactNoop.render(<Counter count={0} />);
       expect(ReactNoop.flush()).toEqual(['Count: 0']);
       expect(ReactNoop.getChildren()).toEqual([span('Count: 0')]);
-      flushPassiveEffects();
+      ReactNoop.flushPassiveEffects();
       expect(ReactNoop.clearYields()).toEqual(['Mount A [0]', 'Mount B [0]']);
 
       ReactNoop.render(<Counter count={1} />);
       expect(ReactNoop.flush()).toEqual(['Count: 1']);
       expect(ReactNoop.getChildren()).toEqual([span('Count: 1')]);
-      flushPassiveEffects();
+      ReactNoop.flushPassiveEffects();
       expect(ReactNoop.clearYields()).toEqual([
         'Unmount A [0]',
         'Unmount B [0]',
@@ -1174,7 +1151,7 @@ describe('ReactHooksWithNoopRenderer', () => {
       ReactNoop.render(<Counter count={0} />);
       expect(ReactNoop.flush()).toEqual(['Count: 0']);
       expect(ReactNoop.getChildren()).toEqual([span('Count: 0')]);
-      expect(() => flushPassiveEffects()).toThrow('Oops');
+      expect(() => ReactNoop.flushPassiveEffects()).toThrow('Oops');
       expect(ReactNoop.clearYields()).toEqual([
         'Mount A [0]',
         'Oops!',
@@ -1208,14 +1185,14 @@ describe('ReactHooksWithNoopRenderer', () => {
       ReactNoop.render(<Counter count={0} />);
       expect(ReactNoop.flush()).toEqual(['Count: 0']);
       expect(ReactNoop.getChildren()).toEqual([span('Count: 0')]);
-      flushPassiveEffects();
+      ReactNoop.flushPassiveEffects();
       expect(ReactNoop.clearYields()).toEqual(['Mount A [0]', 'Mount B [0]']);
 
       // This update will trigger an errror
       ReactNoop.render(<Counter count={1} />);
       expect(ReactNoop.flush()).toEqual(['Count: 1']);
       expect(ReactNoop.getChildren()).toEqual([span('Count: 1')]);
-      expect(() => flushPassiveEffects()).toThrow('Oops');
+      expect(() => ReactNoop.flushPassiveEffects()).toThrow('Oops');
       expect(ReactNoop.clearYields()).toEqual([
         'Unmount A [0]',
         'Unmount B [0]',
@@ -1250,14 +1227,14 @@ describe('ReactHooksWithNoopRenderer', () => {
       ReactNoop.render(<Counter count={0} />);
       expect(ReactNoop.flush()).toEqual(['Count: 0']);
       expect(ReactNoop.getChildren()).toEqual([span('Count: 0')]);
-      flushPassiveEffects();
+      ReactNoop.flushPassiveEffects();
       expect(ReactNoop.clearYields()).toEqual(['Mount A [0]', 'Mount B [0]']);
 
       // This update will trigger an errror
       ReactNoop.render(<Counter count={1} />);
       expect(ReactNoop.flush()).toEqual(['Count: 1']);
       expect(ReactNoop.getChildren()).toEqual([span('Count: 1')]);
-      expect(() => flushPassiveEffects()).toThrow('Oops');
+      expect(() => ReactNoop.flushPassiveEffects()).toThrow('Oops');
       expect(ReactNoop.clearYields()).toEqual([
         'Oops!',
         // B unmounts even though an error was thrown in the previous effect
@@ -1351,7 +1328,7 @@ describe('ReactHooksWithNoopRenderer', () => {
       ]);
       expect(committedText).toEqual('1');
 
-      flushPassiveEffects();
+      ReactNoop.flushPassiveEffects();
       expect(ReactNoop.clearYields()).toEqual([
         'Unmount normal [current: 1]',
         'Mount normal [current: 1]',
@@ -1705,7 +1682,7 @@ describe('ReactHooksWithNoopRenderer', () => {
 
       ReactNoop.render(<App showMore={false} />);
       expect(ReactNoop.flush()).toEqual([]);
-      flushPassiveEffects();
+      ReactNoop.flushPassiveEffects();
       expect(ReactNoop.clearYields()).toEqual(['Mount A']);
 
       ReactNoop.render(<App showMore={true} />);
@@ -1714,7 +1691,7 @@ describe('ReactHooksWithNoopRenderer', () => {
       }).toThrow('Rendered more hooks than during the previous render');
 
       // Uncomment if/when we support this again
-      // flushPassiveEffects();
+      // ReactNoop.flushPassiveEffects();
       // expect(ReactNoop.clearYields()).toEqual(['Mount B']);
 
       // ReactNoop.render(<App showMore={false} />);

packages/react-reconciler/src/forks/ReactFiberHostConfig.custom.js

@@ -56,6 +56,8 @@ export const shouldYield = $$$hostConfig.shouldYield;
 export const scheduleTimeout = $$$hostConfig.setTimeout;
 export const cancelTimeout = $$$hostConfig.clearTimeout;
 export const noTimeout = $$$hostConfig.noTimeout;
+export const schedulePassiveEffects = $$$hostConfig.schedulePassiveEffects;
+export const cancelPassiveEffects = $$$hostConfig.cancelPassiveEffects;
 export const now = $$$hostConfig.now;
 export const isPrimaryRenderer = $$$hostConfig.isPrimaryRenderer;
 export const supportsMutation = $$$hostConfig.supportsMutation;

packages/react-test-renderer/src/ReactTestHostConfig.js

@@ -210,6 +210,8 @@ export const shouldYield = TestRendererSchedulingShouldYield;
 export const scheduleTimeout = setTimeout;
 export const cancelTimeout = clearTimeout;
 export const noTimeout = -1;
+export const schedulePassiveEffects = scheduleDeferredCallback;
+export const cancelPassiveEffects = cancelDeferredCallback;
 
 // -------------------
 //     Mutation