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

[treeshaking]: fix treeshaking and add esm and umd versions #1045

Merged
merged 31 commits into from
Apr 2, 2020

Conversation

davidicus
Copy link
Collaborator

@davidicus davidicus commented Mar 29, 2020

Closes #993 closes #1011

Summary

Changes to our build to allow for es and umd versions and to allow for treeshaking in consuming applications.

Change List (commits, features, bugs, etc)

  • items_here

Acceptance Test (how to verify the PR)

yarn build should output a lib, es, umd, and scss folder.

Using the create-iot-react-app cli I was able to test the baseline build. Like the issue opened on this reports the original build was around 2.74 MB. With tree shaking and dead code elimination this new build is now able to deliver the same app in under 314 KB which is around an 88.5% savings.

Before tree shaking After tree shaking
image image

@netlify
Copy link

netlify bot commented Mar 29, 2020

Deploy preview for carbon-addons-iot-react ready!

Built with commit 64f8d53

https://deploy-preview-1045--carbon-addons-iot-react.netlify.com

Copy link
Member

@tay1orjones tay1orjones left a comment

Choose a reason for hiding this comment

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

A couple small things here.

Additionally, the builds are being generated in a src folder. Is there any way to remove that unecessary second level of nesting so we publish just lib/index.js, es/index.js, es/components/ComponentName/ComponentName.js, etc?

rollup.config.js Outdated Show resolved Hide resolved
src/globals/_charts.scss Outdated Show resolved Hide resolved
src/index.js Show resolved Hide resolved
babel.config.js Show resolved Hide resolved
.eslintignore Show resolved Hide resolved
jest.config.js Show resolved Hide resolved
@davidicus
Copy link
Collaborator Author

davidicus commented Mar 30, 2020

A couple small things here.

Additionally, the builds are being generated in a src folder. Is there any way to remove that unecessary second level of nesting so we publish just lib/index.js, es/index.js, es/components/ComponentName/ComponentName.js, etc?

I believe this is the case because of the preserve modules flag. We can only dictate a directory with this property set to true but luckily we tell consuming apps where to find the entry point and all paths like ones used for styles are still valid.

rollup.config.js Outdated Show resolved Hide resolved
rollup.config.js Outdated Show resolved Hide resolved
Copy link
Member

@tay1orjones tay1orjones left a comment

Choose a reason for hiding this comment

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

LGTM 🎉

@scottdickerson scottdickerson merged commit 2101a52 into master Apr 2, 2020
@tay1orjones
Copy link
Member

🎉 This PR is included in version 2.61.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

@davidicus
Copy link
Collaborator Author

after adding changes to CLI to add d3 as dep for our esm module version we can see that d3 is still not shipped with the application
image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Tree shaking is not working in the PAL addons repo Is tree-shaking working?
3 participants