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

fix(package.json): mark react-transition-group as side-effect free for webpack tree shaking #472

Merged
merged 1 commit into from
Mar 14, 2019

Conversation

setsun
Copy link
Contributor

@setsun setsun commented Mar 13, 2019

What

Add "sideEffects": false to package.json to hint to webpack based workflows that the package is side effect free. This allows react-transition-group to allow tree-shaking of its modules.

Reference: https://webpack.js.org/guides/tree-shaking/#mark-the-file-as-side-effect-free

Copy link
Collaborator

@silvenon silvenon left a comment

Choose a reason for hiding this comment

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

Wow, thank you!!!

@silvenon silvenon merged commit b81dc89 into reactjs:master Mar 14, 2019
jquense pushed a commit that referenced this pull request Mar 14, 2019
## [2.6.1](v2.6.0...v2.6.1) (2019-03-14)

### Bug Fixes

* **package.json:** mark react-transition-group as side-effect free for webpack tree shaking ([#472](#472)) ([b81dc89](b81dc89))
@jquense
Copy link
Collaborator

jquense commented Mar 14, 2019

🎉 This PR is included in version 2.6.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

@billyjanitsch
Copy link

@setsun @silvenon Unfortunately this doesn't have any effect because react-transition-group only ships a CommonJS build. It would have to also ship an ESM build (and point to it with the module field in package.json) for {sideEffects: true} to do anything. Module-level tree-shaking isn't possible with CommonJS.

@setsun
Copy link
Contributor Author

setsun commented Mar 15, 2019

Ah interesting, that's good to know. This may warrant a follow up PR

@silvenon
Copy link
Collaborator

silvenon commented Mar 17, 2019

Actually, #455 already adds ESM (but it was missing sideEffects!), we just haven't merged it yet. I was really busy and honestly not really knowledgeable about modules, but now I'll take some time to properly review and merge it if it's all good. 🙂 I would appreciate another pair of eyes on that PR if anyone wants to help, GitHub allows non-collaborators to also review PRs. 😎

johnfrench3 pushed a commit to johnfrench3/transition-group-react that referenced this pull request Nov 2, 2022
## [2.6.1](reactjs/react-transition-group@v2.6.0...v2.6.1) (2019-03-14)

### Bug Fixes

* **package.json:** mark react-transition-group as side-effect free for webpack tree shaking ([#472](reactjs/react-transition-group#472)) ([b81dc89](reactjs/react-transition-group@b81dc89))
patrickm68 added a commit to patrickm68/react-transition-group-developer that referenced this pull request Dec 1, 2022
## [2.6.1](reactjs/react-transition-group@v2.6.0...v2.6.1) (2019-03-14)

### Bug Fixes

* **package.json:** mark react-transition-group as side-effect free for webpack tree shaking ([#472](reactjs/react-transition-group#472)) ([b81dc89](reactjs/react-transition-group@b81dc89))
shaikdev2 pushed a commit to shaikdev2/transition-group-react that referenced this pull request Jun 9, 2023
## [2.6.1](reactjs/react-transition-group@v2.6.0...v2.6.1) (2019-03-14)

### Bug Fixes

* **package.json:** mark react-transition-group as side-effect free for webpack tree shaking ([#472](reactjs/react-transition-group#472)) ([b81dc89](reactjs/react-transition-group@b81dc89))
GreenCat1996 added a commit to GreenCat1996/react-transition-group that referenced this pull request Aug 1, 2023
## [2.6.1](reactjs/react-transition-group@v2.6.0...v2.6.1) (2019-03-14)

### Bug Fixes

* **package.json:** mark react-transition-group as side-effect free for webpack tree shaking ([#472](reactjs/react-transition-group#472)) ([b81dc89](reactjs/react-transition-group@b81dc89))
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants