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.

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

chore: replace chalk with colorette #189

Closed
wants to merge 1 commit into from

Conversation

bryanjtc
Copy link
Contributor

@bryanjtc bryanjtc commented Mar 2, 2023

Colorette is faster and smaller than chalk. With this change there will be a performance gain.

@skovy
Copy link
Owner

skovy commented Mar 14, 2023

I'm curious if this will have a material impact on the performance of this tool? The log level can be turned down / off so very few logs are printed meaning this would result in almost no performance benefit since it's not used?

From a cost/benefit perspective, I see the cost of this introducing potential bugs/edge cases that don't exist today because this is all new code. Whereas the benefit is minimal (or zero if no logs)?

I'd rather leave the code that is working fine as it is unless we have data to prove otherwise. chalk is also more widely used and maintained.

@bryanjtc
Copy link
Contributor Author

That's reasonable. I will look into measuring the performance of the tool. There should be a way to see if the tool is faster when a change is made.

@RoystonS
Copy link

It's also worth looking at the relative sustainability, number of contributors, etc for the packages.
e.g. https://snyk.io/advisor/npm-package/chalk and https://snyk.io/advisor/npm-package/colorette

@skovy
Copy link
Owner

skovy commented Mar 16, 2023

Yes, thank you @RoystonS - for the listed reasons above and general health rating I believe it makes sense to stick with the current package due to its benefits and no known downsides in practice.

@bryanjtc bryanjtc closed this Jun 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants