From c35e37aabd01c15b006334da7e32171948ea941e Mon Sep 17 00:00:00 2001 From: Sunil Pai Date: Wed, 3 Apr 2019 17:11:24 +0100 Subject: [PATCH 1/4] mark react-events as private so we publish script skips it for now --- packages/react-events/package.json | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/react-events/package.json b/packages/react-events/package.json index 0950a38579bcf..e293b88fed26b 100644 --- a/packages/react-events/package.json +++ b/packages/react-events/package.json @@ -1,5 +1,6 @@ { "name": "react-events", + "private": true, "description": "React is a JavaScript library for building user interfaces.", "keywords": [ "react" From d2c26ee44318de2ac11751d4587e8633d4451157 Mon Sep 17 00:00:00 2001 From: Sunil Pai Date: Sat, 27 Apr 2019 12:00:07 +0100 Subject: [PATCH 2/4] don't flush effects/tasks till the outermost one exits --- .../src/__tests__/ReactTestUtilsAct-test.js | 30 +++++++++++-------- .../src/test-utils/ReactTestUtilsAct.js | 28 +++++++++++------ .../src/createReactNoop.js | 30 +++++++++++++------ .../src/ReactTestRendererAct.js | 28 +++++++++++------ 4 files changed, 76 insertions(+), 40 deletions(-) diff --git a/packages/react-dom/src/__tests__/ReactTestUtilsAct-test.js b/packages/react-dom/src/__tests__/ReactTestUtilsAct-test.js index 461cf6dfda08f..833c55db6eaf8 100644 --- a/packages/react-dom/src/__tests__/ReactTestUtilsAct-test.js +++ b/packages/react-dom/src/__tests__/ReactTestUtilsAct-test.js @@ -285,14 +285,13 @@ describe('ReactTestUtils.act()', () => { return ctr; } const el = document.createElement('div'); + act(() => { + ReactDOM.render(, el); + }); await act(async () => { - act(() => { - ReactDOM.render(, el); - }); - await sleep(100); - expect(el.innerHTML).toBe('1'); }); + expect(el.innerHTML).toBe('1'); }); it('can handle async/await', async () => { @@ -371,18 +370,23 @@ describe('ReactTestUtils.act()', () => { let ctr = 0; const div = document.createElement('div'); - await act(async () => { - act(() => { - ReactDOM.render( ctr++} />, div); - }); - expect(div.innerHTML).toBe('0'); - expect(ctr).toBe(1); + act(() => { + ReactDOM.render( ctr++} />, div); }); - // this may seem odd, but it matches user behaviour - - // a flash of "0" followed by "1" + expect(div.innerHTML).toBe('0'); + expect(ctr).toBe(1); + + await act(async () => {}); // #just-react-things expect(div.innerHTML).toBe('1'); expect(ctr).toBe(2); + + // this may seem odd, but it matches browser behaviour - + // a flash of "0" followed by "1" + + // now you probably wouldn't write a test like this for your app + // you'd await act(async () => { ReactDOM.render( }) + // and assert on final render }); it('propagates errors', async () => { diff --git a/packages/react-dom/src/test-utils/ReactTestUtilsAct.js b/packages/react-dom/src/test-utils/ReactTestUtilsAct.js index 99cb73ede7c10..2160589c8380d 100644 --- a/packages/react-dom/src/test-utils/ReactTestUtilsAct.js +++ b/packages/react-dom/src/test-utils/ReactTestUtilsAct.js @@ -119,14 +119,21 @@ function act(callback: () => Thenable) { called = true; result.then( () => { - flushEffectsAndMicroTasks((err: ?Error) => { + if (actingUpdatesScopeDepth === 1) { + // we're about to exit the act() scope, + // now's the time to flush tasks/effects + flushEffectsAndMicroTasks((err: ?Error) => { + onDone(); + if (err) { + reject(err); + } else { + resolve(); + } + }); + } else { onDone(); - if (err) { - reject(err); - } else { - resolve(); - } - }); + resolve(); + } }, err => { onDone(); @@ -145,9 +152,12 @@ function act(callback: () => Thenable) { ); } - // flush effects until none remain, and cleanup try { - while (flushPassiveEffects()) {} + if (actingUpdatesScopeDepth === 1) { + // we're about to exit the act() scope, + // now's the time to flush effects + while (flushPassiveEffects()) {} + } onDone(); } catch (err) { onDone(); diff --git a/packages/react-noop-renderer/src/createReactNoop.js b/packages/react-noop-renderer/src/createReactNoop.js index 3ac1c7021de5d..a129890b38912 100644 --- a/packages/react-noop-renderer/src/createReactNoop.js +++ b/packages/react-noop-renderer/src/createReactNoop.js @@ -650,6 +650,8 @@ function createReactNoop(reconciler: Function, useMutation: boolean) { // this act() implementation should be exactly the same in // ReactTestUtilsAct.js, ReactTestRendererAct.js, createReactNoop.js + // we track the 'depth' of the act() calls with this counter, + // so we can tell if any async act() calls try to run in parallel. let actingUpdatesScopeDepth = 0; function flushEffectsAndMicroTasks(onDone: (err: ?Error) => void) { @@ -727,14 +729,21 @@ function createReactNoop(reconciler: Function, useMutation: boolean) { called = true; result.then( () => { - flushEffectsAndMicroTasks((err: ?Error) => { + if (actingUpdatesScopeDepth === 1) { + // we're about to exit the act() scope, + // now's the time to flush tasks/effects + flushEffectsAndMicroTasks((err: ?Error) => { + onDone(); + if (err) { + reject(err); + } else { + resolve(); + } + }); + } else { onDone(); - if (err) { - reject(err); - } else { - resolve(); - } - }); + resolve(); + } }, err => { onDone(); @@ -753,9 +762,12 @@ function createReactNoop(reconciler: Function, useMutation: boolean) { ); } - // flush effects until none remain, and cleanup try { - while (flushPassiveEffects()) {} + if (actingUpdatesScopeDepth === 1) { + // we're about to exit the act() scope, + // now's the time to flush effects + while (flushPassiveEffects()) {} + } onDone(); } catch (err) { onDone(); diff --git a/packages/react-test-renderer/src/ReactTestRendererAct.js b/packages/react-test-renderer/src/ReactTestRendererAct.js index 37ced3fb04c04..109d166010be4 100644 --- a/packages/react-test-renderer/src/ReactTestRendererAct.js +++ b/packages/react-test-renderer/src/ReactTestRendererAct.js @@ -100,14 +100,21 @@ function act(callback: () => Thenable) { called = true; result.then( () => { - flushEffectsAndMicroTasks((err: ?Error) => { + if (actingUpdatesScopeDepth === 1) { + // we're about to exit the act() scope, + // now's the time to flush tasks/effects + flushEffectsAndMicroTasks((err: ?Error) => { + onDone(); + if (err) { + reject(err); + } else { + resolve(); + } + }); + } else { onDone(); - if (err) { - reject(err); - } else { - resolve(); - } - }); + resolve(); + } }, err => { onDone(); @@ -126,9 +133,12 @@ function act(callback: () => Thenable) { ); } - // flush effects until none remain, and cleanup try { - while (flushPassiveEffects()) {} + if (actingUpdatesScopeDepth === 1) { + // we're about to exit the act() scope, + // now's the time to flush effects + while (flushPassiveEffects()) {} + } onDone(); } catch (err) { onDone(); From f7aa6ca69361eabfc9f9f5129eb0a8e5e174be48 Mon Sep 17 00:00:00 2001 From: Sunil Pai Date: Sat, 27 Apr 2019 21:25:35 +0100 Subject: [PATCH 3/4] do scope depth mutations in prod too --- packages/react-dom/src/test-utils/ReactTestUtilsAct.js | 7 +++---- packages/react-noop-renderer/src/createReactNoop.js | 7 +++---- packages/react-test-renderer/src/ReactTestRendererAct.js | 7 +++---- 3 files changed, 9 insertions(+), 12 deletions(-) diff --git a/packages/react-dom/src/test-utils/ReactTestUtilsAct.js b/packages/react-dom/src/test-utils/ReactTestUtilsAct.js index 2160589c8380d..f4fa358a8cd67 100644 --- a/packages/react-dom/src/test-utils/ReactTestUtilsAct.js +++ b/packages/react-dom/src/test-utils/ReactTestUtilsAct.js @@ -60,16 +60,15 @@ function flushEffectsAndMicroTasks(onDone: (err: ?Error) => void) { } function act(callback: () => Thenable) { - let previousActingUpdatesScopeDepth; + let previousActingUpdatesScopeDepth = actingUpdatesScopeDepth; + actingUpdatesScopeDepth++; if (__DEV__) { - previousActingUpdatesScopeDepth = actingUpdatesScopeDepth; - actingUpdatesScopeDepth++; ReactShouldWarnActingUpdates.current = true; } function onDone() { + actingUpdatesScopeDepth--; if (__DEV__) { - actingUpdatesScopeDepth--; if (actingUpdatesScopeDepth === 0) { ReactShouldWarnActingUpdates.current = false; } diff --git a/packages/react-noop-renderer/src/createReactNoop.js b/packages/react-noop-renderer/src/createReactNoop.js index a129890b38912..3f3f0b3be5045 100644 --- a/packages/react-noop-renderer/src/createReactNoop.js +++ b/packages/react-noop-renderer/src/createReactNoop.js @@ -670,16 +670,15 @@ function createReactNoop(reconciler: Function, useMutation: boolean) { } function act(callback: () => Thenable) { - let previousActingUpdatesScopeDepth; + let previousActingUpdatesScopeDepth = actingUpdatesScopeDepth; + actingUpdatesScopeDepth++; if (__DEV__) { - previousActingUpdatesScopeDepth = actingUpdatesScopeDepth; - actingUpdatesScopeDepth++; ReactShouldWarnActingUpdates.current = true; } function onDone() { + actingUpdatesScopeDepth--; if (__DEV__) { - actingUpdatesScopeDepth--; if (actingUpdatesScopeDepth === 0) { ReactShouldWarnActingUpdates.current = false; } diff --git a/packages/react-test-renderer/src/ReactTestRendererAct.js b/packages/react-test-renderer/src/ReactTestRendererAct.js index 109d166010be4..f8575f91943c1 100644 --- a/packages/react-test-renderer/src/ReactTestRendererAct.js +++ b/packages/react-test-renderer/src/ReactTestRendererAct.js @@ -41,16 +41,15 @@ function flushEffectsAndMicroTasks(onDone: (err: ?Error) => void) { } function act(callback: () => Thenable) { - let previousActingUpdatesScopeDepth; + let previousActingUpdatesScopeDepth = actingUpdatesScopeDepth; + actingUpdatesScopeDepth++; if (__DEV__) { - previousActingUpdatesScopeDepth = actingUpdatesScopeDepth; - actingUpdatesScopeDepth++; ReactShouldWarnActingUpdates.current = true; } function onDone() { + actingUpdatesScopeDepth--; if (__DEV__) { - actingUpdatesScopeDepth--; if (actingUpdatesScopeDepth === 0) { ReactShouldWarnActingUpdates.current = false; } From 8f88e96c96a85ced9c2d11987f4294fa18c456df Mon Sep 17 00:00:00 2001 From: Sunil Pai Date: Sat, 27 Apr 2019 21:42:05 +0100 Subject: [PATCH 4/4] early return --- .../src/test-utils/ReactTestUtilsAct.js | 24 +++++++++---------- .../src/createReactNoop.js | 24 +++++++++---------- .../src/ReactTestRendererAct.js | 24 +++++++++---------- 3 files changed, 36 insertions(+), 36 deletions(-) diff --git a/packages/react-dom/src/test-utils/ReactTestUtilsAct.js b/packages/react-dom/src/test-utils/ReactTestUtilsAct.js index f4fa358a8cd67..60e1378adc83e 100644 --- a/packages/react-dom/src/test-utils/ReactTestUtilsAct.js +++ b/packages/react-dom/src/test-utils/ReactTestUtilsAct.js @@ -118,21 +118,21 @@ function act(callback: () => Thenable) { called = true; result.then( () => { - if (actingUpdatesScopeDepth === 1) { - // we're about to exit the act() scope, - // now's the time to flush tasks/effects - flushEffectsAndMicroTasks((err: ?Error) => { - onDone(); - if (err) { - reject(err); - } else { - resolve(); - } - }); - } else { + if (actingUpdatesScopeDepth !== 1) { onDone(); resolve(); + return; } + // we're about to exit the act() scope, + // now's the time to flush tasks/effects + flushEffectsAndMicroTasks((err: ?Error) => { + onDone(); + if (err) { + reject(err); + } else { + resolve(); + } + }); }, err => { onDone(); diff --git a/packages/react-noop-renderer/src/createReactNoop.js b/packages/react-noop-renderer/src/createReactNoop.js index 3f3f0b3be5045..bda86017b06b9 100644 --- a/packages/react-noop-renderer/src/createReactNoop.js +++ b/packages/react-noop-renderer/src/createReactNoop.js @@ -728,21 +728,21 @@ function createReactNoop(reconciler: Function, useMutation: boolean) { called = true; result.then( () => { - if (actingUpdatesScopeDepth === 1) { - // we're about to exit the act() scope, - // now's the time to flush tasks/effects - flushEffectsAndMicroTasks((err: ?Error) => { - onDone(); - if (err) { - reject(err); - } else { - resolve(); - } - }); - } else { + if (actingUpdatesScopeDepth !== 1) { onDone(); resolve(); + return; } + // we're about to exit the act() scope, + // now's the time to flush tasks/effects + flushEffectsAndMicroTasks((err: ?Error) => { + onDone(); + if (err) { + reject(err); + } else { + resolve(); + } + }); }, err => { onDone(); diff --git a/packages/react-test-renderer/src/ReactTestRendererAct.js b/packages/react-test-renderer/src/ReactTestRendererAct.js index f8575f91943c1..842ffd4bb22e0 100644 --- a/packages/react-test-renderer/src/ReactTestRendererAct.js +++ b/packages/react-test-renderer/src/ReactTestRendererAct.js @@ -99,21 +99,21 @@ function act(callback: () => Thenable) { called = true; result.then( () => { - if (actingUpdatesScopeDepth === 1) { - // we're about to exit the act() scope, - // now's the time to flush tasks/effects - flushEffectsAndMicroTasks((err: ?Error) => { - onDone(); - if (err) { - reject(err); - } else { - resolve(); - } - }); - } else { + if (actingUpdatesScopeDepth !== 1) { onDone(); resolve(); + return; } + // we're about to exit the act() scope, + // now's the time to flush tasks/effects + flushEffectsAndMicroTasks((err: ?Error) => { + onDone(); + if (err) { + reject(err); + } else { + resolve(); + } + }); }, err => { onDone();