From 5b186a693c280363cc0126938602c00ccba71bad Mon Sep 17 00:00:00 2001
From: Sunil Pai <threepointone@gmail.com>
Date: Tue, 28 May 2019 23:38:30 +0100
Subject: [PATCH 1/6] warn when using the wrong renderer's act around another
renderer's updates
like it says. it uses a real object as the sigil (instead of just a boolean). specifically, it uses a renderer's flushPassiveEffects as the sigil. We also run tests for this separate from our main suite (which doesn't allow loading multiple renderers in a suite), but makes sure to run this in CI as well.
---
fixtures/dom/package.json | 2 +-
fixtures/dom/public/act-dom.html | 65 ++++++-----
fixtures/dom/src/index.test.js | 107 ++++++++++++++++++
package.json | 1 +
.../src/test-utils/ReactTestUtilsAct.js | 13 +--
.../src/createReactNoop.js | 13 +--
.../react-reconciler/src/ReactFiberHooks.js | 2 +
.../src/ReactFiberReconciler.js | 7 ++
.../src/ReactFiberWorkLoop.js | 40 ++++++-
.../src/ReactTestRendererAct.js | 13 +--
.../react/src/ReactActingRendererSigil.js | 19 ++++
packages/react/src/ReactSharedInternals.js | 4 +-
12 files changed, 227 insertions(+), 59 deletions(-)
create mode 100644 fixtures/dom/src/index.test.js
create mode 100644 packages/react/src/ReactActingRendererSigil.js
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 @@
<!DOCTYPE html>
<html>
-<head>
- <title>sanity test for ReactTestUtils.act</title>
-</head>
-<body>
- this page tests whether act runs properly in a browser.
- <br/>
- your console should say "5"
- <script src='scheduler-unstable_mock.development.js'></script>
- <script src='react.development.js'></script>
- <script type="text/javascript">
- window.React.__SECRET_INTERNALS_DO_NOT_USE_OR_YOU_WILL_BE_FIRED.Scheduler = window.SchedulerMock
- </script>
- <script src='react-dom.development.js'></script>
- <script src='react-dom-test-utils.development.js'></script>
- <script>
- async function run(){
+ <head>
+ <title>sanity test for ReactTestUtils.act</title>
+ </head>
+ <body>
+ this page tests whether act runs properly in a browser.
+ <br />
+ your console should say "5"
+ <script src="scheduler-unstable_mock.development.js"></script>
+ <script src="react.development.js"></script>
+ <script type="text/javascript">
+ window.React.__SECRET_INTERNALS_DO_NOT_USE_OR_YOU_WILL_BE_FIRED.Scheduler =
+ window.SchedulerMock;
+ </script>
+ <script src="react-dom.development.js"></script>
+ <script src="react-dom-test-utils.development.js"></script>
+ <script>
// from ReactTestUtilsAct-test.js
function App() {
let [state, setState] = React.useState(0);
@@ -23,23 +23,22 @@
await null;
setState(x => x + 1);
}
- React.useEffect(
- () => {
- ticker();
- },
- [Math.min(state, 4)],
- );
+ React.useEffect(() => {
+ ticker();
+ }, [Math.min(state, 4)]);
return state;
}
- 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);
- }
- run();
-
- </script>
-</body>
+
+ 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();
+ </script>
+ </body>
</html>
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(<App />);
+ });
+ expect(spy).not.toHaveBeenCalled();
+});
+
+it("doesn't warn when you use the right act + renderer: test", () => {
+ TestRenderer.act(() => {
+ TestRenderer.create(<App />);
+ });
+ expect(spy).not.toHaveBeenCalled();
+});
+
+it('works with createRoot().render combo', () => {
+ const root = ReactDOM.unstable_createRoot(document.createElement('div'));
+ TestRenderer.act(() => {
+ root.render(<App />);
+ });
+ confirmWarning();
+});
+
+it('warns when using the wrong act version - test + dom: render', () => {
+ TestRenderer.act(() => {
+ TestUtils.renderIntoDocument(<App />);
+ });
+ 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(<Counter />);
+ TestRenderer.act(() => {
+ setCtr(1);
+ });
+ confirmWarning();
+});
+
+it('warns when using the wrong act version - dom + test: .create()', () => {
+ TestUtils.act(() => {
+ TestRenderer.create(<App />);
+ });
+ 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(<App key="one" />);
+ });
+ TestUtils.act(() => {
+ root.update(<App key="two" />);
+ });
+ 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(<Counter />);
+ TestUtils.act(() => {
+ setCtr(1);
+ });
+ confirmWarning();
+});
diff --git a/package.json b/package.json
index 97998b6b6d4f7..03ba206a0d3bd 100644
--- a/package.json
+++ b/package.json
@@ -107,6 +107,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/test-utils/ReactTestUtilsAct.js b/packages/react-dom/src/test-utils/ReactTestUtilsAct.js
index bf4472b009745..b0e0ebe80fd1e 100644
--- a/packages/react-dom/src/test-utils/ReactTestUtilsAct.js
+++ b/packages/react-dom/src/test-utils/ReactTestUtilsAct.js
@@ -37,7 +37,7 @@ const [
const batchedUpdates = ReactDOM.unstable_batchedUpdates;
-const {ReactShouldWarnActingUpdates} = ReactSharedInternals;
+const {ReactActingRendererSigil} = ReactSharedInternals;
// this implementation should be exactly the same in
// ReactTestUtilsAct.js, ReactTestRendererAct.js, createReactNoop.js
@@ -85,17 +85,16 @@ let actingUpdatesScopeDepth = 0;
function act(callback: () => Thenable) {
let previousActingUpdatesScopeDepth = actingUpdatesScopeDepth;
+ let previousActingUpdatesSigil = ReactActingRendererSigil.current;
actingUpdatesScopeDepth++;
- if (__DEV__) {
- ReactShouldWarnActingUpdates.current = true;
- }
+ // we use the function flushPassiveEffects directly as the sigil,
+ // since it's unique to a renderer
+ ReactActingRendererSigil.current = flushPassiveEffects;
function onDone() {
actingUpdatesScopeDepth--;
+ ReactActingRendererSigil.current = previousActingUpdatesSigil;
if (__DEV__) {
- if (actingUpdatesScopeDepth === 0) {
- ReactShouldWarnActingUpdates.current = false;
- }
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 a21a9d95c1bab..60a702a0b9eb4 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 {ReactActingRendererSigil} = ReactSharedInternals;
const NO_CONTEXT = {};
const UPPERCASE_CONTEXT = {};
@@ -698,17 +698,16 @@ function createReactNoop(reconciler: Function, useMutation: boolean) {
function act(callback: () => Thenable) {
let previousActingUpdatesScopeDepth = actingUpdatesScopeDepth;
+ let previousActingUpdatesSigil = ReactActingRendererSigil.current;
actingUpdatesScopeDepth++;
- if (__DEV__) {
- ReactShouldWarnActingUpdates.current = true;
- }
+ // we use the function flushPassiveEffects directly as the sigil,
+ // since it's unique to a renderer
+ ReactActingRendererSigil.current = flushPassiveEffects;
function onDone() {
actingUpdatesScopeDepth--;
+ ReactActingRendererSigil.current = previousActingUpdatesSigil;
if (__DEV__) {
- if (actingUpdatesScopeDepth === 0) {
- ReactShouldWarnActingUpdates.current = false;
- }
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..aa3b10ed7463a 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';
@@ -1211,6 +1212,7 @@ function dispatchAction<S, A>(
// 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.
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..fd313a8e48632 100644
--- a/packages/react-reconciler/src/ReactFiberReconciler.js
+++ b/packages/react-reconciler/src/ReactFiberReconciler.js
@@ -56,6 +56,7 @@ import {
discreteUpdates,
flushDiscreteUpdates,
flushPassiveEffects,
+ warnIfNotScopedWithMatchingAct,
} from './ReactFiberWorkLoop';
import {createUpdate, enqueueUpdate} from './ReactUpdateQueue';
import ReactFiberInstrumentation from './ReactFiberInstrumentation';
@@ -303,6 +304,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,
diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.js b/packages/react-reconciler/src/ReactFiberWorkLoop.js
index c93985e536cea..75582654f5a9e 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,
+ ReactActingRendererSigil,
} = ReactSharedInternals;
type WorkPhase = 0 | 1 | 2 | 3 | 4 | 5 | 6;
@@ -2276,11 +2276,47 @@ function warnAboutInvalidUpdatesOnClassComponentsInDEV(fiber) {
}
}
+export function warnIfNotScopedWithMatchingAct(fiber: Fiber): void {
+ if (__DEV__) {
+ if (
+ ReactActingRendererSigil.current !== null &&
+ // use the function flushPassiveEffects directly as the sigil
+ // so this comparison is expected here
+ ReactActingRendererSigil.current !== flushPassiveEffects
+ ) {
+ // 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),
+ );
+ }
+ }
+}
+
+// in a test-like environment, we want to warn if dispatchAction() is
+// called outside of a TestUtils.act(...)/batchedUpdates/render call.
+// so we have a a step counter for when we descend/ascend from
+// act() calls, and test on it for when to warn
+// It's a tuple with a single value. Look into ReactTestUtilsAct as an
+// example of how we change the value
+
function warnIfNotCurrentlyActingUpdatesInDEV(fiber: Fiber): void {
if (__DEV__) {
if (
workPhase === NotWorking &&
- ReactShouldWarnActingUpdates.current === false
+ ReactActingRendererSigil.current !== flushPassiveEffects
) {
warningWithoutStack(
false,
diff --git a/packages/react-test-renderer/src/ReactTestRendererAct.js b/packages/react-test-renderer/src/ReactTestRendererAct.js
index eafcae3ba79d2..98eee873317c0 100644
--- a/packages/react-test-renderer/src/ReactTestRendererAct.js
+++ b/packages/react-test-renderer/src/ReactTestRendererAct.js
@@ -18,7 +18,7 @@ import {warnAboutMissingMockScheduler} from 'shared/ReactFeatureFlags';
import enqueueTask from 'shared/enqueueTask';
import * as Scheduler from 'scheduler';
-const {ReactShouldWarnActingUpdates} = ReactSharedInternals;
+const {ReactActingRendererSigil} = ReactSharedInternals;
// this implementation should be exactly the same in
// ReactTestUtilsAct.js, ReactTestRendererAct.js, createReactNoop.js
@@ -66,17 +66,16 @@ let actingUpdatesScopeDepth = 0;
function act(callback: () => Thenable) {
let previousActingUpdatesScopeDepth = actingUpdatesScopeDepth;
+ let previousActingUpdatesSigil = ReactActingRendererSigil.current;
actingUpdatesScopeDepth++;
- if (__DEV__) {
- ReactShouldWarnActingUpdates.current = true;
- }
+ // we use the function flushPassiveEffects directly as the sigil,
+ // since it's unique to a renderer
+ ReactActingRendererSigil.current = flushPassiveEffects;
function onDone() {
actingUpdatesScopeDepth--;
+ ReactActingRendererSigil.current = previousActingUpdatesSigil;
if (__DEV__) {
- if (actingUpdatesScopeDepth === 0) {
- ReactShouldWarnActingUpdates.current = false;
- }
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/ReactActingRendererSigil.js b/packages/react/src/ReactActingRendererSigil.js
new file mode 100644
index 0000000000000..a40ecca4c6604
--- /dev/null
+++ b/packages/react/src/ReactActingRendererSigil.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 ReactActingRendererSigil = {
+ current: (null: null | (() => boolean)),
+};
+export default ReactActingRendererSigil;
diff --git a/packages/react/src/ReactSharedInternals.js b/packages/react/src/ReactSharedInternals.js
index 1bc95cffaad4e..de4a14adb3dbf 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 ReactActingRendererSigil from './ReactActingRendererSigil';
const ReactSharedInternals = {
ReactCurrentDispatcher,
ReactCurrentBatchConfig,
ReactCurrentOwner,
- // used by act()
- ReactShouldWarnActingUpdates: {current: false},
+ ReactActingRendererSigil,
// Used by renderers to avoid bundling object-assign twice in UMD bundles:
assign,
};
From e789900ae1e0557e394f9d7d0154b54c47f3c1a1 Mon Sep 17 00:00:00 2001
From: Sunil Pai <threepointone@gmail.com>
Date: Tue, 28 May 2019 23:41:52 +0100
Subject: [PATCH 2/6] unneeded (and wrong) comment
---
packages/react-reconciler/src/ReactFiberWorkLoop.js | 7 -------
1 file changed, 7 deletions(-)
diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.js b/packages/react-reconciler/src/ReactFiberWorkLoop.js
index 75582654f5a9e..a0edd5a1b8a4e 100644
--- a/packages/react-reconciler/src/ReactFiberWorkLoop.js
+++ b/packages/react-reconciler/src/ReactFiberWorkLoop.js
@@ -2305,13 +2305,6 @@ export function warnIfNotScopedWithMatchingAct(fiber: Fiber): void {
}
}
-// in a test-like environment, we want to warn if dispatchAction() is
-// called outside of a TestUtils.act(...)/batchedUpdates/render call.
-// so we have a a step counter for when we descend/ascend from
-// act() calls, and test on it for when to warn
-// It's a tuple with a single value. Look into ReactTestUtilsAct as an
-// example of how we change the value
-
function warnIfNotCurrentlyActingUpdatesInDEV(fiber: Fiber): void {
if (__DEV__) {
if (
From a2dae63111014ac26910196fa332dcb8d19d7d3f Mon Sep 17 00:00:00 2001
From: Sunil Pai <threepointone@gmail.com>
Date: Wed, 29 May 2019 00:00:07 +0100
Subject: [PATCH 3/6] run the dom fixture on CI
---
.circleci/config.yml | 1 +
1 file changed, 1 insertion(+)
diff --git a/.circleci/config.yml b/.circleci/config.yml
index 75f249e839a0b..a390575b20289 100644
--- a/.circleci/config.yml
+++ b/.circleci/config.yml
@@ -164,6 +164,7 @@ jobs:
- *restore_yarn_cache
- *run_yarn
- run: yarn test-build --maxWorkers=2
+ - run: yarn test-dom-fixture
test_fuzz:
docker: *docker
From 6d6419443e9a91e385437c183a659b17633bba0c Mon Sep 17 00:00:00 2001
From: Sunil Pai <threepointone@gmail.com>
Date: Wed, 29 May 2019 12:16:36 +0100
Subject: [PATCH 4/6] update the sigil only in __DEV__
---
packages/react-dom/src/test-utils/ReactTestUtilsAct.js | 9 ++++++---
packages/react-noop-renderer/src/createReactNoop.js | 9 ++++++---
packages/react-test-renderer/src/ReactTestRendererAct.js | 9 ++++++---
3 files changed, 18 insertions(+), 9 deletions(-)
diff --git a/packages/react-dom/src/test-utils/ReactTestUtilsAct.js b/packages/react-dom/src/test-utils/ReactTestUtilsAct.js
index b0e0ebe80fd1e..f1fe7646626a7 100644
--- a/packages/react-dom/src/test-utils/ReactTestUtilsAct.js
+++ b/packages/react-dom/src/test-utils/ReactTestUtilsAct.js
@@ -85,16 +85,19 @@ let actingUpdatesScopeDepth = 0;
function act(callback: () => Thenable) {
let previousActingUpdatesScopeDepth = actingUpdatesScopeDepth;
- let previousActingUpdatesSigil = ReactActingRendererSigil.current;
+ let previousActingUpdatesSigil;
actingUpdatesScopeDepth++;
// we use the function flushPassiveEffects directly as the sigil,
// since it's unique to a renderer
- ReactActingRendererSigil.current = flushPassiveEffects;
+ if (__DEV__) {
+ previousActingUpdatesSigil = ReactActingRendererSigil.current;
+ ReactActingRendererSigil.current = flushPassiveEffects;
+ }
function onDone() {
actingUpdatesScopeDepth--;
- ReactActingRendererSigil.current = previousActingUpdatesSigil;
if (__DEV__) {
+ ReactActingRendererSigil.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 60a702a0b9eb4..3ca5aaa545765 100644
--- a/packages/react-noop-renderer/src/createReactNoop.js
+++ b/packages/react-noop-renderer/src/createReactNoop.js
@@ -698,16 +698,19 @@ function createReactNoop(reconciler: Function, useMutation: boolean) {
function act(callback: () => Thenable) {
let previousActingUpdatesScopeDepth = actingUpdatesScopeDepth;
- let previousActingUpdatesSigil = ReactActingRendererSigil.current;
+ let previousActingUpdatesSigil;
actingUpdatesScopeDepth++;
// we use the function flushPassiveEffects directly as the sigil,
// since it's unique to a renderer
- ReactActingRendererSigil.current = flushPassiveEffects;
+ if (__DEV__) {
+ previousActingUpdatesSigil = ReactActingRendererSigil.current;
+ ReactActingRendererSigil.current = flushPassiveEffects;
+ }
function onDone() {
actingUpdatesScopeDepth--;
- ReactActingRendererSigil.current = previousActingUpdatesSigil;
if (__DEV__) {
+ ReactActingRendererSigil.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-test-renderer/src/ReactTestRendererAct.js b/packages/react-test-renderer/src/ReactTestRendererAct.js
index 98eee873317c0..39087f111028e 100644
--- a/packages/react-test-renderer/src/ReactTestRendererAct.js
+++ b/packages/react-test-renderer/src/ReactTestRendererAct.js
@@ -66,16 +66,19 @@ let actingUpdatesScopeDepth = 0;
function act(callback: () => Thenable) {
let previousActingUpdatesScopeDepth = actingUpdatesScopeDepth;
- let previousActingUpdatesSigil = ReactActingRendererSigil.current;
+ let previousActingUpdatesSigil;
actingUpdatesScopeDepth++;
// we use the function flushPassiveEffects directly as the sigil,
// since it's unique to a renderer
- ReactActingRendererSigil.current = flushPassiveEffects;
+ if (__DEV__) {
+ previousActingUpdatesSigil = ReactActingRendererSigil.current;
+ ReactActingRendererSigil.current = flushPassiveEffects;
+ }
function onDone() {
actingUpdatesScopeDepth--;
- ReactActingRendererSigil.current = previousActingUpdatesSigil;
if (__DEV__) {
+ ReactActingRendererSigil.current = previousActingUpdatesSigil;
if (actingUpdatesScopeDepth > previousActingUpdatesScopeDepth) {
// if it's _less than_ previousActingUpdatesScopeDepth, then we can assume the 'other' one has warned
warningWithoutStack(
From 669d92e2aa128af986abe2304c5aaad643b25bdf Mon Sep 17 00:00:00 2001
From: Sunil Pai <threepointone@gmail.com>
Date: Wed, 29 May 2019 12:20:27 +0100
Subject: [PATCH 5/6] remove the obnoxious comment
---
packages/react-reconciler/src/ReactFiberHooks.js | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/packages/react-reconciler/src/ReactFiberHooks.js b/packages/react-reconciler/src/ReactFiberHooks.js
index aa3b10ed7463a..2946c931cdc58 100644
--- a/packages/react-reconciler/src/ReactFiberHooks.js
+++ b/packages/react-reconciler/src/ReactFiberHooks.js
@@ -1208,9 +1208,7 @@ function dispatchAction<S, A>(
}
}
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);
From c0dc3989e8c40595610d684636f9b32e1c5d0754 Mon Sep 17 00:00:00 2001
From: Sunil Pai <threepointone@gmail.com>
Date: Wed, 29 May 2019 20:27:30 +0100
Subject: [PATCH 6/6] use an explicit export for the sigil
---
packages/react-dom/src/client/ReactDOM.js | 2 ++
packages/react-dom/src/fire/ReactFire.js | 2 ++
.../react-dom/src/test-utils/ReactTestUtils.js | 4 +++-
.../src/test-utils/ReactTestUtilsAct.js | 11 +++++------
.../react-noop-renderer/src/createReactNoop.js | 16 +++++++++-------
.../react-reconciler/src/ReactFiberReconciler.js | 2 ++
.../react-reconciler/src/ReactFiberWorkLoop.js | 14 ++++++++++----
.../src/ReactTestRendererAct.js | 11 +++++------
...gil.js => ReactCurrentActingRendererSigil.js} | 4 ++--
packages/react/src/ReactSharedInternals.js | 4 ++--
10 files changed, 42 insertions(+), 28 deletions(-)
rename packages/react/src/{ReactActingRendererSigil.js => ReactCurrentActingRendererSigil.js} (82%)
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 f1fe7646626a7..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 {ReactActingRendererSigil} = ReactSharedInternals;
+const {ReactCurrentActingRendererSigil} = ReactSharedInternals;
// this implementation should be exactly the same in
// ReactTestUtilsAct.js, ReactTestRendererAct.js, createReactNoop.js
@@ -87,17 +88,15 @@ function act(callback: () => Thenable) {
let previousActingUpdatesScopeDepth = actingUpdatesScopeDepth;
let previousActingUpdatesSigil;
actingUpdatesScopeDepth++;
- // we use the function flushPassiveEffects directly as the sigil,
- // since it's unique to a renderer
if (__DEV__) {
- previousActingUpdatesSigil = ReactActingRendererSigil.current;
- ReactActingRendererSigil.current = flushPassiveEffects;
+ previousActingUpdatesSigil = ReactCurrentActingRendererSigil.current;
+ ReactCurrentActingRendererSigil.current = ReactActingRendererSigil;
}
function onDone() {
actingUpdatesScopeDepth--;
if (__DEV__) {
- ReactActingRendererSigil.current = previousActingUpdatesSigil;
+ 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 3ca5aaa545765..89276a0b494fb 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 {ReactActingRendererSigil} = 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 = '<default>';
- const {flushPassiveEffects, batchedUpdates} = NoopRenderer;
+ const {
+ flushPassiveEffects,
+ batchedUpdates,
+ ReactActingRendererSigil,
+ } = NoopRenderer;
// this act() implementation should be exactly the same in
// ReactTestUtilsAct.js, ReactTestRendererAct.js, createReactNoop.js
@@ -700,17 +704,15 @@ function createReactNoop(reconciler: Function, useMutation: boolean) {
let previousActingUpdatesScopeDepth = actingUpdatesScopeDepth;
let previousActingUpdatesSigil;
actingUpdatesScopeDepth++;
- // we use the function flushPassiveEffects directly as the sigil,
- // since it's unique to a renderer
if (__DEV__) {
- previousActingUpdatesSigil = ReactActingRendererSigil.current;
- ReactActingRendererSigil.current = flushPassiveEffects;
+ previousActingUpdatesSigil = ReactCurrentActingRendererSigil.current;
+ ReactCurrentActingRendererSigil.current = ReactActingRendererSigil;
}
function onDone() {
actingUpdatesScopeDepth--;
if (__DEV__) {
- ReactActingRendererSigil.current = previousActingUpdatesSigil;
+ 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/ReactFiberReconciler.js b/packages/react-reconciler/src/ReactFiberReconciler.js
index fd313a8e48632..de84dd7b67244 100644
--- a/packages/react-reconciler/src/ReactFiberReconciler.js
+++ b/packages/react-reconciler/src/ReactFiberReconciler.js
@@ -57,6 +57,7 @@ import {
flushDiscreteUpdates,
flushPassiveEffects,
warnIfNotScopedWithMatchingAct,
+ ReactActingRendererSigil,
} from './ReactFiberWorkLoop';
import {createUpdate, enqueueUpdate} from './ReactUpdateQueue';
import ReactFiberInstrumentation from './ReactFiberInstrumentation';
@@ -339,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 a0edd5a1b8a4e..a9eea7c154b5d 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,
- ReactActingRendererSigil,
+ ReactCurrentActingRendererSigil,
} = ReactSharedInternals;
type WorkPhase = 0 | 1 | 2 | 3 | 4 | 5 | 6;
@@ -2276,13 +2276,19 @@ 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 (
- ReactActingRendererSigil.current !== null &&
+ ReactCurrentActingRendererSigil.current !== null &&
// use the function flushPassiveEffects directly as the sigil
// so this comparison is expected here
- ReactActingRendererSigil.current !== flushPassiveEffects
+ ReactCurrentActingRendererSigil.current !== ReactActingRendererSigil
) {
// it looks like we're using the wrong matching act(), so log a warning
warningWithoutStack(
@@ -2309,7 +2315,7 @@ function warnIfNotCurrentlyActingUpdatesInDEV(fiber: Fiber): void {
if (__DEV__) {
if (
workPhase === NotWorking &&
- ReactActingRendererSigil.current !== flushPassiveEffects
+ ReactCurrentActingRendererSigil.current !== ReactActingRendererSigil
) {
warningWithoutStack(
false,
diff --git a/packages/react-test-renderer/src/ReactTestRendererAct.js b/packages/react-test-renderer/src/ReactTestRendererAct.js
index 39087f111028e..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 {ReactActingRendererSigil} = ReactSharedInternals;
+const {ReactCurrentActingRendererSigil} = ReactSharedInternals;
// this implementation should be exactly the same in
// ReactTestUtilsAct.js, ReactTestRendererAct.js, createReactNoop.js
@@ -68,17 +69,15 @@ function act(callback: () => Thenable) {
let previousActingUpdatesScopeDepth = actingUpdatesScopeDepth;
let previousActingUpdatesSigil;
actingUpdatesScopeDepth++;
- // we use the function flushPassiveEffects directly as the sigil,
- // since it's unique to a renderer
if (__DEV__) {
- previousActingUpdatesSigil = ReactActingRendererSigil.current;
- ReactActingRendererSigil.current = flushPassiveEffects;
+ previousActingUpdatesSigil = ReactCurrentActingRendererSigil.current;
+ ReactCurrentActingRendererSigil.current = ReactActingRendererSigil;
}
function onDone() {
actingUpdatesScopeDepth--;
if (__DEV__) {
- ReactActingRendererSigil.current = previousActingUpdatesSigil;
+ 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/ReactActingRendererSigil.js b/packages/react/src/ReactCurrentActingRendererSigil.js
similarity index 82%
rename from packages/react/src/ReactActingRendererSigil.js
rename to packages/react/src/ReactCurrentActingRendererSigil.js
index a40ecca4c6604..09e05c8362d7b 100644
--- a/packages/react/src/ReactActingRendererSigil.js
+++ b/packages/react/src/ReactCurrentActingRendererSigil.js
@@ -13,7 +13,7 @@
* so we can track identity of the renderer.
*/
-const ReactActingRendererSigil = {
+const ReactCurrentActingRendererSigil = {
current: (null: null | (() => boolean)),
};
-export default ReactActingRendererSigil;
+export default ReactCurrentActingRendererSigil;
diff --git a/packages/react/src/ReactSharedInternals.js b/packages/react/src/ReactSharedInternals.js
index de4a14adb3dbf..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 ReactActingRendererSigil from './ReactActingRendererSigil';
+import ReactCurrentActingRendererSigil from './ReactCurrentActingRendererSigil';
const ReactSharedInternals = {
ReactCurrentDispatcher,
ReactCurrentBatchConfig,
ReactCurrentOwner,
- ReactActingRendererSigil,
+ ReactCurrentActingRendererSigil,
// Used by renderers to avoid bundling object-assign twice in UMD bundles:
assign,
};