sentry-wizard icon indicating copy to clipboard operation
sentry-wizard copied to clipboard

chore: migrate from `chalk` to `picocolors`

Open outslept opened this issue 4 months ago • 6 comments

image

vs

image

outslept avatar Aug 12 '25 20:08 outslept

Seems picocolors posts extra ansi escape sequences. Investigating.

UPD: chalk disables colors in CI by default and picocolors doesn't. I'll add NO_COLOR to vitest.config

outslept avatar Aug 12 '25 20:08 outslept

Codecov Report

:x: Patch coverage is 16.39676% with 413 lines in your changes missing coverage. Please review. :white_check_mark: Project coverage is 29.73%. Comparing base (719b3fd) to head (a375ca4).

Files with missing lines Patch % Lines
src/utils/clack/index.ts 26.31% 42 Missing :warning:
src/nextjs/nextjs-wizard.ts 0.00% 38 Missing :warning:
src/flutter/code-tools.ts 4.34% 22 Missing :warning:
src/nuxt/sdk-setup.ts 0.00% 18 Missing :warning:
src/nextjs/templates.ts 5.55% 17 Missing :warning:
src/flutter/flutter-wizard.ts 0.00% 16 Missing :warning:
src/sourcemaps/tools/wrangler.ts 6.25% 15 Missing :warning:
src/remix/sdk-setup.ts 6.66% 14 Missing :warning:
src/sourcemaps/sourcemaps-wizard.ts 6.66% 14 Missing :warning:
src/angular/sdk-setup.ts 7.14% 13 Missing :warning:
... and 37 more
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              
Flag Coverage Δ
unit-tests 29.73% <16.39%> (+0.08%) :arrow_up:

Flags with carried forward coverage won't be shown. Click here to find out more.

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

:rocket: New features to boost your workflow:
  • :snowflake: Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • :package: JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

codecov[bot] avatar Aug 12 '25 20:08 codecov[bot]

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).

outslept avatar Aug 12 '25 21:08 outslept

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 Installing @sentry/cli but receiving [35m◒[39m Installing [1m[36m@sentry/cli[39m[22m with [1mYarn V1[22m... with ANSI escape sequences that it cannot parse, causing the problem.

If you'd like to provide striping on clifty side, you could use Node.js built-in util.stripVTControlCharacters(str) (https://nodejs.org/api/util.html#utilstripvtcontrolcharactersstr) to strip ANSI sequences. It should apply to this https://github.com/Lms24/clifty/blob/0d7ae7c4aa8442cd1ce6b75edd6d41fd57e32fbc/packages/clifty/src/scenarioBuilder.ts#L159-L178

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);
  }
});

outslept avatar Aug 16 '25 11:08 outslept

This is the workflow run from my fork (content doesn't differ from this branch, I just removed NO_COLOR from vitest config): https://github.com/outslept/sentry-wizard/actions/runs/17007457214/job/48219026060?pr=1

Open logs and look for (cloudflare) :

2025-08-16T10:38:59.2196791Z Waiting for: 
2025-08-16T10:38:59.2197230Z Installing @sentry/cli

And all the ansi hell

2025-08-16T10:38:59.2327378Z [999D[J[35m◒[39m  Installing [1m[36m@sentry/cli[39m[22m with [1mYarn V1[22m[999D[J[35m◐[39m  Installing [1m[36m@sentry/cli[39m[22m with [1mYarn V1[22m[999D[...

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.

outslept avatar Aug 16 '25 11:08 outslept

@outslept apologies for the late reply! I added your suggestions to clifty and updated to the latest version in this issue. Might be worth giving this PR a rebase to check if this fixed the issue. Either way, thanks a lot for letting me know about stripVTControlCharacters. TIL!

Lms24 avatar Oct 14 '25 10:10 Lms24