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

Add sideEffects flag to react-is for tree shaking #27701

Merged
merged 1 commit into from
Dec 1, 2023

Conversation

markerikson
Copy link
Contributor

Summary

This PR:

  • Adds the "sideEffects": false flag to the react-is package, to enable proper tree-shaking

How did you test this change?

React-Redux v9 beta switches its artifacts from separate JS files to pre-bundled artifacts. While testing builds locally, I noticed that our use of react-is was no longer getting tree-shaken from Vite builds, despite react-is only being used by our connect API and connect itself getting shaken out of the final bundle.

Hand-adding "sideEffects": false to the react-is package locally convinced Vite (+Rollup) to properly tree-shake react-is out of the bundle when it wasn't being used.

I'd love to see this published as v18.2.1 as soon as this PR is merged - we're hoping to release React-Redux v9 in the next few weeks!

@david-alvarez-rosa
Copy link

LGTM.

@markerikson
Copy link
Contributor Author

FWIW I actually ended up just copy-pasting and inlining the few react-is methods and values we need in React-Redux, out of the latest react-is canary build. (I realize this is fragile and the actual package could change, but the checks are just isMemo, isContextConsumer, and isValidElementType, and these are only used inside of connect(). Inlining those made the code tree-shake properly out of example app bundles, and we are hoping to ship React-Redux 9.0 this week, so I'm fine with that.

If this ever gets merged and released I can try switching back.

@kassens
Copy link
Member

kassens commented Dec 1, 2023

Thanks, makes sense!

@kassens kassens merged commit 108fd8c into facebook:main Dec 1, 2023
34 of 35 checks passed
EdisonVan pushed a commit to EdisonVan/react that referenced this pull request Apr 15, 2024
## Summary

This PR:

- Adds the `"sideEffects": false` flag to the `react-is` package, to
enable proper tree-shaking

## How did you test this change?

React-Redux v9 beta switches its artifacts from separate JS files to
pre-bundled artifacts. While testing builds locally, I noticed that our
use of `react-is` was no longer getting tree-shaken from Vite builds,
despite `react-is` only being used by our `connect` API and `connect`
itself getting shaken out of the final bundle.

Hand-adding `"sideEffects": false` to the `react-is` package locally
convinced Vite (+Rollup) to properly tree-shake `react-is` out of the
bundle when it wasn't being used.

I'd love to see this published as v18.2.1 as soon as this PR is merged -
we're hoping to release React-Redux v9 in the next few weeks!
bigfootjon pushed a commit that referenced this pull request Apr 18, 2024
## Summary

This PR:

- Adds the `"sideEffects": false` flag to the `react-is` package, to
enable proper tree-shaking

## How did you test this change?

React-Redux v9 beta switches its artifacts from separate JS files to
pre-bundled artifacts. While testing builds locally, I noticed that our
use of `react-is` was no longer getting tree-shaken from Vite builds,
despite `react-is` only being used by our `connect` API and `connect`
itself getting shaken out of the final bundle.

Hand-adding `"sideEffects": false` to the `react-is` package locally
convinced Vite (+Rollup) to properly tree-shake `react-is` out of the
bundle when it wasn't being used.

I'd love to see this published as v18.2.1 as soon as this PR is merged -
we're hoping to release React-Redux v9 in the next few weeks!

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

None yet

4 participants