[yuchi]

While I can understand your skepticism on the first one, the second package is actually very useful. It doesn't just concatenate strings filtering out falsy values, it supports the whole old AngularJS class format. You can, for example, pass an object {string:booleanish} and it will add only the keys whose values are truthy.

Pretty neat, actually.

[anonytrary]

> You can, for example, pass an object {string:booleanish} and it will add only the keys whose values are truthy.

Why is this something you can't do on your own in a few lines of code?

  const classNames = (o, c=[]) => {
    for (let k in o) if (o[k]) c.push(k) // booleanish
    return c.join(" ")
  } 

This is not cool at all! Why import a trivial function over NPM to do this for you? Better to lower your dependency count and have a "util.js" file for helpers like this.

[franciscop]

I am starting to follow Sindre's philosophy [1] more and more. I published this morning `fch` [2], a 10-15 LoC library (note: undocumented for now). I will test it well and make sure it works everywhere. In exchange, I removed basically the same code from 3 projects (so far), some of them with subtle bugs.

The cost of using it is the same as locally, and creation cost is lower per-project if you let npm manage your own libs. Plus, bug fixes are automatically propagated.

[1] https://github.com/sindresorhus/ama/issues/10#issuecomment-1...

[2] https://github.com/franciscop/fetch

[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.

[darekkay]

I get the "dependency hell" issue in Javascript world, but why would I rewrite this particular utility? It's well tested (both unit and field tested), includes edge cases and has zero dependencies. As a dependency, I will also get any future bug fixes or features if I wish to. An even better example is lodash. Would you really rewrite every util you need just for the sake of not having external dependencies?

[Aeolun]

Probably because the scope is so small that you _could_ actually rewrite it yourself. And then together with all these other toy libraries, stick it in one big one that covers everything you need in terms of convenience functions.

[jhall1468]

So much this. It's 30 lines of code. The remaining code is comment fluff and 15 lines dedicated to the various types of export utilities (AMD, NPM, window). Seriously this is such an odd package to start these arguments with.

[alexchamberlain]

Playing devil's advocate, why is lowering your dependency count a good thing?

[bthornbury]

Dependencies have cost. You have to monitor for updates, notify the maintainer(s) of any bugs, keep an eye out for security vulnerabilities, and sometimes (gasp) even step through them with a debugger.

Doing that for one dependency is bad enough, but for 100s it's a nightmare.

Personally, I prefer to just pull the pieces I need out of an open source library (unless it's very well maintained, or huge). It's like doing a code review at the same time, so you're aware of what's going on in your application.

[jhall1468]

Copy/pasting has a nearly identical cost (aside from notifying the maintainer), except you have to catch all of them, you removed an easy upgrade path when other people catch the issues and you're spending vastly more time "code reviewing" every dependency you have rather than actually producing the software you're trying to write.

Don't know about you, but we have actual goals and I'd never meet them if I tried your method.

[anonytrary]

I think he might be talking about trivial dependencies, like leftpad, classNames, arrify, etc. In the case of trivial dependencies, rolling your 30-line "util.js" file to remove 3 dependencies is definitely worth it. No one is suggesting you roll your own diffing algorithm.

[jhall1468]

I think I already responded to you elsewhere, but in case I'm incorrect: No it is not worth it. You are re-inventing the wheel so you can pat yourself on the back. Nobody hires a software engineer to write leftpad. Rolling your own diffing algorithm is exactly what actual engineering entails.

[giobox]

> Personally, I prefer to just pull the pieces I need out of an open source library (unless it's very well maintained, or huge). It's like doing a code review at the same time, so you're aware of what's going on in your application.

To quote yourself: Doing that for one dependency is bad enough, but for 100s it's a nightmare.

'Forking' a project's dependencies (which is effectively what this is...) is often fraught with as much trouble as just having the damn dependency in the first place.

[Aeolun]

I think the thing is that the one library you are forking contains 50 frivolous ones, so you’d really only be doing it twice.

[sumnulu]

Replace “dependency” with “code / function” in your post and you will see the problem.

[orhanhenrik]

What’s nice about it is that a lot of projects use the same util, meaning it is very easy to read or edit if you are familiar with the library. If everybody writes a custom function you will need to learn how to do classNames separately in every project you work on.

[BigJono]

Yeah this argument falls flat in the case of classnames where we're comparing reading a two liner function with reading a page of docs for a library.

It's like everyone in JS land is allergic to writing a piece of code that isn't just playing lego with things written by a real developer.

[nathan_f77]

Yeah, my React code is currently littered with conditions and `classNames.join(' ')`. I'm going to start using this library.

[sumnulu]

In ideal world all functions should be pure and be a dependency and all code should be just pure functions.

[siftikha]

Second one is used only once in the code base to concat two strings together afaik