JustinVoitel / svelte-hero-icons

Heroicons for SvelteKit (Project based on heroicons)
MIT License
118 stars 7 forks source link

Not working in vite.js #2

Closed niklasgrewe closed 3 years ago

niklasgrewe commented 3 years ago

Hi, thanks for this library.

At the moment i play around with the new sveltekit and vite.js. When i try to use an Icon lik this:

<script>
    import { ArrowUp } from 'svelte-hero-icons';
</script>

<ArrowUp />

i get this error from vite:

[vite] Error when evaluating SSR module src/routes/login.svelte:
/path/to/project/node_modules/.pnpm/svelte-hero-icons@1.5.0/node_modules/svelte-hero-icons/src/index.js:1
export { default as AcademicCap } from './icons/AcademicCap.svelte'
^^^^^^

SyntaxError: Unexpected token 'export'

Can this error be fixed or is the error related to sveltekit or vite.js?

JustinVoitel commented 3 years ago

I haven't tested it in Vite yet but I had similar problems with kit(snowpack). I could fix it by lazy loading them in onMount:

<script>
let phone
let mail

onMount(async () => {
  const { Phone, Mail } = await import('svelte-hero-icons')
  phone = Phone
  mail = Mail
})
</script>

<svelte:component this={phone} solid />

I know pretty unsvelty but until I come up with a solution this is how you can use them :)

niklasgrewe commented 3 years ago

@JustinVoitel thanks for quick reply. Your temporaliy solution also works in Vite 👍 yeah pretty unsvelty, but it works :) glad to hear you are looking for a solution though. Feel free to let me know if there is any news here...

JustinVoitel commented 3 years ago

I finally had some time to figure out what's going on with this issue. Vite apparently has difficulties when you import svelte files into your project. I could solve this issue by adding this to vite.config.js (tested with @sveltejs/kit@next but vanilla vite should work aswell) in 1.6.0-next.4:

//vite.config.js
export default {
  ssr: {
      noExternal: ["svelte-hero-icons"],
  },
  ...
}

Then you can do this again:

<script>
    import { ArrowUp } from 'svelte-hero-icons';
</script>

<ArrowUp />

Unfortunately this approach ends up in a slow initial start because vite now loads every of the 200+ icons https://user-images.githubusercontent.com/30449535/110818338-73384800-828d-11eb-8a24-b7b9a1a65781.mov

What do you think of that approach

<script>
    import { Icon } from 'svelte-hero-icons';
    import { ArrowUp } from 'svelte-hero-icons/icons'; // import json data of icon
</script>

<Icon src={ArrowUp} solid />

This will still import all icons but only from one file (e.g icons.json), so there is no import waterfall! Tell me what you think

niklasgrewe commented 3 years ago

@JustinVoitel hi, thanks for your update. I think your approach might be the best solution right now, because as you mentioned, we don't have any import waterfall. Unfortunately, I can't think of a better solution at the moment either. I have previously worked with SVG Sprite. But the svg sprite version is already 95 KB big.

one thing bothers me a bit though.... and that is that we need two import statements: one for the icon component and one for the icons. Is there any way to combine them or would it also be possible to work with a dynamic import in the icon component like this? (so you only need to import the Icon)

<Icon src="ArrowUp" solid /> // the ArrowUp Icon would be dynamic imported from the icon component

Otherwise I would be happy if you could implement it that way 👍

And one more thing: I work a lot with TailwindCSS. Would it be possible that you could give the Icon Component a class prop, so i can use tailwind classes for the size like:

<Icon src={ArrowUp} size="h-5 w-5" solid />
JustinVoitel commented 3 years ago

I like your proposal and I'll see what I can do to make it optimal in every possible way :)

As for tailwind I also love using it on my projects (you should try out windicss which has a much nicer and lighter integration with svelte) and you can already use:

<ArrowUp class="h-5 w-5 text-blue-500" solid /> 

The size property is really just convenience to use e.g 100 (gets converted to px) | 100px | 100% | 100rem Will close this issue since its kinda working in vite, but will open up a new one for the rework

JustinVoitel commented 3 years ago

3