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

Tree shake big bundle size #180

Closed
frederikhors opened this issue Aug 24, 2020 · 19 comments
Closed

Tree shake big bundle size #180

frederikhors opened this issue Aug 24, 2020 · 19 comments
Assignees
Labels
performance Issues that impact performance.

Comments

@frederikhors
Copy link

frederikhors commented Aug 24, 2020

I'm trying the amazing Shoelace for the first time today.

I'm working on a project with Rollup.

Everything works and is amazing.

One thing I don't like though.

Just adding the following lines of code to my project increases it by 232 KB (from 747 KB to 979 KB) javascript side only.

import { SlButton } from '@shoelace-style/shoelace'

customElements.define('sl-button', SlButton)

image

If I use this instead it increases by 383 KB (from 747 KB to 1130 KB).

import { defineCustomElements } from '@shoelace-style/shoelace'

defineCustomElements()

image

As you can see from the images we have @stencil/core/internal/client/index.js of ~ 16 KB. Is it really needed?

Is it really necessary to have those 232 KB all together? Can we tree-shaking?

@frederikhors frederikhors added the feature Feature requests. label Aug 24, 2020
@claviska
Copy link
Member

I'm seeing a similar result with webpack, and I assume these are core utilities + dependencies getting pulled in. However, the project doesn't have a lot of dependencies:

  "dependencies": {
    "@popperjs/core": "^2.1.1",
    "@stencil/core": "^1.17.3",
    "color": "^3.1.2",
    "resize-observer-polyfill": "^1.5.1"
  }

The @stencil/core/internal/client/index.js dependency is necessary, as it provides boilerplate code that's shared across all components. I can't imagine the others would be loaded just by importing button — unless the bundle includes all shared utilities. I don't know enough about how bundlers work under the hood to make this determination.

If anyone can shed light on this and provide a reasonable way to optimize it, I'm all ears. In the meantime, if you want a lighter initial footprint, the lazy loader is also a good option.

@claviska claviska changed the title Tree shaked big bundle size Tree shake big bundle size Aug 26, 2020
@claviska
Copy link
Member

claviska commented Aug 30, 2020

Looks like TypeScript is starting work to include the ResizerObserver types. Technically, we don't need the polyfill since browser support for that feature matches browser support for Shoelace. It's mostly being used for types now, so that will save some bytes from the core dependencies.

I think the rest of this is coming from the color package, which is only used by the color picker (so it shouldn't be included with your code above, but maybe it is). With some work, it could be replaced with pure-color, a smaller, tree-shakeable library for color conversions.

Aside from that, I don't think there's much more to remove from the core bundle. I believe Popper is only ~7 kB and Stencil core isn't negotiable.

@EugeneDae
Copy link

EugeneDae commented Oct 22, 2020

Even this code that doesn't involve any components:

import { setAssetPath } from '@shoelace-style/shoelace';
setAssetPath(document.location.origin + '/static/shoelace/');

adds +207 KB to Webpack bundle size 🤯

Interestingly:
https://cdn.jsdelivr.net/npm/@shoelace-style/shoelace@2.0.0-beta.21/dist/shoelace/shoelace.esm.js
is only 6 KB!

I'm a complete noob when it comes to JS code compilation / optimization. I really hope someone knowledgeable about this matter can help this awesome project.

@KonnorRogers
Copy link
Collaborator

So i just found out today you can use the minified version above with Webpack. you have to add this as a Webpack loader:

https://www.npmjs.com/package/@open-wc/webpack-import-meta-loader

And then once you have the import-meta-loader, you can do:

import "@shoelace-style/shoelace/dist/shoelace/shoelace.esm"

and now you can use web components! You dont even have to define anything.

@frederikhors
Copy link
Author

and what about the size?

@KonnorRogers
Copy link
Collaborator

KonnorRogers commented Oct 30, 2020

Bundles are still not great.

With import "@shoelace-style/shoelace/dist/shoelace/shoelace.esm" im getting about 518kb parsed, 120kb gzipped.

with the recommended:

import * as shoelace from defineCustomElements()
shoelace.setAssetPath(document.currentScript.src)
shoelace.defineCustomElements()

im getting 979kb parsed, 198kb gzipped.

@brgrz
Copy link
Contributor

brgrz commented Nov 11, 2020

I'm using Shoelace with Svelte (Rollup config). The starter Svelte template is around 20kb.

I add this

import { setAssetPath, SlButton, SlDrawer } from '@shoelace-style/shoelace';
import App from './App.svelte';

and it goes to 390kb.

If I then add these two lines to make it actually work

customElements.define('sl-button', SlButton);
customElements.define('sl-drawer', SlDrawer);

it goes to 470kb.

?

@claviska claviska added help wanted Ready for a contributor to tackle. performance Issues that impact performance. and removed feature Feature requests. labels Nov 18, 2020
@claviska
Copy link
Member

claviska commented Nov 18, 2020

Analyzing the few dependencies Shoelace has:

  • @stencil/core is required, nothing we can do about that. Not sure how large it is on its own, but a "hello world" component lib is only ~1 KB so it doesn't seem to be bloating things out of the box.

  • @popperjs/core is only ~3 KB so I'm not concerned about that given how useful it is.

  • resize-observer-polyfill weighs in at around 36 KB and it's only necessary for types because of this. Removed in 81ae77f 🥳

  • color is ~11 KB — not too bad but it could probably be swapped out with pure-color so only the required functions are imported. That might save some bytes, but still not a terribly large number. I'm not sure that it's worth the effort.

So realistically the dependencies only account for a small percentage of these bytes. I wonder if there are additional optimizations that can be made through webpack (or whatever bundler you choose to use) when importing the custom elements bundle.

@claviska
Copy link
Member

claviska commented Dec 28, 2020

I don't know why I didn't think of this before, but the Animista animations that come bundled with the animation component are ~200KB. 😳

I think dropping them from the default bundle and providing them as an opt-in import will be better. There will still be ~80 animations from animate.css which seem to be more generic and common. I suspect for some reason these imports are contributing to this, so we could see a nice drop from this change.

Edit: this has been fixed. View the discussion

@danny-andrews
Copy link
Contributor

Removing the animation component is a good move, but the root of the problem is that the animation component should not have been included into bundles which do not import it. E.g., people should be able to just use sl-button without importing the code for all the other components.

Tools like rollup can't tree-shake the modules exported by this library, because they are all concatenated together dist/custom-elements/bundle/index.js. I have an idea on how to make this work, and will take a stab at it in the next few days.

@danny-andrews
Copy link
Contributor

danny-andrews commented Feb 1, 2021

Disregard my last message. 😅 Tree-shaking is working great on beta.26.

Output from rollup
image

@moos
Copy link

moos commented Feb 18, 2021

Tree shaking is not working with esbuild, an extremely fast bundler:

$ echo "import {SlBadge} from '@shoelace-style/shoelace'" | npx esbuild --loader=js --bundle --outfile=out.js
1 warning  (can be ignored!)
$ ls -lh out.js
-rwxrwxrwx 1 foo foo **367K** Feb 18 13:44 out.js

Looking at the content of out.js:

$ grep node_modules out.js
  // node_modules/@shoelace-style/shoelace/node_modules/@stencil/core/internal/app-data/index.js
  // node_modules/@shoelace-style/shoelace/node_modules/@stencil/core/internal/client/index.js
  // node_modules/@shoelace-style/shoelace/dist/custom-elements-bundle/index.js

client/index.js is about 119KB on disk and gets shaken down to ~7KB in out.js. The bulk of out.js comes from custom-elements-bundle/index.js which is 397KB on disk. The only thing that seems to be shaken out of that (other than comments) is the

const SlAlert = /* @__PURE__ */ ...
const SlAnimation = /*@__PURE__*/ ...
const SlAvatar = /*@__PURE__*/ ..
...

at the end. The rest of it is pretty much intact.

esbuild: 0.8.44 (possibly related?? esbuild/#175)
shoelace: beta.27

@moos
Copy link

moos commented Feb 18, 2021

The culprit with esbuild seems to be that shoelace components are extending HTMLElement. I've opened an issue for esbuild to tree-shake known globals. Hopefully it can be addressed soon.

As possible work-around, wrapping the class definition in a PURE function in custom-elements-bundle/index.js seems to work:

const Alert = /*@__PURE__*/ (() => class extends HTMLElement { ... })();

@claviska
Copy link
Member

I'm working on an experimental build using Shoemaker and esbuild that resolves this. The end result is a collection of ES modules that are properly tree-shaken and split into chunks. The nice thing about this is that the same bundle can be served from a browser and into webpack/Rollup/etc.

I'll post more details once I get a bit further with it.

@claviska claviska removed the help wanted Ready for a contributor to tackle. label Feb 26, 2021
@claviska
Copy link
Member

This has been resolved in beta 28 which no longer uses Stencil. The new dist contains a collection of ES modules so you can load everything, cherry pick components, or import with a bundler.

Part of the solution was adding the sideEffects property in Shoelace's package.json so bundlers will see it. This enables tree-shaking in Rollup with zero config, but webpack users need to add this flag in their config. Not sure about other bundlers.

@frederikhors
Copy link
Author

@claviska so now what's the difference in size?

@claviska
Copy link
Member

It depends entirely on what you import, whether you use all.shoelace.js, cherry pick, or bundle it. In my testing, I did notice that webpack’s bundle seemed to be slightly larger than Rollup’s, but that could be from its boilerplate. From what I could tell, it was only including what it should have with optimizations > side effects enabled.

Feel free to run additional tests and post an update for the community if you want.

@claviska
Copy link
Member

The original example before minification/gzip:

import { SlButton } from '@shoelace-style/shoelace';
SlButton.register();

// Rollup - 37.9 KB
// webpack - 109 KB

Of course, not all of this is button code. You're getting the bundler's boilerplate and shared chunks that button requires (like the renderer and Shoemaker), but these are only incurred once.

@moos
Copy link

moos commented Feb 27, 2021

$ time echo "import { SlButton } from '@shoelace-style/shoelace'; SlButton.register();" | npx esbuild --loader=js --bundle --outfile=out.js
// esbuild - 39 KB
// real time: 0.46 sec

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

No branches or pull requests

7 participants