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

Request to add support for tree-shaking in @primer/octicons-v2-react #404

Closed
dmarcey opened this issue Apr 21, 2020 · 0 comments
Closed
Labels

Comments

@dmarcey
Copy link
Contributor

dmarcey commented Apr 21, 2020

After consuming @primer/octicons-v2-react in the memex package, I noticed that the bundle size jumped about 130 kb (gzipped / minified). I thought that perhaps we didn't have tree-shaking working properly in our build system, so I played around with it a little bit, but couldn't quite get anything working.

So, I decided to start with a clean repo (based on create-react-app), and see if I could get it working there. When I was unable to, I decided to pull in a different icon library - react-feather to see if I could get tree-shaking working with it. I was able to with no extra configuration.

Here are the repro steps and the bundle size differences I encountered.

  1. git clone [email protected]:dmarcey/tree-shaking-test.git (https://github.com/dmarcey/tree-shaking-test)
  2. cd tree-shaking-test
  3. yarn install
  4. yarn build Bundle size: 39.39 KB build/static/js/2.a430f49c.chunk.js
  5. Uncomment lines 3 and 26 (import and usage of Camera).
  6. yarn build Bundle size: 40.17 KB (+802 B) build/static/js/2.b6d6f8f9.chunk.js
  7. Recomment lines 3 and 26
  8. yarn build
  9. Uncomment lines 4 and 27 (import and usage of RepoIcon)
  10. yarn build Bundle size: 168.39 KB (+129 KB) build/static/js/2.6febd43a.chunk.js

This quick test shows us that including a single react-feather icon in your bundle adds about 800 bytes, whereas adding a single @primer/octicons-v2-react icon to your bundle adds 129 KB - so tree-shaking is not happening.

I stumbled across #335 by chance - where it looks like there was an attempt made to have each icon in its own file - which looks to be the same approach taken by react-feather, which are then all exported individual at a top-level index.js.
https://github.com/feathericons/react-feather/tree/master/src/icons

Let me know if any more info is needed - we'd love to help get this resolved before octicons v2 goes live!

cc/ @colebemis @emplums

@dmarcey dmarcey changed the title Tree-Shaking doesn't work for @primer/octicons-v2-react Request to add support for tree-shaking in @primer/octicons-v2-react Apr 21, 2020
@dmarcey dmarcey changed the title Request to add support for tree-shaking in @primer/octicons-v2-react Request to add support for tree-shaking in @primer/octicons-v2-react Apr 21, 2020
@yaili yaili added the fr-skip label Apr 22, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants