Resolves #37695 - Use clsx instead of classnames

[tbradsha]

Resolves #37695 - Use clsx instead of classnames

Proposed changes:

clsx is faster and smaller than classnames. Also, it is now used in Calypso (Automattic/wp-calypso#91408), in Gutenberg (WordPress/gutenberg#61138), and in @wordpress packages.

For consistency, this PR matches those changes:

• a rule was added to prevent future use of classnames: 6b5518d
• classnames was replaced with clsx: b45ef16
• classnames/dedupe does not have an equivalent (a dedupe version? lukeed/clsx#13), so its one use was refactored using @wordpress/token-list: a060f54

Caveats:

• jetpack-mu-wpcom has its own implementation of classNames; I've left that intact for now. If desired, we can take a look in a separate PR.
• @automattic/social-previews is a dependency, and the latest published version (2.0.1-beta.13) still has a classnames dependency. This was removed in the aforementioned Calypso PR, but as of yet has not been published.

Internal reference: p4TIVU-aYc-p2

Other information:

• Have you written new tests for your changes, if applicable?
• Have you checked the E2E test CI results, and verified that your changes do not break them?
• Have you tested your changes on WordPress.com, if applicable (if so, you'll see a generated comment below with a script to run)?

Jetpack product discussion

Does this pull request change what data or activity we track or use?

Testing instructions:

Check the code for any typos or mistakes. Pay particularly close attention to the dedupe refactor.

[anomiex]

Looks reasonable to me. A few suggestions for simplifying the dedupe replacement inline.

[anomiex]

LGTM. Not hitting "Approve" just yet to give Agora a chance to review.

[DaniGuardiola]

Just a quick drive-by comment, I was the one who originally started this transition and was personally responsible for the Gutenberg PR. I managed to replace the dedupe usages without much complex logic or the new dependency you're using here. The code affected seems very similar, probably copy-pasted. I suggest you take a look at what I did there in case it helps you simplify this PR and achieve better consistency.

Note that I don't remember the exact approach I took, but I might have decided to simply not dedupe at all in some cases, since it doesn't really affect any functionality and the trade-off of doing the computation is probably not worth it in the first place.

Just a suggestion :) thanks for applying this transition here <3

[anomiex]

but I might have decided to simply not dedupe at all, since it doesn't really affect any functionality

You may want to double-check that.

$ node -e 'const classnames = require( "classnames" ); console.log( classnames( "aa bb cc", { bb: false } ) );'
aa bb cc
$ node -e 'const clsx = require( "clsx" ); console.log( clsx( "aa bb cc", { bb: false } ) );'
aa bb cc
$ node -e 'const classnames = require( "classnames/dedupe" ); console.log( classnames( "aa bb cc", { bb: false } ) );'
aa cc

[DaniGuardiola]

Ah, yep I replaced that behavior with a very very simple alternative. If you dig in my PR you'll find my approach. With that sentence in my comment I meant cases where the resulting class name is something like "class class".

[anomiex]

Glancing at your PR, the following seem likely to be broken to me

• https://github.com/WordPress/gutenberg/blob/200f7c7a210c35eb9c004da9a3e4f1062f471d57/packages/block-library/src/cover/edit.native.js#L301-L303
• https://github.com/WordPress/gutenberg/blob/200f7c7a210c35eb9c004da9a3e4f1062f471d57/packages/block-library/src/embed/embed-preview.js#L119-L121

Also, in https://github.com/WordPress/gutenberg/blob/200f7c7a210c35eb9c004da9a3e4f1062f471d57/packages/block-library/src/embed/util.js#L191-L193 you might mangle things if a class in outputClassNames has one of the classes in aspectRatioClassNames as a substring. For example, pass in "foo wp-has-aspect-ratio-xxx bar", you'll get back "foo -xxx bar".

[tbradsha]

I managed to replace the dedupe usages without much complex logic or the new dependency you're using here.

The one dedupe instance here required negation (removal of various classes). We don't use clsx or classnames for it. I'm not sure what new dependency you're referring to?

The code affected seems very similar, probably copy-pasted. I suggest you take a look at what I did there in case it helps you simplify this PR and achieve better consistency.

I can't speak to the origin of the code, but in this case it was an independent refactor; looking now at that other PR it seems the goal is slightly different, but luckily it isn't too complex!

[renatoagds]

Just got an eyes on VideoPress and AI stuffs. LGTM

[tbradsha]

Internal announcement: pdWQjU-Mt-p2