class: directives do not work when used on elements with both bind:this and prop spreading

[mtlewis]

I've noticed that when elements have the bind:this directive as well as props spread onto the element, class: directives do not result in the appropriate class being applied. This can be worked around by using a class attribute with an expression (e.g. class={primary ? 'primary' : ''} works, but class:primary does not).

Here's a demonstration of the issue. You'll note that the anchor will be green as expected if you do any of the following three things:

• delete line 3 (bind:this={anchor}),
• delete line 4 ({...{ other: 'props' }}), or
• change line 2 to class={primary ? 'primary' : ''}.

Thanks for all the great work you've done on this library!

[EmilTholin]

Hi @mtlewis! Thank you so much for this great research and example.

The reason why it's not working is because the component fragment change handler contains this code:

p: function update(changed, ctx) {
  // ...

  set_attributes(a, get_spread_update(a_levels, [
    { other: "props" },
    { href: "/foo" },
    { class: "svelte-1qq9alg" }
  ]));

  if (changed.primary) {
    toggle_class(a, "primary", primary);
  }
}

set_attribute will result in the anchor tag having the class svelte-1qq9alg, but since primary hasn't changed it will not be applied to the anchor tag again.

TL;DR: I think it boils down to a bug in how Svelte generates code for class: directives.

The reason why it works when you...

1. Delete the bind:this={anchor}
   ... is because anchor can only be given the DOM node as value after the component has been rendered, so the component is made dirty and the update handler above is called to give anchor its value. Without bind:this={anchor} the change handler isn't called after the initial render, and primary is therefore never overwritten.

2. Delete the {...{ other: 'props' }}
   ... is because there is no longer a set_attributes call in the update handler that overrides the class svelte-1qq9alg primary with just svelte-1qq9alg like the code above.

3. Change class:primary to class={primary ? 'primary' : ''}
   ... is because the class gets added directly to the set_attributes argument rather than added in a toggle_class call like above, so primary is never overwritten.

set_attributes(a, get_spread_update(a_levels, [
  (changed.primary) && { class: "" + (primary ? 'primary' : '') + " svelte-1qq9alg" },
  { other: 'props' },
  { href: "/foo" }
]));

[TheBosZ]

Is this the same as the following error?

Error: ValidationError: Classes can only be applied to DOM elements, not components (56:31)
54:     <div class="btn-group" role="group">
55:       {#each Object.keys(types) as type}
56:         <Button on:click={setType} class:active="{types[type] === endpoint.type}" value={types[type]}>{type}</Button>
                                       ^
57:       {/each}
58:     </div>
    at preproces[omitted]

Essentially, I'm working on Bootstrap components and need the button to be "active" (have the class "active") when the type matches. Also, for some reason, I can't get it to work if I use the computed class:

<Button on:click={setType} class="{types[type] === endpoint.type ? 'active' : ''}" value={types[type]}>{type}</Button>

Should I open a new issue for this?

[varholak-peter]

@TheBosZ Does it make sense to pass the class itself into a component when CSS is component scoped? 🤔
My solution would be to pass the computed value into the component directly and handle whatever class you need to include in the component in there. Something like:

// Parent
<Button on:click={setType} active={types[type] === endpoint.type} value={types[type]}>{type}</Button>

// Button.js
<script>
  export let active = false;
</script>

<button class:active>Kobzol</button>

[TheBosZ]

Unfortunately, this is for a Bootstrap library where we need to allow any classes to be set on the component itself.

It looks like classes on components don't really work the way I would expect. I can open a new ticket with it.

[varholak-peter]

In that case, I believe the most viable option for you would be to implement it similarly to how React handles passing down classes to components:

// Parent
<Button className={types[type] === endpoint.type ? 'active' : ''}>{type}</Button>

// Button.js
<script>
  export let className = '';
</script>

<button class={className}>Kobzol</button>

You have to use className (or something similar) since class is a reserved word so this would be the most viable option in my opinion, given we didn't add a custom Svelte keyword for class-drilling.

[TheBosZ]

To try to put it in my own words, the special class handling for Svelte is really only useful for regular DOM elements, not components.

Is that correct?

If that's the case, what's the point of the docs mentioning how to use class as a prop like this?

// you can use export { ... as ... } to have
// props whose names are reserved keywords
let clazz;
export { clazz as class };

[EmilTholin]

@varholak-peter @TheBosZ I don't think this discussion is relevant for the original issue. Consider opening up a new issue to discuss the possibility of adding support for the class: directive on components.

[TheBosZ]

Sorry to pollute this ticket! I figured it out: I need to mark things as reactive.

Still not used to that!

[Conduitry]

Fixed back in 3.6.8 via #3242.

[mtlewis]

@Conduitry I think this should be reopened - I just retested in 3.6.8, and the issue is still occurring in my example above.

[mtlewis]

Seems to still be broken in latest master. I just opened #3668, which adds a failing test for this issue.

[Conduitry]

It looks like #3421 was another form of this same issue, and testing locally, this is also fixed by #3781 (merged but not yet released).