Class directives for components

[RedHatter]

This PR implements class: directives for components. This is especially useful for components that are a simple wrapper around a dom element.

<script>
  import Link from './Link.svelte'
  let active = true
</script>

<Link class:active>Hello</Link>

<script>
  let className
  export { className as class }
</script>
<a class={className}><slot /></a>

[dimfeld]

I played around with the tests a bit in your branch. It looks like this specifically alters the class attribute on the component, and doesn't, by itself, change anything about the rendering. Is that right?

That is, if I remove class={className} from the Child component, and/or remove the class/className prop entirely, then the class:active syntax doesn't have any effect since nothing is rendering the class prop anywhere. I think that's fine and probably even preferred, but probably worth documenting somewhere.

edit: also thank you for implementing this. I'll definitely find it useful :)

[RedHatter]

Yes, that's right. It gives the consuming component freedom of what to actually do with the classes.

[antony]

Thank you for your PR - but I'm a bit confused about the purpose of this PR.

Couldn't you just implement the same thing by yourself with a prop called "class"?

I'm not sure this PR is the right solution for a number of reasons, but feel free to correct me:

• The syntax looks very similar to the class directive which already exists, but the behaviour is completely different, which is a bit confusing.
  You still have to implement half of the code yourself, meaning that code which used it might have undefined behaviour as a side-effect of trying to pass a "class" to a dom element
• The code doesn't guarantee where in the child component the class will be applied, so as an API, it's usage would be a bit mystery-meat anyway.
• Because of the above, it wouldn't be useful for third-party components, because you could never guarantee it's behaviour or implementation.
• Therefore only place the attribute would be useful would be in code where you control both the parent and the child, in which case you could just use a prop?

[dimfeld]

My take is that it's essentially an shortcut to let you write class:active instead of class={active ? 'active' : ''}, in the same way that the you can for the class attribute on a DOM element in Svelte right now.

The "when does this apply" question does present some challenges. On the one hand, it's an unofficial but widely-used practice to do let classNames=''; export { classNames as class }; in components and then apply that class on the top-level DOM element in the component. On the other hand, there are plenty of components out there that don't use this idiom, or use it on other elements than the outermost, and so it could lead to confusion when an officially-supported construct doesn't apply everywhere or in the same fashion. Presumably dev mode warnings on nonexistent properties and good documentation would help some here, but it's not a panacea.

There are also challenges in changing this feature to support components that don't export a class property. The naive solution would be to apply the given classes on all of the top-level DOM elements of the component. That becomes tricky when there is more than one top-level element and doubly so if those elements are generated via an #each. There's also the question of how components are treated differently if they have their own class prop or not. So we probably end up with too many edge cases here, both from a technical perspective and for developer experience.

In my mind, the primary argument for this is that it's a very common thing to need and cleaner than the current alternative of string manipulation or wrapping the child component in a <div class:active><Child /></div>.

Having thought through a lot of this as I write this comment. I like the convenience of this shorthand and the consistency of being able to use class: on both DOM elements and components, but I certainly see the issues in other inconsistencies that this additional syntax raises. So I'm unsure of how I feel about it now.

[RedHatter]

I've added code that warn's in dev mode if a class directive is used on a component that doesn't export a class property. This will erroneously warn if the the component spreads props instead of explicitly passing the class but I'm not sure if not warning when $$props or $$restProps is used is the best approach here.

@antony I've tried my best to answer your concerns and better explain my thought process.

Couldn't you just implement the same thing by yourself with a prop called "class"?

Yes and no. This implementation already does require a class prop. Technically I could have the same functionality by preforming the string manipulation myself user side but it's not as clean and can have a surprising number of edge cases when implemented naively.

The primary motivation for this change is to have the same behavior between dom elements and wrapper components. Class directives are extremely convenient but that convenience is lost when a section of code needs to be converted to a component.

The nice thing with this implementation is that it doesn't have any overhead or modified behavior for code that doesn't use the feature. If you don't like, no harm done, you can just ignore it.

The syntax looks very similar to the class directive which already exists, but the behaviour is completely different, which is a bit confusing.

I don't know I would say that the behavior is any different. The implementation is different sure, but the user facing functionality is the same: conditionally modifying the class "attribute". Yes components handle that class differently than dom elements but from the users perspective the directive is still doing the same thing. Additionally in most cases where the class prop is applied to the root dom element than the end result is also the same.

You still have to implement half of the code yourself, meaning that code which used it might have undefined behaviour as a side-effect of trying to pass a "class" to a dom element

You're not trying to pass a class to a dom element. You're passing a class to a component. It's up to the component to define what that means for the components use case. In most cases it would be passed to a dom element.

From the components point of view it's no different than if I were to explicitly pass a class prop.

<Link class={classnames({ active })}>Click me</Link>

Any side-effects this causes would have to be documented by the component but again this is no different than the current situation.

Because of the above, it wouldn't be useful for third-party components, because you could never guarantee it's behaviour or implementation.

Warning when a component doesn't export a class property should help with this. It still doesn't guarantee implementation but it would no longer silently fail.

It sounds like most of the objections revolve around the idea that this might not work exactly the same for all components. The thing is, this is true for any framework feature a component uses. The framework gives component authors the tools to write components, and to write the api to consume those components, it's up to the authors themselves to use those tools to the fullest and implement best practices.

Passing the class prop to the root dom element is already a wide spread practice and I think a section in the tutorial encouraging component authors to do so as well and good documentation of this feature would make this a very useful feature to have.

[AhmadMayo]

Adding my 2 cents to the discussion. Adding a class prop to a component doesn't necessarily mean it should apply the style to the root element, but it makes sense that it should apply it the main element visually. Let's take a modal component as an example.
A modal component might look like this:

<div class="overlay">
  <div class="wrapper">
    <div class="modal">
      <div class="modal-header">...</div>
      <div class="modal-body">...</div>
      <div class="modal-footer">...</div>
    </div>
  </div>
</div>

When a user passes a class prop, I think we all agree that the normal behavior is to apply it to the modal div, not the overlay one. So letting the component developer decide how to use the prop is the best thing to do IMHO.

And normally, accepting a class prop is documented in the component's props, so passing a class prop to a component that doesn't utilize it, and see no effect is not surprising at all, in the sense that passing any undocumented prop normally mean that it will have no effect.

[milkbump]

I'm a bit against this.

Svelte currently has no opinion as to what you store in an exported class prop of a component. It will not necessarily be used a class attribute.

Also, what if I want to name my class prop rainbow or tree (...assume I have a good technical reason)? It's a convention in Svelte to export { className as class } inspired from docs, but it's certainly not required by the compiler, so I don't think the class:directive can/should be assumed here.

I'd be less hesitant about component class directives if either:

1. This PR implemented a general anyprop:directive, but I don't yet see a use case for this outside of class attributes
2. Or if we formally took a stance that the class prop is THE ordained way to pass class attributes, though I don't think this functionality warrants this restriction.

This might be nit-picky, but I think a valid concern.

[Conduitry]

Sorry to let this PR hang around for so long, but I'm going to close this. I don't think we want to do anything to make people think we have special handling for the class prop on components. It's more explicit and more flexible to use separate props for this - or to have an object with boolean values - and then to do whatever you want with the value inside the component.