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

Local <style scoped> #32

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Local <style scoped> #32

wants to merge 2 commits into from

Conversation

Rich-Harris
Copy link
Member

@Rich-Harris Rich-Harris changed the title add local styles RFC Local <style scoped> Sep 10, 2020
@pngwn
Copy link
Member

pngwn commented Sep 10, 2020

I'm concerned that this will make components incredibly confusing to reason about. I think it improves the authoring experience at the cost of worsening the reading experience.

Currently it is very easy to work out where the styles are defined, there is only one place that can happen. If you use the Svelte extension for VSCode then that is guaranteed consistent, it will be at the bottom or top or middle depending on your settings.

With this proposal we can have various style elements littered throughout a component. Not only do we have to find them all but we also have to reason about their scope and specificity. This might be easily 'teachable' (which i'm not convinced of) but that is an issue in and of itself: style tags don't currently need to be taught outside of 'this is scoped and safe'. Requiring some documentation on how styles are scoped and how deeper styles interact with higher level styles introduces significant complexity to what is an incredibly simple mental model right now.

@antony
Copy link
Member

antony commented Sep 10, 2020

I'm also not in favour of this. I think it also becomes confusing because the notion of scoped on a tag implies that the top level tag would be unscoped, which is not true. This would be the case if you were using Vue, which makes users who are using both or migrating, extra confused.

I once liked the notion of manual scoping and multiple style tags:

<h1>Hello</h1>
<div scope="b">
  <h1>World</h1>
</div>

<style>
  h1 {
    color: red;
  }
</style>

<style scope="b">
  h1 {
    color: blue;
  }
</style>

however, I think we gain a lot from the simplicity of single file components with three known tags in them, and it would be a great shame to lose this.

I think if people are building megacomponents so large that we need to have multiple scopes for styling within them, then it should be encouraged that the onus is upon them to architect their frontend code better.

@Rich-Harris
Copy link
Member Author

it will be at the bottom or top or middle depending on your settings

oh? i've been trying to figure out how to get it to be at the bottom 😬

I think it also becomes confusing because the notion of scoped on a tag implies that the top level tag would be unscoped, which is not true. This would be the case if you were using Vue, which makes users who are using both or migrating, extra confused.

Fair. Does <style local> solve that at all?

@lukeed
Copy link
Member

lukeed commented Sep 10, 2020

I don't like this either TBH:

  1. Style blocks are scoped by default already, making <style scoped> meaningfully redundant, and gives the impression that top-level <style>s aren't scoped. (Edit: sorry, same as @antony's first point)

  2. RE:

    with the priority determined by the order of the declarations in the outputted CSS

    Yes, we use CSS. I thought a Svelte mantra was to use the existing language(s). This is fundamental CSS

  3. All examples are more legible and easier to author using the namespaced equivalents. The "namespace" class is literally the equivalent of what the <style scope> produces, it's just saving you from choosing a classname (and in some cases, a container). The most "challenging" of the examples presented is this:

    {#if condition}
      <p class="green">condition green</p>
    {:else}
      <p class="red">condition red</p>
    {/if}
  4. The only place where I see extra <style> tags making sense is inside of inline components (Inline components #34) – and even then, that'd be a single <style> tag (no scope) to define the component.

@antony
Copy link
Member

antony commented Sep 10, 2020

I think it also becomes confusing because the notion of scoped on a tag implies that the top level tag would be unscoped, which is not true. This would be the case if you were using Vue, which makes users who are using both or migrating, extra confused.

Fair. Does <style local> solve that at all?

Yes, that makes more sense to me when I'm seeing a style tag in the middle of my markup, but only to the extent of making sense of a style tag in the middle of my markup... which doesn't feel like it makes much sense, right now :)

@non25
Copy link

non25 commented Sep 10, 2020

Personally I would do nothing.

I feel like the correct solution is to encourage developers to have smaller components and keep things DRY.

Developers are discouraged to use leaf components like buttons, checkboxes, radios, inputs, because of the need to worry about interface (passing attributes, making visual variants), passing relevant events and styling them (the most important is positioning in the parent component).

So they just slap tags instead of making leaf components to save time and that's how you get to huge components.

I see two ways of styling tags which are not satisfactory:

  1. Making css global outside of components for tags where you could use leaf components with styles in them (you need to choose classnames carefully)
  2. Writing css in any component that uses tags instead of leaf components (components have more extra css, possibly duplicated)

@ryanatkn
Copy link

it will be at the bottom or top or middle depending on your settings

oh? i've been trying to figure out how to get it to be at the bottom 😬

see svelteSortOrder in prettier-plugin-svelte

@PaulMaly
Copy link

Actually, I'm with the other commenters - not sure the problem exists. In my projects on Svelte, we adhere to the "budget" of the component in 200 loc with styles. If the component goes these limits, we just take out styles in a separate file using svelte-preprocess. But this is not often the case.

However, we've another unresolved problem - passing parent's styles to child components. Perhaps, we can steal a similar approach to solve that problem?

// Parent.svelte

<Child />

<style scope="Child">
  .passing-class { ... } 
</style>

OR even:

<Child>
  <style>
     .passing-class { ... } 
  </style>
</Child>

// Child.svelte

<div class="passing-class">
 ...
</div>

I'm not sure is it good or not, but it looks obvious enough to me.

@firefish5000
Copy link

firefish5000 commented Sep 27, 2020

I'm with the others here, not sure the issue exists. And if it does, this seems more like aiding the problem(you like giant components? Let me help you!) than solving it.

What I understand to be the goal of this, eliminating the abuse of class, seems a bit unnecessary to me. But assuming that this is a noble goal worthy of further thought. I am not sure this is the solution. I admit with a larger, long/deeply nested codebase or more complex style, this might start to make sense. But with the simple examples provided, I would definitely prefer classes 100% of the time for readability.

But the most notable issue to me is this introduces reasons to put css inside the markup section which is a visually offsetting. When typing HTML, devs tend to put most (all if they can) styles/scripts at the top of the document inside head, or bottom at the end of body for a reason. It is easier to read as the document is separated by language, and we always know where to look for scripts/styles. This rfc adds&legitimizes reasons to break this coding convention!
I still don't quite get the use case, but I will say that imho putting styles closer to the markup it changes does not make code more readable when the language/syntax is switching mid file, and giving reasons to do so seems counterproductive on the front of improving readability, especially if multiple devs works on the codebase.
Even if svelte/vue have js in their markup, they make it as minimal as possible. That is why we do almost everything in script and, as rule of thumb, do not typically introduce multi-line js blocks inside the markup section. Even if you want to override 1 directive of 1 element, I really can't see a well formatted style tag achieving that.

However, we've another unresolved problem - passing parent's styles to child components. Perhaps, we can steal a similar approach to solve that problem?

This issue I understand! Using .wraperDiv>:global(.childClass) is not ideal since it can hit everything under it, including slots we pass to Child.

Using this approach to solve it does make sense, especially with that slotted syntax. But I would prefer to somehow integrate it into that 1 style tag if at all possible for the same reasons outlined above.

<style>
:Child(.works-exactly-like-global-but-only-hits-Child-component)
:component-Child(.same>.as>.above)
:template-TemplateName(.works-exactly-like-global-but-only-hits-svelte-template-contents)
</style>

Hopefully something like those is usable, though it is a bit more limiting than slotted style tag approach suggestion. It would need to only work on components that are imported, using the name we gave it on import. Ideally it would work even if we don't use the component in our markup (that is, it should be applicable to nested slots). Regardless, scoped styling of child components is certainly something I hope to see improvement on.
Now svelte:template looks interesting! I had been looking for that functionality for times when creating another component doesn't quite make sense

@non25
Copy link

non25 commented Sep 28, 2020

@firefish5000

Personally, I like this approach better: sveltejs/svelte#2888 (comment)
It is also very small in the form of a patch for a compiled version.
This makes component style composition easier and with event forwarding completely negates stated problem.
The reason that makes ending up with big components is that for now it is much easier to work with tags.
You don't need to think about styling APIs with verbose css custom properties, you can use use:, on:event, class: and friends.
Any time you are wrapping a button in a Button component to encapsulate styles and DRY some logic, you are losing all that.

I think I finally understood why they are afraid of passing scoped classes to a components.
They made complex/wildcard selectors with hashing to get regular css feel.

.someclass * {
  margin-top: .5rem;
}

which in return generates

.someclass .svelte-hash {
  margin-top: .5rem;
}

And this will make .svelte-hash, passed to components below .someclass with some other class to catch this style.

So it is not an approach problem, it is svelte class hashing implementation problem.

The way to be completely safe with passing scoped classes to components requires not relying on safe wildcard selectors.

Anyway, if you are using some of the wildcard selectors, it makes sense to :global it to get predictable behavior, because this can be confusing:

<style>
  h1 + * {
    margin-top: .5rem;
  }
</style>

<h1>Test</h1>
<Component />
h1 + .svelte-hash {
  margin-top: .5rem;
}

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

Successfully merging this pull request may close these issues.

None yet

8 participants