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

Convert more Suspense tests to use act #26602

Merged
merged 1 commit into from
Apr 11, 2023

Conversation

acdlite
Copy link
Collaborator

@acdlite acdlite commented Apr 11, 2023

Many of our Suspense-related tests were written before the act API was introduced, and use the lower level waitFor helpers instead. So they are less resilient to changes in implementation details than they could be.

This converts some of our test suite to use act in more places. I found these while working on a PR to expand our fallback throttling mechanism to include all renders that result from a promise resolving, even if there are no more fallbacks in the tree. This isn't all the affected tests, just some of them — I'll be sharding the changes across multiple PRs.

Many of our Suspense-related tests were written before the `act` API was
introduced, and use the lower level `waitFor` helpers instead. So they
are less resilient to changes in implementation details than they
could be.

This converts some of our test suite to use `act` in more places. I
found these while working on a PR to expand our fallback throttling
mechanism to include all renders that result from a promise resolving,
even if there are no more fallbacks in the tree. This isn't all the
affected tests, just some of them — I'll be sharding the changes across
multiple PRs.
@facebook-github-bot facebook-github-bot added CLA Signed React Core Team Opened by a member of the React Core Team labels Apr 11, 2023
@react-sizebot
Copy link

Comparing: 58742c2...fa8bfa9

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 = 164.01 kB 164.01 kB = 51.43 kB 51.43 kB
oss-experimental/react-dom/cjs/react-dom.production.min.js = 166.40 kB 166.40 kB = 51.99 kB 51.98 kB
facebook-www/ReactDOM-prod.classic.js = 562.84 kB 562.84 kB = 98.87 kB 98.87 kB
facebook-www/ReactDOM-prod.modern.js = 546.67 kB 546.67 kB = 96.18 kB 96.18 kB

Significant size changes

Includes any change greater than 0.2%:

(No significant changes)

Generated by 🚫 dangerJS against fa8bfa9

Comment on lines +248 to +250
// Next, we'll flush the complete content.
await waitForAll(['Bar', 'A', 'B']);
});
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should you pull this out as assertLog(...) after act has finished?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

doesn't really matter

@acdlite acdlite merged commit f9de24a into facebook:main Apr 11, 2023
kassens pushed a commit to kassens/react that referenced this pull request Apr 17, 2023
Many of our Suspense-related tests were written before the `act` API was
introduced, and use the lower level `waitFor` helpers instead. So they
are less resilient to changes in implementation details than they could
be.

This converts some of our test suite to use `act` in more places. I
found these while working on a PR to expand our fallback throttling
mechanism to include all renders that result from a promise resolving,
even if there are no more fallbacks in the tree. This isn't all the
affected tests, just some of them — I'll be sharding the changes across
multiple PRs.
EdisonVan pushed a commit to EdisonVan/react that referenced this pull request Apr 15, 2024
Many of our Suspense-related tests were written before the `act` API was
introduced, and use the lower level `waitFor` helpers instead. So they
are less resilient to changes in implementation details than they could
be.

This converts some of our test suite to use `act` in more places. I
found these while working on a PR to expand our fallback throttling
mechanism to include all renders that result from a promise resolving,
even if there are no more fallbacks in the tree. This isn't all the
affected tests, just some of them — I'll be sharding the changes across
multiple PRs.
bigfootjon pushed a commit that referenced this pull request Apr 18, 2024
Many of our Suspense-related tests were written before the `act` API was
introduced, and use the lower level `waitFor` helpers instead. So they
are less resilient to changes in implementation details than they could
be.

This converts some of our test suite to use `act` in more places. I
found these while working on a PR to expand our fallback throttling
mechanism to include all renders that result from a promise resolving,
even if there are no more fallbacks in the tree. This isn't all the
affected tests, just some of them — I'll be sharding the changes across
multiple PRs.

DiffTrain build for commit f9de24a.
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