Skip to content

Support Tree Shaking / Modularized Imports (React) #359

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

Closed
mgreenw opened this issue Nov 4, 2022 · 41 comments · Fixed by #378 or #468
Closed

Support Tree Shaking / Modularized Imports (React) #359

mgreenw opened this issue Nov 4, 2022 · 41 comments · Fixed by #378 or #468

Comments

@mgreenw
Copy link

mgreenw commented Nov 4, 2022

Currently, the React build produces a single index.cjs.js / index.esm.js file including all icons. This is fine for some use cases but can cause bundle bloat / dev server slowness. It would be awesome if each icon was additionally put in its own file so they can be tree shaken effectively by a bundler. This would also allow something like Next.js to use the modularizeImports feature to not bundle every single icon in both the development and production build.

Example:

"@tabler/icons": {
  "transform": "@tabler/icons/{{member}}"
}
@AlonMiz
Copy link

AlonMiz commented Nov 14, 2022

That's pretty important

@weimin1992
Copy link

I hope this feature too! This package's size shows below:
image

@m1911star
Copy link

Looking forward to seeing progress too

@codecalm
Copy link
Member

codecalm commented Nov 22, 2022

I'm working on it here: #378 :)

Screenshot 2022-11-22 at 13 18 04

@mgreenw
Copy link
Author

mgreenw commented Nov 22, 2022

You're awesome @codecalm!

codecalm added a commit that referenced this issue Jan 5, 2023
…s-preact`, `@tabler/icons-svelte`, `@tabler/icons-vue`, `@tabler/icons-solidjs`, `@tabler/icons-png`, `@tabler/icons-pdf`, `@tabler/icons-eps` (#378)

resolves #359
@markitosgv
Copy link

thanks @codecalm , what's the best option now using v2 on next for decrease bundle size?

@anthonyalayo
Copy link

Much needed, thank you!

@anthonyalayo
Copy link

anthonyalayo commented Jan 10, 2023

@codecalm after testing out v2, it looks like typescript has issues with it? I found an associated issue here:
#424

Would it be possible to throw the fix into the v2 branch for testing?

Update: this works, no problems here

@markitosgv
Copy link

Is it possible to use with modularize imports from next? I'm using 2.0.0-beta from react-icons but it seems I cannot remove Icon prefix from member

import { Icon24Hours } from '@tabler/icons-react'

This is the route for Icon:

'@tabler/icons-react/dist/esm/icons/24-hours'

Is it possible to transform? I cannot remove (Icon)24-hours



        '@tabler/icons-react': {
            transform:
                '@tabler/icons-react/dist/esm/icons/{{ kebabCase member }}',
        },

Thanks!

@codecalm
Copy link
Member

@markitosgv check @tabler/icons-react package :)

@anthonyalayo
Copy link

anthonyalayo commented Jan 24, 2023

Thank you for the hard work on that huge release @codecalm !

Unfortunately, it didn't work. I took a look at the package release, and I can see why.

The naming is not consistent between the exported value and the file itself:

export { default as IconMicrophone } from './icons/microphone.js';

And so a configuration like this will just point to a non-existent file:

  modularizeImports: {
    '@tabler/icons-react': {
      transform: '@tabler/icons-react/dist/esm/icons/{{member}}',
    },
  },

@mui/material-icons made the names consistent, and that's why it worked over there. I don't yet know if there is more complicated regex I can do here (like remove the prefix Icon and lowercase). Can anyone else chime in?

I noticed that @markitosgv already warned us about this from the beta release last week. I can confirm that the prefix is the issue here, I can't remove it, and so I can't modularize imports.

@codecalm
Copy link
Member

so, if I ranamed icons from microphone.js to Microphone.js will be okay?

@anthonyalayo
Copy link

@codecalm we can get around the casing with {{ kebabCase member }} but we can't get around the prefix Icon unfortunately

@codecalm
Copy link
Member

so it should be IconMicrophone.js?

@anthonyalayo
Copy link

@codecalm exactly, same as the import. But before you put any work in, I think I found a workaround, they do support regex:
https://nextjs.org/docs/advanced-features/compiler#handlebars-variables-and-helper-functions

Let me attempt that right now and follow up

@codecalm
Copy link
Member

yeah, I read this article and regex section and I think that will be problem with IconName convertion to icon-name

@anthonyalayo
Copy link

@codecalm agreed, I gave it my best shot but unfortunately I think there's nothing we can do without a rename.

  • I looked into the regex side, but it only applies to the import itself and not the members.
  • I looked into other helpers, and I can't find anything like substring that could help make the naming line up
  • Did a quick grep inside next.js, didn't see anything there either

@anthonyalayo
Copy link

Also just to clarify for this ticket, the issue isn't tree shaking, but of the import tree. Webpack needs to traverse the entire import tree regardless of tree shaking (which happens during minification). This slows down the build process whether you are performing a dev or prod build, but it creates noticeable performance problems in dev when they all get loaded in memory.

I filled a ticket on @mui for the same problem: mui/material-ui#35840

@markitosgv
Copy link

so it should be IconMicrophone.js?

Yes it could be icon-xxx or IconXxx but we need Icon prefix for nextjs transformation work . And I think is more consistent naming

Thanks

@anthonyalayo
Copy link

@codecalm would this be considered a 2.1.X, or a 2.0.X?

@codecalm
Copy link
Member

@anthonyalayo 2.0.1

@codecalm
Copy link
Member

@anthonyalayo I've opened PR: https://github.com/tabler/tabler-icons/pull/468/files

what do you think? do I need add something to package.json?

@markitosgv
Copy link

@anthonyalayo I've opened PR: https://github.com/tabler/tabler-icons/pull/468/files

what do you think? do I need add something to package.json?

I think it's ok! new names for files will be IconHome.js ?

@codecalm
Copy link
Member

yes, exactly

@anthonyalayo
Copy link

@codecalm i'll try it out right now locally

@anthonyalayo
Copy link

@codecalm i'm struggling to get the build going with ruby (osx compile issues). If the outputted files are the same name as the variables, we're good to go

@codecalm
Copy link
Member

I've compiled it and everything looks good :)

@anthonyalayo
Copy link

@codecalm i'm ready to test whenever the new version gets released on npm

@codecalm
Copy link
Member

I'm testing new build and I'll release it as soon as possible.

btw, new version will have number 2.1.0, because there is a lot of improvemnts and new icons

@anthonyalayo
Copy link

Added it to a random project I was working on.

Before:

> next dev

ready - started server on 0.0.0.0:3000, url: http://localhost:3000
event - compiled client and server successfully in 7.2s (5205 modules)
wait  - compiling /samplers/auction (client and server)...
event - compiled client and server successfully in 731 ms (5208 modules)

Adding these to my next.config.js:

  modularizeImports: {
    '@tabler/icons-react': {
      transform: '@tabler/icons-react/dist/esm/icons/{{member}}',
    },
  },
  transpilePackages: ['@tabler/icons-react'],

After:

> next dev

ready - started server on 0.0.0.0:3000, url: http://localhost:3000
event - compiled client and server successfully in 6.5s (2007 modules)
wait  - compiling /samplers/auction (client and server)...
event - compiled client and server successfully in 192 ms (2010 modules)

Looks fantastic, thank you @codecalm ! I believe I had to add the project to transpilePackages as well since Next.js server code doesn't support ESM yet.

@anthonyalayo
Copy link

@codecalm, I was looking into the necessity of transpilePackages: ['@tabler/icons-react'] by comparing it with @mui/icons-material which doesn't require it.

The magic is that @mui/icons-material is not bundling their CJS (require) code. Is there a reason @tabler/icons-react CJS is bundled? If the expected consumer of that flavor is other UI frameworks, then not bundling it would actually be better.

image

image

@markitosgv
Copy link

markitosgv commented Jan 26, 2023

To me @anthonyalayo it works without transpilePackages option (only put on modularizeimports), using nextjs 13.1.3. Doing a webpack analyzer in dev environment seems that only a few icons that I use are loading.

@anthonyalayo
Copy link

@markitosgv its definitely working with only loading a few icons, but the no transpile part sounds odd. Are you running with type="module"?

@anthonyalayo
Copy link

I'm not sure what tricks @markitosgv did, but it's definitely not working without transpilePackages. I just cloned a fresh Next.js project and it fails in the same way:

image

I'll file a follow up ticket @codecalm since it's definitely considered another feature.

@markitosgv
Copy link

I'm using a monorepo with turborepo, I don't know if it's something with that

@anthonyalayo
Copy link

@markitosgv most likely something there is performing the same process, you can reproduce it with a vanilla app like I did above.

@markitosgv
Copy link

markitosgv commented Jan 27, 2023

@anthonyalayo I've got a UI package with tabler icons dependency too, and for use this UI package in my project inside a monorepo I'm using transpilePackages option in next.config so I think is the reason that works for me

@anthonyalayo
Copy link

@markitosgv that confirms it. Thanks!

@codecalm
Copy link
Member

@markitosgv @anthonyalayo will there be any more changes needed in the tabler icons code to improve the process?

@anthonyalayo
Copy link

@codecalm yup, that follow up issue is the last one:
#471

@matthewoates
Copy link

Is it possible to use with modularize imports from next? I'm using 2.0.0-beta from react-icons but it seems I cannot remove Icon prefix from member

import { Icon24Hours } from '@tabler/icons-react'

This is the route for Icon:

'@tabler/icons-react/dist/esm/icons/24-hours'

Is it possible to transform? I cannot remove (Icon)24-hours



        '@tabler/icons-react': {
            transform:
                '@tabler/icons-react/dist/esm/icons/{{ kebabCase member }}',
        },

Thanks!

I don't think you want to use kebab-case here. This worked for me:

'@tabler/icons-react': {
    transform: '@tabler/icons-react/dist/esm/icons/{{ camelCase member }}'
},

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment