From 5a653dee4ef348ad49a69dce03095b99e3ac8c2d Mon Sep 17 00:00:00 2001 From: Andrew Clark Date: Fri, 12 Feb 2021 01:19:56 -0600 Subject: [PATCH 1/2] Add `supportsMicrotasks` to the host config MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Only certain renderers support scheduling a microtask, so we need a renderer specific flag that we can toggle. That way it's off for some renderers and on for others. I copied the approach we use for the other optional parts of the host config, like persistent mode and test selectors. Why isn't the feature flag sufficient? The feature flag modules, confusingly, are not renderer-specific, at least when running the our tests against the source files. They are meant to correspond to a release channel, not a renderer, but we got confused at some point and haven't cleaned it up. For example, when we run `yarn test`, Jest loads the flags from the default `ReactFeatureFlags.js` module, even when we import the React Native renderer — but in the actual builds, we load a different feature flag module, `ReactFeatureFlags.native-oss.js.` There's no way in our current Jest load a different host config for each renderer, because they all just import the same module. We should solve this by creating separate Jest project for each renderer, so that the flags loaded when running against source are the same ones that we use in the compiled bundles. The feature flag (`enableDiscreteMicrotasks`) still exists — it's used to set the React DOM host config's `supportsMicrotasks` flag to `true`. (Same for React Noop) The important part is that turning on the feature flag does *not* affect the other renderers, like React Native. The host config will likely outlive the feature flag, too, since the feature flag only exists so we can gradually roll it out and measure the impact in production; once we do, we'll remove it. Whereas the host config flag may continue to be used to disable the discrete microtask behavior for RN, because RN will likely use a native (non-JavaScript) API to schedule its tasks. --- packages/react-art/src/ReactARTHostConfig.js | 4 +-- .../src/client/ReactDOMHostConfig.js | 6 +++++ .../src/ReactFabricHostConfig.js | 4 +-- .../src/ReactNativeHostConfig.js | 4 +-- .../src/createReactNoop.js | 7 ++++- .../ReactFiberHostConfigWithNoMicrotasks.js | 26 +++++++++++++++++++ .../src/ReactFiberWorkLoop.new.js | 6 ++--- .../src/ReactFiberWorkLoop.old.js | 6 ++--- .../src/forks/ReactFiberHostConfig.custom.js | 8 +++++- .../src/ReactTestHostConfig.js | 17 ++---------- scripts/error-codes/codes.json | 3 ++- 11 files changed, 58 insertions(+), 33 deletions(-) create mode 100644 packages/react-reconciler/src/ReactFiberHostConfigWithNoMicrotasks.js diff --git a/packages/react-art/src/ReactARTHostConfig.js b/packages/react-art/src/ReactARTHostConfig.js index de20fe5182c6c..cadf311d54ffd 100644 --- a/packages/react-art/src/ReactARTHostConfig.js +++ b/packages/react-art/src/ReactARTHostConfig.js @@ -249,6 +249,7 @@ export * from 'react-reconciler/src/ReactFiberHostConfigWithNoPersistence'; export * from 'react-reconciler/src/ReactFiberHostConfigWithNoHydration'; export * from 'react-reconciler/src/ReactFiberHostConfigWithNoScopes'; export * from 'react-reconciler/src/ReactFiberHostConfigWithNoTestSelectors'; +export * from 'react-reconciler/src/ReactFiberHostConfigWithNoMicrotasks'; export function appendInitialChild(parentInstance, child) { if (typeof child === 'string') { @@ -338,9 +339,6 @@ export function getChildHostContext() { export const scheduleTimeout = setTimeout; export const cancelTimeout = clearTimeout; export const noTimeout = -1; -export function scheduleMicrotask(callback: Function) { - invariant(false, 'Not implemented.'); -} export function shouldSetTextContent(type, props) { return ( diff --git a/packages/react-dom/src/client/ReactDOMHostConfig.js b/packages/react-dom/src/client/ReactDOMHostConfig.js index aca426ce08ecb..0f27a15db3d70 100644 --- a/packages/react-dom/src/client/ReactDOMHostConfig.js +++ b/packages/react-dom/src/client/ReactDOMHostConfig.js @@ -67,6 +67,7 @@ import { enableCreateEventHandleAPI, enableScopeAPI, enableNewReconciler, + enableDiscreteEventMicroTasks, } from 'shared/ReactFeatureFlags'; import {HostComponent, HostText} from 'react-reconciler/src/ReactWorkTags'; import {listenToAllSupportedEvents} from '../events/DOMPluginEventSystem'; @@ -399,6 +400,11 @@ export const scheduleTimeout: any = export const cancelTimeout: any = typeof clearTimeout === 'function' ? clearTimeout : (undefined: any); export const noTimeout = -1; + +// ------------------- +// Microtasks +// ------------------- +export const supportsMicrotasks = enableDiscreteEventMicroTasks; export const scheduleMicrotask: any = typeof queueMicrotask === 'function' ? queueMicrotask diff --git a/packages/react-native-renderer/src/ReactFabricHostConfig.js b/packages/react-native-renderer/src/ReactFabricHostConfig.js index 27b7076377fc4..e06652048e3da 100644 --- a/packages/react-native-renderer/src/ReactFabricHostConfig.js +++ b/packages/react-native-renderer/src/ReactFabricHostConfig.js @@ -193,6 +193,7 @@ export * from 'react-reconciler/src/ReactFiberHostConfigWithNoMutation'; export * from 'react-reconciler/src/ReactFiberHostConfigWithNoHydration'; export * from 'react-reconciler/src/ReactFiberHostConfigWithNoScopes'; export * from 'react-reconciler/src/ReactFiberHostConfigWithNoTestSelectors'; +export * from 'react-reconciler/src/ReactFiberHostConfigWithNoMicrotasks'; export function appendInitialChild( parentInstance: Instance, @@ -360,9 +361,6 @@ export const warnsIfNotActing = false; export const scheduleTimeout = setTimeout; export const cancelTimeout = clearTimeout; export const noTimeout = -1; -export function scheduleMicrotask(callback: Function) { - invariant(false, 'Not implemented.'); -} // ------------------- // Persistence diff --git a/packages/react-native-renderer/src/ReactNativeHostConfig.js b/packages/react-native-renderer/src/ReactNativeHostConfig.js index 7f11358556287..d626a175d9340 100644 --- a/packages/react-native-renderer/src/ReactNativeHostConfig.js +++ b/packages/react-native-renderer/src/ReactNativeHostConfig.js @@ -97,6 +97,7 @@ export * from 'react-reconciler/src/ReactFiberHostConfigWithNoPersistence'; export * from 'react-reconciler/src/ReactFiberHostConfigWithNoHydration'; export * from 'react-reconciler/src/ReactFiberHostConfigWithNoScopes'; export * from 'react-reconciler/src/ReactFiberHostConfigWithNoTestSelectors'; +export * from 'react-reconciler/src/ReactFiberHostConfigWithNoMicrotasks'; export function appendInitialChild( parentInstance: Instance, @@ -255,9 +256,6 @@ export const warnsIfNotActing = true; export const scheduleTimeout = setTimeout; export const cancelTimeout = clearTimeout; export const noTimeout = -1; -export function scheduleMicrotask(callback: Function) { - invariant(false, 'Not implemented.'); -} export function shouldSetTextContent(type: string, props: Props): boolean { // TODO (bvaughn) Revisit this decision. diff --git a/packages/react-noop-renderer/src/createReactNoop.js b/packages/react-noop-renderer/src/createReactNoop.js index 2aa0043d9e24a..26c58ffd8445c 100644 --- a/packages/react-noop-renderer/src/createReactNoop.js +++ b/packages/react-noop-renderer/src/createReactNoop.js @@ -27,7 +27,10 @@ import { LegacyRoot, } from 'react-reconciler/src/ReactRootTags'; -import {enableNativeEventPriorityInference} from 'shared/ReactFeatureFlags'; +import { + enableNativeEventPriorityInference, + enableDiscreteEventMicroTasks, +} from 'shared/ReactFeatureFlags'; import ReactSharedInternals from 'shared/ReactSharedInternals'; import enqueueTask from 'shared/enqueueTask'; const {IsSomeRendererActing} = ReactSharedInternals; @@ -372,6 +375,8 @@ function createReactNoop(reconciler: Function, useMutation: boolean) { scheduleTimeout: setTimeout, cancelTimeout: clearTimeout, noTimeout: -1, + + supportsMicrotasks: enableDiscreteEventMicroTasks, scheduleMicrotask: typeof queueMicrotask === 'function' ? queueMicrotask diff --git a/packages/react-reconciler/src/ReactFiberHostConfigWithNoMicrotasks.js b/packages/react-reconciler/src/ReactFiberHostConfigWithNoMicrotasks.js new file mode 100644 index 0000000000000..4d88494862016 --- /dev/null +++ b/packages/react-reconciler/src/ReactFiberHostConfigWithNoMicrotasks.js @@ -0,0 +1,26 @@ +/** + * 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. + * + * @flow + */ + +import invariant from 'shared/invariant'; + +// Renderers that don't support microtasks +// can re-export everything from this module. + +function shim(...args: any) { + invariant( + false, + 'The current renderer does not support microtasks. ' + + 'This error is likely caused by a bug in React. ' + + 'Please file an issue.', + ); +} + +// Test selectors (when unsupported) +export const supportsMicrotasks = false; +export const scheduleMicrotask = shim; diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.new.js b/packages/react-reconciler/src/ReactFiberWorkLoop.new.js index b21e951cbc857..6935bb4b1f18d 100644 --- a/packages/react-reconciler/src/ReactFiberWorkLoop.new.js +++ b/packages/react-reconciler/src/ReactFiberWorkLoop.new.js @@ -93,8 +93,9 @@ import { warnsIfNotActing, afterActiveInstanceBlur, clearContainer, - scheduleMicrotask, getCurrentEventPriority, + supportsMicrotasks, + scheduleMicrotask, } from './ReactFiberHostConfig'; import { @@ -219,7 +220,6 @@ import { syncNestedUpdateFlag, } from './ReactProfilerTimer.new'; -import {enableDiscreteEventMicroTasks} from 'shared/ReactFeatureFlags'; // DEV stuff import getComponentName from 'shared/getComponentName'; import ReactStrictModeWarnings from './ReactStrictModeWarnings.new'; @@ -756,7 +756,7 @@ function ensureRootIsScheduled(root: FiberRoot, currentTime: number) { performSyncWorkOnRoot.bind(null, root), ); } else if ( - enableDiscreteEventMicroTasks && + supportsMicrotasks && newCallbackPriority === InputDiscreteLanePriority ) { scheduleMicrotask(performSyncWorkOnRoot.bind(null, root)); diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.old.js b/packages/react-reconciler/src/ReactFiberWorkLoop.old.js index c700acc4efe54..2b57becf8bd79 100644 --- a/packages/react-reconciler/src/ReactFiberWorkLoop.old.js +++ b/packages/react-reconciler/src/ReactFiberWorkLoop.old.js @@ -93,8 +93,9 @@ import { warnsIfNotActing, afterActiveInstanceBlur, clearContainer, - scheduleMicrotask, getCurrentEventPriority, + supportsMicrotasks, + scheduleMicrotask, } from './ReactFiberHostConfig'; import { @@ -219,7 +220,6 @@ import { syncNestedUpdateFlag, } from './ReactProfilerTimer.old'; -import {enableDiscreteEventMicroTasks} from 'shared/ReactFeatureFlags'; // DEV stuff import getComponentName from 'shared/getComponentName'; import ReactStrictModeWarnings from './ReactStrictModeWarnings.old'; @@ -756,7 +756,7 @@ function ensureRootIsScheduled(root: FiberRoot, currentTime: number) { performSyncWorkOnRoot.bind(null, root), ); } else if ( - enableDiscreteEventMicroTasks && + supportsMicrotasks && newCallbackPriority === InputDiscreteLanePriority ) { scheduleMicrotask(performSyncWorkOnRoot.bind(null, root)); diff --git a/packages/react-reconciler/src/forks/ReactFiberHostConfig.custom.js b/packages/react-reconciler/src/forks/ReactFiberHostConfig.custom.js index 0e19b6b70414d..a2f065102d05f 100644 --- a/packages/react-reconciler/src/forks/ReactFiberHostConfig.custom.js +++ b/packages/react-reconciler/src/forks/ReactFiberHostConfig.custom.js @@ -54,7 +54,6 @@ export const shouldSetTextContent = $$$hostConfig.shouldSetTextContent; export const createTextInstance = $$$hostConfig.createTextInstance; export const scheduleTimeout = $$$hostConfig.scheduleTimeout; export const cancelTimeout = $$$hostConfig.cancelTimeout; -export const scheduleMicrotask = $$$hostConfig.scheduleMicrotask; export const noTimeout = $$$hostConfig.noTimeout; export const now = $$$hostConfig.now; export const isPrimaryRenderer = $$$hostConfig.isPrimaryRenderer; @@ -75,6 +74,13 @@ export const prepareScopeUpdate = $$$hostConfig.preparePortalMount; export const getInstanceFromScope = $$$hostConfig.getInstanceFromScope; export const getCurrentEventPriority = $$$hostConfig.getCurrentEventPriority; +// ------------------- +// Microtasks +// (optional) +// ------------------- +export const supportsMicrotasks = $$$hostConfig.supportsMicrotasks; +export const scheduleMicrotask = $$$hostConfig.scheduleMicrotask; + // ------------------- // Test selectors // (optional) diff --git a/packages/react-test-renderer/src/ReactTestHostConfig.js b/packages/react-test-renderer/src/ReactTestHostConfig.js index 60de8a05a17ea..a787d31840af3 100644 --- a/packages/react-test-renderer/src/ReactTestHostConfig.js +++ b/packages/react-test-renderer/src/ReactTestHostConfig.js @@ -58,6 +58,7 @@ export type RendererInspectionConfig = $ReadOnly<{||}>; export * from 'react-reconciler/src/ReactFiberHostConfigWithNoPersistence'; export * from 'react-reconciler/src/ReactFiberHostConfigWithNoHydration'; export * from 'react-reconciler/src/ReactFiberHostConfigWithNoTestSelectors'; +export * from 'react-reconciler/src/ReactFiberHostConfigWithNoMicrotasks'; const NO_CONTEXT = {}; const UPDATE_SIGNAL = {}; @@ -230,21 +231,7 @@ export const warnsIfNotActing = true; export const scheduleTimeout = setTimeout; export const cancelTimeout = clearTimeout; -export const scheduleMicrotask = - typeof queueMicrotask === 'function' - ? queueMicrotask - : typeof Promise !== 'undefined' - ? (callback: Function) => - Promise.resolve(null) - .then(callback) - .catch(handleErrorInNextTick) - : scheduleTimeout; // TODO: Determine the best fallback here. - -function handleErrorInNextTick(error) { - setTimeout(() => { - throw error; - }); -} + export const noTimeout = -1; // ------------------- diff --git a/scripts/error-codes/codes.json b/scripts/error-codes/codes.json index e758c79be2e58..c29a136a1e1ec 100644 --- a/scripts/error-codes/codes.json +++ b/scripts/error-codes/codes.json @@ -373,5 +373,6 @@ "382": "This query has received more parameters than the last time the same query was used. Always pass the exact number of parameters that the query needs.", "383": "This query has received fewer parameters than the last time the same query was used. Always pass the exact number of parameters that the query needs.", "384": "Refreshing the cache is not supported in Server Components.", - "385": "A mutable source was mutated while the %s component was rendering. This is not supported. Move any mutations into event handlers or effects." + "385": "A mutable source was mutated while the %s component was rendering. This is not supported. Move any mutations into event handlers or effects.", + "386": "The current renderer does not support microtasks. This error is likely caused by a bug in React. Please file an issue." } From 0c2c7f2c81052560e7b71d607dfc87adf6f92847 Mon Sep 17 00:00:00 2001 From: Andrew Clark Date: Fri, 12 Feb 2021 15:05:57 -0600 Subject: [PATCH 2/2] Add `supportsMicrotask` to react-reconciler README --- packages/react-reconciler/README.md | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/packages/react-reconciler/README.md b/packages/react-reconciler/README.md index ce5fddb0eea91..fd87321f27f4e 100644 --- a/packages/react-reconciler/README.md +++ b/packages/react-reconciler/README.md @@ -203,9 +203,12 @@ You can proxy this to `clearTimeout` or its equivalent in your environment. This is a property (not a function) that should be set to something that can never be a valid timeout ID. For example, you can set it to `-1`. +#### `supportsMicrotask` + +Set this to true to indicate that your renderer supports `scheduleMicrotask`. We use microtasks as part of our discrete event implementation in React DOM. If you're not sure if your renderer should support this, you probably should. The option to not implement `scheduleMicrotask` exists so that platforms with more control over user events, like React Native, can choose to use a different mechanism. #### `scheduleMicrotask(fn)` -You can proxy this to `queueMicrotask` or its equivalent in your environment. +Optional. You can proxy this to `queueMicrotask` or its equivalent in your environment. #### `isPrimaryRenderer`