From ee3d60e0425538dfbda92219c089dc35215b4866 Mon Sep 17 00:00:00 2001 From: Sunil Pai Date: Tue, 4 Jun 2019 04:22:59 -0700 Subject: [PATCH] allow nested acts from different renderers 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. --- fixtures/dom/src/index.test.js | 9 +++++++++ .../react-dom/src/test-utils/ReactTestUtilsAct.js | 6 ++---- .../react-noop-renderer/src/createReactNoop.js | 6 ++---- .../react-reconciler/src/ReactFiberWorkLoop.js | 14 ++++++++++---- .../src/__tests__/ReactNoopRendererAct-test.js | 2 -- .../src/ReactTestRendererAct.js | 6 ++---- .../react/src/ReactCurrentActingRendererSigil.js | 8 ++++---- 7 files changed, 29 insertions(+), 22 deletions(-) diff --git a/fixtures/dom/src/index.test.js b/fixtures/dom/src/index.test.js index ad10e99d36b7b..002ee04e5908c 100644 --- a/fixtures/dom/src/index.test.js +++ b/fixtures/dom/src/index.test.js @@ -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); @@ -158,3 +159,11 @@ it('does not warn when nesting react-act inside react-test-renderer', () => { TestRenderer.create(); }); }); + +it("doesn't warn if you use nested acts from different renderers", () => { + TestRenderer.act(() => { + TestUtils.act(() => { + TestRenderer.create(); + }); + }); +}); diff --git a/packages/react-dom/src/test-utils/ReactTestUtilsAct.js b/packages/react-dom/src/test-utils/ReactTestUtilsAct.js index c12f396795966..0f43cca235a4d 100644 --- a/packages/react-dom/src/test-utils/ReactTestUtilsAct.js +++ b/packages/react-dom/src/test-utils/ReactTestUtilsAct.js @@ -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( diff --git a/packages/react-noop-renderer/src/createReactNoop.js b/packages/react-noop-renderer/src/createReactNoop.js index f1d7fc419548d..b60703a761796 100644 --- a/packages/react-noop-renderer/src/createReactNoop.js +++ b/packages/react-noop-renderer/src/createReactNoop.js @@ -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( diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.js b/packages/react-reconciler/src/ReactFiberWorkLoop.js index 1308452a77850..9eabc8756dd39 100644 --- a/packages/react-reconciler/src/ReactFiberWorkLoop.js +++ b/packages/react-reconciler/src/ReactFiberWorkLoop.js @@ -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, @@ -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, @@ -2476,7 +2480,9 @@ function warnIfNotCurrentlyActingUpdatesInDEV(fiber: Fiber): void { if ( shouldWarnUnactedUpdates === true && executionContext === NoContext && - ReactCurrentActingRendererSigil.current !== ReactActingRendererSigil + ReactCurrentActingRendererSigil.current.indexOf( + ReactActingRendererSigil, + ) === -1 ) { warningWithoutStack( false, diff --git a/packages/react-reconciler/src/__tests__/ReactNoopRendererAct-test.js b/packages/react-reconciler/src/__tests__/ReactNoopRendererAct-test.js index 38be78044cc8e..6b7c0103a9f41 100644 --- a/packages/react-reconciler/src/__tests__/ReactNoopRendererAct-test.js +++ b/packages/react-reconciler/src/__tests__/ReactNoopRendererAct-test.js @@ -31,7 +31,6 @@ describe('ReactNoop.act()', () => { />, ); }); - expect(Scheduler).toFlushWithoutYielding(); expect(calledLog).toEqual([0]); }); @@ -54,7 +53,6 @@ describe('ReactNoop.act()', () => { ReactNoop.render(); }); expect(Scheduler).toHaveYielded(['stage 1', 'stage 2']); - expect(Scheduler).toFlushWithoutYielding(); expect(ReactNoop.getChildren()).toEqual([{text: '1', hidden: false}]); }); }); diff --git a/packages/react-test-renderer/src/ReactTestRendererAct.js b/packages/react-test-renderer/src/ReactTestRendererAct.js index 1e47e33583a52..8ffd52f66c501 100644 --- a/packages/react-test-renderer/src/ReactTestRendererAct.js +++ b/packages/react-test-renderer/src/ReactTestRendererAct.js @@ -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( diff --git a/packages/react/src/ReactCurrentActingRendererSigil.js b/packages/react/src/ReactCurrentActingRendererSigil.js index 09e05c8362d7b..c579464b7f67c 100644 --- a/packages/react/src/ReactCurrentActingRendererSigil.js +++ b/packages/react/src/ReactCurrentActingRendererSigil.js @@ -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;