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

Pass attributes to Svelte components #5

Closed
vabatta opened this issue May 31, 2021 · 9 comments
Closed

Pass attributes to Svelte components #5

vabatta opened this issue May 31, 2021 · 9 comments
Labels
enhancement New feature or request

Comments

@vabatta
Copy link

vabatta commented May 31, 2021

Hi,

I was wondering if it would be possible to pass attributes to the SVG when loaded as Svelte components.
For example:

<script>
import FacebookLogo from 'line-awesome/svg/facebook-f.svg';
</script>

<FacebookLogo class="my-class" />
@poppa
Copy link
Owner

poppa commented Jun 2, 2021

I will take a look at it in the coming days 👍

@poppa poppa added the enhancement New feature or request label Jun 2, 2021
@poppa
Copy link
Owner

poppa commented Jun 9, 2021

@vabatta I don't know if this is such a good idea. I think it will be difficult to manage this in an efficient way since you need to parse all attributes in the SVG to merge the values if the attribute passed from the Svelte component already exists in the SVG node. Take your class attribute for example: It's not unthinkable that an SVG already has a class attribute.

What I usually do is wrapping the SVG so I can style it from the parent node:

<div class="svg-wrapper">
  <MySvg />
</div>

<style>
.svg-wrapper {
  :global(svg) {
    // svg style
  }
}
</style>

Let me know if you have a good argument for forwarding attributes to the SVG and I will reconsider 👍

@vabatta
Copy link
Author

vabatta commented Jun 9, 2021

I would leave the job of making sure class and attributes don't clash to the developer, honestly. For example, you as a developer might know that the svg being used in a certain context have a fill attribute which makes useless a class that changes the color (unless fill="currentColor"). Also, when it comes to class merging is more about the stylesheet generation order that makes the classes get applied to the SVG; which again is the dev job to make sure they are the right order for what you need (I am thinking of Tailwind that does the job for you already :) ).
Finally, for merging prop classes and SVG ones, I would use a tiny runtime library (228B!) to not have other implementation laying around.

With this said, I would definitely add the possibility and then the dev needs to make sure he's not breaking things :)

@aradalvand
Copy link
Contributor

aradalvand commented Nov 28, 2021

Hi @poppa, I was just palying around with this plugin and I expected this to have already been implemented honestly.

For instance, vite-plugin-svelte-svg and rollup-plugin-svelte-svg have this feature.

It's very straightforward to implement, all you have to do is add a {...props} to the end of the opening tag before passing the string to the Svelte compiler.

Here's how "vite-plugin-svelte-svg" does it: see
Here's how "rollup-plugin-svelte-svg" does it: see

As simple as it gets.

@aradalvand
Copy link
Contributor

aradalvand commented Nov 28, 2021

since you need to parse all attributes in the SVG to merge the values if the attribute passed from the Svelte component already exists

You don't, just put {...props} after all the attributes and the user-defined attributes will take precedence, Svelte already takes care of stuff like that.

@poppa
Copy link
Owner

poppa commented Nov 28, 2021

Hi @AradAral. You are more than welcome to create a PR 👍

@Tropix126
Copy link

since you need to parse all attributes in the SVG to merge the values if the attribute passed from the Svelte component already exists

You don't, just put {...props} after all the attributes and the user-defined attributes will take precedence, Svelte already takes care of stuff like that.

{...$$restProps}* in this case

@aradalvand
Copy link
Contributor

aradalvand commented Dec 1, 2021

@Tropix126 Well, you use $$restProps when your component itself has props, in which case ‍$$restProps‍ would contain the "rest of the props" passed to the component, as the name implies. In this case, where the component doesn't have any props, both $$props and $$restProps would be the same.

@poppa poppa closed this as completed in f0fa2e1 Dec 1, 2021
@aradalvand
Copy link
Contributor

aradalvand commented Dec 1, 2021

Thank you @poppa for implementing this! 💙

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

4 participants