chore: migrate from chalk to picocolors
#1069
Conversation
|
Seems picocolors posts extra ansi escape sequences. Investigating. UPD: |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #1069 +/- ##
==========================================
+ Coverage 29.64% 29.73% +0.08%
==========================================
Files 128 128
Lines 14686 14630 -56
Branches 969 969
==========================================
- Hits 4354 4350 -4
+ Misses 10315 10263 -52
Partials 17 17
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
Alright, the problem should be fixed. Could you please re-run the jobs? It failed due to flaky e2e tests unrelated to this change (had the same-ish behavior in #1068). |
There was a problem hiding this comment.
I re-ran e2e tests. If the same test fails (cloudflare source maps), I need to strip out ansi color/format characters in my testing library that's used by the cloudflare source maps e2e test. I'm surprised we handle this in the other e2e tests though 😅
EDIT: looks like that's the problem. Unfortunately, we can't set NO_COLOR=1 during build for the e2e tests because they use the same build artifacts that we end up publishing (kinda the point of an e2e test xD). So I'll fix this in clifty in a bit.
Update: Not so sure if this is the problem anymore. Still need to do some more debugging but don't have time right now. If you get a chance, you could try running the cloudflare wrangler source maps e2e test locally and check if you spot any obvious problems. Will revisit this as soon as I can.
|
I duplicated the branch to create a PR in my own fork to check the CI logs first before providing feedback. The CI log shows the issue - clifty is expecting If you'd like to provide striping on clifty side, you could use Node.js built-in And it would look like this: child.stdout.on("data", (data) => {
const cleanData = stripVTControlCharacters(data.toString());
this.#stdoutBuffer += cleanData;
this.output.dispatchEvent(
new CustomEvent("stdout", { detail: cleanData })
);
if (this.#testEnv.debug) {
console.log(this.#stdErrBuffer);
}
});child.stderr.on("data", (data) => {
const cleanData = stripVTControlCharacters(data.toString());
this.#stdErrBuffer += cleanData;
this.output.dispatchEvent(
new CustomEvent("stderr", { detail: cleanData })
);
if (this.#testEnv.debug) {
console.log(this.#stdErrBuffer);
}
}); |
|
This is the workflow run from my fork (content doesn't differ from this branch, I just removed Open logs and look for (cloudflare) : And all the ansi hell The reason there are so many ANSI codes is because the CLI spinner updates every x ms with new animation frames, and each update gets accumulated in stdout buffer. While clifty waits for the timeout, the spinner continuously outputs new frames with cursor movement commands and color codes, creating this ANSI sequence spam. UPD: Maybe this is deeper than I assume. I'm trying to localize the issue, but couldn't provide a reproduction. For some reason it seems really easy on paper, but weird in practice. Weird, but I'll keep going. |
vs