Skip to content

changed classnames module with clsx #1064

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Nov 28, 2023

Conversation

NemeZZiZZ
Copy link

@NemeZZiZZ NemeZZiZZ commented Jul 24, 2023

This PR changes classnames dependecy to clsx - smaller and newer alternative
https://github.com/lukeed/clsx

@gabrieljablonski
Copy link
Member

gabrieljablonski commented Jul 24, 2023

Thanks for the great suggestion.

Personally, I'd leave this open and merge whenever we either launch v6 (probably a few months down the road), or make some other bigger changes which would make sense to include this one.

It probably wouldn't be that big of a deal overall, but it just feels weird to randomly switch out an entire package on a minor version just to save on 100-some bytes of bundlesize (if there were a breaking bug or something like that, it would be a different story).

@gabrieljablonski gabrieljablonski added the V6 It might get fixed/merged before, but most likely only on V6's release. label Aug 2, 2023
@ivan-palatov
Copy link
Contributor

ivan-palatov commented Oct 16, 2023

Might I suggest also changing the object style classes with the boolean comparison ones for a sligh performance boost clsx bench?

E.g. change

clsx('react-tooltip-arrow',
coreStyles['arrow'],
styles['arrow'],
classNameArrow, 
{ [coreStyles['noArrow']]: noArrow, }
)

To this:

clsx('react-tooltip-arrow',
coreStyles['arrow'],
styles['arrow'],
classNameArrow,
noArrow && coreStyles['noArrow'])

@danielbarion danielbarion changed the base branch from master to V6 November 22, 2023 13:25
@danielbarion
Copy link
Member

Hi guys,

We will start the development of V6 in our available time, so I created a new branch and I changed the target of this PR from master to V6.

Can you guys please fix the conflicts?

This should be merged soon.

Thanks!

@danielbarion danielbarion merged commit 2f05d22 into ReactTooltip:V6 Nov 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Enhancement V6 It might get fixed/merged before, but most likely only on V6's release.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants