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

Warn if you try to use act() in prod #16282

Merged
merged 1 commit into from
Aug 5, 2019

Conversation

threepointone
Copy link
Contributor

We have behaviour divergence for act() between prod and dev (specifically, act() + concurrent mode does not flush fallbacks in prod. This doesn't affect anyone in OSS yet.)

We also don't have a good story for writing tests in prod (and what from what I gather, nobody really writes tests in prod mode).

We could have wiped out act() in prod builds, except that we ourselves use act() for our tests when we run them in prod mode.

This PR is a compromise to all of this. We will log a warning if you try to use act() in prod mode, and we silence it in our test suites. (Any discrepancies in behaviour will still be caught by assertions).

We will revisit this in a future version.

@sizebot
Copy link

sizebot commented Aug 2, 2019

No significant bundle size changes to report.

Generated by 🚫 dangerJS

@acdlite
Copy link
Collaborator

acdlite commented Aug 5, 2019

Tests?

@threepointone
Copy link
Contributor Author

doh! 😅 fixing asap

We have behaviour divergence for act() between prod and dev (specifically, act() + concurrent mode does not flush fallbacks in prod. This doesn't affect anyone in OSS yet)

We also don't have a good story for writing tests in prod (and what from what I gather, nobody really writes tests in prod mode).

We could have wiped out act() in prod builds, except that _we_ ourselves use act() for our tests when we run them in prod mode.

This PR is a compromise to all of this. We will log a warning if you try to use act() in prod mode, and we silence it in our test suites.
@threepointone
Copy link
Contributor Author

Added a test.

@acdlite
Copy link
Collaborator

acdlite commented Aug 5, 2019

Merging so I can sync it to www

@acdlite acdlite merged commit a1dbb85 into facebook:master Aug 5, 2019
@threepointone threepointone deleted the act-prod-warning branch August 5, 2019 21:18
@henryqdineen
Copy link
Contributor

henryqdineen commented Sep 22, 2019

from what I gather, nobody really writes tests in prod mode

Hey @threepointone! We currently run our tests in prod mode in the CI and I noticed this warning while upgrading to 16.9.0. Will this be removed in a future version without deprecations? Any suggestions other than running tests against the DEV build? It would be a major infra change for us at this point. Thanks!

NMinhNguyen referenced this pull request in enzymejs/react-shallow-renderer Jan 29, 2020
We have behaviour divergence for act() between prod and dev (specifically, act() + concurrent mode does not flush fallbacks in prod. This doesn't affect anyone in OSS yet)

We also don't have a good story for writing tests in prod (and what from what I gather, nobody really writes tests in prod mode).

We could have wiped out act() in prod builds, except that _we_ ourselves use act() for our tests when we run them in prod mode.

This PR is a compromise to all of this. We will log a warning if you try to use act() in prod mode, and we silence it in our test suites.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants