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

Multi Import #299

Open
kthornbloom opened this issue Apr 13, 2024 · 6 comments
Open

Multi Import #299

kthornbloom opened this issue Apr 13, 2024 · 6 comments

Comments

@kthornbloom
Copy link

Hi! Thanks for making this.

I might be missing something, but when I need 10+ icons, the imports get pretty redonkulous. Your readme talks about aliasing things in the webpack config, but could it possibly work like other icon libraries where you can import multiple components at once from a folder?

i.e.
import { BeakerIcon, Cog6ToothIcon, HamburgerIcon } from '@heroicons/vue/24/solid';

@robcresswell
Copy link
Owner

Hey @kthornbloom! I'll be honest, I don't know; I haven't used Vue or Webpack for some years now. If there's a way to change the build script to output them with better imports I'd be happy to make the change. Way back when I first wrote this, tree-shaking was much more naive and bundling things into single files to export tended to cause build bloat, which is why I went the single file route. Fewer foot-guns :)

@andreiculda
Copy link

andreiculda commented Apr 29, 2024

Recommended VueJs usage has changed in the material-design-icons (pictogrmmers)

You can then build your own (code not tested, just an example).

Something in the lines of:

<!-- MyIconComponent -->
<template>
   <SvgIcon 
        type="mdi" 
        :path
        :size 
    />
</template>
<script setup lang="ts">
import SvgIcon from '@jamescoyle/vue-icon'
import {  mdiAndroid, mdiAccount, mdiFullScreen } from '@mdi/js'

const icons = { mdiAndroid, mdiAccount, mdiFullScreen }

const props = defineProps<{
    icon: string,
    size: number,
}>()

// returns the icon name with first letter uppercased
const normalizedIconName = computed(() => `${props.icon.charAt(0).toUpperCase()}${props.icon.slice(1)}`)
const path = computed(() => icons[`mdi${normalizedIconName}`])
</script>

Then you could probably use it like this:

<MyIconComponent icon="android" :size="18">

@robcresswell
Copy link
Owner

@andreiculda Thats exactly the type of pattern that should be avoided with tree shaking, from what I remember. Because of the dynamic name, you're going to end up with one "mega import" that cannot be split apart. In your example, as soon as a page uses MyIconComponent it will import every single icon and hugely bloat your bundles. This is why singular imports are nice; a little verbose, but much easier for build tools to optimise. It also becomes much easier for devs to see when an icon isn't used, because your editor will flag the import as unused (you could even lint against it)

My knowledge may be outdated though; I haven't done any serious frontend work for several years now.

@robcresswell
Copy link
Owner

Also, this is not to discourage people from using other icon libraries, please do! I sure there are much better implementations than what I've made 😅

@andreiculda
Copy link

@andreiculda Thats exactly the type of pattern that should be avoided with tree shaking, from what I remember. Because of the dynamic name, you're going to end up with one "mega import" that cannot be split apart. In your example, as soon as a page uses MyIconComponent it will import every single icon and hugely bloat your bundles. This is why singular imports are nice; a little verbose, but much easier for build tools to optimise. It also becomes much easier for devs to see when an icon isn't used, because your editor will flag the import as unused (you could even lint against it)

My knowledge may be outdated though; I haven't done any serious frontend work for several years now.

I did add a disclaimer that the code is not tested and should be treated as such, but if you look closely at my example the icons are imported as tree shaked.

@robcresswell
Copy link
Owner

@andreiculda

I did add a disclaimer that the code is not tested and should be treated as such, but if you look closely at my example the icons are imported as tree shaked.

I'm not sure what you mean by imported as tree-shaked; I don't know if tools can detect how they're being used dynamically like that and drop them from the bundles, because they won't know how the call sites are executing the icons (I think). Perhaps they're smart enough now though.

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

3 participants