Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

allow nested act()s from different renderers #16039

Merged
merged 2 commits into from Jul 2, 2019
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
18 changes: 10 additions & 8 deletions fixtures/dom/src/index.test.js
Expand Up @@ -70,10 +70,7 @@ it('warns when using the wrong act version - test + dom: updates', () => {
TestRenderer.act(() => {
setCtr(1);
});
}).toWarnDev([
'An update to Counter inside a test was not wrapped in act',
"It looks like you're using the wrong act()",
]);
}).toWarnDev(["It looks like you're using the wrong act()"]);
});

it('warns when using the wrong act version - dom + test: .create()', () => {
Expand Down Expand Up @@ -109,10 +106,7 @@ it('warns when using the wrong act version - dom + test: updates', () => {
TestUtils.act(() => {
setCtr(1);
});
}).toWarnDev([
'An update to Counter inside a test was not wrapped in act',
"It looks like you're using the wrong act()",
]);
}).toWarnDev(["It looks like you're using the wrong act()"]);
});

const {Surface, Group, Shape} = ReactART;
Expand Down Expand Up @@ -158,3 +152,11 @@ it('does not warn when nesting react-act inside react-test-renderer', () => {
TestRenderer.create(<ARTTest />);
});
});

it("doesn't warn if you use nested acts from different renderers", () => {
TestRenderer.act(() => {
TestUtils.act(() => {
TestRenderer.create(<App />);
});
});
});
2 changes: 1 addition & 1 deletion packages/react-art/src/ReactARTHostConfig.js
Expand Up @@ -347,7 +347,7 @@ export function shouldSetTextContent(type, props) {
export const isPrimaryRenderer = false;

// The ART renderer shouldn't trigger missing act() warnings
export const shouldWarnUnactedUpdates = false;
export const warnsIfNotActing = false;

export const supportsMutation = true;

Expand Down
4 changes: 2 additions & 2 deletions packages/react-dom/src/client/ReactDOM.js
Expand Up @@ -38,7 +38,7 @@ import {
findHostInstance,
findHostInstanceWithWarning,
flushPassiveEffects,
ReactActingRendererSigil,
ReactRendererIsActing,
} from 'react-reconciler/inline.dom';
import {createPortal as createPortalImpl} from 'shared/ReactPortal';
import {canUseDOM} from 'shared/ExecutionEnvironment';
Expand Down Expand Up @@ -817,7 +817,7 @@ const ReactDOM: Object = {
dispatchEvent,
runEventsInBatch,
flushPassiveEffects,
ReactActingRendererSigil,
ReactRendererIsActing,
],
},
};
Expand Down
2 changes: 1 addition & 1 deletion packages/react-dom/src/client/ReactDOMHostConfig.js
Expand Up @@ -348,7 +348,7 @@ export function createTextInstance(
}

export const isPrimaryRenderer = true;
export const shouldWarnUnactedUpdates = true;
export const warnsIfNotActing = true;
// This initialization code may run even on server environments
// if a component just imports ReactDOM (e.g. for findDOMNode).
// Some environments might not have setTimeout or clearTimeout.
Expand Down
4 changes: 2 additions & 2 deletions packages/react-dom/src/fire/ReactFire.js
Expand Up @@ -43,7 +43,7 @@ import {
findHostInstance,
findHostInstanceWithWarning,
flushPassiveEffects,
ReactActingRendererSigil,
ReactRendererIsActing,
} from 'react-reconciler/inline.fire';
import {createPortal as createPortalImpl} from 'shared/ReactPortal';
import {canUseDOM} from 'shared/ExecutionEnvironment';
Expand Down Expand Up @@ -823,7 +823,7 @@ const ReactDOM: Object = {
dispatchEvent,
runEventsInBatch,
flushPassiveEffects,
ReactActingRendererSigil,
ReactRendererIsActing,
],
},
};
Expand Down
2 changes: 1 addition & 1 deletion packages/react-dom/src/test-utils/ReactTestUtils.js
Expand Up @@ -44,7 +44,7 @@ const [
runEventsInBatch,
/* eslint-disable no-unused-vars */
flushPassiveEffects,
ReactActingRendererSigil,
ReactRendererIsActing,
/* eslint-enable no-unused-vars */
] = ReactDOM.__SECRET_INTERNALS_DO_NOT_USE_OR_YOU_WILL_BE_FIRED.Events;

Expand Down
16 changes: 10 additions & 6 deletions packages/react-dom/src/test-utils/ReactTestUtilsAct.js
Expand Up @@ -33,12 +33,12 @@ const [
runEventsInBatch,
/* eslint-enable no-unused-vars */
flushPassiveEffects,
ReactActingRendererSigil,
ReactRendererIsActing,
] = ReactDOM.__SECRET_INTERNALS_DO_NOT_USE_OR_YOU_WILL_BE_FIRED.Events;

const batchedUpdates = ReactDOM.unstable_batchedUpdates;

const {ReactCurrentActingRendererSigil} = ReactSharedInternals;
const {ReactIsActing} = ReactSharedInternals;

// this implementation should be exactly the same in
// ReactTestUtilsAct.js, ReactTestRendererAct.js, createReactNoop.js
Expand Down Expand Up @@ -86,17 +86,21 @@ let actingUpdatesScopeDepth = 0;

function act(callback: () => Thenable) {
let previousActingUpdatesScopeDepth = actingUpdatesScopeDepth;
let previousActingUpdatesSigil;
let previousReactIsActing;
let previousReactRendererIsActing;
actingUpdatesScopeDepth++;
if (__DEV__) {
previousActingUpdatesSigil = ReactCurrentActingRendererSigil.current;
ReactCurrentActingRendererSigil.current = ReactActingRendererSigil;
previousReactIsActing = ReactIsActing.current;
previousReactRendererIsActing = ReactIsActing.current;
ReactIsActing.current = true;
ReactRendererIsActing.current = true;
threepointone marked this conversation as resolved.
Show resolved Hide resolved
}

function onDone() {
actingUpdatesScopeDepth--;
if (__DEV__) {
ReactCurrentActingRendererSigil.current = previousActingUpdatesSigil;
ReactIsActing.current = previousReactIsActing;
ReactRendererIsActing.current = previousReactRendererIsActing;
if (actingUpdatesScopeDepth > previousActingUpdatesScopeDepth) {
// if it's _less than_ previousActingUpdatesScopeDepth, then we can assume the 'other' one has warned
warningWithoutStack(
Expand Down
Expand Up @@ -352,7 +352,7 @@ export function shouldSetTextContent(type: string, props: Props): boolean {
export const isPrimaryRenderer = false;

// The Fabric renderer shouldn't trigger missing act() warnings
export const shouldWarnUnactedUpdates = false;
export const warnsIfNotActing = false;

export const scheduleTimeout = setTimeout;
export const cancelTimeout = clearTimeout;
Expand Down
Expand Up @@ -247,7 +247,7 @@ export function resetAfterCommit(containerInfo: Container): void {
}

export const isPrimaryRenderer = true;
export const shouldWarnUnactedUpdates = true;
export const warnsIfNotActing = true;

export const scheduleTimeout = setTimeout;
export const cancelTimeout = clearTimeout;
Expand Down
18 changes: 11 additions & 7 deletions packages/react-noop-renderer/src/createReactNoop.js
Expand Up @@ -65,7 +65,7 @@ type TextInstance = {|
|};
type HostContext = Object;

const {ReactCurrentActingRendererSigil} = ReactSharedInternals;
const {ReactIsActing} = ReactSharedInternals;

const NO_CONTEXT = {};
const UPPERCASE_CONTEXT = {};
Expand Down Expand Up @@ -393,7 +393,7 @@ function createReactNoop(reconciler: Function, useMutation: boolean) {
now: Scheduler.unstable_now,

isPrimaryRenderer: true,
shouldWarnUnactedUpdates: true,
warnsIfNotActing: true,
supportsHydration: false,

mountEventComponent(): void {
Expand Down Expand Up @@ -566,7 +566,7 @@ function createReactNoop(reconciler: Function, useMutation: boolean) {
const {
flushPassiveEffects,
batchedUpdates,
ReactActingRendererSigil,
ReactRendererIsActing,
} = NoopRenderer;

// this act() implementation should be exactly the same in
Expand Down Expand Up @@ -615,17 +615,21 @@ function createReactNoop(reconciler: Function, useMutation: boolean) {

function act(callback: () => Thenable) {
let previousActingUpdatesScopeDepth = actingUpdatesScopeDepth;
let previousActingUpdatesSigil;
let previousReactIsActing;
let previousReactRendererIsActing;
actingUpdatesScopeDepth++;
if (__DEV__) {
previousActingUpdatesSigil = ReactCurrentActingRendererSigil.current;
ReactCurrentActingRendererSigil.current = ReactActingRendererSigil;
previousReactIsActing = ReactIsActing.current;
previousReactRendererIsActing = ReactIsActing.current;
ReactIsActing.current = true;
ReactRendererIsActing.current = true;
}

function onDone() {
actingUpdatesScopeDepth--;
if (__DEV__) {
ReactCurrentActingRendererSigil.current = previousActingUpdatesSigil;
ReactIsActing.current = previousReactIsActing;
ReactRendererIsActing.current = previousReactRendererIsActing;
if (actingUpdatesScopeDepth > previousActingUpdatesScopeDepth) {
// if it's _less than_ previousActingUpdatesScopeDepth, then we can assume the 'other' one has warned
warningWithoutStack(
Expand Down
4 changes: 2 additions & 2 deletions packages/react-reconciler/src/ReactFiberReconciler.js
Expand Up @@ -57,7 +57,7 @@ import {
flushDiscreteUpdates,
flushPassiveEffects,
warnIfNotScopedWithMatchingAct,
ReactActingRendererSigil,
ReactRendererIsActing,
} from './ReactFiberWorkLoop';
import {createUpdate, enqueueUpdate} from './ReactUpdateQueue';
import ReactFiberInstrumentation from './ReactFiberInstrumentation';
Expand Down Expand Up @@ -345,7 +345,7 @@ export {
flushControlled,
flushSync,
flushPassiveEffects,
ReactActingRendererSigil,
ReactRendererIsActing,
};

export function getPublicRootInstance(
Expand Down
26 changes: 12 additions & 14 deletions packages/react-reconciler/src/ReactFiberWorkLoop.js
Expand Up @@ -55,7 +55,7 @@ import {
scheduleTimeout,
cancelTimeout,
noTimeout,
shouldWarnUnactedUpdates,
warnsIfNotActing,
} from './ReactFiberHostConfig';

import {createWorkInProgress, assignFiberPropertiesInDEV} from './ReactFiber';
Expand Down Expand Up @@ -174,7 +174,7 @@ const ceil = Math.ceil;
const {
ReactCurrentDispatcher,
ReactCurrentOwner,
ReactCurrentActingRendererSigil,
ReactIsActing,
} = ReactSharedInternals;

type ExecutionContext = number;
Expand Down Expand Up @@ -2420,18 +2420,14 @@ 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 const ReactRendererIsActing = {current: (false: boolean)};

export function warnIfNotScopedWithMatchingAct(fiber: Fiber): void {
if (__DEV__) {
if (
shouldWarnUnactedUpdates === true &&
ReactCurrentActingRendererSigil.current !== null &&
ReactCurrentActingRendererSigil.current !== ReactActingRendererSigil
warnsIfNotActing === true &&
ReactIsActing.current === true &&
ReactRendererIsActing.current !== true
) {
warningWithoutStack(
false,
Expand All @@ -2456,8 +2452,9 @@ export function warnIfNotScopedWithMatchingAct(fiber: Fiber): void {
export function warnIfNotCurrentlyActingEffectsInDEV(fiber: Fiber): void {
if (__DEV__) {
if (
shouldWarnUnactedUpdates === true &&
ReactCurrentActingRendererSigil.current !== ReactActingRendererSigil
warnsIfNotActing === true &&
ReactIsActing.current === false &&
ReactRendererIsActing.current === false
) {
warningWithoutStack(
false,
Expand All @@ -2482,9 +2479,10 @@ export function warnIfNotCurrentlyActingEffectsInDEV(fiber: Fiber): void {
function warnIfNotCurrentlyActingUpdatesInDEV(fiber: Fiber): void {
if (__DEV__) {
if (
shouldWarnUnactedUpdates === true &&
warnsIfNotActing === true &&
executionContext === NoContext &&
ReactCurrentActingRendererSigil.current !== ReactActingRendererSigil
ReactIsActing.current === false &&
ReactRendererIsActing.current === false
) {
warningWithoutStack(
false,
Expand Down
Expand Up @@ -59,7 +59,7 @@ export const cancelTimeout = $$$hostConfig.clearTimeout;
export const noTimeout = $$$hostConfig.noTimeout;
export const now = $$$hostConfig.now;
export const isPrimaryRenderer = $$$hostConfig.isPrimaryRenderer;
export const shouldWarnUnactedUpdates = $$$hostConfig.shouldWarnUnactedUpdates;
export const warnsIfNotActing = $$$hostConfig.warnsIfNotActing;
export const supportsMutation = $$$hostConfig.supportsMutation;
export const supportsPersistence = $$$hostConfig.supportsPersistence;
export const supportsHydration = $$$hostConfig.supportsHydration;
Expand Down
2 changes: 1 addition & 1 deletion packages/react-test-renderer/src/ReactTestHostConfig.js
Expand Up @@ -217,7 +217,7 @@ export function createTextInstance(
}

export const isPrimaryRenderer = false;
export const shouldWarnUnactedUpdates = true;
export const warnsIfNotActing = true;

export const scheduleTimeout = setTimeout;
export const cancelTimeout = clearTimeout;
Expand Down
16 changes: 10 additions & 6 deletions packages/react-test-renderer/src/ReactTestRendererAct.js
Expand Up @@ -11,15 +11,15 @@ import type {Thenable} from 'react-reconciler/src/ReactFiberWorkLoop';
import {
batchedUpdates,
flushPassiveEffects,
ReactActingRendererSigil,
ReactRendererIsActing,
} from 'react-reconciler/inline.test';
import ReactSharedInternals from 'shared/ReactSharedInternals';
import warningWithoutStack from 'shared/warningWithoutStack';
import {warnAboutMissingMockScheduler} from 'shared/ReactFeatureFlags';
import enqueueTask from 'shared/enqueueTask';
import * as Scheduler from 'scheduler';

const {ReactCurrentActingRendererSigil} = ReactSharedInternals;
const {ReactIsActing} = ReactSharedInternals;

// this implementation should be exactly the same in
// ReactTestUtilsAct.js, ReactTestRendererAct.js, createReactNoop.js
Expand Down Expand Up @@ -67,17 +67,21 @@ let actingUpdatesScopeDepth = 0;

function act(callback: () => Thenable) {
let previousActingUpdatesScopeDepth = actingUpdatesScopeDepth;
let previousActingUpdatesSigil;
let previousReactIsActing;
let previousReactRendererIsActing;
actingUpdatesScopeDepth++;
if (__DEV__) {
previousActingUpdatesSigil = ReactCurrentActingRendererSigil.current;
ReactCurrentActingRendererSigil.current = ReactActingRendererSigil;
previousReactIsActing = ReactIsActing.current;
previousReactRendererIsActing = ReactIsActing.current;
ReactIsActing.current = true;
ReactRendererIsActing.current = true;
}

function onDone() {
actingUpdatesScopeDepth--;
if (__DEV__) {
ReactCurrentActingRendererSigil.current = previousActingUpdatesSigil;
ReactIsActing.current = previousReactIsActing;
ReactRendererIsActing.current = previousReactRendererIsActing;
if (actingUpdatesScopeDepth > previousActingUpdatesScopeDepth) {
// if it's _less than_ previousActingUpdatesScopeDepth, then we can assume the 'other' one has warned
warningWithoutStack(
Expand Down
19 changes: 0 additions & 19 deletions packages/react/src/ReactCurrentActingRendererSigil.js

This file was deleted.

17 changes: 17 additions & 0 deletions packages/react/src/ReactIsActing.js
@@ -0,0 +1,17 @@
/**
* 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 inside an act() scope.
*/

const ReactIsActing = {
current: (false: boolean),
};
export default ReactIsActing;