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

Library imported bundle size #15

Open
SamusElderg opened this issue Nov 1, 2023 · 3 comments
Open

Library imported bundle size #15

SamusElderg opened this issue Nov 1, 2023 · 3 comments

Comments

@SamusElderg
Copy link
Collaborator

SamusElderg commented Nov 1, 2023

I am by far not an expert on tree-shaking and library optimization, however i suspect (based on import size when using a single component in this library) there is some unintended bloat coming in with imports even when tree-shaking in specific components. Most individual components seem to be ~150k imported by themselves and when you tack on multiple it doesn't really increase much, so i think in general we have large minimum-import size for one reason or another.

As we shift into using this library in place of optimized 3rd party deps we should focus on solving this, for instance, the react-avatar import was/is ~59k and we have built a lighter-weight in-house one in the frontend repo, if we migrate it to this component library and import from there instead we will bloat our NET position (for that component) by ~3x vs using the initial react-avatar and lose a portion of the advantage to doing things in-house

Im also not very familiar with vite 😂 so this is super 'not-my-wheelhouse', but i suspect some first places to look for optimizing and properly enforcing tree-shaking to its fullest would be:

  • vite.config.js
  • public folder (probably irrelevant if we get config nailed)
  • lets maybe run a bundle size analyzer before and each step of the way to nail down the best ways to optimize things as for myself at least would be mainly guessing optimization tactics thru trial and error (maybe even make it apart of the github workflow so we can compare with every PR so this never slips as a priority item in the future when reviewing PRs)

As a surface level skim of the actual structure of the library appears to be well suited to tree-shaking (in my minimal-experience opinion)

One thing we should be careful with when we get to the 'icon' component is to make sure only the intended icon SVG/s are apart of the bundle when being imported as i remember having trouble with this in the past

@SamusElderg SamusElderg changed the title Import size Library imported bundle size Nov 1, 2023
@SamusElderg
Copy link
Collaborator Author

99% sure its because the font files are being base64'd and inlined into the style.css file. Vite should handle this appropriately with files above 4KB by default but i could not get the build process to obey the expected default behavior nor even when i enforced it in the vite config 🤷‍♀️

Removing the entire @font-face rule however brings down the gzip'd size of style.css from 160kb to 7kb which is suspiciously close to the minimum bundle import size we are seeing, seems extremely likely at least that is the culprit.

@SamusElderg
Copy link
Collaborator Author

99% sure its because the font files are being base64'd and inlined into the style.css file. Vite should handle this appropriately with files above 4KB by default but i could not get the build process to obey the expected default behavior nor even when i enforced it in the vite config 🤷‍♀️

Removing the entire @font-face rule however brings down the gzip'd size of style.css from 160kb to 7kb which is suspiciously close to the minimum bundle import size we are seeing, seems extremely likely at least that is the culprit.

For starters, i think we should drop .ttf from the fonts completely, almost all browsers support .woff2. The very small portion of super old browsers that dont support .woff2 do support .woff so if we keep anything more than .woff2 we can fallback to .woff? ideally we drop all the way back to exclusively .woff2 and fallback to a common local browser font that is most similar though to keep the bundle size smaller?

For instance we go from:

  src: url("../fonts/Satoshi-Variable.woff2") format("woff2"),
    url("../fonts/Satoshi-Variable.woff") format("woff"),
    url("../fonts/Satoshi-Variable.ttf") format("truetype");

to something like:

  src: url("../fonts/Satoshi-Variable.woff2") format("woff2"),
    local(Arial),
    local(Helvetica);

@SamusElderg
Copy link
Collaborator Author

Ignore the above, seem to have a solution:

  • move font files to the public folder
  • reference them instead in the .scss file

Will make a quick PR and we can test if it reduces the bundle size (and instead adds a http request for the font file)

SamusElderg added a commit that referenced this issue Nov 6, 2023
- moved the font files to the public folder
- changes the typography css rules to point to them so that they can be served as-is without being base64'd and added inline to style.css on build

This might solve a large portion of #15 but we should leave the issue open to tackle this a little deeper even if it does
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

No branches or pull requests

1 participant