Update the package using the clsx implementation?

[oliviertassinari]

clsx by @CLSx provides an interesting improvement over the actual classnames package implementation, it's faster and lighter. He has done an awesome job!

However, given how popular classnames is and the relatively small performance improvement of clsx compare to the other dependency we are dealing with (often ~10 kB gzipped & ~10k ops/s). I have some doubt about the advantage of migrating from classnames to clsx: mui/material-ui#14152. It's very likely that people will see classnames and clsx in their bundle, resulting in a negative outcome.

@JedWatson Do you have any plan updating this package implementation using clsx as inspiration? I think that it's a path that would really benefit the React community. Thank you.

[eps1lon]

The lighter link should include the classnames size. clsx has 76% of the size of classnames. You save 86B gzipped. People need to decide if this warrants a change.

[jquense]

If we don't think of this as replacing a package with another one, I think the concept here is pretty uncontroversial right? As in if someone showed up to one of my libs with a PR that made the package smaller and faster without changing the public API in any way, I'd likely merge it in, unless it made the code siginficantly harder to maintain.

In this case the existing code is already so minimal and fairly static at this point I don't think maintenance is a big concern, tho I won't presume for @JedWatson

It's always weird and presumptuous when someone shows up to your project say we should replace something. I hope tho in this case it stikes us like that because the library is so small already.

Obviously also this package is HEAVILY depended on so any change should be thought through :)

[TrySound]

@jquense And this is the problem. To not break so much existing packages dependant on this package it's easier to go with the new one. I see this PR and don't believe it will be ever merged.

[jquense]

It's easier for one project to go with something else, it's infinitely harder for the whole ecosystem to switch, which is what is required for there to be anything other than a net lose for most people

Like if the problem here is maintaince lets see if anyone is willing to transfer ownership? All I'm getting it is the market share of this package is such that evolving it's going to far more effective in the outcome than the alternatives. At least worth exhausting those options yeah? I mean we are all looking for the same practical outcome :)

[vladshcherbin]

There is also classcat package with the same purpose and benefits (size, speed, esm build, etc) so we might end up with bundles with classnames, classcat and clsx inside 🤦‍♂️

[oliviertassinari]

@vladshcherbin classcat has a different API. I wouldn't consider it a viable option.

[vladshcherbin]

@oliviertassinari yep, the api is different, but my point is same as yours - if there will be many packages with same purpose, a package creator can take any of them (depending on what google/npm/etc will give you first) so we might end up with bundles bloated with packages doing the same thing.

It would be really nice to see this package updated with best solutions from other packages instead of being replaced by clsx/classcat/etc in other packages.

[TrySound]

I don't see a problem here. Package authors should update to new major version of classnames (if it will be ever changed) or change it to clsx. In both cases you will get "bundle bloat".

[xobotyi]

@vladshcherbin or cnbuilder same API with classnames, ESM build, size, and speed comparable or superios the classcat.

[TrySound]

@xobotyi Why did you created it? Why not contribute to clsx? Why pollute npm with one more package?

[TrySound]

They exist because classnames cannot accept new features and new distributions. We already tried.
#150 (comment)

Nothing stopped you from talking with maintainer of clsx.

[xobotyi]

@TrySound shit%) instead of "edit" pressed "delete" =\

for the history, my comment was:

TrySound maybe because my one at least faster that clsx? correctly handle nulls and does not generate some insane output when unexpected input sneaked?

clsx(()=>{console.log("HELLO")}), null, "WORLD!"); // => "()=>{console.log("HELLO");} null WORLD!"

And if to follow your logic: why clsx created? why classcat created? why they're polluting npm while there were classnames?

The answer is because their authors thought that they creating something better than original lib.
clsx - faster on strings and objects input and ESM-compatible.
classcat - superiorly faster than classnames and clsx but has less handy API and ESM-compatible.
cnbuilder - has same API as classnames, but in some cases even superior classcat in speed, ESM-compatible and written in TS.

Nothing stopped you from talking with maintainer of clsx.

1. none will rewrite their lib to TS (cause it'll force to change the whole development pipeline).
2. i'll be the guy who made the PR but not the guy who made the lib.

[TrySound]

You probably missed the point of open source.

[xobotyi]

You probably missed the point of open source.

Oh i get the point right..
Changes i've planned to make were equal to creating whole thing from scratch (codebases completely different).
So I'm wasting my time - while working i'm getting money for it, while open-source developing i'm getting... lets call it "the fame".
If i'll make a PR - i wont be in collaborators or elsewhere on front page, only in commits and PR history. If there wont be my name on it - it completely does not worth my time.

The point of opensource is to workfor credits, instead of money. Work for completely nothing even if you joy it.. well.. it's quite stupid when you can have some benefit from it. Even if it wont work out - at least you've tried.

[JedWatson]

Thanks for raising this @oliviertassinari

I've read through the clsx implementation and have some questions - it is probably faster because it does a lot less safety checks (e.g skips hasOwnProperty and Array.isArray in favour of detecting .push on objects)

Things like that make me concerned because although it's faster and lighter, it's pretty easy to imagine those shortcuts causing problems out there in the real world.

So I wouldn't take the implementation directly but will see if any other optimisations can be made safely. If anyone else wants to suggest something please do.

As a side-note, someone kindly reached out to me and pointed out that I've been missing a bunch of notifications from this repo and it really deserves some love, so I'm fixing that ❤️

[oliviertassinari]

@JedWatson This sounds awesome!

[oliviertassinari]

For people interested in performance, they might enjoy https://github.com/merceyz/babel-plugin-optimize-clsx. We are using it on Material-UI, it has yielded a nice free small bundle size reduction.