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

Passing class to components #2870

Closed
wysher opened this issue May 26, 2019 · 132 comments
Closed

Passing class to components #2870

wysher opened this issue May 26, 2019 · 132 comments
Labels
awaiting submitter needs a reproduction, or clarification

Comments

@wysher
Copy link

wysher commented May 26, 2019

I'm not sure if this isn't related to other issues, but I checked many of them and neither covers what I need.

I come from React and CSS-Modules, where passing down styles is really easy. Just pass down className prop, and css-modules will figure out and hash styles to avoid conflicts.

In svelte I cannot pass down classes to component through class prop, like so:
https://svelte.dev/repl/1d0b80898137445da24cf565830ce3f7?version=3.4.2

There are some solutions to pass styles, like using :global, but it's fragile: if anywhere inside child component will have class named same, it will cause problems. Sure, we can use BEM or another approach, be very careful with global styles, but I imagine it can happen automagically (like with CSS-Modules).

Another approach is to add add divs inside componetns, but consider Col component from example above. If we have:

<style>
    div {
        background: tomato;
    }
</style>

<Col>
    <div>content</div>
</Col>

content will get background, but also will have unwanted gap caused by Col padding. And quickly code will have many unwanted divs.

Why I think it matters?
I wrote hundreds of Rect components and what I learned is that Componets should be able to be styled by developer who is using it. Adding BEM layer or sth else is for me unnecessary work. And may cause conflicts. Imagine dev who use BEM. There may be rare cases when his (global) styles are named same as component's and interfere.

in conclusion
It will be great to pass down classes, not be global, but with some unique id.

PS. I found modular-css package, but it's for svelte2, and I'm not sure if it will works with passing down styles to components.

@tivac
Copy link
Contributor

tivac commented May 26, 2019

Modular CSS works fine with svelte3, we already use it in daily development.

@wysher
Copy link
Author

wysher commented May 26, 2019

@tivac probably not the best place for this, but i tried to setup modular-css with rollup in sapper. Readme seems confusing. I tried few different approach, but without success. Do you have some example config with rollup?

@tivac
Copy link
Contributor

tivac commented May 26, 2019

I'll see if I can put something together for you, I haven't looked at sapper in a while but I don't know of a reason why it wouldn't work.

@nikku
Copy link
Contributor

nikku commented May 26, 2019

I think the important bit to note is that svelte does perform CSS optimizations, removing any unused class declarations.

Most simple example:

<script>
  import ChildComponent from './Child.svelte';
</script>

<style>
  .class-to-add {
    background-color: tomato;
  }
</style>

<ChildComponent class="class-to-add" />

...compiles to CSS without the class-to-add declaration, as svelte currently does not recognize the class name as being used.

I'd expect

  • class-to-add is bundled with all nested style declarations
  • class-to-add is passed to ChildComponent as class-to-add svelte-HASH

This looks like a bug / missing feature to me. Not sure how modular-css would help me here.

@wysher
Copy link
Author

wysher commented May 27, 2019

Nikku got the point with simpler example.

Although modular-css is some solution, I'd rather expect passing hashed classes down as a built in feature. Why? I assume that soon developers will publish more components for svelte, and any possible class conflict is problematic. Yeah, it's rare case, but it is.

Also Svelte is so great because developer do not need to worry about class names conflict, except of passing (global) classes to component (sic!).

Some React components uses Css-modules to avoid theese conflicts, but in my opinion it's an overkill for simple (and hopefully small) published libs/components.

nikku added a commit to nikku/svelte that referenced this issue May 27, 2019
nikku added a commit to nikku/svelte that referenced this issue May 27, 2019
@nikku
Copy link
Contributor

nikku commented May 27, 2019

This should get addressed via #2888.

nikku added a commit to nikku/svelte that referenced this issue May 27, 2019
nikku added a commit to nikku/svelte that referenced this issue May 28, 2019
nikku added a commit to nikku/svelte that referenced this issue May 28, 2019
nikku added a commit to nikku/svelte that referenced this issue May 28, 2019
@nikku
Copy link
Contributor

nikku commented May 28, 2019

Classes on components are currently conciously not supported.

I wonder why is that?

@Conduitry
Copy link
Member

There's no guaranteed single root element in a component - and the markup in a component should be up to that component anyway, not its parent.

@nikku
Copy link
Contributor

nikku commented May 30, 2019

There's no guaranteed single root element in a component

Could you provide an example / markup to show where that is a problem?

and the markup in a component should be up to that component anyway, not its parent.

Agreed. And what about if you'd like to customize, lets say the margin of a component, dependent on where it is in the parent? If we look into how customizations in in component frameworks works, passing classes is exactly that.

@tcrowe
Copy link
Contributor

tcrowe commented Jun 3, 2019

I was curious about this too so I've provided a work-around in case you want to try it.

./App.svelte

<Anchor hash="#/products" one=true>Products</Anchor>
<Anchor hash="#/services" one=true>Services</Anchor>

./Anchor.svelte

<script>
export let hash = "";
export let one = false;
</script>

<--                     v---here 'one' is used as a conditional class -->
<a href={hash}  class:one={one}>
  <slot/>
</a>

@nikku
Copy link
Contributor

nikku commented Jun 3, 2019

Unfortunately this is not a workaround for the general case of overriding component look and feel provided by UI frameworks.

@tcrowe
Copy link
Contributor

tcrowe commented Jun 3, 2019

@nikku Yeah, it's not ideal.

There's another one I just tried. Let's say we are passing in Bootstrap styles.

./App.svelte

<Anchor hash="#/products" klass="nav-item nav-link">Products</Anchor>
<Anchor hash="#/services" klass="nav-item nav-link">Services</Anchor>

./Anchor.svelte

<script>
export let hash = "";
export let klass = "";
</script>

<a href={hash}  class={klass}><slot/></a>

@tcrowe
Copy link
Contributor

tcrowe commented Jun 3, 2019

Using this trick from the docs: https://svelte.dev/docs#script

Then export { klass as class }; (this breaks my syntax highlighting but the code works)

@wysher
Copy link
Author

wysher commented Jun 3, 2019

you can do both, but if you do you also need do :global(.foo) in parent component, because .foo is scoped to parent. This has some constraints, e.g you need to be careful with naming, to avoid clashes.

Ideally svelte will provide some mechanism for passing down class (or object of classes) which can be used in child component markup

@nikku
Copy link
Contributor

nikku commented Jun 3, 2019

That is an easy one too. However, what if you'd like to define your own overrides in your parent component? And pass these to the child component? These will be happily removed aka optimized away by Svelte at the moment.

My workaround is to use your second approach + declare the styles defined in the parent component as :global(.override-style). Ugly but works.

@adrian-marcelo-gallardo

One approach that works is by doing:
<button class="some-class {$$props.class}">

I would love to be able to create some custom directive like use:parentClass where I can access $$props and the node element and abstract that logic there, but I think that is not possible.

@nikku
Copy link
Contributor

nikku commented Jul 15, 2019

One approach that works is by doing:
<button class="some-class {$$props.class}">

Does this prevent classes defined in the parent scope from being optimized away? I believe not.

@adrian-marcelo-gallardo

@nikku you are completely right.. the approach that I mentioned was working for me as I'm using some global CSS library, but sadly it doesn't work for scoped styles.

I believe both things are needed:

  • A way for the optimizer to recognize class attributes applied to Components
  • A way to tell svelte where to apply those styles assigned to parent (e.g. the use:parentClass suggestion on my previous comment)

@nikku
Copy link
Contributor

nikku commented Jul 16, 2019

#2888 does exactly what you mention, in the most lean way.

@Conduitry Conduitry added awaiting submitter needs a reproduction, or clarification proposal labels Jul 29, 2019
@pngwn
Copy link
Member

pngwn commented Dec 14, 2021

@AradAral no petitions, no begging for upvotes.

@sveltejs sveltejs deleted a comment from aradalvand Dec 14, 2021
@tycobbb
Copy link

tycobbb commented Dec 15, 2021

@non25 i haven't looked at yr solution yet. i'm sure it's fine, i may even use it in the mean time. however, it's the sort of thing that the framework should implement. the bigger a tool is the more responsibility it has to be a good citizen of its ecosystem (the web, the world, &c) by default. in this case the blessed path, the one most will use, generates pretty illegible and bloated markup, trips up newcomers, deviates from the expected way the web works, &c. imo, that isn't it.

@windyclory
Copy link

windyclory commented Jan 3, 2022

Replied to

There's no guaranteed single root element in a component.

You don't need it. If a class name is added from the parent to the child component via the class prop, it just needs a random class name added to the child component's element(the element that the class prop is used in).

The markup in a component should be up to that component anyway, not its parent.

I understood wrong or is this a joke? You do change everything related to a component from its parent(where the component is imported) based on where it's being used.

@shynome
Copy link

shynome commented Feb 11, 2022

hi, today I resolve it with this comment
#2870 (comment)

here is my solution: https://gist.github.com/shynome/601c74deeab113b2063eea58ef5d73d8

@tycobbb
Copy link

tycobbb commented Feb 11, 2022

yeesh, still sad about this. perhaps i just don't vibe with or understand svelte's goals. rather than passing classes to components, jumping through hoops to find an idelogically pure solution at the expense of productivity, deviating from common web practices and violating the principle of least surprise, discarding useful prior art like bem and suit, producing unreadable markup, reaching for tight control rather than allowing for leaky abstractions. let the web be free!

@matthewsimo
Copy link

Ok, just finished reading through the 3 years of comments from this issue - quite a lot! Sad to see a lot of people here talking past each other and not really proposing things that wouldn't break the encapsulation model or fully taking into account that a component doesn't necessarily === one element.

Really a testament that the convo is still going though and a clear sign the community is active and engaged in solving practical problems. I do see the perspective of both sides but I think having an explicit component interface is probably the more conservative/safer choice so I see why things are the way they are...

Personally, I see this ultimately breaking down into two needs:

  • "I have a bunch of stuff I need the child to change and therefor want to pass a class" (which is just an open ended set of styles that can be as large as necessary). I think this is a little misguided and ultimately a bad idea b/c it is kind of a code smell that the incorrect abstraction is being used. If you're in this scenario, create an explicit prop interface for your child component.
  • "I have a tweak here, or tweak there I need so the child can render appropriately in the parent's context". I think passing custom properties goes a long way to solve this problem and makes this pretty trivial/a non-issue now. https://svelte.dev/repl/d720e3afad1042e08d7899567216ad42?version=3.48.0

I'm fairly new to the Svelte ecosystem but that's my perspective.. Is there another need that I'm not seeing?

@lukaszpolowczyk
Copy link

@matthewsimo I would need to for repetitive CSS code.
I currently copy CSS code in several places, and when I fix it I have to do it in several places....
Sometimes I use an external global style, but that's not very Svelt...

@tycobbb
Copy link

tycobbb commented Jun 17, 2022

I think this is a little misguided and ultimately a bad idea b/c it is kind of a code smell that the incorrect abstraction is being used

it's not the framework's job to make that decision, it's providing a piece of general-purpose infrastructure! it can't possibly know about what sort of abstractions are appropriate in my-or-your-or-anyone-else's applications.

it reminds me of this post about the "generic repository trap". sure, it's probably better to have a specific contract at the seam, something that maps from enum-valued data to one of a handful of css classes. but if my application's particular use-cases also demand a more generalized abstraction under the hood, that's my business! svelte provides an infrastructural abstraction and is trying to simultaneously enforce application-layer contracts without any knowledge of those applications' particular constraints.

everyone in this thread is saying "hey this doesn't suit my application's particular needs" for a reason, the framework doesn't know better.

@madeleineostoja
Copy link

"I have a bunch of stuff I need the child to change and therefor want to pass a class" (which is just an open ended set of styles that can be as large as necessary). I think this is a little misguided and ultimately a bad idea b/c it is kind of a code smell that the incorrect abstraction is being used. If you're in this scenario, create an explicit prop interface for your child component.

I also disagree with this. Yes such an interface is a potential way to introduce code smell if used sloppily, but as @tycobbb said that's not the framework's job to decide. For instance the currently most suggested workaround is using :global, which can be a far bigger footgun than consuming a scoped class.

Take something like a Button or Img component, that almost always at least need some positioning styles or similar when used. Would the better abstraction there be to allow the consumer to pass a class to it, or to expose a whole bunch of arbitrary CSS props that have ultimately nothing to do with the component, but instead the context it's used in? Especially since I imagine consuming scoped classes would be an opt-in feature (given that there is no single guaranteed root element in a svelte component), so if you wanted to have a stricter style interface you could.

I totally agree that there is a lot of talking past each other and finger pointing in these issues, but I think a better solution to that is to focus on the technical issues rather than philosophical ones. The fact this argument is still going after all this time is a testament to how important this feature is for a lot of Svelte's users, and brushing all of that off as "you're trying to use a bad pattern" is unhelpful imo.

@merchako
Copy link

merchako commented Jul 1, 2023

Take something like a Button or Img component, that almost always at least need some positioning styles or similar when used.

If someone can provide good, specific idioms

  • to position Button and Image components and
  • to apply grid-area for grid-template-areas

without explicitly passing classes or styles exported by the child component, I will follow in your ways and stop asking for a solution to this issue.

BTW: What might it look like to add syntactic sugar for components that contain one element?

@aradalvand
Copy link

aradalvand commented Jul 1, 2023

If anyone here is using SvelteKit, I've found that Vite's CSS Modules feature has partly solved this problem for me, give it a try. Although it comes with the downside that you have to have a separate .css file for every .svelte component and you can't use the <style> tag inside the component itself, but it nonetheless makes it possible to pass scoped classes down to child components, which is huge for me.

@benmccann
Copy link
Member

The way we decided to solve this for the image component case is to use a preprocessor: sveltejs/kit#10788. That solves this styling issue as well as the event handling issue.

@madeleineostoja
Copy link

@benmccann image preprocessing looks great, but I think that's a very narrow use-case, in sveltekit only no less, that doesn't solve this broader issue at all

@benmccann
Copy link
Member

I'll note that it's not just SvelteKit and any Vite-based project can use it, but I understand there are many additional use cases where you may have similar needs. I wanted to share in case others found the approach useful

@Mooncat25
Copy link

After watching the video about Svelte 5 runes, can we have something like $classProps() to tell the compiler the props are used as CSS classes?

Example:

<!-- Component.svelte -->
<script lang="ts">
  let { class: string, divClass: string } = $classProps()
</script>

<!-- App.svelte -->
<script lang="ts">
  import Component from './Component.svelte';
</script>

<Component class="myClass" divClass="myDiv"/>

<style>
  .myClass {
    color: red;
  }

  .myDiv {
    color: blue;
  }
</style>

Not an expert that can actually implement a working prototype, so I will just throw the idea here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
awaiting submitter needs a reproduction, or clarification
Projects
None yet
Development

Successfully merging a pull request may close this issue.