tailwindlabs / heroicons

A set of free MIT-licensed high-quality SVG icons for UI development.
https://heroicons.com
MIT License
21.22k stars 1.27k forks source link

Tree shaking and ESM Support not useful #309

Closed rhysemmerson closed 2 years ago

rhysemmerson commented 3 years ago

I've been trying to get tree shaking to work with heroicons in our app using webpack.

When importing I would expect unused icons to be tree shaken, but in order to do so the esm version has to be explicitly imported.

import { MenuIcon } from '@heroicons/react/solid'; // no tree shaking
import { MenuIcon } from '@heroicons/react/solid/esm'; // tree shaking works

This is not expected as other libraries manage to make this work. This is a real issue in that the runtime/bundler can't decide for itself which loader to use. In our app we have a browser bundle, a SSR bundle and we run our code via jest, all these environments need to dynamically choose which loader to use and they can't do that if we have to hard code one in the code.

After some research I believe the package.json needs a main and module key for the bundler to be able to select which build to use https://github.com/rollup/rollup/wiki/pkg.module

I don't think it's that easy though as you can't specify multiple entrypoints and heroicons has 2, solid/esm/index.js and outline/esm/index.js

In the meantime we have to import specific icons to avoid loading the entire library. This isn't ideal as it's hard to enforce and IDEs will often "help" by collapsing them.

import MenuIcon from '@heroicons/react/solid/MenuIcon';
vvo commented 3 years ago

Also experimenting this, here's my ANALYZED bundle from Next.js (https://www.npmjs.com/package/@next/bundle-analyzer).

image

vvo commented 3 years ago

On second thought, while the Next bundle analyzer shows heroicons is using a lot of space. When running next build there's no difference between the two versions (with or without full import path).

πŸ€·β€β™€οΈ

benatshippabo commented 2 years ago

Any thoughts on this @adamwathan?

adamwathan commented 2 years ago

Unfortunately just not a priority right now, happy to accept a PR otherwise will get to it when we get to it. Sorry 😞

jasonthorsness commented 2 years ago

I'm glad I found this issue! I have a blog based on https://github.com/timlrx/tailwind-nextjs-starter-blog - turns out the hero icons were not getting tree shaken correctly in MDX bundling, and I was at 500 KB data size - adding /esm to all imports of hero icons in *.mdx files reduced that to ~50KB, factor of 10 decrease.

patcon commented 2 years ago

This worked for me!

import { QuestionMarkCircleIcon } from "@heroicons/react/solid/esm"; // tree shaking DIDN'T work
import QuestionMarkCircleIcon from "@heroicons/react/solid/esm/QuestionMarkCircleIcon"; // tree shaking works

EDIT: Seems that while bundle-analyzer showed it visually as being a savings, the size wasn't actually any different :/

HarshaVardhanNakkina commented 2 years ago

@adamwathan is this related to package.json file in esm folders? the contents of the package.json file in esm folder after running npm run build-react are like this

{
  "type": "module",
  "sideEffects": false
}

but for tree-shaking to work, I guess, there should be an entry called "module": "<path-to-entry-file>" if this is the problem, I would like to contribute 😊.

bradlc commented 2 years ago

Hey @rhysemmerson. Sorry that you aren't seeing tree-shaking in your project. I'm going to close this issue because tree-shaking is working correctly with webpack in my tests, and without a reproduction it will be impossible to debug. You can see an example project with tree-shaking working fine here: https://github.com/bradlc/heroicons-webpack

If you're still having issues please open a new issue with a reproduction repository, where you already have everything all set up so we can clone it and immediately see the issue without configuring everything by hand.