Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Replace classnames with clsx #1236

Merged
merged 4 commits into from
Sep 12, 2019
Merged

Replace classnames with clsx #1236

merged 4 commits into from
Sep 12, 2019

Conversation

TrySound
Copy link
Collaborator

A few points

  • more performant
  • smaller without iife
  • esm support without hacky conversion to commonjs with module wrapper

Refs https://unpkg.com/classnames@2.2.6/index.js, https://twitter.com/MaterialUI/status/1092928715804299264

@jquense
Copy link
Owner

jquense commented Feb 23, 2019 •
edited
Loading

If there is a problem with classnames wouldn't it make WAY more sense to PR to classnames instead of trying to boil the ocean in the react ecosystem to switch packages?

@TrySound
Copy link
Collaborator Author

It would be good. The problem is that classnames package is stuck. So I just started migrating all packages to clsx. material-ui, material-ui-pickers and react-virtualzied are already migrated.
JedWatson/classnames#150 (comment)

@stale
Copy link

stale bot commented May 23, 2019

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the wontfix label May 23, 2019
@TrySound
Copy link
Collaborator Author

Reactivate.

@stale stale bot removed the wontfix label May 23, 2019
@TrySound
Copy link
Collaborator Author

TrySound commented Jun 9, 2019

classnames package still is not active with this topic. clsx becomes more and more popular. @jquense what do you think?

@stale
Copy link

stale bot commented Aug 9, 2019

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the wontfix label Aug 9, 2019
@TrySound
Copy link
Collaborator Author

TrySound commented Aug 9, 2019

Keep it please

@stale stale bot removed the wontfix label Aug 9, 2019
@TrySound
Copy link
Collaborator Author

@jquense If you ok to go with clsx I can help to migrate all your packages. Just give me a list of them.

@TrySound TrySound requested a review from jquense September 12, 2019 10:28
@TrySound
Copy link
Collaborator Author

Still no movements in classnames. Can we go with this guys @jquense @taion ?

jquense
jquense approved these changes Sep 12, 2019
View reviewed changes
Copy link
Owner

@jquense jquense left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sure

@TrySound TrySound merged commit eb6b770 into jquense:master Sep 12, 2019
@TrySound TrySound deleted the clsx branch September 12, 2019 16:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jquense jquense jquense approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@TrySound @jquense