diff --git a/.circleci/config.yml b/.circleci/config.yml index ba1b117947d6a..2d0dd4e80ad36 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -178,6 +178,7 @@ jobs: - *restore_yarn_cache - *run_yarn - run: yarn test-build --maxWorkers=2 + - run: yarn test-dom-fixture test_fuzz: docker: *docker diff --git a/fixtures/dom/package.json b/fixtures/dom/package.json index 3282c85fa58a7..9bf118f0c29cd 100644 --- a/fixtures/dom/package.json +++ b/fixtures/dom/package.json @@ -18,7 +18,7 @@ }, "scripts": { "start": "react-scripts start", - "prestart": "cp ../../build/node_modules/scheduler/umd/scheduler-unstable_mock.development.js ../../build/node_modules/scheduler/umd/scheduler-unstable_mock.production.min.js ../../build/node_modules/react/umd/react.development.js ../../build/node_modules/react-dom/umd/react-dom.development.js ../../build/node_modules/react/umd/react.production.min.js ../../build/node_modules/react-dom/umd/react-dom.production.min.js ../../build/node_modules/react-dom/umd/react-dom-server.browser.development.js ../../build/node_modules/react-dom/umd/react-dom-server.browser.production.min.js ../../build/node_modules/react-dom/umd/react-dom-test-utils.development.js ../../build/node_modules/react-dom/umd/react-dom-test-utils.production.min.js public/", + "prestart": "cp ../../build/node_modules/scheduler/umd/scheduler-unstable_mock.development.js ../../build/node_modules/scheduler/umd/scheduler-unstable_mock.production.min.js ../../build/node_modules/react/umd/react.development.js ../../build/node_modules/react-dom/umd/react-dom.development.js ../../build/node_modules/react/umd/react.production.min.js ../../build/node_modules/react-dom/umd/react-dom.production.min.js ../../build/node_modules/react-dom/umd/react-dom-server.browser.development.js ../../build/node_modules/react-dom/umd/react-dom-server.browser.production.min.js ../../build/node_modules/react-dom/umd/react-dom-test-utils.development.js ../../build/node_modules/react-dom/umd/react-dom-test-utils.production.min.js public/ && cp -a ../../build/node_modules/. node_modules", "build": "react-scripts build && cp build/index.html build/200.html", "test": "react-scripts test --env=jsdom", "eject": "react-scripts eject" diff --git a/fixtures/dom/public/act-dom.html b/fixtures/dom/public/act-dom.html index 2100026a5ba45..2aa33da465e26 100644 --- a/fixtures/dom/public/act-dom.html +++ b/fixtures/dom/public/act-dom.html @@ -1,21 +1,21 @@ - - sanity test for ReactTestUtils.act - - - this page tests whether act runs properly in a browser. -
- your console should say "5" - - - - - - + + + + + - + + async function testAsyncAct() { + const el = document.createElement("div"); + await ReactTestUtils.act(async () => { + ReactDOM.render(React.createElement(App), el); + }); + // all 5 ticks present and accounted for + console.log(el.innerHTML); + } + + testAsyncAct(); + + diff --git a/fixtures/dom/src/index.test.js b/fixtures/dom/src/index.test.js new file mode 100644 index 0000000000000..0eddb918706b3 --- /dev/null +++ b/fixtures/dom/src/index.test.js @@ -0,0 +1,107 @@ +/** + * 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 + */ + +import React from 'react'; +import ReactDOM from 'react-dom'; +import TestUtils from 'react-dom/test-utils'; +import TestRenderer from 'react-test-renderer'; + +let spy; +beforeEach(() => { + spy = jest.spyOn(console, 'error').mockImplementation(() => {}); +}); + +function confirmWarning() { + expect(spy).toHaveBeenCalledWith( + expect.stringContaining( + "It looks like you're using the wrong act() around your test interactions." + ), + '' + ); +} + +function App(props) { + return 'hello world'; +} + +it("doesn't warn when you use the right act + renderer: dom", () => { + TestUtils.act(() => { + TestUtils.renderIntoDocument(); + }); + expect(spy).not.toHaveBeenCalled(); +}); + +it("doesn't warn when you use the right act + renderer: test", () => { + TestRenderer.act(() => { + TestRenderer.create(); + }); + expect(spy).not.toHaveBeenCalled(); +}); + +it('works with createRoot().render combo', () => { + const root = ReactDOM.unstable_createRoot(document.createElement('div')); + TestRenderer.act(() => { + root.render(); + }); + confirmWarning(); +}); + +it('warns when using the wrong act version - test + dom: render', () => { + TestRenderer.act(() => { + TestUtils.renderIntoDocument(); + }); + confirmWarning(); +}); + +it('warns when using the wrong act version - test + dom: updates', () => { + let setCtr; + function Counter(props) { + const [ctr, _setCtr] = React.useState(0); + setCtr = _setCtr; + return ctr; + } + TestUtils.renderIntoDocument(); + TestRenderer.act(() => { + setCtr(1); + }); + confirmWarning(); +}); + +it('warns when using the wrong act version - dom + test: .create()', () => { + TestUtils.act(() => { + TestRenderer.create(); + }); + confirmWarning(); +}); + +it('warns when using the wrong act version - dom + test: .update()', () => { + let root; + // use the right one here so we don't get the first warning + TestRenderer.act(() => { + root = TestRenderer.create(); + }); + TestUtils.act(() => { + root.update(); + }); + confirmWarning(); +}); + +it('warns when using the wrong act version - dom + test: updates', () => { + let setCtr; + function Counter(props) { + const [ctr, _setCtr] = React.useState(0); + setCtr = _setCtr; + return ctr; + } + const root = TestRenderer.create(); + TestUtils.act(() => { + setCtr(1); + }); + confirmWarning(); +}); diff --git a/package.json b/package.json index 2b7fe127ef9a0..b09bc1df76931 100644 --- a/package.json +++ b/package.json @@ -108,6 +108,7 @@ "test-prod-build": "yarn test-build-prod", "test-build": "cross-env NODE_ENV=development jest --config ./scripts/jest/config.build.js", "test-build-prod": "cross-env NODE_ENV=production jest --config ./scripts/jest/config.build.js", + "test-dom-fixture": "cd fixtures/dom && yarn && yarn prestart && yarn test", "flow": "node ./scripts/tasks/flow.js", "flow-ci": "node ./scripts/tasks/flow-ci.js", "prettier": "node ./scripts/prettier/index.js write-changed", diff --git a/packages/react-dom/src/client/ReactDOM.js b/packages/react-dom/src/client/ReactDOM.js index 3629ed3e1d6aa..1db9e527efe19 100644 --- a/packages/react-dom/src/client/ReactDOM.js +++ b/packages/react-dom/src/client/ReactDOM.js @@ -38,6 +38,7 @@ import { findHostInstance, findHostInstanceWithWarning, flushPassiveEffects, + ReactActingRendererSigil, } from 'react-reconciler/inline.dom'; import {createPortal as createPortalImpl} from 'shared/ReactPortal'; import {canUseDOM} from 'shared/ExecutionEnvironment'; @@ -816,6 +817,7 @@ const ReactDOM: Object = { dispatchEvent, runEventsInBatch, flushPassiveEffects, + ReactActingRendererSigil, ], }, }; diff --git a/packages/react-dom/src/fire/ReactFire.js b/packages/react-dom/src/fire/ReactFire.js index 6b22b7c43d678..bb74931fdc3ef 100644 --- a/packages/react-dom/src/fire/ReactFire.js +++ b/packages/react-dom/src/fire/ReactFire.js @@ -43,6 +43,7 @@ import { findHostInstance, findHostInstanceWithWarning, flushPassiveEffects, + ReactActingRendererSigil, } from 'react-reconciler/inline.fire'; import {createPortal as createPortalImpl} from 'shared/ReactPortal'; import {canUseDOM} from 'shared/ExecutionEnvironment'; @@ -822,6 +823,7 @@ const ReactDOM: Object = { dispatchEvent, runEventsInBatch, flushPassiveEffects, + ReactActingRendererSigil, ], }, }; diff --git a/packages/react-dom/src/test-utils/ReactTestUtils.js b/packages/react-dom/src/test-utils/ReactTestUtils.js index 5e39f754a2523..5f4d96fa9632b 100644 --- a/packages/react-dom/src/test-utils/ReactTestUtils.js +++ b/packages/react-dom/src/test-utils/ReactTestUtils.js @@ -42,8 +42,10 @@ const [ restoreStateIfNeeded, dispatchEvent, runEventsInBatch, - // eslint-disable-next-line no-unused-vars + /* eslint-disable no-unused-vars */ flushPassiveEffects, + ReactActingRendererSigil, + /* eslint-enable no-unused-vars */ ] = ReactDOM.__SECRET_INTERNALS_DO_NOT_USE_OR_YOU_WILL_BE_FIRED.Events; function Event(suffix) {} diff --git a/packages/react-dom/src/test-utils/ReactTestUtilsAct.js b/packages/react-dom/src/test-utils/ReactTestUtilsAct.js index bf4472b009745..e6dc6e38807f0 100644 --- a/packages/react-dom/src/test-utils/ReactTestUtilsAct.js +++ b/packages/react-dom/src/test-utils/ReactTestUtilsAct.js @@ -33,11 +33,12 @@ const [ runEventsInBatch, /* eslint-enable no-unused-vars */ flushPassiveEffects, + ReactActingRendererSigil, ] = ReactDOM.__SECRET_INTERNALS_DO_NOT_USE_OR_YOU_WILL_BE_FIRED.Events; const batchedUpdates = ReactDOM.unstable_batchedUpdates; -const {ReactShouldWarnActingUpdates} = ReactSharedInternals; +const {ReactCurrentActingRendererSigil} = ReactSharedInternals; // this implementation should be exactly the same in // ReactTestUtilsAct.js, ReactTestRendererAct.js, createReactNoop.js @@ -85,17 +86,17 @@ let actingUpdatesScopeDepth = 0; function act(callback: () => Thenable) { let previousActingUpdatesScopeDepth = actingUpdatesScopeDepth; + let previousActingUpdatesSigil; actingUpdatesScopeDepth++; if (__DEV__) { - ReactShouldWarnActingUpdates.current = true; + previousActingUpdatesSigil = ReactCurrentActingRendererSigil.current; + ReactCurrentActingRendererSigil.current = ReactActingRendererSigil; } function onDone() { actingUpdatesScopeDepth--; if (__DEV__) { - if (actingUpdatesScopeDepth === 0) { - ReactShouldWarnActingUpdates.current = false; - } + ReactCurrentActingRendererSigil.current = previousActingUpdatesSigil; if (actingUpdatesScopeDepth > previousActingUpdatesScopeDepth) { // if it's _less than_ previousActingUpdatesScopeDepth, then we can assume the 'other' one has warned warningWithoutStack( diff --git a/packages/react-noop-renderer/src/createReactNoop.js b/packages/react-noop-renderer/src/createReactNoop.js index 3d4ecf32e06f6..25effd1c2049e 100644 --- a/packages/react-noop-renderer/src/createReactNoop.js +++ b/packages/react-noop-renderer/src/createReactNoop.js @@ -81,7 +81,7 @@ type TextInstance = {| |}; type HostContext = Object; -const {ReactShouldWarnActingUpdates} = ReactSharedInternals; +const {ReactCurrentActingRendererSigil} = ReactSharedInternals; const NO_CONTEXT = {}; const UPPERCASE_CONTEXT = {}; @@ -650,7 +650,11 @@ function createReactNoop(reconciler: Function, useMutation: boolean) { const roots = new Map(); const DEFAULT_ROOT_ID = ''; - const {flushPassiveEffects, batchedUpdates} = NoopRenderer; + const { + flushPassiveEffects, + batchedUpdates, + ReactActingRendererSigil, + } = NoopRenderer; // this act() implementation should be exactly the same in // ReactTestUtilsAct.js, ReactTestRendererAct.js, createReactNoop.js @@ -698,17 +702,17 @@ function createReactNoop(reconciler: Function, useMutation: boolean) { function act(callback: () => Thenable) { let previousActingUpdatesScopeDepth = actingUpdatesScopeDepth; + let previousActingUpdatesSigil; actingUpdatesScopeDepth++; if (__DEV__) { - ReactShouldWarnActingUpdates.current = true; + previousActingUpdatesSigil = ReactCurrentActingRendererSigil.current; + ReactCurrentActingRendererSigil.current = ReactActingRendererSigil; } function onDone() { actingUpdatesScopeDepth--; if (__DEV__) { - if (actingUpdatesScopeDepth === 0) { - ReactShouldWarnActingUpdates.current = false; - } + ReactCurrentActingRendererSigil.current = previousActingUpdatesSigil; if (actingUpdatesScopeDepth > previousActingUpdatesScopeDepth) { // if it's _less than_ previousActingUpdatesScopeDepth, then we can assume the 'other' one has warned warningWithoutStack( diff --git a/packages/react-reconciler/src/ReactFiberHooks.js b/packages/react-reconciler/src/ReactFiberHooks.js index c3e9acdaa611f..2946c931cdc58 100644 --- a/packages/react-reconciler/src/ReactFiberHooks.js +++ b/packages/react-reconciler/src/ReactFiberHooks.js @@ -35,6 +35,7 @@ import { flushPassiveEffects, requestCurrentTime, warnIfNotCurrentlyActingUpdatesInDev, + warnIfNotScopedWithMatchingAct, markRenderEventTimeAndConfig, } from './ReactFiberWorkLoop'; @@ -1207,10 +1208,9 @@ function dispatchAction( } } if (__DEV__) { - // jest isn't a 'global', it's just exposed to tests via a wrapped function - // further, this isn't a test file, so flow doesn't recognize the symbol. So... - // $FlowExpectedError - because requirements don't give a damn about your type sigs. + // $FlowExpectedError - jest isn't a global, and isn't recognized outside of tests if ('undefined' !== typeof jest) { + warnIfNotScopedWithMatchingAct(fiber); warnIfNotCurrentlyActingUpdatesInDev(fiber); } } diff --git a/packages/react-reconciler/src/ReactFiberReconciler.js b/packages/react-reconciler/src/ReactFiberReconciler.js index 16214aa0332f5..de84dd7b67244 100644 --- a/packages/react-reconciler/src/ReactFiberReconciler.js +++ b/packages/react-reconciler/src/ReactFiberReconciler.js @@ -56,6 +56,8 @@ import { discreteUpdates, flushDiscreteUpdates, flushPassiveEffects, + warnIfNotScopedWithMatchingAct, + ReactActingRendererSigil, } from './ReactFiberWorkLoop'; import {createUpdate, enqueueUpdate} from './ReactUpdateQueue'; import ReactFiberInstrumentation from './ReactFiberInstrumentation'; @@ -303,6 +305,12 @@ export function updateContainer( ): ExpirationTime { const current = container.current; const currentTime = requestCurrentTime(); + if (__DEV__) { + // $FlowExpectedError - jest isn't a global, and isn't recognized outside of tests + if ('undefined' !== typeof jest) { + warnIfNotScopedWithMatchingAct(current); + } + } const suspenseConfig = requestCurrentSuspenseConfig(); const expirationTime = computeExpirationForFiber( currentTime, @@ -332,6 +340,7 @@ export { flushControlled, flushSync, flushPassiveEffects, + ReactActingRendererSigil, }; export function getPublicRootInstance( diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.js b/packages/react-reconciler/src/ReactFiberWorkLoop.js index 52beda4e5c8cb..270299d5f731f 100644 --- a/packages/react-reconciler/src/ReactFiberWorkLoop.js +++ b/packages/react-reconciler/src/ReactFiberWorkLoop.js @@ -173,7 +173,7 @@ const ceil = Math.ceil; const { ReactCurrentDispatcher, ReactCurrentOwner, - ReactShouldWarnActingUpdates, + ReactCurrentActingRendererSigil, } = ReactSharedInternals; type WorkPhase = 0 | 1 | 2 | 3 | 4 | 5 | 6; @@ -2271,11 +2271,46 @@ function warnAboutInvalidUpdatesOnClassComponentsInDEV(fiber) { } } +// We export a simple object here to be used by a renderer/test-utils +// as the value of ReactCurrentActingRendererSigil.current +// This identity lets us identify (ha!) when the wrong renderer's act() +// wraps anothers' updates/effects +export const ReactActingRendererSigil = {}; + +export function warnIfNotScopedWithMatchingAct(fiber: Fiber): void { + if (__DEV__) { + if ( + ReactCurrentActingRendererSigil.current !== null && + // use the function flushPassiveEffects directly as the sigil + // so this comparison is expected here + ReactCurrentActingRendererSigil.current !== ReactActingRendererSigil + ) { + // it looks like we're using the wrong matching act(), so log a warning + warningWithoutStack( + false, + "It looks like you're using the wrong act() around your test interactions.\n" + + 'Be sure to use the matching version of act() corresponding to your renderer:\n\n' + + '// for react-dom:\n' + + "import {act} from 'react-test-utils';\n" + + '//...\n' + + 'act(() => ...);\n\n' + + '// for react-test-renderer:\n' + + "import TestRenderer from 'react-test-renderer';\n" + + 'const {act} = TestRenderer;\n' + + '//...\n' + + 'act(() => ...);' + + '%s', + getStackByFiberInDevAndProd(fiber), + ); + } + } +} + function warnIfNotCurrentlyActingUpdatesInDEV(fiber: Fiber): void { if (__DEV__) { if ( workPhase === NotWorking && - ReactShouldWarnActingUpdates.current === false + ReactCurrentActingRendererSigil.current !== ReactActingRendererSigil ) { warningWithoutStack( false, diff --git a/packages/react-test-renderer/src/ReactTestRendererAct.js b/packages/react-test-renderer/src/ReactTestRendererAct.js index eafcae3ba79d2..ffd55af9eb579 100644 --- a/packages/react-test-renderer/src/ReactTestRendererAct.js +++ b/packages/react-test-renderer/src/ReactTestRendererAct.js @@ -11,6 +11,7 @@ import type {Thenable} from 'react-reconciler/src/ReactFiberWorkLoop'; import { batchedUpdates, flushPassiveEffects, + ReactActingRendererSigil, } from 'react-reconciler/inline.test'; import ReactSharedInternals from 'shared/ReactSharedInternals'; import warningWithoutStack from 'shared/warningWithoutStack'; @@ -18,7 +19,7 @@ import {warnAboutMissingMockScheduler} from 'shared/ReactFeatureFlags'; import enqueueTask from 'shared/enqueueTask'; import * as Scheduler from 'scheduler'; -const {ReactShouldWarnActingUpdates} = ReactSharedInternals; +const {ReactCurrentActingRendererSigil} = ReactSharedInternals; // this implementation should be exactly the same in // ReactTestUtilsAct.js, ReactTestRendererAct.js, createReactNoop.js @@ -66,17 +67,17 @@ let actingUpdatesScopeDepth = 0; function act(callback: () => Thenable) { let previousActingUpdatesScopeDepth = actingUpdatesScopeDepth; + let previousActingUpdatesSigil; actingUpdatesScopeDepth++; if (__DEV__) { - ReactShouldWarnActingUpdates.current = true; + previousActingUpdatesSigil = ReactCurrentActingRendererSigil.current; + ReactCurrentActingRendererSigil.current = ReactActingRendererSigil; } function onDone() { actingUpdatesScopeDepth--; if (__DEV__) { - if (actingUpdatesScopeDepth === 0) { - ReactShouldWarnActingUpdates.current = false; - } + ReactCurrentActingRendererSigil.current = previousActingUpdatesSigil; if (actingUpdatesScopeDepth > previousActingUpdatesScopeDepth) { // if it's _less than_ previousActingUpdatesScopeDepth, then we can assume the 'other' one has warned warningWithoutStack( diff --git a/packages/react/src/ReactCurrentActingRendererSigil.js b/packages/react/src/ReactCurrentActingRendererSigil.js new file mode 100644 index 0000000000000..09e05c8362d7b --- /dev/null +++ b/packages/react/src/ReactCurrentActingRendererSigil.js @@ -0,0 +1,19 @@ +/** + * 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 + */ + +/** + * Used by act() to track whether you're outside an act() scope. + * We use a renderer's flushPassiveEffects as the sigil value + * so we can track identity of the renderer. + */ + +const ReactCurrentActingRendererSigil = { + current: (null: null | (() => boolean)), +}; +export default ReactCurrentActingRendererSigil; diff --git a/packages/react/src/ReactSharedInternals.js b/packages/react/src/ReactSharedInternals.js index 1bc95cffaad4e..1d993139c4194 100644 --- a/packages/react/src/ReactSharedInternals.js +++ b/packages/react/src/ReactSharedInternals.js @@ -10,13 +10,13 @@ import ReactCurrentDispatcher from './ReactCurrentDispatcher'; import ReactCurrentBatchConfig from './ReactCurrentBatchConfig'; import ReactCurrentOwner from './ReactCurrentOwner'; import ReactDebugCurrentFrame from './ReactDebugCurrentFrame'; +import ReactCurrentActingRendererSigil from './ReactCurrentActingRendererSigil'; const ReactSharedInternals = { ReactCurrentDispatcher, ReactCurrentBatchConfig, ReactCurrentOwner, - // used by act() - ReactShouldWarnActingUpdates: {current: false}, + ReactCurrentActingRendererSigil, // Used by renderers to avoid bundling object-assign twice in UMD bundles: assign, };