Allow className to be an array of strings

[chenglou]

FIxes #1179.
className={['a', null, 'b']} becomes 'a b'.
This allows you to do className={['a'].concat(this.props.className)}.
Note that nested arrays are currently forbidden.

[cpojer]

why are you not just using "joinClasses"?

[syranide]

Shouldn't we allow objects as well then?

Also, 👍

Kind of a side-note, wouldn't the same thing be useful for style as well? padding: [0, 0, 0, 10]

[chenglou]

@cpojer
Edit: nvm.

[syranide]

Spoke with @chenglou in chat a bit, and he seemed to agree. I feel like supporting only arrays and strings, but not objects ({A: true, B: true, C: false}), is really arbitrary. Especially as I understand it, you use the object-approach exclusively internally at FB to allow minifying of class names (that's my impression from seeing previous discussions on the subject at least).

Sure, the classSet addon exists, but it's cumbersome and as far as I understand it, the primary reason why it's an addon is because you use cx to identify class name objects basically. So to me it seems like add another addon classArray (or whatever) or support strings, arrays and objects. Because this middle-ground seems really really arbitrary to me.

cc @zpao

[petehunt]

Is this the return of classSet? How about we just have injectable magic props?

[chenglou]

The fact that ['a'].concat(this.props.className) works with both this.props.className as a string and as an array, and is joined correctly, feels not right lol. Interesting, but not right.
I think @sebmarkbage has legit arguments for it though.

[sebmarkbage]

It should be called classList since this corresponds to the classList DOM property. That also allows an incremental upgrade path.

This should use the same type check as concat, namely it should not do an Array.isArray check but rather allow array like objects, and later iterators.

if (typeof n === 'string' || typeof n === 'number' || typeof n === 'boolean') {
  // string
} else if (n == null) {
  // empty
} else {
  // array like
}

Otherwise it has different semantics than the proposed combinator syntax:

<div classList={['a'].concat(this.props.classList)} />

This also excludes the object notation of classSet but that has other issues (objects as map is bad, mkay) and is difficult to statically type. We do use similar features in a helper but this has it's own static analysis. You can't use something like closure compiler to minify classnames. That would be dangerous without having the same logic applied to your CSS files.

I'd like properties to be compatible with generic deep merging functions so that we can use or add language features for this.

It's not clear whether we should even allow both strings/arrays or just arrays because we're not sure we can express this union type well in common type systems. Recursive union types are even less common which is why we'd like to wait on nested arrays. Additionally when this is used with a plain deep merge, it's not clear how you merge a string and an array unless we assume that such values are concatenated in a merge. You also can't merge two strings. That would suddenly mean that two strings are not equivalent to two arrays.

The only valid data structure here is an ES6 Set. Anything else is just sugar which has to pull it's weight.

[syranide]

@sebmarkbage An idea is perhaps to implement classList to the spec then and "require" that you use a ES6 Set-shim if you want to use it?

[sophiebits]

Are we replacing className with classList? If not, transferring props manually gets hairy because you don't know which property actually holds the classes (and what if you specify both?).

Honestly, these don't feel that different to me:

<div classList={['a'].concat(this.props.classList)} />
<div classList={['a', this.props.classList]} />
<div className={'a ' + this.props.className} />

so I'm in favor of leaving things as-is.

[sebmarkbage]

The idea is to replace it, yes.

[sophiebits]

@zpao @sebmarkbage What's happening here?

[chenglou]

tl; dr version is that transforming all these to array is too expensive internally

[sebmarkbage]

Let's have this discussion in react-future so that it's consistent with other plans.
We can leave it open though. reactjs/react-future#11

[brianblocker]

👍 for the ability to pass in className as an array

[sophiebits]

We're not planning to do this for now so I'm going to close it out – we can always reopen later if we do want it.

[Rob-pw]

I preferred the passing an array approach, rather than plopping a load of strings in an attribute and joining them together with spaces. If any of the items in the array is not a valid string, that item won't be added to the classes.