Skip to content
New issue

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

#

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

Already on GitHub? # to your account

Jump to bottom

Support disabling spurious act warnings with a global environment flag #22561

Merged
merged 2 commits into from
Oct 18, 2021
Merged

Support disabling spurious act warnings with a global environment flag #22561

merged 2 commits into from
Oct 18, 2021

Conversation

acdlite
Copy link
Collaborator

@acdlite acdlite commented Oct 14, 2021

Refer to reactwg/react-18#102 for context on overall proposal

This adds support for disabling spurious act warnings using a global. The tentative name is IS_REACT_ACT_ENVIRONMENT, though that may change before release.

If IS_REACT_ACT_ENVIRONMENT is set to false, we will suppress any act warnings. Otherwise, the behavior of act is the same as in React 17: if jest is defined, it warns.

In concurrent roots, the plan is to remove the jest check and only warn if IS_REACT_ACT_ENVIRONMENT is true. I have not implemented that part yet.

@facebook-github-bot facebook-github-bot added the React Core Team Opened by a member of the React Core Team label Oct 14, 2021
@sizebot
Copy link

sizebot commented Oct 14, 2021 •
edited
Loading

Comparing: a45533c...5a0bbca

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 = 130.15 kB 130.15 kB = 41.41 kB 41.41 kB
oss-experimental/react-dom/cjs/react-dom.production.min.js = 132.98 kB 132.98 kB = 42.39 kB 42.39 kB
facebook-www/ReactDOM-prod.classic.js = 414.32 kB 414.32 kB = 76.58 kB 76.58 kB
facebook-www/ReactDOM-prod.modern.js = 402.91 kB 402.91 kB = 74.85 kB 74.85 kB
facebook-www/ReactDOMForked-prod.classic.js = 414.32 kB 414.32 kB = 76.58 kB 76.58 kB

Significant size changes

Includes any change greater than 0.2%:

Expand to show
Name +/- Base Current +/- gzip Base gzip Current gzip
oss-experimental/jest-react/cjs/jest-react.development.js +1.06% 11.28 kB 11.40 kB +1.81% 3.81 kB 3.87 kB
oss-stable-semver/jest-react/cjs/jest-react.development.js +1.06% 11.28 kB 11.40 kB +1.81% 3.81 kB 3.87 kB
oss-stable/jest-react/cjs/jest-react.development.js +1.06% 11.28 kB 11.40 kB +1.81% 3.81 kB 3.87 kB
react-native/implementations/ReactFabric-dev.js +0.38% 724.16 kB 726.88 kB +0.36% 157.29 kB 157.85 kB
facebook-www/ReactART-dev.modern.js +0.37% 724.89 kB 727.61 kB +0.39% 155.17 kB 155.78 kB
facebook-www/ReactART-dev.classic.js +0.37% 735.09 kB 737.81 kB +0.37% 157.30 kB 157.89 kB
react-native/implementations/ReactFabric-dev.fb.js +0.36% 753.65 kB 756.36 kB +0.35% 162.92 kB 163.50 kB
oss-stable-semver/react-art/cjs/react-art.development.js +0.36% 669.49 kB 671.90 kB +0.36% 145.47 kB 145.99 kB
oss-stable/react-art/cjs/react-art.development.js +0.36% 669.49 kB 671.90 kB +0.36% 145.47 kB 145.99 kB
oss-experimental/react-art/cjs/react-art.development.js +0.35% 685.48 kB 687.89 kB +0.35% 148.93 kB 149.46 kB
oss-stable-semver/react-art/umd/react-art.development.js +0.33% 773.28 kB 775.80 kB +0.32% 163.83 kB 164.36 kB
oss-stable/react-art/umd/react-art.development.js +0.33% 773.28 kB 775.80 kB +0.32% 163.83 kB 164.36 kB
oss-experimental/react-art/umd/react-art.development.js +0.32% 790.12 kB 792.64 kB +0.31% 167.28 kB 167.80 kB
facebook-www/React-dev.classic.js = 115.02 kB 114.64 kB = 28.34 kB 28.20 kB
facebook-www/React-dev.modern.js = 114.00 kB 113.62 kB = 28.15 kB 28.00 kB
oss-experimental/react/umd/react.development.js = 113.45 kB 113.06 kB = 29.08 kB 28.93 kB
oss-stable-semver/react/umd/react.development.js = 112.48 kB 112.09 kB = 28.96 kB 28.81 kB
oss-stable/react/umd/react.development.js = 112.48 kB 112.09 kB = 28.96 kB 28.81 kB
facebook-react-native/react/cjs/React-dev.js = 105.12 kB 104.75 kB = 25.68 kB 25.52 kB
oss-experimental/react/cjs/react.development.js = 89.70 kB 89.32 kB = 24.11 kB 23.95 kB
oss-stable-semver/react/cjs/react.development.js = 88.77 kB 88.39 kB = 23.98 kB 23.82 kB
oss-stable/react/cjs/react.development.js = 88.77 kB 88.39 kB = 23.98 kB 23.82 kB
oss-experimental/react/cjs/react-unstable-shared-subset.development.js = 75.39 kB 75.01 kB = 20.68 kB 20.53 kB

Generated by 🚫 dangerJS against 5a0bbca

@acdlite acdlite force-pushed the flag-to-disable-spurious-act-warnings branch from bb01b37 to d5cc3c6 Compare October 14, 2021 19:05
@acdlite
Extract act environment check into function
c677938 
`act` checks the environment to determine whether to fire a warning.
We're changing how this check works in React 18. As a first step, this
refactors the logic into a single function. No behavior changes yet.
@acdlite acdlite mentioned this pull request Oct 14, 2021
Merged
@acdlite acdlite requested a review from eps1lon October 14, 2021 21:19
eps1lon
eps1lon approved these changes Oct 14, 2021
View reviewed changes
Copy link
Collaborator

@eps1lon eps1lon left a comment

Choose a reason for hiding this comment

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

This looks great 👍🏻 Though I don't trust myself evaluating triple && statements (warnsIfNotActing && jestIsDefined && isReactActEnvironmentGlobal !== false) in my head so I'm curious if this integrates well with our Mocha test suite in Material-UI.

packages/react-reconciler/src/ReactFiberAct.new.js Outdated
: undefined;

// TODO: Only check `jest` in legacy mode. In concurrent mode, this heuristic
// is repalced by IS_REACT_ACT_ENVIRONMENT.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
// is repalced by IS_REACT_ACT_ENVIRONMENT.
// is replaced by IS_REACT_ACT_ENVIRONMENT.

packages/react-reconciler/src/ReactFiberAct.old.js Outdated
: undefined;

// TODO: Only check `jest` in legacy mode. In concurrent mode, this heuristic
// is repalced by IS_REACT_ACT_ENVIRONMENT.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
// is repalced by IS_REACT_ACT_ENVIRONMENT.
// is replaced by IS_REACT_ACT_ENVIRONMENT.

@acdlite
Use IS_REACT_ACT_ENVIRONMENT to disable warnings
5a0bbca 
If `IS_REACT_ACT_ENVIRONMENT` is set to `false`, we will suppress
any `act` warnings. Otherwise, the behavior of `act` is the same as in
React 17: if `jest` is defined, it warns.

In concurrent mode, the plan is to remove the `jest` check and only warn
if `IS_REACT_ACT_ENVIRONMENT` is true. I have not implemented that
part yet.
@acdlite acdlite force-pushed the flag-to-disable-spurious-act-warnings branch from d5cc3c6 to 5a0bbca Compare October 18, 2021 13:38
@acdlite acdlite merged commit 163e81c into facebook:main Oct 18, 2021
facebook-github-bot pushed a commit to facebook/react-native that referenced this pull request Nov 2, 2021
@facebook-github-bot
React Native sync for revisions afcb9cd...3fcd81d
e161598 
Summary:
This sync includes the following changes:
- **[5cccacd13](facebook/react@5cccacd13 )**: Upgrade useId to alpha channel ([#22674](facebook/react#22674)) //<Andrew Clark>//
- **[75f3ddebf](facebook/react@75f3ddebf )**: Remove experimental useOpaqueIdentifier API ([#22672](facebook/react#22672)) //<Andrew Clark>//
- **[8c4a05b8f](facebook/react@8c4a05b8f )**: Remove flow pragma comment from module registration start/stop templates ([#22670](facebook/react#22670)) //<Brian Vaughn>//
- **[ebf9ae857](facebook/react@ebf9ae857 )**: useId ([#22644](facebook/react#22644)) //<Andrew Clark>//
- **[a0d991fe6](facebook/react@a0d991fe6 )**: Re-land #22292 (remove uMS from open source build) ([#22664](facebook/react#22664)) //<Andrew Clark>//
- **[6bce0355c](facebook/react@6bce0355c )**: Upgrade useSyncExternalStore to alpha channel ([#22662](facebook/react#22662)) //<Andrew Clark>//
- **[7034408ff](facebook/react@7034408ff )**: Follow-up improvements to error code extraction infra ([#22516](facebook/react#22516)) //<Andrew Clark>//
- **[90e5d3638](facebook/react@90e5d3638 )**: chore: fix comment typo ([#22615](facebook/react#22615)) //<btea>//
- **[3c4c1c470](facebook/react@3c4c1c470 )**: Remove warning for dangling passive effects ([#22609](facebook/react#22609)) //<Andrew Clark>//
- **[d5b6b4b86](facebook/react@d5b6b4b86 )**: Expand act warning to cover all APIs that might schedule React work ([#22607](facebook/react#22607)) //<Andrew Clark>//
- **[fa9bea0c4](facebook/react@fa9bea0c4 )**: Initial implementation of cache cleanup ([#22510](facebook/react#22510)) //<Joseph Savona>//
- **[0e8a5aff3](facebook/react@0e8a5aff3 )**: Scheduling Profiler: Add marks for component effects (mount and unmount) ([#22578](facebook/react#22578)) //<Brian Vaughn>//
- **[4ba20579d](facebook/react@4ba20579d )**: Scheduling Profiler: De-emphasize React internal frames ([#22588](facebook/react#22588)) //<Brian Vaughn>//
- **[cdb8a1d19](facebook/react@cdb8a1d19 )**: [Fizz] Add option to inject bootstrapping script tags after the shell is injected ([#22594](facebook/react#22594)) //<Sebastian Markbåge>//
- **[34e4c9756](facebook/react@34e4c9756 )**: Clear extra nodes if there's a hydration mismatch within a suspense boundary  ([#22592](facebook/react#22592)) //<Sebastian Markbåge>//
- **[02f411578](facebook/react@02f411578 )**: Upgrade useInsertionEffect to stable ([#22589](facebook/react#22589)) //<Andrew Clark>//
- **[2af4a7933](facebook/react@2af4a7933 )**: Hydrate using SuspenseComponent as the parent ([#22582](facebook/react#22582)) //<Sebastian Markbåge>//
- **[b1acff0cc](facebook/react@b1acff0cc )**: Enable cache in test renderer ([#22580](facebook/react#22580)) //<Joseph Savona>//
- **[996da67b2](facebook/react@996da67b2 )**: Replace global `jest` heuristic with `IS_REACT_ACT_ENVIRONMENT` ([#22562](facebook/react#22562)) //<Andrew Clark>//
- **[163e81c1f](facebook/react@163e81c1f )**: Support disabling spurious act warnings with a global environment flag ([#22561](facebook/react#22561)) //<Andrew Clark>//
- **[23b7dfeff](facebook/react@23b7dfeff )**: Enable scheduling profiler for RN FB profiling builds ([#22566](facebook/react#22566)) //<Brian Vaughn>//
- **[61455a25b](facebook/react@61455a25b )**: Enable experimental Cache API in www TestRenderer ([#22554](facebook/react#22554)) //<Joseph Savona>//
- **[7142d110b](facebook/react@7142d110b )**: Bugfix: Nested useOpaqueIdentifier references ([#22553](facebook/react#22553)) //<Andrew Clark>//
- **[1e247ff89](facebook/react@1e247ff89 )**: Enabled scheduling profiler marks for React Native FB target ([#22544](facebook/react#22544)) //<Brian Vaughn>//
- **[c16b005f2](facebook/react@c16b005f2 )**: Update test and stack frame code to support newer V8 stack formats ([#22477](facebook/react#22477)) //<Brian Vaughn>//
- **[55d75005b](facebook/react@55d75005b )**: duplicate value in variable ([#22390](facebook/react#22390)) //<BIKI DAS>//

Changelog:
[General][Changed] - React Native sync for revisions afcb9cd...3fcd81d

jest_e2e[run_all_tests]

Reviewed By: yungsters

Differential Revision: D32065987

fbshipit-source-id: ef2d402835c981aab68ca40a894c66c1630864e9
KamranAsif pushed a commit to KamranAsif/react that referenced this pull request Nov 4, 2021
@acdlite @KamranAsif
Support disabling spurious act warnings with a global environment flag (
b81e99b 
facebook#22561)

* Extract `act` environment check into function

`act` checks the environment to determine whether to fire a warning.
We're changing how this check works in React 18. As a first step, this
refactors the logic into a single function. No behavior changes yet.

* Use IS_REACT_ACT_ENVIRONMENT to disable warnings

If `IS_REACT_ACT_ENVIRONMENT` is set to `false`, we will suppress
any `act` warnings. Otherwise, the behavior of `act` is the same as in
React 17: if `jest` is defined, it warns.

In concurrent mode, the plan is to remove the `jest` check and only warn
if `IS_REACT_ACT_ENVIRONMENT` is true. I have not implemented that
part yet.
facebook-github-bot pushed a commit to facebook/react-native that referenced this pull request Nov 15, 2021
@facebook-github-bot
React Native sync for revisions afcb9cd...c0c71a8
55663ec 
Summary:
This sync includes the following changes:
- **[c0c71a868](facebook/react@c0c71a868 )**: Re-enable useMutableSource in internal RN ([#22750](facebook/react#22750)) //<Ricky>//
- **[cb11155c8](facebook/react@cb11155c8 )**: Add runtime type checks around module boundary code ([#22748](facebook/react#22748)) //<Brian Vaughn>//
- **[a04f13d29](facebook/react@a04f13d29 )**: react-refresh@0.11.0 //<Dan Abramov>//
- **[ff9897d23](facebook/react@ff9897d23 )**: [React Refresh] support typescript namespace syntax ([#22621](facebook/react#22621)) //<irinakk>//
- **[0ddd69d12](facebook/react@0ddd69d12 )**: Throw on hydration mismatch and force client rendering if boundary hasn't suspended within concurrent root ([#22629](facebook/react#22629)) //<salazarm>//
- **[c3f34e4be](facebook/react@c3f34e4be )**: eslint-plugin-react-hooks@4.3.0 //<Dan Abramov>//
- **[827021c4e](facebook/react@827021c4e )**: Changelog for eslint-plugin-react-hooks@4.3.0 //<Dan Abramov>//
- **[8ca3f567b](facebook/react@8ca3f567b )**: Fix module-boundary wrappers ([#22688](facebook/react#22688)) //<Brian Vaughn>//
- **[1bf6deb86](facebook/react@1bf6deb86 )**: Renamed packages/react-devtools-scheduling-profiler to packages/react-devtools-timeline ([#22691](facebook/react#22691)) //<Brian Vaughn>//
- **[51c558aeb](facebook/react@51c558aeb )**: Rename (some) "scheduling profiler" references to "timeline" ([#22690](facebook/react#22690)) //<Brian Vaughn>//
- **[00ced1e2b](facebook/react@00ced1e2b )**: Fix useId in strict mode ([#22681](facebook/react#22681)) //<Andrew Clark>//
- **[5cccacd13](facebook/react@5cccacd13 )**: Upgrade useId to alpha channel ([#22674](facebook/react#22674)) //<Andrew Clark>//
- **[75f3ddebf](facebook/react@75f3ddebf )**: Remove experimental useOpaqueIdentifier API ([#22672](facebook/react#22672)) //<Andrew Clark>//
- **[8c4a05b8f](facebook/react@8c4a05b8f )**: Remove flow pragma comment from module registration start/stop templates ([#22670](facebook/react#22670)) //<Brian Vaughn>//
- **[ebf9ae857](facebook/react@ebf9ae857 )**: useId ([#22644](facebook/react#22644)) //<Andrew Clark>//
- **[a0d991fe6](facebook/react@a0d991fe6 )**: Re-land #22292 (remove uMS from open source build) ([#22664](facebook/react#22664)) //<Andrew Clark>//
- **[6bce0355c](facebook/react@6bce0355c )**: Upgrade useSyncExternalStore to alpha channel ([#22662](facebook/react#22662)) //<Andrew Clark>//
- **[7034408ff](facebook/react@7034408ff )**: Follow-up improvements to error code extraction infra ([#22516](facebook/react#22516)) //<Andrew Clark>//
- **[90e5d3638](facebook/react@90e5d3638 )**: chore: fix comment typo ([#22615](facebook/react#22615)) //<btea>//
- **[3c4c1c470](facebook/react@3c4c1c470 )**: Remove warning for dangling passive effects ([#22609](facebook/react#22609)) //<Andrew Clark>//
- **[d5b6b4b86](facebook/react@d5b6b4b86 )**: Expand act warning to cover all APIs that might schedule React work ([#22607](facebook/react#22607)) //<Andrew Clark>//
- **[fa9bea0c4](facebook/react@fa9bea0c4 )**: Initial implementation of cache cleanup ([#22510](facebook/react#22510)) //<Joseph Savona>//
- **[0e8a5aff3](facebook/react@0e8a5aff3 )**: Scheduling Profiler: Add marks for component effects (mount and unmount) ([#22578](facebook/react#22578)) //<Brian Vaughn>//
- **[4ba20579d](facebook/react@4ba20579d )**: Scheduling Profiler: De-emphasize React internal frames ([#22588](facebook/react#22588)) //<Brian Vaughn>//
- **[cdb8a1d19](facebook/react@cdb8a1d19 )**: [Fizz] Add option to inject bootstrapping script tags after the shell is injected ([#22594](facebook/react#22594)) //<Sebastian Markbåge>//
- **[34e4c9756](facebook/react@34e4c9756 )**: Clear extra nodes if there's a hydration mismatch within a suspense boundary  ([#22592](facebook/react#22592)) //<Sebastian Markbåge>//
- **[02f411578](facebook/react@02f411578 )**: Upgrade useInsertionEffect to stable ([#22589](facebook/react#22589)) //<Andrew Clark>//
- **[2af4a7933](facebook/react@2af4a7933 )**: Hydrate using SuspenseComponent as the parent ([#22582](facebook/react#22582)) //<Sebastian Markbåge>//
- **[b1acff0cc](facebook/react@b1acff0cc )**: Enable cache in test renderer ([#22580](facebook/react#22580)) //<Joseph Savona>//
- **[996da67b2](facebook/react@996da67b2 )**: Replace global `jest` heuristic with `IS_REACT_ACT_ENVIRONMENT` ([#22562](facebook/react#22562)) //<Andrew Clark>//
- **[163e81c1f](facebook/react@163e81c1f )**: Support disabling spurious act warnings with a global environment flag ([#22561](facebook/react#22561)) //<Andrew Clark>//
- **[23b7dfeff](facebook/react@23b7dfeff )**: Enable scheduling profiler for RN FB profiling builds ([#22566](facebook/react#22566)) //<Brian Vaughn>//
- **[61455a25b](facebook/react@61455a25b )**: Enable experimental Cache API in www TestRenderer ([#22554](facebook/react#22554)) //<Joseph Savona>//
- **[7142d110b](facebook/react@7142d110b )**: Bugfix: Nested useOpaqueIdentifier references ([#22553](facebook/react#22553)) //<Andrew Clark>//
- **[1e247ff89](facebook/react@1e247ff89 )**: Enabled scheduling profiler marks for React Native FB target ([#22544](facebook/react#22544)) //<Brian Vaughn>//
- **[c16b005f2](facebook/react@c16b005f2 )**: Update test and stack frame code to support newer V8 stack formats ([#22477](facebook/react#22477)) //<Brian Vaughn>//
- **[55d75005b](facebook/react@55d75005b )**: duplicate value in variable ([#22390](facebook/react#22390)) //<BIKI DAS>//

Changelog:
[General][Changed] - React Native sync for revisions afcb9cd...c0c71a8

jest_e2e[run_all_tests]

Reviewed By: yungsters

Differential Revision: D32395873

fbshipit-source-id: 3afd158f167b1eedcc244e29aba1a2c502d3c9d9
@gaearon gaearon mentioned this pull request Mar 29, 2022
Merged
zhengjitf pushed a commit to zhengjitf/react that referenced this pull request Apr 15, 2022
@acdlite @zhengjitf
Support disabling spurious act warnings with a global environment flag (
a0b7ff6 
facebook#22561)

* Extract `act` environment check into function

`act` checks the environment to determine whether to fire a warning.
We're changing how this check works in React 18. As a first step, this
refactors the logic into a single function. No behavior changes yet.

* Use IS_REACT_ACT_ENVIRONMENT to disable warnings

If `IS_REACT_ACT_ENVIRONMENT` is set to `false`, we will suppress
any `act` warnings. Otherwise, the behavior of `act` is the same as in
React 17: if `jest` is defined, it warns.

In concurrent mode, the plan is to remove the `jest` check and only warn
if `IS_REACT_ACT_ENVIRONMENT` is true. I have not implemented that
part yet.
# for free to join this conversation on GitHub. Already have an account? # to comment
Reviewers

@eps1lon eps1lon

Assignees
No one assigned
Labels
CLA Signed React Core Team Opened by a member of the React Core Team
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@acdlite @sizebot @eps1lon @facebook-github-bot