sveltejs / svelte

Cybernetically enhanced web apps
https://svelte.dev
MIT License
76.98k stars 4k forks source link

Passing class to components #2870

Closed wysher closed 4 years ago

wysher commented 5 years ago

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 commented 5 years ago

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

wysher commented 5 years ago

@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 commented 5 years ago

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 commented 5 years ago

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

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

wysher commented 5 years ago

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 commented 5 years ago

This should get addressed via https://github.com/sveltejs/svelte/pull/2888.

nikku commented 5 years ago

Classes on components are currently conciously not supported.

I wonder why is that?

Conduitry commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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

tcrowe commented 5 years ago

@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 commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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 commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

@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:

nikku commented 4 years ago

https://github.com/sveltejs/svelte/pull/2888 does exactly what you mention, in the most lean way.

AnxiousDarkly commented 4 years ago

This is definetely something that should be considered, the same thing is trivial with css modules & react.

caschbre commented 4 years ago

I've run into this issue as well. To avoid using global styles, I've unfortunately resorted to the following hack. I basically add the desired class to an html element that is hidden.

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

<style>
  :global(.component-class-hack) {
    display: none !important;
  }

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

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

<span class="component-class-hack class-to-add"></span>
nickyhajal commented 4 years ago

Just a note that I've also been trying to figure out how to pass a class to child components in Svelte and found myself here. Seems like still no resolution?

cortopy commented 4 years ago

I would love some sort of solution for this issue too. For now, my hack is to use both {$$props.class} in the child and I what I think is a unique enough class name with :global selector in the parent's <style> like

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

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

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

and then in the child

<div {$$props.class}>
...
</div>
PatrickG commented 4 years ago

My current solution is, let all my components have just one "outer element" with a class of the component name.

<!-- ChildComponent.svelte -->
<div class="ChildComponent">...</div>
<!-- OtherComponent.svelte -->
<script>
  import ChildComponent from './ChildComponent.svelte';
</script>

<style>
  .OtherComponent :global(.ChildComponent) {
    color: red;
  }
</style>

<div class="OtherComponent">
  <ChildComponent />
</div>

This way, the style is only appied to childs of OtherComponent.

If i really need to add a class to the ChildComponent, i use this:

<!-- ChildComponent -->
<script>
  let klass = undefined;
  export { klass as class };
</script>

<div class="ChildComponent {klass}">...</div>
<!-- OtherComponent -->
<script>
  import ChildComponent from './ChildComponent.svelte';
</script>

<style>
  .OtherComponent :global(.my-class) {
    color: red;
  }
</style>

<div class="OtherComponent">
  <ChildComponent class="my-class" />
</div>

I would be happy, if this gets resolved soon. So we don't have to use the :global identifier. +1 for https://github.com/sveltejs/svelte/pull/2888

NSDrowned commented 4 years ago

I would love some sort of solution for this issue too. For now, my hack is to use both {$$props.class} in the parent and I what I think is a unique enough class name with :global selector in <style> like

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

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

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

This worked for me, but I had to use a different name for the prop or else I would get a "ParseError: Unexpected keyword 'class'" from the child component. How did you get it to work with class=""?

cortopy commented 4 years ago

@NSDrowned I've editied my comment as it had an error. The {$$props.class} directive goes in the child.

Also you can't do

<script>
export let class;
</script>

as class is a reserved word. See @PatrickG 's response to see how he brilliantly manages it

If this doesn't fix it, please post your code so that I can have a look

ghost commented 4 years ago

Another way to pass styles to a child (REPL):

In Parent:

<script>
  import Child from './Child.svelte'
  let styles = `background-color: black; color: aqua; text-align: center`
</script>

<Child {styles}/>

In Child:

<script>
  export let styles
</script>

<h1 style={styles}>Hot Salsa!</h1>

Seems pretty flexible. Any obvious limitations to this approach aside from having style strings in your scripts?

AnxiousDarkly commented 4 years ago

@dyslexicon I mean that doesn't achieve what we want, feel free to look up the differences between inline styling and class selectors.

ghost commented 4 years ago

@gibdig

feel free to look up the differences between inline styling and class selectors

Do you mean specific to how the Svelte compiler handles them, or in CSS in general? By all means I do feel free to google but if you could be a little more specific that would help 😉

that doesn't achieve what we want

That's my question and perhaps also why this issue is unresolved—what specifically does applying classes to child components achieve vs using a class on a wrapper div (in the parent) or passing inline styles as props as I demonstrated ? Cleaner syntax, what?

If the use case is...

And what about if you'd like to customize, lets say the margin of a component, dependent on where it is in the parent?

... okay, that I understand, and is in fact the exact issue that lead me here. It would be cleaner IMO to apply classes to children components than to clutter the markup with a bunch of extra wrapper divs. But there are outstanding questions about what exactly it means to apply a class to a nested component: see https://github.com/sveltejs/svelte/issues/289#issuecomment-280351738 by @Rich-Harris

And then this statement from @Conduitry ...

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.

... seems to suggest there's not a common understanding of what exactly is hoped to be achieved by applying classes to nested components.

I agree, the solutions in this thread and others (wrapper divs, global: & {$$props.class}, and inline styles as I showed) are hacks and a proper solution, perhaps https://github.com/sveltejs/svelte/pull/2888, should be considered.

But in the interim, for those who may be stuck on this issue, what styling scenario can't be achieved with some combination of the above methods? Sincere question. See REPL.

cortopy commented 4 years ago

@dyslexicon

That's my question and perhaps also why this issue is unresolved—what specifically does applying classes to child components achieve vs using a class on a wrapper div (in the parent) or passing inline styles as props as I demonstrated ? Cleaner syntax, what?

I'll just give an example of my use case.

I use functional CSS patterns (tachyons, tailwinds) that rely on having one utility class per css property. Following this, I would like to pass classes like bottom-2 or b--black-90 to a component which just renders text within a p tag

I could pass inline styles, but that would force me to stop using functional css. I could also have a div container but this is not quite the same and it would bloat the DOM with unnecessary div elements for every single p I create

ghost commented 4 years ago

In attempt to summarize and kind of codify what's been discussed thus far and to help this issue advance toward a solution, here are some common terms for what's been discussed. If I'm missing anything or if you think any of these are non-issues, please chime in and I'll update this.

Why apply classes to imported components?

Per instance styling

You only want to add a class to a particular instance of a component. For example, you have two <Button /> components inside a <Card /> component and only one <Button /> needs some left margin. This is where you might otherwise use a "wrapper" <div>:

<--! In Card.svelte -->

<--! unstyled instance -->
<Button />

<--! less than ideal styled instance -->
<div class="some-left-margin">
  <Button />
</div>

<--! something like this would be ideal -->
<Button class={some-left-margin} />

The Higher Order Component (HOC) pattern

In React, a HOC is a function that takes a component as an argument and returns a new instance of that component with some additional data or functionality. Its purpose is to act as a factory for easily creating components that share common properties. An HOC pattern is still being worked out for Svelte.

Are HOCs and the passing of scoped styles mutually compatible? Shouldn't they be?

What problems do such classes solve?

Wrapper div pollution

Wrapping a component in a <div> containing a class, for the sole reason that the component inherits the class, creates un-sematic markup in both the DOM and the source-code.

Problems that need to be solved to implement this

class name collision

If, for example, my-class is applied to a component which also contains (or has a descendant that contains) a scoped my-class and both instances of my-class share common properties, should precedent be given to the scoped my-class, or vice versa? Currently, :global(my-class) gives precedent to the scoped my-class.

n top-level elements

Say we have the component <MyComponent /> included in a Parent.svelte component and we want to apply the border-bottom-2 class to it. The <MyComponent /> implementation however is as follows:

<--! in MyComponent.svelte -->

<div>
  <--! some other elements -->...
</div>

<div>
  <AnotherComponent />
  <--! some other elements -->...
</div>

In this case, where n is 2, that is, there are two top-level elements—where does Svelte apply the border-bottom-2 class?

Svelte could, behind the scenes, wrap all top level elements in a <div> and apply the class to that, but that's a big assumption on Svelte's behalf and it brings us back to wrapper div pollution (at least in the DOM).

Another problem arises if MyComponet.svelte has scoped styling which targets an :nth-of-type selector (or something similar), the calculation could be thrown off.

ghost commented 4 years ago

@cortopy

Referencing the terms I defined above, your use case would be considered a per instance styling and you'd like to avoid wrapper div pollution .

Functional CSS patterns can be used with inline styles. Here's how:

Create, for example, a classes.js file with the functional styles

export let ba = 'border-style: solid; border-width: 1px;'
export let p2 = 'padding: 0.5rem;'
// etc ...

In the parent component

<script>
  import Child from './Child.svelte'
  import {ba, p2} from './classes.js'
</script>

<!-- functional CSS -->
<Child classes={ba + p2}/>

In the child component

<script>
  export let classes
</script>

<h1 style={classes}>
  Hello from Child
</h1>

Here's the REPL

Advantages

Disadvantages

Questions

cortopy commented 4 years ago

@dyslexicon

That is an amazing summary of the discussion. Overviews like yours help a lot when issues become long discussions. Thanks so much for taking a step aside to get some clarity, gather thoughts and describe potential problems and solutions.

I've been giving this issue some thought, and I would like to add a couple of comments if I may to your previous post.

Why apply classes to imported components?

Here, I would add yet another case

HOC

Svelte community is still deciding what would be the best pattern for HOC. In the discussions I've seen so far there is no mention of how to pass css class names to a HOC.

When considering a solution for this conversation, I think it would help if it is compatible with whatever solution is found for HOC

My thoughts on this

In my experience with HOC in other frameworks, you can have them as functions (in which case passing styles or even a horrific classNames wouldn't be an issue) or as HTML markup that behaves just like any other component (as discussed in the linked issue). In the latter case, we are going back in a circle since, how is one supposed to pass the css class names to this HTML markup?

cortopy commented 4 years ago

@dyslexicon

I've also been thinking about your JS implementation of functional CSS. The snippets you include in your comment are exactly the pattern I'm using except I use scss to generate utility classes

I don't have particular issues with the disadvantages you mention. Generating a class names list with js sounds a very good idea indeed. And, in most basic cases, what you describe would indeed work.

However, I'm not convinced it's the best option because:

So, my problem still persists of how to pass a bw0-m class defined as :global (i.e: no border width for screens wider thatn the medium breakpoint) to a child component

msfragala commented 4 years ago

I think that {$$props.class} is the best direction. It's hacky now because scoped styles don't work so you have to use :global() and your best attempt at a hopefully-unique-enough classname. But if scoped styles did work on components, then it would really powerful:

<!-- Icon.svelte -->
<p>Super cool icon</p>
<svg class="icon {$$props.class}">...</svg>

<!-- Button.svelte -->
<button>
    <Icon class="icon" />
</button>

<style>
    .icon {
        fill: red;
    }
</style>
nikku commented 4 years ago

:100: for @msfragala summarizing my personal sentiment.

https://github.com/sveltejs/svelte/pull/2888 implements this and I'm happy to give it another polish / or rework missing bits. Until now I'm missing the confirmation by the Svelte core team that they'd like to support such functionality.

Looking into the available alternatives, none of the workarounds presented in this thread sound like compeling way to go for me. Using :global works, kind of, but is a nasty hack, too.

ghost commented 4 years ago

Thanks @cortopy for your kind encouragement 🙂

You've given me a lot to think about with the mention of HOCs, something I hadn't previously been exposed to.

Having had a look at how React handles them, and at the implementation Rich wrote, I wonder: If any single, regular Svelte component can receive a scoped CSS class as a prop, why wouldn't that extend to and be compatible with the functional HOC pattern?

Svelte is a compiler, it can do whatever it wants, it can interpret 🌶 as a semicolon, right? I had a look at some of the source code... very sparse on comments lol. Surprised to see TypeScript! What about harnessing decorators or abstract classes to implement HOCs?

As an API consumer however, I can only really deal with these things as black boxes. If Svelte already included <MyComponent class="..." />, I'd probably take it for granted and not think about how it worked under the hood. Come to think of it, how do slots work in that regard? Can't esssentially the same thing be done for CSS?

jeffersoncostas commented 4 years ago

I think this problem could be solved using something like react styled components. Because all the styles that you want it could be added using props, and the library should be responsible to add a class name.

cortopy commented 4 years ago

@jeffersoncostas styled components would solve some issues, but I don't think it would cover some use cases discussed in this thread:

ianwalter commented 4 years ago

I would love for the default functionality to be:

If the custom component has a single root element, apply all non-defined (defined meaning export let whatever in child's <script>) props to said root element. If there isn't a single root element, let the end-user manually assign the props/attributes.

All of the comments that I read on this thread, understandably, are focusing on the class attribute but what about id? And other attributes like required or placeholder? The parent component should be assigning id to the child component and we shouldn't have to write boilerplate code to get that to work. Or, at least, it would be more intuitive and a more pleasurable development experience if it "just worked".

In Vue.js, this just works. I'm guessing because there's always a single root element. I would make the same trade-off if forced, but I think the logic I outlined above is a good compromise.

EDIT: Same idea in the related PR: https://github.com/sveltejs/svelte/pull/2888#issuecomment-549064835

pngwn commented 4 years ago

Those things should be defined as part of the component interface, passing random attributes down and automatically adding them without involving the component itself breaks the component model.

Just because Vue does it that way, doesn't mean we agree with that design nor does that make it more likely to be implemented.

Regarding classes specifically, that is highly unlikely to be implemented. The styling RFC will likely be the approach we take.

The PR implementing this was closed as we do not feel that passing classes around is the best approach here, so I'm going to close this as well. Some of the problems outlined in this thread would be solved by the CSS properties RFC. If there are other concerns that the RFC doesn't address then by all means open a new issue to discuss them but focus on the problem itself rather than a proposed a solution, we can have a better discussion that way.

Conduitry commented 4 years ago

There's a proposal #2930 for exposing a separate object of all of the 'extra' props, which sounds reasonable enough to me personally. That could be used to explicitly achieve the last suggestion. Even without that, you can still do something like that by using destructuring to manually strip out the 'known' props.

kamalkech commented 4 years ago

@nikku your example not working on reply

jakobrosenberg commented 4 years ago

What's in the way of letting users specify a component hash from the parent if they so choose?

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

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

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

The syntax may not be optimal, but I'm sure the idea is obvious.

ymatuhin commented 4 years ago

Any news on this item? It's really important.

acnologia000 commented 4 years ago

wrapping in divs worked fine for me ( i am ashamed of this but it works)

jddecano commented 4 years ago

I just tried using classnames. It works

code

 <SidebarDivider customClass="my-0" />
<script>
  import cx from 'classnames'

  export let activeClass = 'active';

  let active = 0

</script>

<hr class="{cx('sidebar-divider',  {customClass: active === 1})}" />
nikku commented 4 years ago

Nope, it does not.

If you declare styles for customClass in the outer component these will be stripped of / optimized away / tree shaken or however you call it.

Obviously passing the class down works. Passing any declared styles down / retaining them requires support by the compiler which won't be implemented as explained here.

AnxiousDarkly commented 4 years ago

TBH It is a bit disheartening to see this issue closed when all proposed solutions do not sufficiently solve the issue at hand, I really like svelte but if this is how feature requests are handled I am probably not going to use it in the future.

AlbertMarashi commented 4 years ago

This is a deal-breaker for me.

Vue supports child scoping and svelte should too.

For example, I have a <Link> component that manages javascript routing, and renders an <a> tag.

Say I want to style this javascript routing anchor tag on various pages (some may be buttons, plain links, images) it makes it incredibly difficult. Eg:

<Link href="/about">About Us</Link>
<style>
a {
  color: red; //doesn't apply this rule, because scoping doesn't extend to children
}
</style>

We should be able to apply an attribute to the style to apply scoping to child elements, eg:

<Link href="/about">About Us</Link>
<style children>
a {
  color: red; //applies rule to children
}
</style>

Pleaase do it like vue