-
-
Notifications
You must be signed in to change notification settings - Fork 145
feat: support svelte snippets #400
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
base: main
Are you sure you want to change the base?
Conversation
|
I'll check it tmr, I think the d.ts changes will break existing types |
Thank you 🧡 If the option to import within module will work for you, let's do it this way. I will be able to use my d.ts inside my project anyway and not use it from the library, but the main thing is not to break anything🥺 |
<Icon color='red'/>
UPD: It's okay ✅ |
As I think it is possible to remove And possibly interfering with productivity: |
Can you open an issue about the props issue? Do you have the types in your app.d.ts? https://github.com/unplugin/unplugin-icons/blob/main/examples/sveltekit/src/app.d.ts#L1 In the pr we test it and was working in VSCode and Jetbrains IDE (I tested IntelliJ and WebStorm). |
/cc @dominikg @benmccann |
It is safe since the code will be replaced at build time, the content is just the compiled svg that comes from you local node_modules, otherwise Sveltekit doesn't had that directive. |
Oh, thank you 🙏 I will move the import to module in this PR |
This should be configurable, no idea yet about the perfomance when using script context with the snippet per icon. You are not adding support, you are replacing the current implementation with the snippet. |
If the snippet works better, we can just change the pr title to |
We can render the component itself as before, without |
I think |
It turned out that I suggest to switch completely to snippet to avoid duplicating svg code inside the component. Also, I did a study that confirms that snippets will not affect performance REPL: link Comparison with |
how would that work? The code emitted by the generated svelte5 icon components is pretty minimal already and compresses well. And even if snippet mode performed slightly better, it looks much worse when adding icons <script>
import MyIcon from '...'
</script>
<MyIcon width={24}/> with snippet: <script>
import {snippet as MyIconSnippet} from '...'
</script>
{@render MyIconSnippet({width: 24}) to avoid breaking changes for users, a separate "svelte-snippet" compiler could be added, but currently i still don't follow what kind of benefit it would bring. Depending on the app (amount of different icons, amount of same icon used in different places) it is possible that it has a benefit and i like the exploration here but my gut feeling right now is that snippet isn't the right tool here, at least not if its exposed to the enduser. |
Yeah, I've been thinking about a separate compiler too and am willing to make one Snippet helps me to write components where I need to use "slots" For example: <Button startIcon={isonSnippet}>button<Button/> In general, when the whole code is built on Snippets, it doesn't make much sense to use regular components for “slots”. Of course, you can do it like this, but it doesn't make much sense to me because of the amount of additional code: <Button>
{#snippet startIcon}
<Icon/>
{/startIcon}
button
<Button/> |
Do I need to create additional examples and complete the documentation with svelte-snippet? |
The way you use unplugin-icons is not an issue with this repository, we can add a new compiler to allow use it and you'll be happy ;) , but current compilers are fine and this PR shouldn't break current Svelte compilers and the way consumers use the icons. We can also add mix support for both in the same compiler (runes should be the default) adding a new mode (will require some change in the core), this way consumers will be able to use both runes and snippet compilers. I'm very busy, I'll try to send a PR based on this PR later if I have time, in the meantime you can give it a try. |
New, updated, and removed dependencies detected. Learn more about Socket for GitHub ↗︎
|
Add the new types in the package.json exports just below svelte5 here: https://github.com/unplugin/unplugin-icons/blob/main/package.json#L115-L117 Update also the readme file |
I've added 2 new expample Made separate types for svelte-snippet so that those using the regular svelte compiler don't have unnecessary types Updated Build Tools in the readme: Updated Frameworks in readme |
snippet cannot be exported by default because the svlete 5 compiler does not allow it ( |
Missing render example. Remember that with this solution you can only use runes or snippet compiler, you cannot use both at the same time. |
Do I understand correctly that you mean Frameworks > Svelte? Unfortunately svelte doesn't allow you to export snippets by default to use only them 😢 Right now the example looks like this: imort Icon, {snippet as icon} from '~icons/collection/name' |
in svelte5 you can pass in components as props and render them, so still not sure why you want/need a snippet from unplugin-icons itself dynamic components can also be typed https://svelte.dev/docs/svelte/typescript#The-Component-type so you should be able to restrict the type to an unplugin icon too |
i would recommend you join the svelte discord and create a thread in |
But what if my Or is this the right way to do it? <Button>
{#snippet startIcon}
<Icon/>
{/startIcon}
button
<Button/> But then I don't understand how to pass the icon from the config. const item:ButtonProps {
icon: iconSnippet
}
const menuConfig {
item
}
<Button {...item }/> |
Hi 🧡
I was using Svelte 5 and I had a need to insert icons as snippet. 🚀
Description
I solved this by adding a snippet to the component generation and exporting the snippet from it
The old API is not broken, but now can be used additionally:
I think this is a very nice improvement for those using svelte 5 with runes 🧡
I also updated the
d.ts
file. For reasons I don't understand, Snippet doesn't want to accept props fromSvelteHTMLElements
if it is imported inside a declare module. Because of this, I took it out of it 🥲Linked Issues
I hope this helps a lot of people 😍
Additional context
Yes, please validate the
d.ts
file of svelte 5.This is the only option that works for me 🧡