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

Introduce <style module> — update: forward directive #6972

Open
aradalvand opened this issue Dec 1, 2021 · 58 comments
Open

Introduce <style module> — update: forward directive #6972

aradalvand opened this issue Dec 1, 2021 · 58 comments
Labels
compiler Changes relating to the compiler feature request
Milestone

Comments

@aradalvand
Copy link

aradalvand commented Dec 1, 2021

Also see the update

Describe the problem

Currently, Svelte's default mechanism for handling scoped styles is limited in many ways, and literally dozens if not hundreds of issues have been created over the years complaining about different aspects of it. One thing that stands out however, is the inability to pass down scoped classes to child components.

Here are just a number of issues surrounding this:

Stack Overflow questions:

Some notable comments that beautifully describe this problem:

Arguably, the biggest pain point right now when it comes to Svelte's scoped styling is precisely this, namely the inability to send scoped classes down to child components, and this is a big deal.
This is something that has made me (and I'm pretty sure many others) seriously consider using Vue (although I love other aspects of Svelte too much I can't bring myself to do that), so this isn't trivial, and the workarounds are almost universally terrible.

There's an argument that has been repeatedly made by some (which has also been consistently downvoted by users, the number of thumbs-downs on those comments is very telling) claiming that this would break encapsulation. This is a horrible argument, if you have this in mind, I refer you to the comments I linked above, please take some time to read those and reflect. They debunk this myth eloquently. I'm not going to boringly reiterate what's already been said and discussed ad nauseam; instead, I'm going to focus on the solution I think would solve the aforementioned shortcomings.

Describe the proposed solution

I would propose Svelte add a feature that Vue has had for some time, namely <style module>, which solves nearly all of those problems that users have been suffering over for years now in the Svelte community.
Note that this would be an opt-in feature, meaning that it would be entirely backward-compatible and optional.

<script>
    import Icon from './Icon.svelte';
</script>

<button class={$$styles.button}>
    <!-- This is a component that receives a "class" prop and adds the classes to the root element: -->
    <Icon class={$$styles.icon} />
    <slot />
</button>

<style module>
    .button {
        /* styles */
    }
    .icon {
        /* styles */
    }
</style>

Note the special $$styles variables, this fits Svelte's syntax neatly as we already have things like $$props, $$slots, etc. The name could be anything, $$classes is another option, $$styles is just similar to what Vue uses ($styles). See this in Vue docs.

The other important bit here is that this only works with classes (and also animation names., for that matter), complying with the CSS Modules specs. So any other selectors (e.g. element selectors, IDs, attribute selectors) will be left untouched (or could cause an error, we can decide on this later), and will also not be included in $$styles.

This would give developers the freedom that's currently lacking, it:

  • Enables developers to pass down scoped classes to child components. (FINALLY!)
  • Requires no special treatment for the class prop on components - which was the solution most people were suggesting to solve the problem mentioned above, but this was consistently opposed by the maintainers.

Alternatives considered

Currently, I and many others have had to ditch Svelte's scoped styles entirely because of this limitation.

The alternative approach I've personally taken is I'm essentially using BEM naming and always making the styles global, this of course works, but obviously I'm deprived of all the advantages of using scoped styles; and I also have to worry about name clashes and such all over again. So, it's a huge bummer to have to do this.

I also want to ask you guys not to dismiss this proposal out of hand, please consider the fact that the problem this intends to solve is a real one, which has affected countless users, and you can tell by simply looking at the sheer number of issues created over the years surrounding this very topic.
The problem we're solving here is by no means uncommon or niche, and it's already been solved in other popular frameworks like Vue and React for years, so it must be taken with due seriousness, in my opinion. The current Svelte CSS scoping mechanism is effectively unusable for many people and projects precisely because of this particular limitation.

As I mentioned above, the solution I'm proposing is not a breaking change, as it doesn't intend to modify the current behavior of scoped styles, which means it won't interfere with the workflow of the users who are already using the default scoped styles. They can continue to do so if so they wish. So this is a big pro as well. As far as I've seen, most proposals addressing this problem involves some form of a breaking-change.

Would love to know your thoughts and opinions on this.
Thank you in advance.

Importance

crucial, big pain point

@dummdidumm dummdidumm added compiler Changes relating to the compiler feature request labels Dec 1, 2021
@TheOnlyTails
Copy link

How would this compare to using the :global selector?

@MacFJA
Copy link

MacFJA commented Dec 2, 2021

So the goal is to have this:

<!-- Child.svelte -->
<div class="text-white">I'm the <span>child</span></div>
<style>
    .text-white {
        color: white
    }
</style>
<!-- Parent.svelte -->
<script>
    import Child from './Child.svelte';
</script>

<div>
    <Child class={$$styles.my-identifier} />
</div>

<style module>
    div {
        background: green
    }
    .my-identifier {
        background: red
    }
</style>
<!-- rendered output -->
<div class="svelte-1">
    <div class="svelte-2 my-identifier text-white">I'm the <span>child</span></div>
</div>
<style>
    div.svelte-1 {
        background: green
    }

    .my-identifier.svelte-2 {
        background: red
    }
    
    .text-white.svelte-2 {
        color: white
    }
</style>

  • What happen if Child.svelte don't have root element ? (wrap with a div like with --css-variable ?)
  • What if I want conditional class in <Child /> ? (<Child class={condition ? '$$style.my-class' : ''} /> ?)
  • What if I want several down scoped classes ? (<Child class="$$style.my-class $$style.my-class2" /> ?)
  • What if the <Child /> already have class props ?
  • What if I want to down scope a class for a child of <Child /> component ?

I thinks that they are still too many cases that need to be check.
(And many of them, IMO, are bad component design)


The --css-variable is for me the best solution have it allow to pass variable to down to a component and all it's children.
It the job of the Component designer/creator to define a set of css variable to use.

IMO it allow better control of the visual behavior of the component, as what should be configurable is defined, instead of opening everthing to modifications
(Kinda following the SOLID principle)

@aradalvand
Copy link
Author

aradalvand commented Dec 2, 2021

@MacFJA None of the problems you mentioned is actually a problem. I'll try to explain below:
First of all, as I mentioned in my original comment, class isn't treated specially, it isn't magically added to the root element, the component author has to decide on that. In other words, you'll have to do something with the passed class in the child component. It is like this because Svelte maintainers have always been against special treatment for class.
For example, imagine a real-world scenario where you have a Button component that renders a button, and also a Modal component, that has two buttons inside it, a submit button and a cancel button. So like:

<!-- Button.svelte -->
<button type="button" class="button {$$restProps.class}">
    <slot />
</button>

<style>
    .button {
        /* styles... */
    }
</style>
<!-- Modal.svelte -->
<script>
    import Button from './Button.svelte';
</script>

<div class="modal {$$restProps.class}">
    <!-- Other stuff... -->

    <Button class={$$styles.submitButton}>Submit</Button>
    <Button class={$$styles.cancelButton}>Cancel</Button>
</div>

<style>
    .modal {
        /* styles... */
    }

    .submitButton {
        /* styles... */
    }
    
    .cancelButton {
        /* styles... */
    }
</style>

The resulting class names with be hashed, that's how CSS Modules works, this is unlike Svelte's default scoping mechanism where it adds a special class to elements and then uses compound selectors. So in this case the resulting HTML will look like:

<div class="modal-jdj20d9">
    <!-- Other stuff... -->

    <button type="button" class="button-4h72jd0 submitButton-jdj20d9">Submit</button>
    <button type="button" class="button-4h72jd0 cancelButton-jdj20d9">Cancel</button>
</div>

Now, to answer your questions:

What happen if Child.svelte don't have root element ? (wrap with a div like with --css-variable ?)

As I mentioned multiple times, class isn't treated specially, if you have multiple root elements you can decide what you want to do with the passed class, you can add it to all of them, or only one.

What if I want conditional class in ? (<Child class={condition ? '$$style.my-class' : ''} /> ?)

That's not relevant here, this proposal doesn't change anything in that regard. Yes you could do <Child class={condition ? '$$style.my-class' : ''} /> as you said.

What if I want several down scoped classes ? ( ?)

No problem, you can do that. Again, there's nothing magical about $$styles, it's just an object whose properties are hashed class names, so they're just strings you can pass anywhere.

What if the already have class props ?

Not sure I understand this one?! Well, once again, there's NOTHING magical about class, you can call the prop anything you want, like containerClass, classClassClassHahaha, anything...

What if I want to down scope a class for a child of component ?

You can have another prop. So imagine the example I showed above again, the Button and the Modal, imagine your Button also renders an SVG element acting as the icon, Button can receive an extra prop called iconClass. Although I wouldn't necessarily recommend doing this, it's certainly possible.

I thinks that they are still too many cases that need to be check.
(And many of them, IMO, are bad component design)

I disagree. This proposal is as lightweight and as side-effect-free as it possibly gets.
And regarding the claim that this is "bad component design", absolutely NOT true. Look at the example I just wrote above, how is that "bad component design" exactly?! You have a modal, and in that modal you have two buttons, you need to have access to those buttons in your CSS in the modal component to be able to position them correctly in the modal. This is as clean and as easy-to-reason-about of a component structure as it gets.

@aradalvand
Copy link
Author

aradalvand commented Dec 2, 2021

@TheOnlyTails Well, with the :global selector you're making all the selectors (including classnames) global, which means you'll have to deal with name collisions, this is exactly what any CSS scoping mechanism (such as CSS Modules and also Svelte's current CSS scoping approach) is meant to avoid.
With this approach, a random (not exactly random but you can think of it as random) string will be attached to the end of all classnames within a component to make them unique across the whole app. Kind of similar to what Svelte currently does, the key difference is the $$styles object here, which gives you (the developer) access to the hashed class names and allows to pass them around.

@nosovk
Copy link
Contributor

nosovk commented Dec 3, 2021

Probably it's not a problem, you can use global if you actually need to style child component. But after css variables arrival it looks like we actually stoped using global selector at all in new components...
Scoped styles allows to avoid adopting BEM and mostly stick to elements rather then classes inside scope, which is at least less verbose.

@aradalvand
Copy link
Author

@nosovk

use global if you actually need to style child component

The whole point of this proposal (and scoped styles in general) is that you won't have to use global styling, as making styles global would open the door to countless issues and headaches down the road, unless you rely on a strict naming system like BEM.

@araxemy
Copy link

araxemy commented Dec 3, 2021

I totally sympathize with this proposal, this is a problem that's long been solved by Vue, it's strange that Svelte still has no real solution to this. (I'm talking about solutions, not workarounds like "use global styles", that's not a solution, it's an ugly workaround for something that I would argue should already be possible to natively do in Svelte.)

@aahmadi458
Copy link

actually i've been seeing people demanding a feature like this for a long time now, especially in svelte's discord people ask about this on regular basis, i agree that this would be a nice (necessary ? long overdue ?) addition to svelte.
I mean , just look at what people are currently doing to hack around this:

image

* :global([selector]) haha! clever! 😅

@nosovk
Copy link
Contributor

nosovk commented Dec 4, 2021

@nosovk

use global if you actually need to style child component

The whole point of this proposal (and scoped styles in general) is that you won't have to use global styling, as making styles global would open the door to countless issues and headaches down the road, unless you rely on a strict naming system like BEM.

but global selector works only to nested elements
https://svelte.dev/docs#style

div :global(strong) {
		/* this will apply to all <strong> elements, in any
			 component, that are inside <div> elements belonging
			 to this component */
		color: goldenrod;
	}

repl with demo

@aradalvand
Copy link
Author

aradalvand commented Dec 4, 2021

@nosovk In that example you're selecting ALL strong elements in the scoped div, if there's a strong element ten levels deeper in the component hierarchy this will select it. That's not what we want, we want to have specific control over which, in this case strong elements, are selected.
We all know that doing it this way is already possible, but it's a hacky workaround that introduces new problems and it's irrelevant.

@nosovk
Copy link
Contributor

nosovk commented Dec 5, 2021

@nosovk In that example you're selecting ALL strong elements in the scoped div, if there's a strong element ten levels deeper in the component hierarchy this will select it. That's not what we want, we want to have specific control over which, in this case strong elements, are selected. We all know that doing it this way is already possible, but it's a hacky workaround that introduces new problems and it's irrelevant.

But you can create class if needed? It will be uniq. It will allow selecting exact 'strong'.
I can't understand why that use case requires special treatment. When you need to broke style isolation (which is bad) you have instruments to do that. But in common, doing that is bad practice. Making it easier wouldn't do our code better, if you really require to broke style isolation - use global.

@aradalvand
Copy link
Author

aradalvand commented Dec 5, 2021

But in common, doing that is bad practice. Making it easier wouldn't do our code better, if you really require to broke style isolation - use global.

That is simply not true as I (and others) have explained numerous times. There are lots of CSS properties (margin, top/left/right/bottom, flex, etc.) that should only be set by the parent, it doesn't make sense to set them from within the component itself.
The rationale is perfectly clear, I don't really know what it is that you're struggling to understand here.

You can ignore this issue if you don't like this proposal. There are, however, many people who are in line with it.

@razshare
Copy link

razshare commented Dec 7, 2021

Svelte does have this problem, but that is not a solution imo.
That's a procedular patch in that we would drag along for a long time if it's introduced.

Please don't add this.

  • Code like that will pop up all over the codebases of everything Svelte related.

  • This goes completely against inversion of control.

  • The fact it would be "opt-in" won't minimize the damage it can do, we would drag this along for years.
    Those words "it's just opt-in" should always raise a flag.
    It doesn't matter it's "opt-in", if it's faster in the short run, and really bad design in the long run, people under pressure at work will still adopt that and it will spiral.

  • This would make debugging harder.

The fact it barely works in Vue doesn't make it a good solution in Svelte, it's just bad imo.

How do you even deal with media queries and container queries?
Is the runtime supposed to parse the css?

The creator of the component should provide the api, you shouldn't hack your way into a component to change its appearance or behaviour, I mean for the love of god it's the reason we do this:

<Component on:click={()=>{ ... }}/>

and not this:

<script>
const c1 = document.getElementById("c1")
c1.addEventListener(...)
</script>
<Component id="c1"/>

Components should be closed for changes and open for extensions.
I would also add that this problem should be solved using css instead of using js props.

The issue of course is that the way of writing these apis from the point of view of the "creator" of the component is pretty slow and sloppy right now.

Something like this would make more sense imo:

Component.svelte

<style>
  h1 {
    color: #f45;
  }

  :export(.extra) {} /* The compiler can already detect this empty class declaration */
</style>

<h1 class="extra">hello world</h1>

and then in Parent.svelte

<style of Component>
    .extra{
        color: #000
    }
</style>

<Component />

At an implementation level, everything inside <style of Component> would be provided the hashed class of Component in order to reference the component plus a second hash identifying the context in which Component.svelte is used.

So the output would look like this:

<style>
    .s-123.si-aaa.extra{
        color: #000
    }
</style>
    <!-- 
        "s-123" would be the usual hash of Component.svelte 
        "si-aaa" would be the context hash
    -->
<h1 class="s-123 si-aaa extra">hello world</h1>

The context hash would depend on both parent and child, so it would be as simple has hashing the parent and adding the child as salt.

This means if you're to call Component.svelte in a different component called Parent2 for example, the context hash would be different, allowing you different implementations.

This way you're not bound to the individual instances of the component and you're free to extend whatever extensions the author provides.

If we really want to manipulate individual instances, then simply change the context hashing, it would have to be executed on the actual instance of the component, and in the syntax we could reference these by variable names:

<style of i1>
    .extra{
        color: #000
    }
</style>

<script>
let i1
</script>

<Component bind:this={i1} />

Note that there's no js reference inside the actual css code, it would still remain plain css.


On top of that we could get intellisense aswell.

@araxemy
Copy link

araxemy commented Dec 7, 2021

@tncrazvan I don't like the approach you're proposing at all. I find the OP's suggestion more sensible and here are my responses to your criticisms:

This goes completely against inversion of control.

How?

This would make debugging harder.

How?

It doesn't matter it's "opt-in", if it's faster in the short run, and really bad design in the long run, people under pressure at work will still adopt that and it will spiral.

It's not bad design, that's a statement of utter ignorance. For the thousandth time: There are countless CSS properties that should be declared by the parent and not the component itself, things like margin etc. and the most reasonable way to do that is to give the child component a class and style the class in the parent. The claim that it's "bad design" is a completely empty claim.

The fact it barely works in Vue doesn't make it a good solution in Svelte, it's just bad IMO.

It "barely" works?! What does that mean?! It's been working without issue for years. You're just sneaking in the word "barely" as though it's been a pain point for Vue devs, it hasn't.

How do you even deal with media queries and container queries?

Well! You deal with media queries the way you deal with media queries anywhere, what is the problem?! What's special here?

On top of that we could get intellisense aswell.

We could get intellisense with this approach as well.

The alternative solution you're providing is not ergonomic at all, I mean, ffs why would you have to get a reference to the component to be able to style it individually?! That makes no sense in my opinion. That's what classes are for, you're needlessly reinventing the wheel, and the new wheel you're inventing is much worse than the old one.

@kevmodrome
Copy link
Contributor

This most likely will not happen. It has been discussed to death already. I don't see why you expect this to change. I know you have already read it but here is a comment by @pngwn that goes into the reasoning for anyone else reading this: sveltejs/rfcs#22 (comment)

@aradalvand
Copy link
Author

aradalvand commented Dec 7, 2021

@kevmodrome None of the arguments made by @pngwn justifies the inability to style a component from the parent for layout purposes. Just tell me how you'd do that currently?! Svelte just doesn't have any way of doing that at the moment. Yet this is of one the most common things you'd want to do in CSS.

You guys' continual refusal to even take this issue seriously, quite honestly baffles me. This is a major shortcoming in Svelte, and I don't understand how anyone could deny that; and I absolutely don't understand why it's always been hand-waved away so dismissively.

We should seriously stop pretending like this has been resolved, it hasn't, closing issues that talk about it does not "resolve" anything. This is the biggest pain point when it comes to CSS in Svelte, and it needs a proper solution.

Besides, @pngwn's points were largely responded to multiple times, take a look at some of the comments I linked above, this one succinctly sums up how @pngwn's criticisms and so on — anything along the lines of "it's dangerous" — make no sense.

Quoting @pretzelhammer here:

Yes. Both React and Vue solved this problem years ago. Devs need to be to access the hashed classnames in JS as it enables useful patterns like passing scoped styles from a parent to a child without leaking anything into the global scope.

It's as though Svelte is waiting for "the perfect solution", while also failing to provide an adequate workaround in the meantime.
Svelte should just give this ability to developers who want to use it and know what they're doing, it shouldn't want to police everything, because it utlimately can't, that's just not how web development works. I don't understand why Svelte so desperately wants to be different from React and Vue in this particular respect, when those two (and presumably others) have provided a solution to this for a long time, and I've seen very few people ever complain about the way they're doing it.

@pngwn
Copy link
Member

pngwn commented Dec 7, 2021

This needs an RFC. There is zero chance that this will be agreed upon in an issue.

@pngwn
Copy link
Member

pngwn commented Dec 7, 2021

Also I don’t really think anyone has debunked any ‘myths’. They have just restated the problem, which I have never denied. But you clearly have a different definition of debunked than I do.

Your adversarial tone isn’t going to increase your chances of convincing us, btw, quite the opposite.

@pngwn
Copy link
Member

pngwn commented Dec 7, 2021

I do plan to revisit this issue properly at some point, but won't have time until the new year.

I've worked on component libraries and design systems (large and small) pretty extensively over the past few years and would like to revisit my thoughts.

@aradalvand
Copy link
Author

aradalvand commented Dec 7, 2021

@pngwn Oh here's the antagonist himself! (just kidding! :P)
What I was referring to by the "myth" was the notion, frequently restated either implicitly or explicitly, that Svelte should avoid giving the developer more control in this regard because people "could" screw things up if they don't use the feature mindfully, and I would argue that is a myth, as I think saying "it could be abused" is almost never a great argument against any feature WHEN refusing to provide that feature is likely to produce more frustration, and especially when we're talking about CSS, as CSS almost always gives the developer the opportunity to screw things up, regardless of how much tooling you utilize. So it's ultimately the developer that has to be mindful, and not the tooling, the tooling should, of course, minimize those possibilities as much as is reasonable, but from some point on, it's none of its business anymore, and it should just trust the developer's judgment. And all I'm arguing is that this is certainly one of those cases.

And my tone shouldn't really determine the outcome here, although I'm sorry if it sounds adversarial, it's the underlying reasoning that ultimately matters.
I hope you revisit this as you said and let us know.

@razshare
Copy link

razshare commented Dec 7, 2021

@araxemy

This goes completely against inversion of control.

How?

The suggestion says that you're supposed to pass in a class to the component through a prop and, quoting, "adds the classes to the root element".

Besides the fact there's no guarantee a component has a root element in svelte, op does mention it would be an actual prop the author would decide where to place.
Let's say the author decides where that class goes all the time, be it in the root or somewhere else, you're still injecting a variable class name into the component.
You're setting the dependency yourself, which should never be the case.

Take for example @MacFJA 's question:

What if I want conditional class in ? (<Child class={condition ? '$$style.my-class' : ''} /> ?)

and the answer

That's not relevant here, this proposal doesn't change anything in that regard. Yes you could do <Child class={condition ? '$$style.my-class' : ''} /> as you said.

Except it is relevant, it's not clear exactly what that will imply at scale - will people fill their markup with conditionals like that?
Well I certainly don't wanna read and manage that type of code, scares the shit out of me and brings me back to php templating times.

This would make debugging harder.

How?

You would always have to track down the parent component in order to find out where a class name came from.

It doesn't matter it's "opt-in", if it's faster in the short run, and really bad design in the long run, people under pressure at work will still adopt that and it will spiral.

It's not bad design,

I'll just stop there, you got me.

The fact it barely works in Vue doesn't make it a good solution in Svelte, it's just bad IMO.

It "barely" works?! What does that mean?! It's been working without issue for years. You're just sneaking in the word "barely" as though it's been a pain point for Vue devs, it hasn't.

If you wanna argue semantics be my guest, if you're so in love with Vue that much that you want everything to be Vue go use Vue in all fairness.
I'm trying to point out it's not a good solution for Svelte because of inversion of control and encapsulation

How do you even deal with media queries and container queries?

Well! You deal with media queries the way you deal with media queries anywhere, what is the problem?! What's special here?

I am geniunly asking what the edge cases are.

I mean, ffs why would you have to get a reference to the component to be able to style it individually?!

I'm not trying to be rude, sorry if I come accross like that - if you can't put the effort to read everything I wrote and process it there's no point.


Either way, I completely agree with @pngwn #6972 (comment)
But also this is discouraging #6972 (comment)

@TheOnlyTails
Copy link

After talking with some other people in the community, we thought it'd be better to move this discussion to a Discord thread.

This isn't specifically for this solution, but generally for solutions to this problem.

The thread is "Styling the kids" under the society-chat channel.

@PaulMaly
Copy link
Contributor

PaulMaly commented Jan 5, 2022

@AradAral Please take a look: https://www.npmjs.com/package/svelte-preprocess-cssmodules

Maybe it will solve your problems without a changes in core. The main idea of preprocessors was to give ability do such things in user-land.

@lukaszpolowczyk
Copy link

@jmroon
Copy link

jmroon commented Jan 16, 2022

@AradAral Please take a look: https://www.npmjs.com/package/svelte-preprocess-cssmodules

Maybe it will solve your problems without a changes in core. The main idea of preprocessors was to give ability do such things in user-land.

Unfortunately that doesn't quite help with utility frameworks such as tailwind.

Typically, I want my parent to decide how my child component is laid out. The child might have a preferred width for example, but I might want it to change depending on its parent.

At the moment, the only solutions I've found are either have a wrapper div around all of my components and merge classes and styles, or set the width/height to 100% and wrap it in the parent. Both solutions feel less than ideal.

Vue has the right approach IMO. Inherit parent attributes if the component has a single root (its by default, but I wouldn't mind if it were opt in in the config). If there are multiple roots, then the attributes aren't inherited. This would solve my issues and greatly reduce the amount of boilerplate needed to style components. Global css selectors simply aren't a good fit for utility css frameworks.

@juanmait
Copy link

I think that the svelte team should clarify his philosophy about this in the official documentation or at least in a FAQ section.

I mean, there is no roadmap or intentions to fix this. Only a strong although vague opinion on how we should do things or how bad we are doing things. The React team has also strong opinions but they make those very clear in the official documentation. Although I'm tired of those already that's why i'm here...

Wait.. What I'm going to do when all the smart engineers that I'm trying to convince that svelte is the best framework ever created period, discover that in the documentation??

I'll be toast! Game over man!

I'll better go back to my toy projects, that's a lot of fun indeed!

@JWess
Copy link

JWess commented Jan 11, 2023

One shortcoming of the above proposal is that the parent component can only style elements within the child component (<Button>) if the author of the child component chose to allow it. When using 3rd-party components from component libraries, there are occasional cases where I need to adjust/tweak the inner styles. If the author didn't choose to forward:class the proper element(s), I wouldn't be able to customize anything in the child component.

I know this is probably not possible in the Svelte architecture, but in Angular, they have ::ng-deep .someClass which, causes .someClass to become global, but then they have the :host pseudo-class selector which can be used in a parent component to scope styles to itself and all descendants. Combine it with ::ng-deep like :host ::ng-deep .someClass to affect all elements with the class="someClass" at the host level and all descendants.

@madeleineostoja
Copy link

@JWess that’s no different than component authors choosing not to consume className in React and et al, and should be seen as a feature rather than a downside. You can already delve into the tree of components with :global if you really want, but it’s generally seen as an anti pattern.

@tontonsb
Copy link

This proposal entirely solves all the problems described in this issue

@aradalvand what if you have more than one element in the component?

@JWess
Copy link

JWess commented Jan 12, 2023

@JWess that’s no different than component authors choosing not to consume className in React and et al, and should be seen as a feature rather than a downside. You can already delve into the tree of components with :global if you really want, but it’s generally seen as an anti pattern.

I agree it's something that should be used/needed sparingly, but it's not realistic to imagine a world where it's never necessary to tweak a component's styles in some way that the original author didn't intend/expose. It shouldn't be a first-class citizen, but it should be supported by the framework, with caution flags in the documentation.

As far as using :global, in order to accomplish the behavior I described with :host ::ng-deep .someClass (where the style is not leaked app-wide), the parent component has to wrap the child component with an extra element and put a wrapper class on there like this:

// Parent.svelte

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

<div class="wrapper"> <!-- I wish I didn't have to wrap this component just to override styles -->
  <Child />
</div>

<style>
  .wrapper :global .someClass {
    display: contents;
    /* some style overrides */
  }
</style>
// Child.svelte

<button class="someClass">
  ...
</button>

<style>
    .someClass {
        /* ... */
    }
</style>

@jmroon
Copy link

jmroon commented Jan 12, 2023

@JWess one thing worth noting is that ng-deep has been deprecated for a few releases now. It's also really not that different than :global. The only difference is you'll use your parent component's top level element as the root of your selector, whereas you'd use host in angular. With ng-deep deprecated, the accepted solutions now are either to just write in your global stylesheet, or disable view encapsulation for that component.

@nosovk
Copy link
Contributor

nosovk commented Jan 12, 2023

actually :global should be enough to style child components, if you really know what you are actually doing. But if you have to add styling support - probably CSS Vars is a better option. Actually Angular also insists on CSS Vars instead of ng-deep hack.

@madeleineostoja
Copy link

madeleineostoja commented Jan 12, 2023

There's a reason why tree diving mechanisms like ::ng-deep in angular, :global in svelte, and /deep and ::shadow in web components are either deprecated or seen as hacks — poking into the internals of components to override styles makes them brittle and open to breaking without warning. The mechanism to do exactly that exists in Svelte (:global), it's not pretty but it is supported.

What we're asking for in this issue is simply a way to forward a single scoped class to a component, explicitly to avoid leaky hacks like :global. All other frameworks support this in some way, and the fact this issue comes up again and again and again shows that it's something that users need and constantly run up against, ideological reasoning either way aside. The current situation is the equivalent of not being able to use the className prop in a React component or add a class to the :host of a web component.

As I understand it the main technical issue here is that Svelte components don't require a single root node like React et al. @aradalvand's proposal seems like a great solution to this.

Is it time for a proper RFC finally? Anyone from the svelte team want to weigh in and/or veto before someone goes to the trouble of an RFC? Don't want to ping names but it would be great to get a little momentum on this again after a few years of going in circles.

@jmroon
Copy link

jmroon commented Jan 15, 2023

I was certainly of the opinion early on (as evidenced by my comments above) that I would like to be able to do this. But I don't know anymore. I understand why it's seen as hacky, and why it shouldn't be encouraged, but when you're using component libraries or third party components in general, this is essential. For every single component library I've used, I've always run into a case where I had to override its styling. It's unavoidable.

:global is almost good enough. The main issue is that its a bit hard to use global safely. You can scope your global selectors to your root element in your component, but that relies on you defining a unique class name for your element. What about a :deep selector that would auto-generate a unique class name for the root(s) of your svelte component and automatically prepended it to the selectors you defined? I want a safe and sanctioned way to ensure that my global selectors only select descendants of this component.

e.g.

<div>
  someStuff
</div>
<div>
  someMoreStuff
</div>

<style>
  :deep(button) {
    color: white; // make buttons white
   }
</style>

Through the compiler some autogenerated root classes would be applied to transform it into:

<div class="<autoGenerated>">
  someStuff
</div>
<div class="<autoGenerated>">
  someMoreStuff
</div>

<style>
  :global(<autoGenerated> button) {
    color: white;
   }
</style>

Maybe it's not super feasible if each Svelte compiled is compiled in isolation, therefore it may be hard to avoid class name collisions.

Essentially, it would be nice to say "I want to target all buttons in this component's tree" instead of "I want to target all buttons in the entire app".

@DeepDoge
Copy link

I still can't get my head around why we can't just have have something like this:
Foo.svelte

<div class="a">A</div>
<div class="b">B</div>

App.svelte

<Foo />
<div />
<Foo />

<style>
  :Foo(*) { /* A and B in all Foo(s) under this component */ }
  :Foo(.b) { /* B only in all Foo(s) under this component */ }
  div + :Foo(.a) { /* A only in last Foo under this component */ }
</style>

That way IDE can give warnings. its basically like :global but with warnings.
Something like forward has no use if you are trying to add styles on Components from other libraries, doesn't mean it wouldn't have any use, of course it would be a nice addition.

But we are asking for :global with IDE warnings when the thing we are selecting doesn't exists.
If we are gonna go with "leaky hacks" why do we have to go full "leaky" with :global

@kenbankspeng
Copy link

Ever see a non-Svelte product become brittle through large amounts of global css, such that for every css bug a dev team fixes will inadvertently creates two new css bugs?

Due to the inevitable emotional damage this causes, I now constrain css selectors to typically go no further than the module (ie. css modules) and at most, reaching no further than to an immediate child module.

Having now moved on to use Svelte/Tailwind, I don't use selectors at all. In the few cases where I do need to effect the styling of the child from the parent, I just add class="..." to the child instance within the parent, and use cva/twMerge/$$props.class within the child to realize the styling from the parent. The cva variants cover most needs, $$props.class supports the remaining bespoke custom needs, and twMerge ties it all together appropriately for Tailwind.

I know not everyone has entered into this new Tailwind world view, but for many who have, not only would we not use <style module> if it is added, we'd also be just fine with <style> itself being removed...

@aradalvand aradalvand changed the title Introduce <style module> Introduce <style module> — update: forward directive Jul 25, 2023
@patricknelson
Copy link

patricknelson commented Jul 26, 2023

Move to either:

  1. Close this in favor the forward directive RFC (Implement forward directive rfcs#60) to solve the issue of passing styles directly to child components.
  2. Rephrase the title of this issue to better capture the intent of the issue (instead of including a solution direction in the title, which that proposed solution is one of many possible approaches)

Reason: Reduces confusion, since that RFC is a very different but elegant approach which solves the problem outlined in the original description without leveraging the implementation suggested in the title (i.e. <style module>). This is important because <style module> could actually be useful for other completely different use cases but might have a totally different implementation and feature set.

Edit: Sorry folks for the weird notification; I pressed alt-enter way too early before I was actually ready to post my comment. 😬

@aradalvand
Copy link
Author

aradalvand commented Jul 26, 2023

Close this in favor the forward directive RFC (sveltejs/rfcs#60) to solve the issue of passing styles directly to child components.

No, even if we consider this issue to be specific to the forward directive, it's still useful to have an actual issue tracking the feature (especially considering the fact that this issue is now one of the most upvoted open issues in the repo and that will presumably be a factor when it comes what the maintainers decide to spend time on), what you linked to is the RFC; not the same thing.
I could potentially change the title of this issue to just "Introduce forward directive" though.

This is important because <style module> could actually be useful for other completely different use cases but might have a totally different implementation and feature set.

I don't think so. What would <style module> be useful for, that the forward directive won't cover?

@patricknelson
Copy link

patricknelson commented Jul 26, 2023

No, even if we consider this issue to be specific to the forward directive, it's still useful to have an actual issue tracking the feature

Sorry, just to clarify: After much reading and confusion on my part, I eventually interpreted ticket to being specific to need to pass down scoped classes to child components (which could be implemented either via <style module> or via forward directive).

I could potentially change the title of this issue to just "Introduce forward directive" though.

I accidentally submitted my comment too early and notified a bunch of folks and likely would have refined it even further to that one suggestion. 😬

I don't think so. What would <style module> be useful for that the forward directive won't cover?

See this module https://github.com/micantoine/svelte-preprocess-cssmodules, the source of my confusion. 😅 The reason why is because I'm new to Svelte, never used Vue nor React but I did see svelte-preprocess-cssmodules before I found this issue (which mentioned the module in the <style> tag, which was why it stood out to me). As it turns out, that plugin actually uses a similar syntax to accomplish something very similar but the use case I'm referring to specifically is this one (scoped): https://github.com/micantoine/svelte-preprocess-cssmodules#scoped -- i.e. you can use <style module="scoped"> which explicitly has the con of not being able to pass styles to child components.

Hope this clarifies my perspective!

@KevsRepos
Copy link

Would like to leave my opinion here aswell. Another approach to solve the issue of passing styles could be a :let modifier:

<style>
:let(myClassName) {/* styles */}
</style>

Svelte will do these things:

  1. create a global class and and generate a hashed name for it
  2. create a component scoped javascript variable named myClassName
  3. append the generated class name to the variable myClassName

From there on we have the class name stored in a variable and therefore can do with it whatever we want. Either pass it to a component to style the child from the parent, put it in a context to release the style to all children or anything else. This is what it would look like in a real-world example:
https://svelte.dev/repl/cb117170fdc54a4c969e0a56ab71a225?version=4.1.1

@FeldrinH
Copy link

FeldrinH commented Sep 2, 2023

@aradalvand is there an RFC for forward:class? As far as I can see the original forward directive RFC doesn't touch on forwarding classes. I think it would be useful to have the forward:class as a separate RFC so that it can act as a rallying point for the proposal.

Also, I think the part preprocessor by @valterkraemer is great implementation of the same general idea and is even more flexible and powerful than the proposed forward:class. If that was merged directly into Svelte then I think the problem would be more-or-less fully solved.

@aradalvand
Copy link
Author

aradalvand commented Sep 2, 2023

@aradalvand is there an RFC for forward:class? As far as I can see the original forward directive RFC doesn't touch on forwarding classes. I think it would be useful to have the forward:class as a separate RFC so that it can act as a rallying point for the proposal.

forward:class is an addition to the forward RFC, so I'm not sure if it makes sense to create a separate RFC for it specifically. But no, the current forward RFC doesn't mention forward:class. Maybe @Tropix126 could update it? That would be appreciated.

@FeldrinH
Copy link

FeldrinH commented Sep 2, 2023

I think a separate RFC would be a good idea, because the other forward directives are largely syntax sugar for things that are already possible, but forward:class adds fundamentally new functionality that cannot be easily emulated otherwise.

EDIT: Also, the idea that forward:class is an addition to the forward directive is just a framing choice. The core idea of forward:class could be implemented on its own even if the other forward directives never make it into Svelte.

@Tropix126
Copy link

I think a separate RFC would be a good idea, because the other forward directives are largely syntax sugar for things that are already possible, but forward:class adds fundamentally new functionality that cannot be easily emulated otherwise.

I tend to agree. I like the idea, although keeping the scope of this RFC down is something that i've tried to maintain as much as possible, and it likens the chance that it'll be eventually considered (paving the way for future additions).

@Malix-off
Copy link

This issue should have the popular label

@jnehlmeier
Copy link

I am new to svelte and I was surprised that it is so annoying to style child components within the context of a parent component.

Currently I am heavily using GWT which is an old library/toolkit invented by Google to write JS apps using Java (transpiler). The first version of Gmail was written using GWT so most features of GWT predate modern JS/CSS. But it is interesting to see how they solved CSS obfuscation and encapsulation a long time ago. So I thought I write something up, maybe it is interesting for people.

public class CustomButton extends Widget {

  // Bundles all kind of resources like images, text and css.
  // Bundling means GWT will embed it into the final JS output, except image resources
  // which might become image sprites to save download requests.
  // In this example we only use css.
  public interface Resources extends ClientBundle {

    // All methods here represent css class names and they will
    // all be obfuscated at compile time. So we can't reference them 
    // in other components as we do not know the css class names.
    @ImportWithPrefix("CustomButton") // this prefix can be anything
    public interface Css extends CssResource {
      String root();
      String icon();
      String text();
    }
  
    // Contains .root/.icon/.text css classes.
    // They will be obfuscated during Java -> JS compilation.
    @Source("CustomButton.css")
    Css css();
  }

  private static final Resources RES = GWT.create(Resources.class);

  public CustomButton() {
    setElement(Document.get().createButtonElement());
    // here we apply the root CSS class to the component
    setStyleName(RES.css().root());

    // some additional code to append an icon element and the button text
    // ....
  }
}



public class CustomToolbar extends Composite {

  interface Resources extends ClientBundle {

    @ImportWithPrefix("CustomToolbar")
    interface Css extends CssResource {
      String root();
      String okButton();
      String cancelButton();
    }
  
    // This imports all css classes defined in the interface into CustomToolbar.css
    // using the prefix "CustomButton" as defined in CustomButton component.
    @Import(CustomButton.Resources.Css.class)
    @Source("CustomToolbar.css")
    Css css();
  }

  private static final Resources RES = GWT.create(Resources.class);

  public CustomToolbar() {
    FlowPanel root = new FlowPanel();

    CustomButton okButton = new CustomButton();
    CustomButton cancelButton = new CustomButton();

    root.add(okButton);
    root.add(cancelButton);

    // Here we can add additional css classes to custom button.
    // However this only allows us to change css rules at the root element of custom button 
    // and its child elements. We cannot explicitly change the .icon css class of a 
    // custom button because the css class name is obfuscated at runtime.
    // 
    // In order to target the .icon class we need to import the CSS of custom button
    // into the context of CustomToolbar component. To do so we used the @Import 
    // statement above on the CustomToolbar.Resources.css() method.
    // 
    okButton.addStyleNames(RES.css().okButton());
    cancelButton.addStyleNames(RES.css().cancelButton());

    initWidget(root);
    setStyleName(RES.css().root());
  }

}


CustomToolbar.css:

.root {
  // toolbar 
}

// this only targets the root of custom button
.okButton,
.cancelButton {
  margin: 10px;
}

.okButton {
  background: green;
}

.cancelButton {
  background: grey;
}

// here we target explicitly the .icon css class of CustomButton
// by using syntax .<import-prefix>-<class-name>
.okButton .CustomButton-icon {
  color: pink;
}

While the example is pretty long it shows two things:

  • You can add css classes to the child component to adjust css rules at the root level of the child component (and possibly child elements, e.g. .root div)
    • It is a bit dangerous for child components that use display:flex and someone wants to use it as an inline block element. Using display: inline-block would break the child component layout. So the child component needs to defend itself against it. Either provide an inline version right away or wrap itself in a div as a layer of protection.
  • To target a specific css class inside a child component you need to import the child CSS into the parent CSS context.

I have used both patterns above extensively in GWT apps and it feels natural to import public CSS classes/api from a child component up into the parent component if you want to adjust some details.

So it is relatively similar to what @DeepDoge has written up in #6972 (comment) and I kind of like it. Maybe the naming convention in the example of the linked comment can be extended so that a component can indeed hide some css classes. Maybe by using __ as a prefix or similar for private css classes. I think this can be useful if components are a little more complex and need to ensure its general layout.

@DeepDoge
Copy link

BTW Svelte's style scoping doesn't work with new CSS features. So it seems that Svelte has to keep up with new css features. I think this can be problematic long term.

Svelte at least should have an option to use new css feature @scope in the complied CSS.

@jnehlmeier
Copy link

After reading a bit about the topic I feel like the svelte compiler should be enhanced to understand part attribute on HTML elements and exportparts attribute on HTML elements (web components) as well as Svelte components and handle them accordingly. Then it would be possible to use the ::part pseudo-element to target specific parts of a component and modify their styling. These attributes are defined in CSS Shadow Parts to support styling parts of web components.

Because Svelte components can also be compiled to web components it could work for them naturally.

However because we need a CSS reference to a component in order to use ::part I think it must be combined with the forward:class directive proposed by @aradalvand . In that case it would need to be important that forward:class can only be used on the root element of a component. As a consequence if a component author wants specific parts of a component to be accessible via ::part that component must have a root element.

In the following example I used suffix -private for all CSS classes that cannot be targeted from the outside. Suffix -public on the other hand can be targeted from the outside of the component.

PersonCard.svelte:

<div forward:class> <!-- extra element to protect against changing display:flex of the card -->
  <div class="person-card-private">
    <div class="picture-private">....</div>
    <div class="data-private">
      <div class="name-private">
        <span part="name-public">....</span>
      </div>
      <div class="address-private">
        <span part="address-public">...</span>
      </div>
    </div>
  </div>
<div>
<style>
  /* 
     *-private CSS classes to define the general structure of the component. 
     Cannot be modified easily from the outside.

     .person-card-private {
       display: flex;
     }
     ....
   */
</style>

Example.svelte:

<script>
  import PersonCard from './PersonCard.svelte';
</script>

<PersonCard class="my-person-card" class:inactive={someCondition}></PersonCard>

<style>
  .my-person-card {
    margin: 15px;
  }
  .my-person-card::part(name-public) {
    color: black;
  }
  .my-person-card::part(address-public) {
    color: #aaa;
  }
  .inactive {
    opacity: 0.5;
  }
</style>

The only downside is that a component author must define what can be changed. Basically these "parts" are part of the public API of a component.

If PersonCard would use a component itself, lets say an Address component then PersonCard can use exportparts on the nested Address component to export the Address parts, e.g.

...
    <div class="data-private">
      <div class="name-private">
        <span part="name-public">....</span>
      </div>
      <Address exportparts="street-public, city-public: person-city-public">
    </div>
...

The above makes street-public of the Address component accessible from outside of PersonCard using ::part(street-public). In addition it makes city-public accessible but renames it to person-city-public so it can be targeted as ::part(person-city-public). It works the same way if PersonComponent would use a nested web component that has parts defined.

References:

@Rich-Harris Rich-Harris added this to the 5.0 milestone Apr 2, 2024
@dummdidumm dummdidumm modified the milestones: 5.0, 5.x May 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
compiler Changes relating to the compiler feature request
Projects
None yet
Development

No branches or pull requests