Introduce `<style module>` — update: `forward` directive

aradalvand commented 2 years ago

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

TheOnlyTails commented 2 years ago

How would this compare to using the :global selector?

MacFJA commented 2 years ago

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

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

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

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

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

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

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:

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

nosovk commented 2 years ago

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

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

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

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

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

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

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: https://github.com/sveltejs/rfcs/pull/22#issuecomment-664047806

aradalvand commented 2 years ago

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

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

pngwn commented 2 years ago

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

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

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

@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 https://github.com/sveltejs/svelte/issues/6972#issuecomment-987920097 But also this is discouraging https://github.com/sveltejs/svelte/issues/6972#issuecomment-987903813

TheOnlyTails commented 2 years ago

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.