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
Changes from 1 commit
Commits
File filter...
Filter file types
Jump to…
Jump to file or symbol
Failed to load files and symbols.

Always

Just for now

@@ -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()', () => {
@@ -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;
@@ -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 />);
});
});
});
@@ -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;

@@ -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';
@@ -817,7 +817,7 @@ const ReactDOM: Object = {
dispatchEvent,
runEventsInBatch,
flushPassiveEffects,
ReactActingRendererSigil,
ReactRendererIsActing,
],
},
};
@@ -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.
@@ -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';
@@ -823,7 +823,7 @@ const ReactDOM: Object = {
dispatchEvent,
runEventsInBatch,
flushPassiveEffects,
ReactActingRendererSigil,
ReactRendererIsActing,
],
},
};
@@ -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;

@@ -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
@@ -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;
This conversation was marked as resolved by threepointone

This comment has been minimized.

Copy link
@acdlite

acdlite Jul 2, 2019

Member

Naming nit: IsSomeRendererActing and IsThisRendererActing?

}

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(
@@ -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;
@@ -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;
@@ -65,7 +65,7 @@ type TextInstance = {|
|};
type HostContext = Object;

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

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

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

mountEventComponent(): void {
@@ -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
@@ -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(
@@ -57,7 +57,7 @@ import {
flushDiscreteUpdates,
flushPassiveEffects,
warnIfNotScopedWithMatchingAct,
ReactActingRendererSigil,
ReactRendererIsActing,
} from './ReactFiberWorkLoop';
import {createUpdate, enqueueUpdate} from './ReactUpdateQueue';
import ReactFiberInstrumentation from './ReactFiberInstrumentation';
@@ -345,7 +345,7 @@ export {
flushControlled,
flushSync,
flushPassiveEffects,
ReactActingRendererSigil,
ReactRendererIsActing,
};

export function getPublicRootInstance(
@@ -55,7 +55,7 @@ import {
scheduleTimeout,
cancelTimeout,
noTimeout,
shouldWarnUnactedUpdates,
warnsIfNotActing,
} from './ReactFiberHostConfig';

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

type ExecutionContext = number;
@@ -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,
@@ -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,
@@ -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,
@@ -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;
@@ -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;
@@ -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
@@ -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(

This file was deleted.

@@ -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;
ProTip! Use n and p to navigate between commits in a pull request.
You can’t perform that action at this time.