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 picocolors #15487

Draft
wants to merge 39 commits into
base: main
Choose a base branch
from

Conversation

pralkarz
Copy link

@pralkarz pralkarz commented Feb 3, 2025

Summary

With #15197 as a starting point, this PR completes the migration from chalk to picocolors for most packages (the remaining ones use more advanced chalk features not present in picocolors, e.g. Truecolor support). Failing tests are largely snapshots stemming from ANSI escape codes being put in different places by these two libraries, e.g.:

- <red>Array [</color>
- <red>  0,</color>
- <red>  1,</color>
- <red>]</color>
+ <red>Array [
+   0,
+   1,
+ ]</color>

Chalk seems to operate line-by-line, while picocolors sets the color once and clears it once. I could just update the snapshots, but that would result in a massive diff. What's the suggested course of action here? I could split this PR and go package-by-package instead, updating the snapshots as I go.

Test plan

TBD.

ishon19 and others added 30 commits July 17, 2024 07:21
Copy link

netlify bot commented Feb 3, 2025

Deploy Preview for jestjs ready!

Built without sensitive environment variables

Name Link
🔨 Latest commit 1e02600
🔍 Latest deploy log https://app.netlify.com/sites/jestjs/deploys/67a13fb9173a950008a9127a
😎 Deploy Preview https://deploy-preview-15487--jestjs.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@pralkarz
Copy link
Author

pralkarz commented Feb 6, 2025

I've opened #15490 to make it easier to gauge what snapshot changes are needed to go forward with this migration. I think it's gonna be much more comfortable to review when done package-by-package rather than opening one huge PR with thousands of lines of snapshot changes.

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.

2 participants