-
Notifications
You must be signed in to change notification settings - Fork 78
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
Is tree-shaking working? #993
Comments
Do you have webpack configuration that you can post? I think this could be a potential easy bug to miss. Also, this statement sounds like you are looking for this configuration in our packages.json. This would need to be in the consuming applications package.json
|
I am trying to test to see if I am able to get the packages to tree shake. Will update once I have more information. |
@scottdickerson was able to confirm that they are able to achieve tree shaking without having to change any configuration. |
Hey @ilovepumpkin i think we need to add the sideEffects property in our Package.json like you mention above. Will test and update. |
@ilovepumpkin thank you for making us look at this again. I am testing a fix and should have a PR open soon. |
Oh, great! I am glad you have found the solution so quickly. Just let me know once you fix it - I can not wait to try it, :D |
just an FYI @ilovepumpkin and @davidicus I don't think the Source Bundle Analyzer really helps you understand tree shaking in webpack 4: webpack-contrib/webpack-bundle-analyzer#161 I had to actually look at my uglified production file size to verify the tree was shaken. |
Hi @scottdickerson, actually I am using source-map-analyzer for the above screenshot, not webpack-bundle-analyzer. |
🎉 This issue has been resolved in version 2.61.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
@davidicus Today when I was testing your new build in my application, I was very glad to see that the bundle size issue is fixed (thank you so much!), but the carbon-addons-iot-react styles are all gone - carbon-addons-iot-react.css is imported in my App.jsx and it works well with previous version. After investigation, I found the sideEffects should be set in the following way so it can work. With '"sideEffects": false', if I import carbon-addons-iot-react.css in my project main.scss, it also works. But you don't know how consuming applications use it, so I would suggest your fix it. Will you? |
I see my issue #793 is already closed. I tried the steps mentioned in it with latest version 2.55.2, but I don't think tree-shaking is working. Then I tried 'npx create-iot-react-app' to create a hello world app, and run 'yarn build', I got the following after running source map analyzer against the biggest bundle files generated.
In App.js, only 5 components are imported, but the bundled index.js is 1.34MB. So I doubt if tree-shaking is really working, or I need apply any additional steps in the created simple app.
Per my understanding, to support tree-shaking, carbon-addons-iot-react should be bundled in es module, i.e. there should be "module" entry in package.json, and "sideEffects:false" as well. But I didn't find them.
And I see lots of unused components are bundled from carbon-component-react even they are not used in App.js. If I directly import components from carbon-component-react, tree-shaking works well.
I really need to get the answer since after importing carbon-addons-iot-react, our bundle becomes extremely bigger than before and I am still only using Stateful component. Thanks!
The text was updated successfully, but these errors were encountered: