Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support use in act testing API #25523

Merged
merged 1 commit into from
Oct 21, 2022
Merged

Conversation

acdlite
Copy link
Collaborator

@acdlite acdlite commented Oct 20, 2022

use can avoid suspending on already resolved data by yielding to microtasks. In a real, browser environment, we do this by scheduling a platform task (i.e. postTask).

In a test environment, tasks are scheduled on a special internal queue so that they can be flushed by the act testing API. So we need to add support for this in act.

This behavior only works if you await the thenable returned by the act call. We currently do not require that users do this. So I added a warning, but it only fires if use was called. The old Suspense pattern will not trigger a warning. This is to avoid breaking existing tests that use Suspense.

The implementation of act has gotten extremely complicated because of the subtle changes in behavior over the years, and our commitment to maintaining backwards compatibility. We really should consider being more restrictive in a future major release.

The changes are a bit confusing so I did my best to add inline comments explaining how it works.

Test plan

I ran this against Facebook's internal Jest test suite to confirm nothing broke

@sizebot
Copy link

sizebot commented Oct 20, 2022

Comparing: 9cdf8a9...65d6fc6

Critical size changes

Includes critical production bundles, as well as any change greater than 2%:

Name +/- Base Current +/- gzip Base gzip Current gzip
oss-stable/react-dom/cjs/react-dom.production.min.js = 135.99 kB 135.99 kB = 43.54 kB 43.54 kB
oss-experimental/react-dom/cjs/react-dom.production.min.js = 151.46 kB 151.46 kB = 48.23 kB 48.23 kB
facebook-www/ReactDOM-prod.classic.js = 493.51 kB 493.51 kB = 87.83 kB 87.83 kB
facebook-www/ReactDOM-prod.modern.js = 478.77 kB 478.77 kB = 85.64 kB 85.64 kB
facebook-www/ReactDOMForked-prod.classic.js = 493.51 kB 493.51 kB = 87.83 kB 87.83 kB
oss-stable-semver/react/cjs/react.development.js +5.44% 88.33 kB 93.14 kB +6.18% 23.86 kB 25.33 kB
oss-stable/react/cjs/react.development.js +5.44% 88.35 kB 93.16 kB +6.17% 23.88 kB 25.36 kB
oss-experimental/react/cjs/react.development.js +5.16% 93.27 kB 98.08 kB +5.83% 25.16 kB 26.63 kB
facebook-react-native/react/cjs/React-dev.js +4.62% 106.56 kB 111.48 kB +12.52% 26.27 kB 29.56 kB
oss-stable-semver/react/umd/react.development.js +4.44% 111.68 kB 116.63 kB +5.19% 28.65 kB 30.14 kB
oss-stable/react/umd/react.development.js +4.44% 111.70 kB 116.66 kB +5.19% 28.67 kB 30.16 kB
oss-experimental/react/umd/react.development.js +4.24% 116.93 kB 121.89 kB +5.00% 30.04 kB 31.54 kB
facebook-www/React-dev.modern.js +4.13% 119.27 kB 124.19 kB +11.20% 29.68 kB 33.00 kB
facebook-www/React-dev.classic.js +4.09% 120.36 kB 125.28 kB +11.17% 29.89 kB 33.23 kB

Significant size changes

Includes any change greater than 0.2%:

Expand to show
Name +/- Base Current +/- gzip Base gzip Current gzip
oss-stable-semver/react/cjs/react.development.js +5.44% 88.33 kB 93.14 kB +6.18% 23.86 kB 25.33 kB
oss-stable/react/cjs/react.development.js +5.44% 88.35 kB 93.16 kB +6.17% 23.88 kB 25.36 kB
oss-experimental/react/cjs/react.development.js +5.16% 93.27 kB 98.08 kB +5.83% 25.16 kB 26.63 kB
facebook-react-native/react/cjs/React-dev.js +4.62% 106.56 kB 111.48 kB +12.52% 26.27 kB 29.56 kB
oss-stable-semver/react/umd/react.development.js +4.44% 111.68 kB 116.63 kB +5.19% 28.65 kB 30.14 kB
oss-stable/react/umd/react.development.js +4.44% 111.70 kB 116.66 kB +5.19% 28.67 kB 30.16 kB
oss-experimental/react/umd/react.development.js +4.24% 116.93 kB 121.89 kB +5.00% 30.04 kB 31.54 kB
facebook-www/React-dev.modern.js +4.13% 119.27 kB 124.19 kB +11.20% 29.68 kB 33.00 kB
facebook-www/React-dev.classic.js +4.09% 120.36 kB 125.28 kB +11.17% 29.89 kB 33.23 kB
oss-experimental/react/cjs/react.shared-subset.development.js +0.28% 77.34 kB 77.55 kB +0.38% 21.36 kB 21.44 kB

Generated by 🚫 dangerJS against 65d6fc6

`use` can avoid suspending on already resolved data by yielding to
microtasks. In a real, browser environment, we do this by scheduling
a platform task (i.e. postTask).

In a test environment, tasks are scheduled on a special internal queue
so that they can be flushed by the `act` testing API. So we need to add
support for this in `act`.

This behavior only works if you `await` the thenable returning by
`act` call. We currently do not require that users do this. So I added
a warning, but it only fires if `use` was called. The old Suspense
pattern will not trigger a warning. This is to avoid breaking existing
tests that use Suspense.

The implementation of `act` has gotten extremely complicated because of
the subtle changes in behavior over the years, and our commitment to
maintaining backwards compatibility. We really should consider being
more restrictive in a future major release.

The changes are a bit confusing so I did my best to add inline comments
explaining how it works.
@acdlite acdlite marked this pull request as ready for review October 20, 2022 16:36
@acdlite acdlite merged commit c635807 into facebook:main Oct 21, 2022
poteto added a commit that referenced this pull request Oct 21, 2022
facebook-github-bot pushed a commit to facebook/react-native that referenced this pull request Nov 1, 2022
Summary:
This sync includes the following changes:
- **[ab075a232](facebook/react@ab075a232 )**: Do not unmount layout effects on initial Offscreen mount ([#25592](facebook/react#25592)) //<Samuel Susla>//
- **[765805bf8](facebook/react@765805bf8 )**: Fix type check for null ([#25595](facebook/react#25595)) //<Sebastian Markbåge>//
- **[2ac77aab9](facebook/react@2ac77aab9 )**: Clean up vestige of useOpaqueIdentifier ([#25587](facebook/react#25587)) //<Andrew Clark>//
- **[bdd3d0807](facebook/react@bdd3d0807 )**: Extract logic for detecting bad fallback to helper //<Andrew Clark>//
- **[952dfff3f](facebook/react@952dfff3f )**: Split suspended work loop logic into separate functions //<Andrew Clark>//
- **[d2c0ab10d](facebook/react@d2c0ab10d )**: In work loop, add enum of reasons for suspending //<Andrew Clark>//
- **[5450dd409](facebook/react@5450dd409 )**: Strict Mode: Reuse memoized result from first pass ([#25583](facebook/react#25583)) //<Andrew Clark>//
- **[d2a0176a1](facebook/react@d2a0176a1 )**: Detect and warn if use(promise) is wrapped with try/catch block ([#25543](facebook/react#25543)) //<Andrew Clark>//
- **[cf3932be5](facebook/react@cf3932be5 )**: Remove old react-fetch, react-fs and react-pg libraries ([#25577](facebook/react#25577)) //<Sebastian Markbåge>//
- **[28a574ea8](facebook/react@28a574ea8 )**: Try assigning fetch to globalThis if global assignment fails ([#25571](facebook/react#25571)) //<Sebastian Markbåge>//
- **[09def5990](facebook/react@09def5990 )**: [Float] handle noscript context for Resources ([#25559](facebook/react#25559)) //<Josh Story>//
- **[17204056d](facebook/react@17204056d )**: [Float] fix coordination of resource identity and hydration ([#25569](facebook/react#25569)) //<Josh Story>//
- **[d925a8d0b](facebook/react@d925a8d0b )**: Flight client error stack ([#25560](facebook/react#25560)) //<Josh Story>//
- **[996b00b78](facebook/react@996b00b78 )**: [Tiny] Fixed incorrect import in `react-server-dom-webpack` ([#25554](facebook/react#25554)) //<Leo Lamprecht>//
- **[e7c5af45c](facebook/react@e7c5af45c )**: Update cache() and use() to the canary aka next channel ([#25502](facebook/react#25502)) //<Sebastian Markbåge>//
- **[fa77f52e7](facebook/react@fa77f52e7 )**: Unify promise switch statements //<Andrew Clark>//
- **[7572e4931](facebook/react@7572e4931 )**: Track thenable state in work loop //<Andrew Clark>//
- **[7fc3eefd8](facebook/react@7fc3eefd8 )**: Revert yieldy behavior for non-use Suspense (in Flight, too) //<Andrew Clark>//
- **[61f9b5e97](facebook/react@61f9b5e97 )**: [Float] support <base> as Resource ([#25546](facebook/react#25546)) //<Josh Story>//
- **[1d3fc9c9c](facebook/react@1d3fc9c9c )**: Bug fix when resolving cache ([#25545](facebook/react#25545)) //<Sebastian Markbåge>//
- **[cce18e350](facebook/react@cce18e350 )**: [Flight] Use AsyncLocalStorage to extend the scope of the cache to micro tasks ([#25542](facebook/react#25542)) //<Sebastian Markbåge>//
- **[caa84c8da](facebook/react@caa84c8da )**: Revert fetch instrumentation to only RSC ([#25540](facebook/react#25540)) //<Sebastian Markbåge>//
- **[0c11baa6a](facebook/react@0c11baa6a )**: add warnings for non-resources rendered outside body or head ([#25532](facebook/react#25532)) //<Josh Story>//
- **[9236abdb5](facebook/react@9236abdb5 )**: when float is enabled only push title and script as a single unit ([#25536](facebook/react#25536)) //<Josh Story>//
- **[dd5c20825](facebook/react@dd5c20825 )**: Revert yieldy behavior for non-use Suspense ([#25537](facebook/react#25537)) //<Andrew Clark>//
- **[934177598](facebook/react@934177598 )**: fix transposed escape functions ([#25534](facebook/react#25534)) //<Josh Story>//
- **[d1ced9fd5](facebook/react@d1ced9fd5 )**: [Float] support all links as Resources ([#25515](facebook/react#25515)) //<Josh Story>//
- **[973b90bdf](facebook/react@973b90bdf )**: [Float] support meta tags as Resources ([#25514](facebook/react#25514)) //<Josh Story>//
- **[79c582981](facebook/react@79c582981 )**: Let ReactDOM initialize in RSC ([#25503](facebook/react#25503)) //<Sebastian Markbåge>//
- **[1f7a2f577](facebook/react@1f7a2f577 )**: [Float] support title tags as Resources ([#25508](facebook/react#25508)) //<Josh Story>//
- **[c63580787](facebook/react@c63580787 )**: Support `use` in `act` testing API ([#25523](facebook/react#25523)) //<Andrew Clark>//
- **[65e32e58b](facebook/react@65e32e58b )**: Add fetch Instrumentation to Dedupe Fetches ([#25516](facebook/react#25516)) //<Sebastian Markbåge>//
- **[9336e29d9](facebook/react@9336e29d9 )**: [useEvent] Lint for presence of useEvent functions in dependency lists ([#25512](facebook/react#25512)) //<lauren>//
- **[3cc792bfb](facebook/react@3cc792bfb )**: [useEvent] Non-stable function identity ([#25473](facebook/react#25473)) //<lauren>//
- **[987292815](facebook/react@987292815 )**: Remove feature flag enableStrictEffects ([#25387](facebook/react#25387)) //<Samuel Susla>//
- **[8e2bde6f2](facebook/react@8e2bde6f2 )**: Add cache() API ([#25506](facebook/react#25506)) //<Sebastian Markbåge>//
- **[9cdf8a99e](facebook/react@9cdf8a99e )**: [Codemod] Update copyright header to Meta ([#25315](facebook/react#25315)) //<Andrew Clark>//
- **[e54015e26](facebook/react@e54015e26 )**: Refactor: fill in the flow missing type ([#25496](facebook/react#25496)) //<c0dedance>//
- **[3b1fd5767](facebook/react@3b1fd5767 )**: refactor: Flow: typing of Scheduler ([#25485](facebook/react#25485)) //<bubucuo>//
- **[14072ce64](facebook/react@14072ce64 )**: Add detach to Offscreen component ([#25265](facebook/react#25265)) //<Samuel Susla>//
- **[3bb71dfd4](facebook/react@3bb71dfd4 )**: Rename react-server-dom-webpack entry points to /client and /server ([#25504](facebook/react#25504)) //<Sebastian Markbåge>//
- **[71f2c8cf1](facebook/react@71f2c8cf1 )**: move resource acquisition to mutation phase ([#25500](facebook/react#25500)) //<Josh Story>//
- **[500bea532](facebook/react@500bea532 )**: Add option to load Fizz runtime from external file ([#25499](facebook/react#25499)) //<Andrew Clark>//
- **[4494f2a86](facebook/react@4494f2a86 )**: [Float] add support for scripts and other enhancements ([#25480](facebook/react#25480)) //<Josh Story>//
- **[9ecf84ed7](facebook/react@9ecf84ed7 )**: Bugfix: Suspending in shell during discrete update ([#25495](facebook/react#25495)) //<Andrew Clark>//

Changelog:
[General][Changed] - React Native sync for revisions 54f297a...ab075a2

jest_e2e[run_all_tests]

Reviewed By: kassens

Differential Revision: D40897093

fbshipit-source-id: 6a040315834dea5c0ab994ea94d91f5605b9d6b0
@manakuro
Copy link

@acdlite Thank you for your great work!

I'm trying some tests on [email protected], and I noticed that the test that uses jest.useFakeTimers and promise inside custom hooks got stuck in the flushActQueue function.

It seems like the performConcurrentWorkOnRoot got stuck in there and I changed the callback(false) to callback(true), then it worked.
https://github.com/facebook/react/pull/25523/files#diff-74fadc1ab3f56ca2a5ae8932ba9876711d5d718c5367ba1cf4faf286c236c353R266

Screen.Recording.2022-11-18.at.9.33.07.mov

I'm wondering if it might be related to the changes on this PR, so it would be helpful if you could check that out.

I set up the test environment, so you can see this behavior through this.
https://github.com/manakuro/test-react-experimental

It works well on the [email protected] like here.
https://github.com/manakuro/test-react-experimental/tree/18.3.0-next-65e32e58b-20221019

rickhanlonii pushed a commit that referenced this pull request Dec 3, 2022
`use` can avoid suspending on already resolved data by yielding to
microtasks. In a real, browser environment, we do this by scheduling a
platform task (i.e. postTask).

In a test environment, tasks are scheduled on a special internal queue
so that they can be flushed by the `act` testing API. So we need to add
support for this in `act`.

This behavior only works if you `await` the thenable returned by the
`act` call. We currently do not require that users do this. So I added a
warning, but it only fires if `use` was called. The old Suspense pattern
will not trigger a warning. This is to avoid breaking existing tests
that use Suspense.

The implementation of `act` has gotten extremely complicated because of
the subtle changes in behavior over the years, and our commitment to
maintaining backwards compatibility. We really should consider being
more restrictive in a future major release.

The changes are a bit confusing so I did my best to add inline comments
explaining how it works.

## Test plan

I ran this against Facebook's internal Jest test suite to confirm
nothing broke
OlimpiaZurek pushed a commit to OlimpiaZurek/react-native that referenced this pull request May 22, 2023
Summary:
This sync includes the following changes:
- **[ab075a232](facebook/react@ab075a232 )**: Do not unmount layout effects on initial Offscreen mount ([facebook#25592](facebook/react#25592)) //<Samuel Susla>//
- **[765805bf8](facebook/react@765805bf8 )**: Fix type check for null ([facebook#25595](facebook/react#25595)) //<Sebastian Markbåge>//
- **[2ac77aab9](facebook/react@2ac77aab9 )**: Clean up vestige of useOpaqueIdentifier ([facebook#25587](facebook/react#25587)) //<Andrew Clark>//
- **[bdd3d0807](facebook/react@bdd3d0807 )**: Extract logic for detecting bad fallback to helper //<Andrew Clark>//
- **[952dfff3f](facebook/react@952dfff3f )**: Split suspended work loop logic into separate functions //<Andrew Clark>//
- **[d2c0ab10d](facebook/react@d2c0ab10d )**: In work loop, add enum of reasons for suspending //<Andrew Clark>//
- **[5450dd409](facebook/react@5450dd409 )**: Strict Mode: Reuse memoized result from first pass ([facebook#25583](facebook/react#25583)) //<Andrew Clark>//
- **[d2a0176a1](facebook/react@d2a0176a1 )**: Detect and warn if use(promise) is wrapped with try/catch block ([facebook#25543](facebook/react#25543)) //<Andrew Clark>//
- **[cf3932be5](facebook/react@cf3932be5 )**: Remove old react-fetch, react-fs and react-pg libraries ([facebook#25577](facebook/react#25577)) //<Sebastian Markbåge>//
- **[28a574ea8](facebook/react@28a574ea8 )**: Try assigning fetch to globalThis if global assignment fails ([facebook#25571](facebook/react#25571)) //<Sebastian Markbåge>//
- **[09def5990](facebook/react@09def5990 )**: [Float] handle noscript context for Resources ([facebook#25559](facebook/react#25559)) //<Josh Story>//
- **[17204056d](facebook/react@17204056d )**: [Float] fix coordination of resource identity and hydration ([facebook#25569](facebook/react#25569)) //<Josh Story>//
- **[d925a8d0b](facebook/react@d925a8d0b )**: Flight client error stack ([facebook#25560](facebook/react#25560)) //<Josh Story>//
- **[996b00b78](facebook/react@996b00b78 )**: [Tiny] Fixed incorrect import in `react-server-dom-webpack` ([facebook#25554](facebook/react#25554)) //<Leo Lamprecht>//
- **[e7c5af45c](facebook/react@e7c5af45c )**: Update cache() and use() to the canary aka next channel ([facebook#25502](facebook/react#25502)) //<Sebastian Markbåge>//
- **[fa77f52e7](facebook/react@fa77f52e7 )**: Unify promise switch statements //<Andrew Clark>//
- **[7572e4931](facebook/react@7572e4931 )**: Track thenable state in work loop //<Andrew Clark>//
- **[7fc3eefd8](facebook/react@7fc3eefd8 )**: Revert yieldy behavior for non-use Suspense (in Flight, too) //<Andrew Clark>//
- **[61f9b5e97](facebook/react@61f9b5e97 )**: [Float] support <base> as Resource ([facebook#25546](facebook/react#25546)) //<Josh Story>//
- **[1d3fc9c9c](facebook/react@1d3fc9c9c )**: Bug fix when resolving cache ([facebook#25545](facebook/react#25545)) //<Sebastian Markbåge>//
- **[cce18e350](facebook/react@cce18e350 )**: [Flight] Use AsyncLocalStorage to extend the scope of the cache to micro tasks ([facebook#25542](facebook/react#25542)) //<Sebastian Markbåge>//
- **[caa84c8da](facebook/react@caa84c8da )**: Revert fetch instrumentation to only RSC ([facebook#25540](facebook/react#25540)) //<Sebastian Markbåge>//
- **[0c11baa6a](facebook/react@0c11baa6a )**: add warnings for non-resources rendered outside body or head ([facebook#25532](facebook/react#25532)) //<Josh Story>//
- **[9236abdb5](facebook/react@9236abdb5 )**: when float is enabled only push title and script as a single unit ([facebook#25536](facebook/react#25536)) //<Josh Story>//
- **[dd5c20825](facebook/react@dd5c20825 )**: Revert yieldy behavior for non-use Suspense ([facebook#25537](facebook/react#25537)) //<Andrew Clark>//
- **[934177598](facebook/react@934177598 )**: fix transposed escape functions ([facebook#25534](facebook/react#25534)) //<Josh Story>//
- **[d1ced9fd5](facebook/react@d1ced9fd5 )**: [Float] support all links as Resources ([facebook#25515](facebook/react#25515)) //<Josh Story>//
- **[973b90bdf](facebook/react@973b90bdf )**: [Float] support meta tags as Resources ([facebook#25514](facebook/react#25514)) //<Josh Story>//
- **[79c582981](facebook/react@79c582981 )**: Let ReactDOM initialize in RSC ([facebook#25503](facebook/react#25503)) //<Sebastian Markbåge>//
- **[1f7a2f577](facebook/react@1f7a2f577 )**: [Float] support title tags as Resources ([facebook#25508](facebook/react#25508)) //<Josh Story>//
- **[c63580787](facebook/react@c63580787 )**: Support `use` in `act` testing API ([facebook#25523](facebook/react#25523)) //<Andrew Clark>//
- **[65e32e58b](facebook/react@65e32e58b )**: Add fetch Instrumentation to Dedupe Fetches ([facebook#25516](facebook/react#25516)) //<Sebastian Markbåge>//
- **[9336e29d9](facebook/react@9336e29d9 )**: [useEvent] Lint for presence of useEvent functions in dependency lists ([facebook#25512](facebook/react#25512)) //<lauren>//
- **[3cc792bfb](facebook/react@3cc792bfb )**: [useEvent] Non-stable function identity ([facebook#25473](facebook/react#25473)) //<lauren>//
- **[987292815](facebook/react@987292815 )**: Remove feature flag enableStrictEffects ([facebook#25387](facebook/react#25387)) //<Samuel Susla>//
- **[8e2bde6f2](facebook/react@8e2bde6f2 )**: Add cache() API ([facebook#25506](facebook/react#25506)) //<Sebastian Markbåge>//
- **[9cdf8a99e](facebook/react@9cdf8a99e )**: [Codemod] Update copyright header to Meta ([facebook#25315](facebook/react#25315)) //<Andrew Clark>//
- **[e54015e26](facebook/react@e54015e26 )**: Refactor: fill in the flow missing type ([facebook#25496](facebook/react#25496)) //<c0dedance>//
- **[3b1fd5767](facebook/react@3b1fd5767 )**: refactor: Flow: typing of Scheduler ([facebook#25485](facebook/react#25485)) //<bubucuo>//
- **[14072ce64](facebook/react@14072ce64 )**: Add detach to Offscreen component ([facebook#25265](facebook/react#25265)) //<Samuel Susla>//
- **[3bb71dfd4](facebook/react@3bb71dfd4 )**: Rename react-server-dom-webpack entry points to /client and /server ([facebook#25504](facebook/react#25504)) //<Sebastian Markbåge>//
- **[71f2c8cf1](facebook/react@71f2c8cf1 )**: move resource acquisition to mutation phase ([facebook#25500](facebook/react#25500)) //<Josh Story>//
- **[500bea532](facebook/react@500bea532 )**: Add option to load Fizz runtime from external file ([facebook#25499](facebook/react#25499)) //<Andrew Clark>//
- **[4494f2a86](facebook/react@4494f2a86 )**: [Float] add support for scripts and other enhancements ([facebook#25480](facebook/react#25480)) //<Josh Story>//
- **[9ecf84ed7](facebook/react@9ecf84ed7 )**: Bugfix: Suspending in shell during discrete update ([facebook#25495](facebook/react#25495)) //<Andrew Clark>//

Changelog:
[General][Changed] - React Native sync for revisions 54f297a...ab075a2

jest_e2e[run_all_tests]

Reviewed By: kassens

Differential Revision: D40897093

fbshipit-source-id: 6a040315834dea5c0ab994ea94d91f5605b9d6b0
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed React Core Team Opened by a member of the React Core Team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants