Skip to content

refactor: replace cli-color with picocolors#256

Merged
nathanhleung merged 3 commits intonathanhleung:masterfrom
JounQin:refactor/replace_cli-color
Apr 13, 2025
Merged

refactor: replace cli-color with picocolors#256
nathanhleung merged 3 commits intonathanhleung:masterfrom
JounQin:refactor/replace_cli-color

Conversation

@JounQin
Copy link
Copy Markdown
Contributor

@JounQin JounQin commented Apr 3, 2025

cli-color -> es5-ext detected as a virus

medikoo/es5-ext#186

@ljharb
Copy link
Copy Markdown
Collaborator

ljharb commented Apr 3, 2025

Does anything but Kaspersky report it as a virus? The comments and links on that issue suggest that any antivirus company controllable by a hostile nation-state shouldn't be trusted, which includes Kaspersky.

@JounQin
Copy link
Copy Markdown
Contributor Author

JounQin commented Apr 3, 2025

@ljharb I noticed this issue via https://github.com/apps/socket-security.

By the way, picocolors is much lighter.

@ljharb
Copy link
Copy Markdown
Collaborator

ljharb commented Apr 3, 2025

Unfortunately picocolors does not declare an engines field; does it support node 8+?

@JounQin
Copy link
Copy Markdown
Contributor Author

JounQin commented Apr 3, 2025

Unfortunately picocolors does not declare an engines field; does it support node 8+?

https://app.unpkg.com/picocolors@1.1.1/files/picocolors.js

I think Node 6+ is supported, only let + Array#includes required.

Copy link
Copy Markdown
Collaborator

@ljharb ljharb left a comment

Choose a reason for hiding this comment

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

LGTM then

@JounQin
Copy link
Copy Markdown
Contributor Author

JounQin commented Apr 3, 2025

I don't think the CI broken is related to this PR:

Run npm install -g pnpm@3 yarn@1
npm WARN notice [SECURITY] pnpm has the following vulnerabilities: 2 high, 1 moderate. Go here for more details: https://github.com/advisories?query=pnpm - Run `npm i npm@latest -g` to upgrade your npm version, and then `npm audit` to get more info.
npm ERR! path C:\npm\prefix\yarn.cmd
npm ERR! code EEXIST
npm ERR! Refusing to delete C:\npm\prefix\yarn.cmd: is outside C:\npm\prefix\node_modules\yarn and not a link
npm ERR! File exists: C:\npm\prefix\yarn.cmd
npm ERR! Move it away, and try again.

npm ERR! A complete log of this run can be found in:
npm ERR!     C:\npm\cache\_logs\2025-0[4](https://github.com/nathanhleung/install-peerdeps/actions/runs/14250611119/job/39943052401?pr=256#step:6:5)-03T19_10_31_722Z-debug.log

@ljharb
Copy link
Copy Markdown
Collaborator

ljharb commented Apr 3, 2025

I agree, but it passed 2 weeks ago, so I'm not sure what the issue is. It'll have to wait until that's resolved.

@JounQin
Copy link
Copy Markdown
Contributor Author

JounQin commented Apr 3, 2025

I agree, but it passed 2 weeks ago, so I'm not sure what the issue is. It'll have to wait until that's resolved.

@ljharb Maybe just retry failed workflows? I noticed that Windows + Node 8/10/12+ all works.

@ljharb
Copy link
Copy Markdown
Collaborator

ljharb commented Apr 3, 2025

That doesn't seem to do it.

@JounQin
Copy link
Copy Markdown
Contributor Author

JounQin commented Apr 3, 2025

yarnpkg/yarn#2816 (comment)

@ljharb Maybe --force is needed?

@nathanhleung
Copy link
Copy Markdown
Owner

For some reason, Node 9 and 11 (but no other versions) have been consistently failing on Windows and I haven't had time to figure out exactly why (see previous PRs: #254, #255).

@JounQin wanna open a PR updating the workflow and we can see if the solution in the linked issue works?

@JounQin
Copy link
Copy Markdown
Contributor Author

JounQin commented Apr 3, 2025

@nathanhleung Just tried 7efd77b (#256)

@nathanhleung
Copy link
Copy Markdown
Owner

@JounQin approved your workflow

@JounQin
Copy link
Copy Markdown
Contributor Author

JounQin commented Apr 3, 2025

Sadly it doesn't work, I'll revert back.

@JounQin
Copy link
Copy Markdown
Contributor Author

JounQin commented Apr 3, 2025

@nathanhleung So are we good to merge with the two known broken workflows?

@JounQin
Copy link
Copy Markdown
Contributor Author

JounQin commented Apr 9, 2025

Friendly ping @nathanhleung

@nathanhleung nathanhleung merged commit d1232d2 into nathanhleung:master Apr 13, 2025
34 of 36 checks passed
@nathanhleung
Copy link
Copy Markdown
Owner

Friendly ping @nathanhleung

@JounQin thanks, merged and will cut a new release shortly.

@JounQin JounQin deleted the refactor/replace_cli-color branch April 13, 2025 17:51
@nathanhleung
Copy link
Copy Markdown
Owner

Released in v3.0.7

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.

3 participants