From 65697def79da230ff95301d43bb6d43f215ff48e Mon Sep 17 00:00:00 2001 From: Andrew Clark Date: Sat, 26 Jun 2021 12:17:59 -0400 Subject: [PATCH] `act`: Resolve to return value of scope function MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When migrating some internal tests I found it annoying that I couldn't return anything from the `act` scope. You would have to declare the variable on the outside then assign to it. But this doesn't play well with type systems — when you use the variable, you have to check the type. Before: ```js let renderer; act(() => { renderer = ReactTestRenderer.create(); }) // Type system can't tell that renderer is never undefined renderer?.root.findByType(Component); ``` After: ```js const renderer = await act(() => { return ReactTestRenderer.create(); }) renderer.root.findByType(Component); ``` --- .../src/__tests__/ReactIsomorphicAct-test.js | 31 +++++++++++++++ .../src/ReactTestRenderer.js | 2 +- packages/react/src/ReactAct.js | 38 ++++++++++++------- 3 files changed, 56 insertions(+), 15 deletions(-) diff --git a/packages/react-reconciler/src/__tests__/ReactIsomorphicAct-test.js b/packages/react-reconciler/src/__tests__/ReactIsomorphicAct-test.js index 08d9bc568837e..001474e935217 100644 --- a/packages/react-reconciler/src/__tests__/ReactIsomorphicAct-test.js +++ b/packages/react-reconciler/src/__tests__/ReactIsomorphicAct-test.js @@ -47,4 +47,35 @@ describe('isomorphic act()', () => { }); expect(root).toMatchRenderedOutput('B'); }); + + // @gate __DEV__ + test('return value – sync callback', async () => { + expect(await act(() => 'hi')).toEqual('hi'); + }); + + // @gate __DEV__ + test('return value – sync callback, nested', async () => { + const returnValue = await act(() => { + return act(() => 'hi'); + }); + expect(returnValue).toEqual('hi'); + }); + + // @gate __DEV__ + test('return value – async callback', async () => { + const returnValue = await act(async () => { + return await Promise.resolve('hi'); + }); + expect(returnValue).toEqual('hi'); + }); + + // @gate __DEV__ + test('return value – async callback, nested', async () => { + const returnValue = await act(async () => { + return await act(async () => { + return await Promise.resolve('hi'); + }); + }); + expect(returnValue).toEqual('hi'); + }); }); diff --git a/packages/react-test-renderer/src/ReactTestRenderer.js b/packages/react-test-renderer/src/ReactTestRenderer.js index d8b6b08611fef..eb00975506956 100644 --- a/packages/react-test-renderer/src/ReactTestRenderer.js +++ b/packages/react-test-renderer/src/ReactTestRenderer.js @@ -51,7 +51,7 @@ import {ConcurrentRoot, LegacyRoot} from 'react-reconciler/src/ReactRootTags'; import {allowConcurrentByDefault} from 'shared/ReactFeatureFlags'; const act_notBatchedInLegacyMode = React.unstable_act; -function act(callback: () => Thenable): Thenable { +function act(callback: () => T): Thenable { return act_notBatchedInLegacyMode(() => { return batchedUpdates(callback); }); diff --git a/packages/react/src/ReactAct.js b/packages/react/src/ReactAct.js index 885e803b027c5..99156f85fde4c 100644 --- a/packages/react/src/ReactAct.js +++ b/packages/react/src/ReactAct.js @@ -15,7 +15,7 @@ import enqueueTask from 'shared/enqueueTask'; let actScopeDepth = 0; let didWarnNoAwaitAct = false; -export function act(callback: () => Thenable): Thenable { +export function act(callback: () => T | Thenable): Thenable { if (__DEV__) { // `act` calls can be nested, so we track the depth. This represents the // number of `act` scopes on the stack. @@ -41,21 +41,22 @@ export function act(callback: () => Thenable): Thenable { typeof result === 'object' && typeof result.then === 'function' ) { + const thenableResult: Thenable = (result: any); // The callback is an async function (i.e. returned a promise). Wait // for it to resolve before exiting the current scope. let wasAwaited = false; - const thenable = { + const thenable: Thenable = { then(resolve, reject) { wasAwaited = true; - result.then( - () => { + thenableResult.then( + returnValue => { popActScope(prevActScopeDepth); if (actScopeDepth === 0) { // We've exited the outermost act scope. Recursively flush the // queue until there's no remaining work. - recursivelyFlushAsyncActWork(resolve, reject); + recursivelyFlushAsyncActWork(returnValue, resolve, reject); } else { - resolve(); + resolve(returnValue); } }, error => { @@ -88,6 +89,7 @@ export function act(callback: () => Thenable): Thenable { } return thenable; } else { + const returnValue: T = (result: any); // The callback is not an async function. Exit the current scope // immediately, without awaiting. popActScope(prevActScopeDepth); @@ -100,7 +102,7 @@ export function act(callback: () => Thenable): Thenable { } // Return a thenable. If the user awaits it, we'll flush again in // case additional work was scheduled by a microtask. - return { + const thenable: Thenable = { then(resolve, reject) { // Confirm we haven't re-entered another `act` scope, in case // the user does something weird like await the thenable @@ -108,18 +110,22 @@ export function act(callback: () => Thenable): Thenable { if (ReactCurrentActQueue.current === null) { // Recursively flush the queue until there's no remaining work. ReactCurrentActQueue.current = []; - recursivelyFlushAsyncActWork(resolve, reject); + recursivelyFlushAsyncActWork(returnValue, resolve, reject); + } else { + resolve(returnValue); } }, }; + return thenable; } else { // Since we're inside a nested `act` scope, the returned thenable // immediately resolves. The outer scope will flush the queue. - return { + const thenable: Thenable = { then(resolve, reject) { - resolve(); + resolve(returnValue); }, }; + return thenable; } } } else { @@ -142,7 +148,11 @@ function popActScope(prevActScopeDepth) { } } -function recursivelyFlushAsyncActWork(resolve, reject) { +function recursivelyFlushAsyncActWork( + returnValue: T, + resolve: T => mixed, + reject: mixed => mixed, +) { if (__DEV__) { const queue = ReactCurrentActQueue.current; if (queue !== null) { @@ -152,17 +162,17 @@ function recursivelyFlushAsyncActWork(resolve, reject) { if (queue.length === 0) { // No additional work was scheduled. Finish. ReactCurrentActQueue.current = null; - resolve(); + resolve(returnValue); } else { // Keep flushing work until there's none left. - recursivelyFlushAsyncActWork(resolve, reject); + recursivelyFlushAsyncActWork(returnValue, resolve, reject); } }); } catch (error) { reject(error); } } else { - resolve(); + resolve(returnValue); } } }