Pretty neat, actually.
Pretty neat, actually.
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.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.
Don't know about you, but we have actual goals and I'd never meet them if I tried your method.
Not quite, that would imply time and effort. I'm talking about reinventing the toothpick (2-3 lines of code), which is much different than reinventing a complex library for the sake of reinventing it.
> Rolling your own diffing algorithm is exactly what actual engineering entails
That's nice, but you're addressing a straw man. No one actually said rolling your own diffing algorithm is not what engineering entails. It was said that no one is suggesting you recreate a non-trivial dependency for the sake of reducing your dependency count.
I can use it in an example if it helps: If you are hired to write a basic website and you choose React, then you probably shouldn't reinvent React as your first step in writing the website. React is a fine dependency because it does a lot.
Humor me -- the next time you use classNames for a small project, try and see if you can't replace all of your calls to classNames with very basic template strings or other trivial code. It should not take any effort. For example I've seen things similar to "classNames({myClass: true})" in code before. Can you see what is wrong with this line? When people get comfortable with abstractions, they completely forget how to write the simplest code.
Re-inventing 1000 versions of a tooth pick is reinventing the wheel.
> That's nice, but you're addressing a straw man.
You keep using that term and I'm not sure you understand what it means.
> It was said that no one is suggesting you recreate a non-trivial dependency for the sake of reducing your dependency count.
And my argument is that dependency count is one of the dumbest metrics to be interested in. Size, sure as that impacts bundle size, which in-turn, effects latency. But "count" is meaningless.
> Humor me -- the next time you use classNames for a small project
I use styled-components exclusively for React projects now.
> For example I've seen things similar to "classNames({myClass: true})" in code before.
So? Bad code is bad, that's not a library failure. Now that is a straw man.
> Can you see what is wrong with this line?
Of course I can, because I'm actually good at my job. I also wouldn't do that. Using your toaster in a bathtub is also REALLY stupid, but that doesn't mean we should ban toasters.
> When people get comfortable with abstractions, they completely forget how to write the simplest code.
No that's called laziness and it has nothing to do with dependencies. You have twice in this thread accused me of straw men inaccurately, and now wrote a paragraph dedicated to a huge straw man.
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.
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.
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…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.
That's a whole lot of complaints for 50 lines of code.
> 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 themNeed is definitely not the right word here. The classNames utility is a completely unnecessary abstraction.
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.
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.
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.
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...