Skip to content

Commit

Permalink
allow nested acts from different renderers
Browse files Browse the repository at this point in the history
sometimes apps can be made wtih multiple renderers. a specific usecase is react-art embedded inside react-dom. in these cases, we want to be able to wrap an update with acts corrsponding to both the renderers. In this PR, we change ReactCurrentActingRendererSigil to hold a stack of sigils (instead of just the current active one), pushing and popping onto it as required.

This commit also fixes the dom fixtures tests, which were broken? ugh.
  • Loading branch information
threepointone committed Jun 28, 2019
1 parent eb2ace1 commit ee3d60e
Show file tree
Hide file tree
Showing 7 changed files with 29 additions and 22 deletions.
9 changes: 9 additions & 0 deletions fixtures/dom/src/index.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import ARTSVGMode from 'art/modes/svg';
import ARTCurrentMode from 'art/modes/current';
import TestUtils from 'react-dom/test-utils';
import TestRenderer from 'react-test-renderer';
global.__DEV__ = process.env.NODE_ENV !== 'production';

ARTCurrentMode.setCurrent(ARTSVGMode);

Expand Down Expand Up @@ -158,3 +159,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 />);
});
});
});
6 changes: 2 additions & 4 deletions packages/react-dom/src/test-utils/ReactTestUtilsAct.js
Original file line number Diff line number Diff line change
Expand Up @@ -86,17 +86,15 @@ let actingUpdatesScopeDepth = 0;

function act(callback: () => Thenable) {
let previousActingUpdatesScopeDepth = actingUpdatesScopeDepth;
let previousActingUpdatesSigil;
actingUpdatesScopeDepth++;
if (__DEV__) {
previousActingUpdatesSigil = ReactCurrentActingRendererSigil.current;
ReactCurrentActingRendererSigil.current = ReactActingRendererSigil;
ReactCurrentActingRendererSigil.current.push(ReactActingRendererSigil);
}

function onDone() {
actingUpdatesScopeDepth--;
if (__DEV__) {
ReactCurrentActingRendererSigil.current = previousActingUpdatesSigil;
ReactCurrentActingRendererSigil.current.pop();
if (actingUpdatesScopeDepth > previousActingUpdatesScopeDepth) {
// if it's _less than_ previousActingUpdatesScopeDepth, then we can assume the 'other' one has warned
warningWithoutStack(
Expand Down
6 changes: 2 additions & 4 deletions packages/react-noop-renderer/src/createReactNoop.js
Original file line number Diff line number Diff line change
Expand Up @@ -615,17 +615,15 @@ function createReactNoop(reconciler: Function, useMutation: boolean) {

function act(callback: () => Thenable) {
let previousActingUpdatesScopeDepth = actingUpdatesScopeDepth;
let previousActingUpdatesSigil;
actingUpdatesScopeDepth++;
if (__DEV__) {
previousActingUpdatesSigil = ReactCurrentActingRendererSigil.current;
ReactCurrentActingRendererSigil.current = ReactActingRendererSigil;
ReactCurrentActingRendererSigil.current.push(ReactActingRendererSigil);
}

function onDone() {
actingUpdatesScopeDepth--;
if (__DEV__) {
ReactCurrentActingRendererSigil.current = previousActingUpdatesSigil;
ReactCurrentActingRendererSigil.current.pop();
if (actingUpdatesScopeDepth > previousActingUpdatesScopeDepth) {
// if it's _less than_ previousActingUpdatesScopeDepth, then we can assume the 'other' one has warned
warningWithoutStack(
Expand Down
14 changes: 10 additions & 4 deletions packages/react-reconciler/src/ReactFiberWorkLoop.js
Original file line number Diff line number Diff line change
Expand Up @@ -2422,8 +2422,10 @@ export function warnIfNotScopedWithMatchingAct(fiber: Fiber): void {
if (__DEV__) {
if (
shouldWarnUnactedUpdates === true &&
ReactCurrentActingRendererSigil.current !== null &&
ReactCurrentActingRendererSigil.current !== ReactActingRendererSigil
ReactCurrentActingRendererSigil.current.length !== 0 &&
ReactCurrentActingRendererSigil.current.indexOf(
ReactActingRendererSigil,
) === -1
) {
warningWithoutStack(
false,
Expand All @@ -2449,7 +2451,9 @@ export function warnIfNotCurrentlyActingEffectsInDEV(fiber: Fiber): void {
if (__DEV__) {
if (
shouldWarnUnactedUpdates === true &&
ReactCurrentActingRendererSigil.current !== ReactActingRendererSigil
ReactCurrentActingRendererSigil.current.indexOf(
ReactActingRendererSigil,
) === -1
) {
warningWithoutStack(
false,
Expand All @@ -2476,7 +2480,9 @@ function warnIfNotCurrentlyActingUpdatesInDEV(fiber: Fiber): void {
if (
shouldWarnUnactedUpdates === true &&
executionContext === NoContext &&
ReactCurrentActingRendererSigil.current !== ReactActingRendererSigil
ReactCurrentActingRendererSigil.current.indexOf(
ReactActingRendererSigil,
) === -1
) {
warningWithoutStack(
false,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@ describe('ReactNoop.act()', () => {
/>,
);
});
expect(Scheduler).toFlushWithoutYielding();
expect(calledLog).toEqual([0]);
});

Expand All @@ -54,7 +53,6 @@ describe('ReactNoop.act()', () => {
ReactNoop.render(<App />);
});
expect(Scheduler).toHaveYielded(['stage 1', 'stage 2']);
expect(Scheduler).toFlushWithoutYielding();
expect(ReactNoop.getChildren()).toEqual([{text: '1', hidden: false}]);
});
});
6 changes: 2 additions & 4 deletions packages/react-test-renderer/src/ReactTestRendererAct.js
Original file line number Diff line number Diff line change
Expand Up @@ -67,17 +67,15 @@ let actingUpdatesScopeDepth = 0;

function act(callback: () => Thenable) {
let previousActingUpdatesScopeDepth = actingUpdatesScopeDepth;
let previousActingUpdatesSigil;
actingUpdatesScopeDepth++;
if (__DEV__) {
previousActingUpdatesSigil = ReactCurrentActingRendererSigil.current;
ReactCurrentActingRendererSigil.current = ReactActingRendererSigil;
ReactCurrentActingRendererSigil.current.push(ReactActingRendererSigil);
}

function onDone() {
actingUpdatesScopeDepth--;
if (__DEV__) {
ReactCurrentActingRendererSigil.current = previousActingUpdatesSigil;
ReactCurrentActingRendererSigil.current.pop();
if (actingUpdatesScopeDepth > previousActingUpdatesScopeDepth) {
// if it's _less than_ previousActingUpdatesScopeDepth, then we can assume the 'other' one has warned
warningWithoutStack(
Expand Down
8 changes: 4 additions & 4 deletions packages/react/src/ReactCurrentActingRendererSigil.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,12 @@
*/

/**
* 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.
* We maintain a 'stack' of renderer specific sigils
* corresponding to act() calls, so we can track whenever an update
* happens inside/outside of one.
*/

const ReactCurrentActingRendererSigil = {
current: (null: null | (() => boolean)),
current: ([]: Array<{}>),
};
export default ReactCurrentActingRendererSigil;

0 comments on commit ee3d60e

Please sign in to comment.