[yuchi]

Your little utility here doesn’t cover the whole 'classnames' API.

  export default function classNames(...args) {
    return args.filter(valid).map(single);
  }

  function valid(arg) {
    return arg && (!Array.isArray(arg) || arg.length);
  }

  function single(arg) {
    if (typeof arg === 'string') {
      return arg;
    } else if (Array.isArray(arg)) {
      return classNames(...arg);
    } else {
      return Object.keys(arg).filter(k => ………

This is getting similar…

[anonytrary]

> Your little utility here doesn’t cover the whole 'classnames' API.

Thats partly the point. The whole API doesn't really need to be covered in order to achieve the same utility. I think the library clearly over-engineered the simple task of string concatenation, and their API is redundant.

Their API caters to too many styles and that increases its complexity. It takes 2 seconds to grok the two lines in my utility, whereas it takes 5 minutes to make sure you know all 50 combinatorial ways to concatenate a string with the classNames library. There is too much indecision in the API, so they decided to support everything they could think of. Now I have to be ready for the possibility that the 5 people on my team can't agree on a convention simply because the library allows every possible convention one could desire.

[jhall1468]

You don't "agree" on a convention, they have different use cases. Sometimes you'll want to merge a bunch of class names so you use array-style. Other times you have class names that have prop dependencies so you use object style and finally you'll want to merge in some always-on defaults so you have string style. You'll often need to combine them, depending on use-case.

That's a whole lot of complaints for 50 lines of code.

[anonytrary]

> Sometimes you'll want to merge a bunch of class names so you use array-style

> Other times you have class names that have prop dependencies so you use object style

> merge in some always-on defaults so you have string style

There's some irony here -- all of this just to avoid dealing with string templates:

  `${class1} ${class2} ${class3}`

> You'll often need to combine them

Need is definitely not the right word here. The classNames utility is a completely unnecessary abstraction.

[jhall1468]

> There's some irony here -- all of this just to avoid dealing with string templates:

By that argument, we can just avoid everything by not making our apps dynamic!

> The classNames utility is a completely unnecessary abstraction.

The only necessary abstraction in programming is assembly since 0's and 1's are hard to reason about. The "need" in this case refers to needing to mix different primitives to make your app dynamic. Your arbitrary example above doesn't do anything because you still have conditional logic to determine what class1 class2 and class3 are, you are just doing a bunch of ugly crap in the render function rather than abstracting it. Or implementing your own version because you're smart.

It's a stupid hill to die on here... you want to argue about importing the entire lodash library because you like .chain, have at it. But I guess you can keep patting yourself on the back for avoiding importing a no-dependency 50-line library.

[wild_preference]

You want `undefined` and `null` to show up in class names? You'd at least need `${class1||''}` and your solution will grow in cases from there despite your one-liner.

Your solution doesn't do anything I'd expect someone wants that lib for, like merging in a map of classes.

I don't think this lib is the best pyre to die on to make a point I generally agree with.

[yuchi]

Ok, no one is shoveling it down your throat. Just avoid it.

But be prepared because it is the de-facto standard way of building classNames in React, also because it was part of the standard utilities provided by React.