await act(async () => ...)#14853
Merged
Merged
Conversation
|
ReactDOM: size: 0.0%, gzip: 0.0% Details of bundled changes.Comparing: 9307932...e49e043 react-dom
react-art
react-native-renderer
react-test-renderer
react-noop-renderer
react-reconciler
Generated by 🚫 dangerJS |
threepointone
commented
Feb 14, 2019
| act = ReactTestUtils.act; | ||
| }); | ||
|
|
||
| describe('sync', () => { |
Contributor
Author
There was a problem hiding this comment.
these are the ones moved in from TestUtils-test
| ); | ||
| }); | ||
| }); | ||
| describe('async', () => { |
Contributor
Author
There was a problem hiding this comment.
these are newly added
threepointone
commented
Feb 14, 2019
| }); | ||
|
|
||
| await sleep(1000); | ||
| expect(console.error).toHaveBeenCalledTimes(1); |
Contributor
Author
There was a problem hiding this comment.
but nuh uh, we warn anyway (todo - match on actual warning message)
gaearon
reviewed
Feb 14, 2019
Contributor
Author
|
Assuming there’s no big problem with the approach, I’m going to close this pr later today and continue working on it in my branch. Please feel free to leave comments! |
Contributor
Author
|
(nevermind, I'll leave it open) |
threepointone
commented
Feb 15, 2019
| ReactTestUtils.renderIntoDocument(<Component />); | ||
| expect(mockArgs.length).toEqual(0); | ||
| }); | ||
|
|
Contributor
Author
There was a problem hiding this comment.
moved these tests into a separate file for .act tests
Contributor
Author
|
spoke with dan, moving the |
Contributor
Author
|
The failing test is odd, investigating. |
threepointone
commented
Feb 15, 2019
threepointone
commented
Feb 15, 2019
added 2 commits
February 15, 2019 21:09
added 2 commits
February 16, 2019 02:25
it's an odd one, because not only does sync act not flush effects correctly, but the async one does (wut). verified it's fine with the dom version.
This was referenced Mar 10, 2020
This was referenced Mar 18, 2020
acra5y
added a commit
to acra5y/n-dilation-frontend
that referenced
this pull request
May 7, 2020
* Initialized wasm submodule * Add npm scripts and webpack config to compile wasm and include to assets * Update express to fix mime type issue with wasm * Update react to fix async act (see react/react#14853) * Changed error message component props for less tight coupling
mrizwanashiq
pushed a commit
to mrizwanashiq/react
that referenced
this pull request
Jun 25, 2026
This took a while, but I'm happy I went through it. Some key moments - recursively flushing effects, flushing microtasks on each async turn, and my team's uncompromising philosophy on code reuse. Really happy with this. I still want to expand test coverage, and I have some more small related todos, but this is good to land. On to the next one. Soundtrack to landing this - https://open.spotify.com/track/0MF8I8OUo8kytiOo8aSHYq?si=gSWqUheKQbiQDXzptCXHTg * hacked up act(async () => {...}) * move stuff around * merge changes * abstract .act warnings and stuff. all renderers. pass all tests. * move testutils.act back into testutils * move into scheduler, rename some bits * smaller bundle * a comment for why we don't do typeof === 'function' * fix test * pass tests - fire, prod * lose actContainerElement * tighter * write a test for TestRenderer it's an odd one, because not only does sync act not flush effects correctly, but the async one does (wut). verified it's fine with the dom version. * lint * rewrote to move flushing logic closer to the renderer the scheduler's `flushPassiveEffects` didn't work as expected for the test renderer, so I decided to go back to the hack (rendering a dumb container) This also makes reactdom not as heavy (by a few bytes, but still). * move it around so the delta isn't too bad * cleanups fix promise chaining propagate errors correctly test for thenable the 'right' way more tests! tidier! ponies! * Stray comment * recursively flush effects * fixed tests * lint, move noop.act into react-reconciler * microtasks when checking if called, s/called/calledLog, cleanup * pass fb lint we could have globally changed our eslint config to assume Promise is available, but that means we expect a promise polyfill on the page, and we don't yet. this code is triggered only in jest anyway, and we're fairly certain Promise will be available there. hence, the once-off disable for the check * shorter timers, fix a test, test for Promise * use global.Promise for existence check * flush microtasks * a version that works in browsers (that support postMessage) I also added a sanity fixture inside fixtures/dom/ mostly for me. * hoist flushEffectsAndMicroTasks * pull out tick logic from ReactFiberScheduler * fix await act (...sync) hanging - fix a hang when awaiting sync logic - a better async/await test for test renderer * feedback changes - use node's setImmediate if available - a warning if MessageChannel isn't available - rename some functions * pass lint/flow checks (without requiring a Promise polyfill/exclusion) * prettier the prettiest, even. * use globalPromise for the missed await warning * __DEV__ check for didWarnAboutMessageChannel * thenables and callbacks instead of promises, pass flow/lint * tinier. better. - pulled most bits out of FiberScheduler - actedUpdates uses callbacks now * pass build validation * augh prettier * golfing 7 more chars * Test that effects are not flushed without also flushing microtasks * export doesHavePendingPassiveEffects, nits * createAct() * dead code * missed in merge? * lose the preflushing bits * ugh prettier * removed `actedUpdates()`, created shared/actingUpdatesScopeDepth * rearrange imports so builds work, remove the hack versions of flushPassiveEffects * represent actingUpdatesScopeDepth as a tuple [number] * use a shared flag on React.__SECRET... * remove createAct, setup act for all relevant renderers * review feedback shared/enqueueTask import ReactSharedInternals from 'shared/ReactSharedInternals'; simpler act() internals ReactSharedInternals.ReactShouldWarnActingUpdates * move act() implementation into createReactNoop * warnIfNotCurrentlyActingUpdatesInDev condition check order
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
15 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
TODO -
(repasting from the group)
I hacked together an asynchronous version of
act(...), and it's kinda nice.You've seen the synchronous version -
This still works, and gives the same warnings. But if you pass an
asyncfunction -Neat! I set it up so if you don't
awaitthe result fromact, a warning gets triggered (withsetImmediate) to do so. That makes it a bit harder to have rogue asyncact()calls in the ether.You can nest
act()calls -I implemented a cheap form of unrolling safety, so if a previous
act()gets closed before any subsequentact()calls, a warning gets triggered. This should prevent most interleaving attempts, and maintain a tree-like shape ofact()blocks.pros -
cons -
act()call... which might be fine?)