Replace classnames with clsx #61138
Replace classnames with clsx #61138
Conversation
DaniGuardiola
requested review from
ellatrix,
fluiddot,
dcalhoun,
talldan,
ajitbohra,
gziolo,
ntwb,
nerrad,
draganescu,
adamziel,
kevin940726,
getdave,
alexstine,
tellthemachines,
artemiomorales,
michalczaplinski,
geriux,
ryanwelcher,
juanmaguitar,
fabiankaegy and
ndiego
as code owners
|
@dcalhoun FYI the bundle size comment finally got posted 🎉 #61138 (comment) |
| const classes = classnames( | ||
| 'components-focal-point-picker__icon_container' | ||
| ); | ||
| const classes = clsx( 'components-focal-point-picker__icon_container' ); |
There was a problem hiding this comment.
For some of those, I wonder why we even need a classnames/clsx utility. Guess it can be removed.
There was a problem hiding this comment.
True, this one's honestly kind of funny lol. Guessing there was a point in using it before and then it was updated and they just didn't remove the classnames call.
| @@ -94,8 +94,7 @@ export const SVG = forwardRef( | |||
| const appliedProps = { | |||
| ...props, | |||
| className: | |||
| classnames( className, { 'is-pressed': isPressed } ) || | |||
| undefined, | |||
| clsx( className, { 'is-pressed': isPressed } ) || undefined, | |||
There was a problem hiding this comment.
I know it was here before, but do we really need the || undefined?
There was a problem hiding this comment.
Yeah makes no sense. Honestly I didn't really look into these, I just search-replaced.
DaniGuardiola
added
the
[Type] Code Quality
|
It looks like this PR is the first one making a change to the I tried rolling that back the update to actions/setup-node, and and it worked for me (#61289). I also moved the debugging steps we added above to that PR to keep this one specific to the task at hand. |
|
Thanks @desrosj, I'm gonna remove the unrelated CI debug changes in order to merge the PR. |
…gnore/clsx-size-test
DaniGuardiola
changed the title
DaniGuardiola
added
Needs Dev Note
PR originally started as a test, see: #61091 (comment)
Then it replaced #61091, see: #61091 (comment)
There are no significant changes in this PR that weren't already part of the previous PR, I only synced with trunk and ensured that all usage of classnames was still consistently replaced with cslx.