jest icon indicating copy to clipboard operation
jest copied to clipboard

feat: Replace `chalk` with `picocolors`

Open ishon19 opened this issue 1 year ago • 19 comments

Summary

This PR is for replacing the chalk package in the codebase with lighter and faster picocolors owing to the small bundle size (~5Kb) of picocolors as compared to the existing chalk (~48Kb).

Test plan

🚧 WIP

Fixes #15189

ishon19 avatar Jul 18 '24 08:07 ishon19

CLA Signed

The committers listed above are authorized under a signed CLA.

  • :white_check_mark: login: ishon19 / name: Shreyans Pathak (820347e2d9deabec325464c9e7ac438ce867ad8e, 95365f55677a22e2327b24e77b02ea451c37c9a9, bddbdb64a606233a960a4b3e2330795b9acf24f4, 90fa15c6e6ae78646d09d965bec0ba318e839a85, 9175c29776479f47ed2363c9e050c40089e133fd, 69e103d817d83fe154a425241eb03b92d5790cf2, 19a46390c863276ab91d823876251f8d0dc7837c, b8531953e7b04eb12d3bdc0dcfec48bc5946a93c, 0900fd3460ae5a1d35bd9c9831b054fbd7a20a9e, 0511891d83aaadf0f4a7022e23ad80aa7f822312, 2f362e5c68f2656a53948ef9f593e9c782a1d14a, 0b2a7fcbad1a3ea54e1708c99625a0676041e18a, b84878886aacd933fe7e2e91b8a4fdbcca03407e, ec3cb590d9d27745a9b05c79e8d718e6c2a0689a, 0fd1645c8575599f404d207a537c4d34a687f015, d32505e8705fc63bb946deb39fd1ce32a89e2db5, 87fb3f3a98ac4d8c16f92b747e5738e75c330b5a, daf66488b8b4d3532e206e9a14dd6480aa34f5b5, 1e7f0e884e76a7161dd083ab13b89ca65b7b561f, 9e09d89545f83b613324b93884d71649036520ea, cc0f080637c0d85c959753801b54623ff4ccd955, 964e3ae24845cc38640256873d9b38c17f33e12d, 73a4051933fcc72fefde8d00a753f6f57024ee51, 47542f69d8dda81abc74c78e68a81914a0433abb, 51baa3e668a354743f282977282b645fd5e1f810, e2044544bb872d8b2b48466c3a5cf988abb2af08)

Deploy Preview for jestjs ready!

Built without sensitive environment variables

Name Link
Latest commit 73a4051933fcc72fefde8d00a753f6f57024ee51
Latest deploy log https://app.netlify.com/sites/jestjs/deploys/66fb05e1e8fb280008ada0ec
Deploy Preview https://deploy-preview-15197--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.

netlify[bot] avatar Jul 18 '24 08:07 netlify[bot]

Probably supports-color can be also replaced by .isColorSupported from picocolors:

https://github.com/jestjs/jest/blob/213c3b2770a3278fe47267c1ace250251954fe4f/packages/jest-worker/src/workers/ChildProcessWorker.ts#L11

mrazauskas avatar Jul 30 '24 08:07 mrazauskas

Probably supports-color can be also replaced by .isColorSupported from picocolors:

https://github.com/jestjs/jest/blob/213c3b2770a3278fe47267c1ace250251954fe4f/packages/jest-worker/src/workers/ChildProcessWorker.ts#L11

That makes sense, thanks for pointing it out! :)

ishon19 avatar Jul 30 '24 21:07 ishon19

this might be a bit late, but maybe it helps? https://github.com/es-tooling/module-replacements-codemods/pull/77

SimenB avatar Aug 02 '24 13:08 SimenB

this might be a bit late, but maybe it helps? es-tooling/module-replacements-codemods#77

Much helpful, thanks for sharing @SimenB!

Planning to wrap the changes up ASAP, right now stuck on yarn build failure as API extractor complains about missing d.ts type declaration file in picocolors as in the screenshot below, which they do have, but it's called picocolors.d.ts and I suspect api-extractor is picking up the wrong file, trying to override that behavior in bundleTs.mjs.

image

ishon19 avatar Aug 02 '24 14:08 ishon19

@ishon19 This looks like a picocolors bug. They are shipping .ts file instead of .d.ts to npm, but a package should only have .js and .d.ts.

I mean, if code is compiled using tsc, these are the only files one will have in a package.

mrazauskas avatar Aug 02 '24 14:08 mrazauskas

@ishon19 This looks like a bug Picolors. They are shipping .ts file instead of .d.ts to npm, but a package should only have .js and .d.ts.

I mean, if code is compiled using tsc, these are the only files one will have in a package.

Thanks @mrazauskas, yes I agree, I can create an issue on their repo for clarification.

ishon19 avatar Aug 02 '24 15:08 ishon19

Hi everyone, I'm really interested with this PR!

@ishon19, have you had any time recently to get things moving? 🙏🏻

Kocal avatar Sep 09 '24 06:09 Kocal

@Kocal If I get it right, this PR is still block because of https://github.com/alexeyraspopov/picocolors/issues/77

mrazauskas avatar Sep 09 '24 06:09 mrazauskas

Thanks @mrazauskas!

Kocal avatar Sep 09 '24 08:09 Kocal

Hi everyone, I'm really interested with this PR!

@ishon19, have you had any time recently to get things moving? 🙏🏻

Thanks @Kocal, I know it's waiting for too long and I don't know when the issue this is waiting on is going to be resolved, so if you have any workaround or a better idea to make this happen, please feel free to link a new PR. :)

ishon19 avatar Sep 09 '24 13:09 ishon19

Hey @ishon19, don't worry, I wasn't trying to blame you or something like that, I'm just super hyped by your PR because I also want to replace chalk by picocolors (see https://github.com/symfony/webpack-encore/pull/1333 & https://github.com/nuxt/friendly-errors-webpack-plugin/pull/21).

I would be really happy to help here, I will take a look today, thanks :)

Kocal avatar Sep 09 '24 13:09 Kocal

I would be really happy to help here, I will take a look today, thanks :)

Yeah, absolutely! Since you worked on similar issues, really appreciate your feedback :)

ishon19 avatar Sep 09 '24 13:09 ishon19

https://github.com/alexeyraspopov/picocolors/pull/82 has been merged

Kocal avatar Sep 30 '24 19:09 Kocal

alexeyraspopov/picocolors#82 have been merged

@Kocal Thanks for the update and the fix as well!

ishon19 avatar Sep 30 '24 20:09 ishon19

@Kocal this change is now available in v1.1.1

alexeyraspopov avatar Oct 16 '24 20:10 alexeyraspopov

@alexeyraspopov thanks! :)

Kocal avatar Oct 17 '24 06:10 Kocal

Hey @Kocal! Since we have the official confirmation now, I believe we're all set for the upgrade!

Unfortunately at this moment, I've got a lot on my plate and I don't want to delay this upgrade any further, please feel free to take this forward if you'd like to. You can open a new PR altogether, or use this one. Let me know if I can help in any way. Appreciate your help :)

ishon19 avatar Oct 17 '24 12:10 ishon19

I did some work with your branch as a starting point and opened a new PR: https://github.com/jestjs/jest/pull/15487.

pralkarz avatar Feb 03 '25 22:02 pralkarz

I did some work with your branch as a starting point and opened a new PR: #15487.

Hi @pralkarz Thanks a lot for picking this up, while I'm still caught up in a lot of things, let me know if you need any assistance, I'll be glad to jump in!

ishon19 avatar Feb 04 '25 19:02 ishon19

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. Please note this issue tracker is not a help forum. We recommend using StackOverflow or our discord channel for questions.

github-actions[bot] avatar Jun 22 '25 00:06 github-actions[bot]