Appending classes to nested components

[shawnhwei]

<div>
  <SomeComponent class="test"></SomeComponent>
</div>

SomeComponent does not get the test class added.

This would be really helpful to have!

edit: doesn't work for id as well.

[mefjuu]

What's the problem doing this yourself in component's markup? E.g.:

<div class="{{className}}">

[shawnhwei]

@mefjuu Nothing really, it just seemed like a natural thing to have. It's a bit nitpicky but currently every component that you want to pass classes to will need be made with this in consideration.

[PaulBGD]

This is one of the things spreading #280 would help with as you can pass all attributes to the child.

[Conduitry]

Ehhhh I don't like the idea behind this. This is essentially saying that certain property/attribute names, when passed to components, will be treated specially. This feels like a gross change, and one that unnecessarily complicates the framework.

[paulocoghi]

I agree with @Conduitry

[Rich-Harris]

As do I — there are a few reasons not to do this. Firstly, a component can have an arbitrary number of top-level elements — what happens in those situations? Does every element receive those attributes?

Secondly, which attributes are applied? If the SomeComponent template looked like this...

<div>{{class}}</div>

...does the <div> still get the class='test' attribute, or not (because it's no longer 'extra')?

Thirdly, what if the component template already specifies a class?

<div class='foo'>...</div>

Does it become foo test? Is it overwritten to become test? Or does it stay as foo?

Fourthly, depending on the answers to the questions above, the implementation gets hairy — every component would have to have extra code to accommodate this behaviour whether or not it benefitted from it, which runs counter to Svelte's philosophy.

Fifthly, it takes control away from the component author — suddenly it's impossible to know how a component will actually be rendered while you're writing it.

Sixthly, it's unnecessary! As @mefjuu points out, this doesn't actually add any expressive power. If you really needed to add arbitrary styles to the top level elements of a child component, you could do it like this:

<div class='component-wrapper'>
  <SomeComponent/>
</div>

<style>
  .component-wrapper > * {
    color: blue;
  }
</style>

For all those reasons I'm afraid I have to close this as wontfix!

[bestguy]

While I agree it's best to not automatically append the class and id attributes, the other point is would be very nice for reusable components to be able to use the specified class attributes like so:

<MyComponent class="yeahWhatever"></MyComponent>

Then in the component definition use it like so:

<div class="my-component some-other {{class}} ">
  ...
</div>

This currently does not work, apparently due to class being reserved word.

While my example may not be convincing, I'm working on Svelte components for bootstrap , but users wrapping every component in a div to add classes is unwieldy, as is 'inventing' an attribute name such as 'className'. Would be great if svelte could provide a way to access easily.

[Conduitry]

Hm that is a fair point. There is precedent for using a className attribute for this (React; properties in the DOM itself) but it would be nice to be able to use {{class}} like that. However, I'm not sure how that could be added with the way Svelte is implemented now. As I understand it, currently it tries to parse {{ }} expressions as-is as js, and then later transforms the js as appropriate. So while class would end up as a property value on an object, which is perfectly legitimate in js, we're initially trying to parse it as an identifier, which is not allowed.

[PaulBGD]

Yeah I actually just ran into this, since class is a reserved keyword you can't use it as an attribute (or in a compute function)

[Rich-Harris]

I wonder if we could tweak readExpression to handle this special case:

src/parse/read/expression.js:

import { parseExpressionAt } from 'acorn';

export default function readExpression ( parser ) {
+	const start = parser.index;
+
+	const name = parser.readUntil( /\s*}}/ );
+	if ( /^\w$/.test( name ) ) {
+		parser.allowWhitespace();
+		parser.eat( '}}', true );
+		return {
+			type: 'Identifier',
+			start,
+			end: start + name.length,
+			name
+		};
+	}
+
+	parser.index = start;

	try {
		const node = parseExpressionAt( parser.template, parser.index );
		parser.index = node.end;

		// TODO check it's a valid expression. probably shouldn't have
		// [arrow] function expressions, etc

		return node;
	} catch ( err ) {
		parser.acornError( err );
	}
}

In other words, bypass Acorn altogether for straightforward cases like {{foo}} and {{class}}. You still wouldn't be able to do {{class.toLowerCase()}} or whatever, but maybe that's okay?

[Rich-Harris]

#383

[Rich-Harris]

Ah, whoops — just noticed @Conduitry reopened this before I created the new issue. I'll close this anyway since #383 is more specific