sveltejs / rfcs

RFCs for changes to Svelte
274 stars 81 forks source link

Passing CSS custom properties to components #13

Closed Rich-Harris closed 3 years ago

Rich-Harris commented 4 years ago

Rendered

rjharmon commented 4 years ago

combining alternatives, can we recommend making the component support style= property for theming, and that authors pass styles through to whatever real dom element(s) it sees fit?

<Slider style="--rail-color: black; --track-color: red" />

Conduitry commented 4 years ago

Hijacking something that's already a valid prop like style would be a breaking change, and so isn't something we'd want to do know.

Actually, I guess technically this RFC is a breaking change, but the probability that someone would be using a prop starting with -- and accessing it with $$props[...] and wouldn't want that to be hijacked like this is pretty remote. I think the intuitiveness of this syntax shouldn't be prevented just because it's technically breaking.

belated edit: I see that this 'technically breaking' thing was actually already acknowledged in the RFC and dismissed, which I think is quite reasonable.

Rich-Harris commented 4 years ago

I think the suggestion was for component authors to be in the habit of doing this:

<script>
  export let style = '';
</script>

<div style="{style}; color: purple">...</div>

I've added it to the Alternatives section in the RFC

mrkishi commented 4 years ago

I think I'm too worried about the explosion of variables, but that's what strikes me the most.

Even in the simple example used in the rfc, we're already missing at least a hover color. As in, any useable component that exposes a thing-color that's applied to something interactive will definitely need a thing-hover-color and possibly a thing-inactive-color, or the component won't be very robust.

halfnelson commented 4 years ago

This is the first instance we have of a property that gets auto-applied to child dom nodes. There are a couple of other use cases for this behaviour such as, spreading slot="name" to children which would allow slot directive on components, users may want to spread a class="material" or even class="disabled" to children, or onclick=()=>some_callback_in_parent(),

Since the style is going to generate a bunch of code for each element already, could we generalize this somehow? instead of style, either have some custom prefix or prop name to specify props to be applied to dom children eg

<Component $slot="name"/>
<!-- or  -->
<Component $childprops={{ slot: "name", class: "active", style; "--track-color: blue}}>

This would allow a more straight forward port of some other framework's components such as vue that always have a root element and passes down attributes https://vuejs.org/v2/guide/components-props.html#Non-Prop-Attributes https://vuejs.org/v2/guide/class-and-style.html#With-Components

PaulMaly commented 4 years ago

Not sure I like this PR more than do nothing. I don't use such kind of customizations of components in most of the cases to keep them isolated. In other cases, I use tools we already have (custom properties, special props, style forwarding etc) and don't see many problems with them. I'm sure I don't want to have additional code to be included in everyone's app.

pngwn commented 4 years ago

Despite being personally attacked, I'm mostly on board with this rfc. The thing that strikes me the most is that it feels reversed in some ways, almost implicit. The parent decides what to pass down and the child component can use it if they want to. Control is still in the hands of the child, which is a good thing but the 'style interface' feels more implicit than explicit. Maybe this is a reasonable trade-off.

Defining an explicit style interface would be cool but I'm not sure how it would work, it would certainly mean taking an approach to styles similar to our approach in the template, CSS+ or CSSx.

Something like this in the child:

<div>
  <p><a href="" />Hi<a/></p>
</div>

<style>
  @expose { 
    --thing-color: 'blue';
    --thing-hover-color: 'pink';
    --thing-active-visited: 'red';
  };

  a {
    color: var(--thing-color);
  }

  a:hover {
    color: var(--thing-hover-color);
  }

  a:visited {
    color: var(--thing-visited-color)
  }
</style>

Which would allow only those properties to be passed in by a parent and used anywhere in the component, subsequent children would probably need to define an interface as well. This maintains encapsulation and is an explicit interface but it is definitely more limiting:

<Child --thing-color="cyan"  --thing-hover-color="magenta"/>

One question I have about the RFC, would there be a specific way to define defaults for these css properties in case one wasn't passed down? If we define them in the component directly then surely the child would overwrite the passed-down values?

@rjharmon Another issue with just using export let style = '' is that when any of those styles changed everything would be reapplied as opposed to more granular updates. Unless they were specifically handled in which case a string seems a poor choice.

@mrkishi This is true to an extent no matter what route we choose, is the specific concern just a lot of styling being done directly in the template? I figure that spreading an object of css properties could be possible, maybe this would at least improve the ergonomics.

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

  const styles = {
    "--thing-color": 'blue',
    "--thing-hover-color": 'red',
    "--thing-inactive-color": 'pink',
    "--thing-border-width": '10px',
  } 
</script>

<Child {...styles} />
kevmodrome commented 4 years ago

I think putting the css properties directly on the component can get crazy quite quickly. Maybe it could be done in combination with something like this (for components that require much more extensive styling):

<style component="Slider">
    .potato-slider {
        color: green;
        ...
    }
    .potato-slider-rail {
        background: rebeccapurple;
        ...
    }
</style>

The idea is to have the classes merge properties with the classes in the child thus giving the parent full control of all styles while maintaining a somewhat readable HTML.

Rich-Harris commented 4 years ago

@halfnelson I worry about that being a bit of a Pandora's box β€” it throws encapsulation completely out the window. The nature of CSS custom properties is that they're inert unless the child chooses to do something with them, which wouldn't be the case for other things.

@pngwn

would there be a specific way to define defaults for these css properties in case one wasn't passed down? If we define them in the component directly then surely the child would overwrite the passed-down values?

Yes, in this situation the --fg property effectively isn't exposed, while the --bg property is exposed but has a fallback value, equivalent to export let bg = 'black':

<div style="--fg: red">
  ...
</div>

<style>
  div {
    color: var(--fg);
    background-color: var(--bg, 'black');
  }
</style>
rob-balfre commented 4 years ago

FYI svelte-select handles custom properties (CSS variables) like this... https://svelte.dev/repl/f3bc0fd6b6f74ea499e5ecb26911bf28?version=3.12.1

Rich-Harris commented 4 years ago

@kevmodrome That has the drawback of breaking encapsulation, and relying on volatile implementation details. You could still define custom properties in <style>, you'd just need a wrapper element (as you can do today):

<div>
  <Slider bind:value/>
</div>

<style>
  div {
    --rail-color: black;
    --thumb-color: red;
  }
</style>
Rich-Harris commented 4 years ago

@rob-balfre perfect, it's already an ecosystem convention! (hopefully the camelCase doesn't catch on though πŸ˜› ). So in essence, this RFC would enable the last example in that demo to change from

<style>
  .themed {
    --border: 3px solid blue;
    --borderRadius: 10px;
    --placeholderColor: blue;
  }
</style>

<div class="themed">
  <h2>Theming</h2>
  <Select {items}></Select> 
</div>

to

<h2>Theming</h2>
<Select
  {items}
  --border="3px solid blue"
  --borderRadius="10px"
  --placeholderColor="blue"
></Select>  
kevmodrome commented 4 years ago

@Rich-Harris

That makes sense. :)

What happens if there are 15 properties to style though (I suppose one could argue that should never happen in the first place, but still)? Wrapping the component probably is a workable solution, it just clutters up the code a bit. Personally I would want that away from the HTML (but then I'm a bit biased since I am a big fan of the styled components way of doing it).

halfnelson commented 4 years ago

for completeness I am adding a comment on how this can be currently achieved with Context with downside it adds some boilerplate to components that want to be styled this way, on the up side it doesn't impact the code generated for intermediate components

Inherited

<WithStyles --rail-color="black" --trac-color="red">.
..bunch of tags nested
<Slider />
</WithStyles>

WithStyles sets a context with the style props

direct

<Slider styles="--rail-color="blue" />

in slider

<script>
   import { inheritedStyles } from 'with-styles'
   export let styles;
   $: all_styles = inheritedStyles.apply(styles)
</script>
<div class="potato-track" style={$all_styles[--track-color]} />

inheritedStyles.apply fetches styles store from WithStyles context and derives a new store based on overriding with local styles.

Rich-Harris commented 4 years ago

@kevmodrome I'd expect that at that point you'd probably want those styles to be themed in a global.css file or similar β€” I'm anticipating this mainly being for limited, targeted overrides. But it's definitely possible that my imagination here is lacking

rob-balfre commented 4 years ago

@Rich-Harris ❀️πŸͺ

Looks good but what other main benefits does it bring other than removing clutter?

I presume the compiler would output the styles as custom properties? If thats the case then postCSS can handle IE11 support.

Rich-Harris commented 4 years ago

I presume the compiler would output the styles as custom properties?

Not exactly β€” it would apply them to top-level elements at runtime, using node.style.setProperty

pngwn commented 4 years ago

@Rich-Harris I'm not sure about that. With many design systems there will be a vareity of themes that are applied declaratively directly in the template. I dont know what that would look like in svelte but in other libraries you might do something like:

import { theme } from './themes.js';

export default () => (
  <SystemProvider theme={theme}>
    <Thing>Hello World!</Thing>
  </SystemProvider>
);

But something similar might be possible with the RFC proposal.

rob-balfre commented 4 years ago

I presume the compiler would output the styles as custom properties?

Not exactly β€” it would apply them to top-level elements at runtime, using node.style.setProperty

Hmm IE11 support is a must have for us - wish it wasn't but 30% of our clients (big corps etc) are still stuck on it. So with your approach you couldn't emit the CSS with the custom properties included?

Rich-Harris commented 4 years ago

The CSS would reference the custom properties, but they wouldn't have values (since those are set at runtime). IOW you'd be left with a) the fallback values, or b) whatever your preprocessor replaced the custom property references with (which is a totally legitimate solution that I think we should encourage, in situations where you know the values ahead of time)

korywka commented 4 years ago

If root component doesn't define hover, active, focus style properties (color, text-decoration, opacity, ...) we can't add them just with the help of CSS constants. Theming is not just changing colours, it is ability to re-define or define new CSS properties. Noting about that in rfc.

korywka commented 4 years ago

@Rich-Harris for more flexible solution something like that:

<script>
   import Slider from './slider.svelte';
</script>

<Slider />

<style data-scope="Slider">
   .rail { color: blue; }
   .rail:before { content: ''; display: block; ... }
   .track:hover { opacity: 0.5; }
</style>

under the hood it works like Object.assign: {...root styles, ...theme styles}

takoyaro commented 4 years ago

This is clever and is something I've been hoping for.


<Component --rail-color="rgba(20,122,255,0.8)"/>

Juste makes so much sense. 
I don't need anything fancy, just this is fine.
rob-balfre commented 4 years ago

The CSS would reference the custom properties, but they wouldn't have values (since those are set at runtime). IOW you'd be left with a) the fallback values, or b) whatever your preprocessor replaced the custom property references with (which is a totally legitimate solution that I think we should encourage, in situations where you know the values ahead of time)

Currently for IE11 support we use https://github.com/postcss/postcss-custom-properties but, after this change, would this do anything useful during Svelte preprocess?

rob-balfre commented 4 years ago

TIL I learnt about CSS Houdini ... https://youtu.be/-oyeaIirVC0?t=894 (Chrome dev conference)

lukeed commented 4 years ago

I maintain that it'd be amazing to share component context across into <style> tags. That way, we have full consistency allowing the entire component (scripts, template, and styles) to bind & be reactive to values.

Why am I bringing this (back) up now? Because 1) we're talking about CSS values being defined via props 2) props are one of our most common gateways into Svelte's reactivity

Right now, that's only feasible through this:

<script>
  let x=0, y=0;
  $: style = `transform: translate(${x}px, ${y}px)`;

  function onMove(ev) {
    x = ev.clientX;
    y = ev.clientY;
  }
</script>

<style>
  .hello {
    width: 100px;
    height: 100px;
    background: red;
  }
</style>

<svelte:window on:mousemove={onMove} />

<div class="hello" {style}>World</div>

Live: https://svelte.dev/repl/6b86d5f813e94107a8825cf4b6ed2730?version=3.12.1

This is fine, and is already leagues ahead of other options (because reasons), but we can do better.

Ideally, we define reactive CSS properties in the same way, using any variables from the <script> environments, just as the template/view would too. This includes store values, too, because they're also available to the rest of the component.

The compiler would hoist & map these reactive properties to a style property, invoking the update logic as needed. This is simply a couple new variables to track/change and sync them as part of existing $$invalidate calls.

Svelte would unequivocally (😏) reign supreme if we could define the same component like this:

<script>
  // using `transform` here because
  //  dunno how we'd want to do string interpolation
  //  inside the CSS layer: translate(${x}px, ${y}px) ?
  $: transform: `translate(${x}px, ${y}px)`;

  function onMove(ev) {
    x = ev.clientX;
    y = ev.clientY;
  }
</script>

<style>
  .hello {
    width: 100px;
    height: 100px;
    background: red;
    transform: $transform;
  }
</style>

<svelte:window on:mousemove={onMove} />

<div class="hello">World</div>

This should output something like this:

<style>
  .hello.svelte-xyz123 {
    width: 100px;
    height: 100px;
    background: red;
    transform: var(--xyz123-1);
  }
</style>

<!-- div.style.cssText = `--${cmp_hash}-${++attr_num}: ${local.transform}`; -->
<div class="hello svelte-xyz123" style="--xyz123-1: translate(5px, 10px)>World</div>

I shared another example, with conditional CSS values here. There I also mentioned what the output would look like, both in --legacy and modern variants, which I think is still important to consider.

lukeed commented 4 years ago

Oh, and this also means that accepting CSS values from props is as easy as this (easy, in the end-user sense):

<script>
  export let color = 'red';
</script>

<style>
  .foobar {
    color: $color;
    /* others */
  }
</style>
Rich-Harris commented 4 years ago

@bravecow not allowing component consumers to set arbitrary CSS properties is a feature, not a bug. Just like you can't mess around with a component's internal state (only its explicitly exported props), you shouldn't generally be mucking around with its styles, other than the ones that the component author has deliberately exposed.

@rob-balfre it would work as it does today, as though you had a wrapper element like this:

<div style="--foo: purple">
  <SomeComponent/>
</div>

PostCSS would be able to rewrite the color: var(--foo) declaration in your component styles, using whatever values it had available, but it wouldn't be able to see the purple in the inline style. This RFC doesn't change that, no. You'd need some other way to pipe in those values.


I've updated the RFC with a recent discovery (thanks @tanhauhau!) β€” we can use display: contents to avoid passing styles around at all. Instead, we can simply wrap components in an element that doesn't contribute to layout. This addresses @PaulMaly's concern (which I share) that this RFC would introduce a universal tax, making it pay-per-play instead. Details here.

Rich-Harris commented 4 years ago

@lukeed 🀯

I do still think CSS custom properties are better for theming, since they're something that can be set globally with local overrides (and the potential for naming conflicts isn't a big deal, since you're not going to use two design systems in a single app). But for dynamic stuff, that idea is pretty sexy.

One immediate hurdle: it's not valid CSS, at least according to css-tree: https://svelte.dev/repl/b3cab13f272a42c4a62b2a1b7d151b05?version=3.14.1

Assuming we could solve that (confident we can) I don't think we could accurately figure out which elements the styles would apply to in all cases (the current matching logic is necessarily fuzzy, because of things like class={whoknows}), so I imagine we'd probably just apply --xyz123-1 styles to top-level elements.

lukeed commented 4 years ago

@Rich-Harris That'd be a limitation on our CSS parser then. It's very much valid: https://codepen.io/lukeed/pen/vYYzjBN

Edit 1: It's not a limitation. It was just missing a closing " πŸ˜… https://svelte.dev/repl/f705224e073d44a3b454c74c6ef94d19?version=3.14.1

Edit 2: Lost track of REPLs.. oops
Having a CSS parser understand $thing shouldn't be too hard because we're already doing that with stores & their shorthand getters/setters. In plain JS, a free-standing $thing would be a syntax error too.

I think this is CSS theming, no? You export a theme-object prop, or individual property values. Either way, you provide default values for the default theme and then easily have a way to pass in props that change/override those defaults.

Remember, too, that part of the component's design is deciding what properties can/can't/shouldn't be configurable from the outside world. That's the current mantra, too.

Whether it's a theme-object prop or individual keys doesn't matter – that's a question of application/component design & I believe Svelte has no business in making that decision.

Because once you enable the ability to take a (JS) prop and turn it into a (CSS) value, we've won. Just need to open up that gateway and the developer is fully in control of any variant they see fit.

<Button
  theme={{
    font: 'Arial',
    fontSize: '16px',
    lineHeight: '1.6',
    primaryColor: 'blanchedalmond'
  }}
/>

<!-- versus -->

<Button
  font="Arial"
  fontSize="16px"
  lineHeight="1.6"
  primaryColor="blanchedalmond"
/>

<!-- If you are in a position -->
<!-- where you're stuck trying to make this decision -->
<!-- you've already won -->

Edit 3: πŸ™„

I think it's still just as easy/nice to have a global theme vs a customized theme.

If we're passing around a theme object, for example, this can live inside a store or any globally_ accessible JS object. After all, it is just an object in this case.

A lot of UI kits will define their theme in this way – like Vuetify. They have an internal set of defaults. You can then override theme values as part of your build config, but essentially this object just gets tossed around the application wherever needed. Components can locally override these at any time.

Alternatively, you can still take the (dynamic?)prop route and always plug in to the global theme. Nothing is lost & I actually think it gains much more πŸŽ‰

/* global.css */
<style>
html {
  --rail-color: black;
  --track-color: red;
}
</style>

<!-- Slider.svelte -->
<script>
  export let railColor = 'local default';
</script>

<style>
  .potato-slider-rail {
    /* this means that $railColor still produces a "--xyz123" for this component */
    background-color: var($railColor, var(--rail-color));
  }
</style>

All that's changed here is that the output no longer auto-wraps the reactive variable with var( and ). We instead expect the developer to specify the var($value) and any defaults.

However, this does lose the ability to have a seamless --legacy fallback since the use of var() is part of the true source code.

tanhauhau commented 4 years ago

I have a few questions πŸ™Œ :

// warn me that `--baz` is unknown props?
<Foo --bar="red" --baz="blue" />

// Foo.svelte
<div />
<style>
   div {
      color: var(--bar);
   }
</style>

<Foo --baz="blue" {...attr} />


because if we are going to wrap component with a `div` in compile time, it would be hard for do spreading, especially mixing between "actual props" and style props
rspieker commented 4 years ago

"The consumer of the component can override them" <Slider --track-color="goldenrod"/>

It feels like this may have the opposite effect of what is said to be the goal; making overrides easier compared to overrides from a global stylesheet. As this (and any direct name override) could break the option to override at all, especially if there's an education task/chapter involved.

Even worse, we'd then have a custom CSS property that may end up needing a lot more specificity to be overruled.

First something to get out of the way, the sentence "It's also brittle, since it treats internal (potentially changeable) implementation details as a public API." seems to skip past the fact that custom CSS properties suffer from the same "brittleness", as they are both a public API and as easy to change as an "implementation detail" like class names.

I'm hoping that a consensus can be found in not trying to teach implementers to set up proper inheritance syntax, as that could (should?) be a task for the Svelte compiler.

Looking at the example suggested with "teaching the component author"

<style>
  .potato-slider-rail {
    background-color: var(--rail-color, var(--potato-theme-color, 'purple'));
  }
</style>

I would think it may be possible for the compiler to figure this out. So an authored component could look like:

<style>
.potato-slider-rail {
    background-color: var(--potato-theme-color, 'purple');
}
</style>

And (if an intent to override by the consumer is present) could be generated as

<style>/* for the component containing the intent to override/hint at a new default*/
.svelte-a3bmb2 {
    --svelte-a3bmb2-potato-theme-color: green;
}
</style>
<style>/* for the component being overridden/hinted to new default values */
.potato-slider-rail {
    background-color: var(--potato-theme-color, var(--svelte-a3bmb2-potato-theme-color, purple));
}
</style>

At a functional level this resembles the syntax used by the svelte-select component mentioned by @rob-balfre

Obviously the added byte-size may come into play at this point (which I think is gzipped just fine) though the compiler could be made aware of anything the component consumer actually passes in, reducing the number of wrapped variables. If there's no intent to override the values from a higher up component, there's no point in providing the wrapper and override. Furthermore I think the override value using the generated class/variable prefix can actually be handled by the component indicating the desire for overrides of the default values.

As for how to specify those overrides; I think it has to be absolutely clear its mechanics are different from setting the style attribute so I would recommend against using any normal (as in: HTML specified) attribute or element. Perhaps this can be a task for <svelte:options> as that would be explicit and hopefully easier to pick up on by both the component author and compiler.

<Slider><svelte:options custom-css-property="--track-color" value="goldenrod" /></Slider>
korywka commented 4 years ago

not allowing component consumers to set arbitrary CSS properties is a feature, not a bug. Just like you can't mess around with a component's internal state (only its explicitly exported props), you shouldn't generally be mucking around with its styles, other than the ones that the component author has deliberately exposed.

Reasonable. I think it’s worth mentioning this in the rfc.

giuseppeg commented 4 years ago

@Rich-Harris fwiw I recently wrote a proposal for styled-jsx that is similar to @lukeed's (https://github.com/sveltejs/rfcs/pull/13#issuecomment-553215007) https://github.com/zeit/styled-jsx/issues/595 I think that's a valid approach.

Rich-Harris commented 4 years ago

@lukeed I was referring to transform: $transform being invalid, but it does look like current CSS parsers can cope with this, so hopefully that's a non-issue.

I do think we're talking about two separate things, both desirable and not at all mutually exclusive. When I talk about global themes, I mean global themes in CSS files, rather than JS files (and for a reasonably-sized design system we could be talking about a lot of theme). Aside from meaning we get to have less stuff in JavaScript, it enables a nicer workflow: open devtools, tweak --potato-theme-color on the root html element, and see the entire app respond.

With this...

<script>
  export let railColor = 'local default';
</script>

<style>
  .potato-slider-rail {
    /* this means that $railColor still produces a "--xyz123" for this component */
    background-color: var($railColor, var(--rail-color));
  }
</style>

...the local default would always take precedence over the global theme colour. And --xyz123 lacks any of the semantic hints that are so useful when you're tweaking stuff in devtools.

Implementation-wise, there's an advantage to treating style properties as syntactic sugar. While this...

<div style="display: contents; --rail-color: black; --track-color: red">
  <Slider
    bind:value
    min={0}
    max={100}
  />
</div>

...does mean an extra DOM element, the compiler can trivially know whether those values will ever change (and in the common case, they won't), and avoid wasting work during update. If the Slider component itself is the one responsible for managing CSS vars based on props that are passed in, it has to assume the worst, and always be checking to see if the vars need updating.

Whether it's a theme-object prop or individual keys doesn't matter – that's a question of application/component design & I believe Svelte has no business in making that decision.

I agree that a framework should be flexible enough to support different styles of development, but I also think there's great value in promoting ecosystem norms, particularly when there are performance/workflow gains to be had. I think that if we say, loudly, 'this is how theming works in Svelte', we make it much easier and more appealing for design systems to flourish in our ecosystem. At the moment there's no 'right' way to solve these problems.

Summary: why-not-both.gif?


@tanhauhau

would passing an unknown css variables get the "Passing unknown props" warning?

Not in this RFC's current state β€” we'd basically be in the same situation we are today if you manually create the wrapper element with the style attribute that the proposed syntax would desugar to.

@pngwn's @expose idea would make it possible to change that. It's a little tricky though since custom properties are inherited β€” the immediate child might not expose a given property, but it could contain a child of its own that does.

will it be spreadable?

I don't see a good way to make that happen, no.


@rspieker

Even worse, we'd then have a custom CSS property that may end up needing a lot more specificity to be overruled.

Not sure I understand what you're saying β€” specificity isn't a concept that applies to custom properties, only selectors.

seems to skip past the fact that custom CSS properties suffer from the same "brittleness", as they are both a public API and as easy to change as an "implementation detail" like class names.

The point is that as a component author you're being explicit that the custom properties are public API, and therefore don't change except in a major version. The same can't be said for arbitrary selectors (of which classnames are just one example), unless the entire structure of your markup is considered public API, which is unrealistic.


@bravecow Added this: 52e20b91ef5b301bd4b3f2a9461b929ac05aca0c


@giuseppeg great minds think alike!

o-t-w commented 4 years ago

While have divs everywhere isn't ideal, I think this is a great API that should ship ASAP.

mrkishi commented 4 years ago

As long as we're desugaring to a display: contents container, what if we also allowed it on elements for consistency?

<Component --color="red">
...turns into...
<svelte-css style="display: contents; --color: red">
    <Component/>
</svelte-css>

<div --color="red"><Component/></div>
...turns into...
<div style="color: red"><Component/></div>

edit: nevermind this!
<div><Component --color="red"></div>
...also turns into the above, since it's an only-child.

edit: disregard the only-child thing; it could mess the parent.

lukeed commented 4 years ago

...the local default would always take precedence over the global theme colour

@Rich-Harris Right, because I told it to! I admit, not a super well-thought example, but I (the dev) defined that behavior.

If I want to conditionally override with local value, then this:

<script>
  export let railColor = '';
</script>

<style>
  .potato-slider-rail {
    /* ternary override for local preference, else global */
    /* Note: playing with syntax. Maybe $prop: signifies reactivity? */
    $background-color: $railColor ? var($railColor) : var(--rail-color);
  }
</style>

I love CSS vars and the often use the workflow you're describing. That's also how all the templates in PWA are configured too. I think this flow is very compatible with that overall workflow too.


Theming is fine. I understand the feature and necessity, especially in terms of CSS vars. I agree that these ideas aren't mutually exclusive at all.

However, I think we need to fully enable exported-props-to-css without any div soup. That's not what I, the dev, ordered. And as mentioned elsewhere, this doesn't have a legacy fallback. We can't put something so critical behind a steep support wall.

I personally still have clients (plural) on IE9 and IE10. Monsters do exist.

When I pass down CSS values, as defined by my component's public interface, I decide how/where that gets used.

To get around this, I'd actually like to see theming be its own component. Explicit, opt-in, Svelte-specific behavior: <svelte:theme />

With this, I can define a local theme for whatever's inside of it. And visually, I can see what inherits what from where.

The output would still rely on display: contents, sure, but it's not arbitrarily sprinkled all throughout my application AND my components are (potentially) pulling down new values for their style vars.

Here's mostly the same - renamed so easier to type - example component, with that ternary conditional, but used 3 different ways:

html {
  --color: red;
}
<!-- Text.svelte -->
<script>
  export let color = '';
</script>

<style>
  p {
    $color: $color ? var($color) : var(--color);
  }
</style>

<p>{ slot }</p>
<!-- App.svelte -->
<Text>I will be red, via global</Text>

<Text color="blue">I will be blue, via local</Text>

<svelte:theme --color="yellow">
  <Text>I will be yellow, via theme</Text>

  <!-- everything else here sees same theme values -->

  <!-- but I can still override if I want to! -->
  <Text color="green">I will be green, via local</Text>
</svelte:theme>

This feels most natural & reads the best. As would the output. And it feels like Svelte vocabulary already.

arxpoetica commented 4 years ago

πŸƒ it may be preferable to use a made-up element name instead

For real safe-guarding, give it an obscure name, one that people would be unlikely to employ as made-up DOM. You know, like <svyles>.

Rich-Harris commented 4 years ago

@lukeed what would this get compiled to? Can you use arbitrary JS here, or just ternaries?

<script>
  export let railColor = '';
</script>

<style>
  .potato-slider-rail {
    /* ternary override for local preference, else global */
    /* Note: playing with syntax. Maybe $prop: signifies reactivity? */
    $background-color: $railColor ? var($railColor) : var(--rail-color);
  }
</style>
PatrickG commented 4 years ago

It somehow feels wrong to set styles to the component directly. It's like how people have written HTML before CSS was a thing.

What about classes, but only allow custom properties. Like so:

<style>
  .my-little-class {
    --rail-color: red;
  }
</style>

<Slider class="my-little-class" />

The result would be the same

<div style="display: contents; --rail-color: red;">...</div>

I know this can be done with a wrapping div, but i don't like to wrap components just to target them in the css

mrkishi commented 4 years ago

With the current proposal, we're still somewhat conflating state and styles (via --var={value}). While that's useful in itself, it sounds like it'd be more versatile with the name-only substitution thing lukeed appeared to have brought up at first (before the js-in-css can of worms!).

What about restricting desugaring to css only? Orthogonal proposals could then mix state and styles via regular props.

selectors Component {
    --background-color: white;
}

/* turns into... */

selectors .svelte-123123.component {
    display: contents;
    --background-color: white;
}

The component selector would have to be the last one, of course, and .svelte-123123.component would target the generated container.

lukeed commented 4 years ago

Sorry, I'm juggling a few ideas once here and they're overlapping a bit. Let me backtrack a bit and focus solely on <style> tag reactivity.


Yes, I'd like reactive properties to be computed in JS and, at compile time, know that their result will be dumped into a specific variable name.

<style>
.foo {
  $color: /*do something reactive, will be a variable */
}
/* output as */
.foo {
  color: var(--xyz123-1);
}

The above could also be written as:

<script>
  $: color_value = /* same reactive stuff as before */
</script>
<style>
  .foo {
    /* syntax undecided, of course */
    $color: $color_value;
  }
</style>

Now, what is color_value? That's where things shift a bit.

In a --legacy build, we can't inject any new var() code. So we'd be injecting the value directly to the color property. ~> style="color: red" As values update, we write the new property value directly to the DOM node once again; ~> style="color: blue"

In a modern build, we are free to speak the language of var()s. So there, we use the component hashid & the property incrementer to generate a CSS var ID ahead of time. I've been using xyz123-1 in my examples for this value.

Because we now have this pre-determined variable ID, and we also control/are aware of the styles, we inject the variable reader in the static stylesheet! ~> color: var(--xyz123-1); As values, update, we write the variable's new value (remember, static ID) to the DOM node; ~> style="--xyz123-1: blue"

Either way, the value is evaluated in JS as part of the normal/current reactivity cycle. It's just a different style target that receives the updated values – it's either a variable or the property itself.

I have live demos of equivalent behaviors for both modern & legacy outputs. The idea here is that these snippets are how the compiler would preprocess/interpret the new ideas.

As you can see, we're really just talking about syntax sugar... shorthands. We're already able to do exactly what I've described, functionally, but I think we can also get Svelte to wire up these as shortcuts for us, making this entire RFC so much easier to use & understand.

korywka commented 4 years ago

display: contents still have one disadvantage at least. If these tags will be inserted by Svelte, user can't treat component's HTML as his own. He should care about that div:

https://svelte.dev/repl/82dc593e315d48eba53768ae0d347765?version=3.14.0

PatrickG commented 4 years ago

Like @bravecow im also not very comfortable by inserting a div for this. Maybe it would be best, to apply this feature only to components which have a single root element, so you can set the custom properties to this root element.

korywka commented 4 years ago

@Rich-Harris How about something like that.

Slider.svelte

/* variables declared inside component or outside (:root). doesn't matter */
.slider {
  --rail-color: while;
  --track-color: yellow;
  --other-color: green;
}
.rail {
  width: 100%;
  height: 3px;
  background: var(--rail-color);
}
.track {
  width: 10px;
  height: 10px;
  border-radius: 50%;
  background: var(--track-color);
  border-color: var(--other-color);
}

App.svelte

<Slider
  bind:value
  min={0}
  max={100}
  --rail-color="black"
  --track-color="red"
/>

Do replace:

  1. var(--rail-color) to black
  2. var(--track-color) to red

Do cleanup:

  1. Find all CSS variables declared in component's style scope:
  2. --rail-color used? (inside var) No. Remove declaration.
  3. --track-color used? (inside var) No. Remove declaration.
  4. --other-color used? (inside var) Yes. Do not remove.

Create new classes for .rail and .track with changed colors. Ideally, if a new Slider will be called with the same constants, it should use already created classes.


Disadvantage: Slider child components (for example SliderRail.svelte and SliderTrack.svelte will not inherit these variables). But compiler I think can fix that (create new css classes for child components too if they use the same css variables).

Rich-Harris commented 4 years ago

@mrkishi there's a couple of drawbacks to the class proposal: firstly, it doesn't allow for anything dynamic (like <Foo --bar={baz}>, where baz changes), unless coupled with @lukeed's proposal (even then there would be limitations because values would have to be component-level β€” no different styles within an each block). Secondly, it would introduce confusion over which declarations applied:

selectors Component {
  /* this works... */
  --background-color: white;

  /* and so does this, because inheritance */
  font-size: 24px;

  /* but this doesn't, because display: contents */
  box-shadow: 2px 2px 2px black;
}

You could mitigate that with compiler warnings, but it starts to feel a bit uncanny valley I think.


@lukeed Unfortunately I'm not sure the idea for supporting legacy environments by setting inline styles would work. Imagine a case like this:

<div class={sometimes_foo}>...</div>
<div class={othertimes_foo}>...</div>

<style>
  .foo {
    $color: /*do something reactive, will be a variable */
  }
</style>

We couldn't blindly do div1.style.color = $color and div2.style.color = $color. We would have to get more sophisticated:

if (div1.matches(selector)) div1.style.color = $color;
if (div2.matches(other_selector)) div2.style.color = $color;

That would work, but now we have to add/remove those styles if the classes change. We could do that. It would be pretty complicated to figure out, but not nearly as much as when you have compound selectors, or :hover/:focus, or pseudo-elements, or :global selectors that reference something outside the component, in which case I guess you have to start hacking around with MutationObserver? In short, the complexity quickly spirals to a level that I think is unrealistic.

Furthermore I don't think we can put JavaScript in our CSS willy-nilly. Earlier you posted this example:

.potato-slider-rail {
  $background-color: $railColor ? var($railColor) : var(--rail-color);
}

$railColor ? var($railColor) : var(--rail-color) isn't valid JavaScript, and in fact most CSS tokens aren't valid JavaScript. There's no reasonable way to parse it. So it would have to be something more like this...

.potato-slider-rail {
  $background-color: $railColor ? `var(${railColor})` : `var(--rail-color)`;
}

...which starts to look pretty gross! I think we'd be much better off sticking with your original idea, albeit with a small syntax modification to accommodate store values β€” using square brackets to indicate 'computed' values:

.potato-slider-rail {
  background-color: [color];
}

This doesn't (AFAICT) give us a way to solve the shadowing problem, of course. So while I genuinely love this proposal and want us to implement it, I don't think it solves the problem this RFC addresses. We need both.


@PatrickG

Maybe it would be best, to apply this feature only to components which have a single root element

The consumer of the component shouldn't know (or care about) that sort of implementation detail


@bravecow

Yes, users of this feature would be expected to know about the mechanism by which it's implemented β€” it would be clearly communicated in the tutorials and docs etc.

Not sure I understand your proposal, I'm afraid. Bear in mind it needs to account for style values that change (possibly frequently).

korywka commented 4 years ago

Ah, didn't take into account that styles should be reactive.

Can someone point me out why simple rename will not work?

Slider.svelte

<style>
  /* default vars are declared inside component's parent Node */
  .slider {
    --rail-color: green;
    --track-color: blue;
  }
  .rail {
    width: 100%;
    height: 3px;
    background: var(--rail-color);
  }
  .track {
    width: 50px;
    height: 50px;
    background: var(--track-color);
  }
</style>

App.svelte

<!-- suffix cat -->
<Slider
  bind:value
  min={0}
  max={100}
  --rail-color="black"
  --track-color="red"
/>

<!-- suffix dog -->
<Slider
  bind:value
  min={0}
  max={100}
  --rail-color="back"
  --track-color="yellow"
/>

will do:

:global {
  /* slider 1 */
  --rail-color-cat: black;
  --track-color-cat: red;
  /* slider 2 */
  --rail-color-dog: black;
  --track-color-dog: yellow;
}

/* slider 1 */
.rail.-cat {
  background: var(--rail-color-cat);
}
.track.-cat {
  background: var(--track-color-cat);
}

/* slider 2 */
.rail.-dog {
  background: var(--rail-color-dog);
}
.track.-dog {
  background: var(--track-color-dog);
}
Rich-Harris commented 4 years ago

@bravecow because now you need coordination between the components, which means extra code for everyone β€” the compiler operates on a single component at a time, so any coordination requires runtime overhead.


Had a realisation about the simple (name-only) form of the JS-in-CSS idea:

<style>
  .foo {
    color: [foreground];
  }
</style>

This would desugar to transform: var(--svelte-xyz123-foreground) which is nice and easy to work with in devtools, compared to a more random-looking hash.

lukeed commented 4 years ago

I'm not sure if I agree/understand with the spiraling complexity bit. All this would desugar into reactive declarations that we can already do today.

In my mind, it's basically a preprocessor step that turns the CSS shorthand into the slightly larger Svelte component, then compiles it.

I like the square brackets.

And I'm fine with tossing that ternary example, but mostly because it wasn't consistent with what color means. It should always be the value (legacy) or the var(--id). I'd rewrite that example more like this:

.potato-slider-rail {
  background-color: [color] || var(--rail-color);
  /* Or maybe */
  background-color: [color || 'var(--rail-color)'];
}

Don't intend for this bit to replace the RFC at all. Definitely supplements. My only comments about this RFC are the legacy support and the <svelte:theme> suggestion.