CLI: Replace chalk with util.styleText wrapper#3106
Merged
Conversation
Collaborator
📊 Performance Test ResultsComparing f013c03 vs trunk app-size
site-editor
site-startup
Results are median values from multiple test runs. Legend: 🟢 Improvement (faster) | 🔴 Regression (slower) | ⚪ No change (<50ms diff) |
Contributor
Author
|
@gcsecsey thanks for the review, nice catch on the Would you mind doing another review, just to be extra safe? |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.






Related issues
How AI was used in this PR
Claude Code drafted the wrapper, tests, and call-site updates. I reviewed the generated code (including a self-review pass that caught a color-depth detection bug in the hex path) and verified behavior manually with
FORCE_COLOR=1/3andNO_COLOR=1.Proposed Changes
Drops the
chalkdependency fromapps/cliin favor of Node's built-inutil.styleText(Node 20+), as suggested on e18e.dev. Since Studio's CLI requires Node ≥22,styleTextis fully available.tools/common/lib/chalk.ts: a thin Proxy-based wrapper that exposes a chalk-compatible surface (named styles, chained access likechalk.bold.red, aliased leaves,chalk.hex(...)/chalk.bgHex(...)) backed byutil.styleText. Hex colors use raw 24-bit ANSI escapes sincestyleTextdoesn't support them, gated onprocess.stdout.getColorDepth()so terminals without truecolor degrade cleanly. HonorsNO_COLOR/FORCE_COLOR/ TTY detection.tools/common/lib/tests/chalk.test.ts: 7 unit tests covering named, chained, aliased, nested, hex fg, hex bg + named, and truecolor stripping.import chalk from 'chalk'→import chalk from '@studio/common/lib/chalk'across the 6 files inapps/clithat use it. No call-site logic changed.chalkfromapps/cli/package.json.Not included
tools/benchmark-site-editorandtools/compare-perfstill depend on chalk. They're standalone dev tools (not bundled into the shipped CLI or desktop app), run via ts-node, and have their own tsconfigs that don't resolve the shared wrapper cleanly. I attempted the migration but rolled it back since there's no user-visible benefit and the type resolution was fiddly. Leaving them as-is.Testing Instructions
npm install(chalk will be removed fromapps/cli).npm run cli:buildnpx vitest run tools/common/lib/tests/chalk.test.ts→ should show 7 passed.node apps/cli/dist/cli/main.mjs codeand verify the AI chat TUI renders with colors (this exercises the largest chalk surface — apps/cli/ai/ui.ts).studio code sessions(or equivalent) to verify the session list styling.NO_COLOR=1 node apps/cli/dist/cli/main.mjs <cmd>→ no ANSI escapes in output.FORCE_COLOR=3 node apps/cli/dist/cli/main.mjs <cmd> | cat→ full truecolor escapes emitted even though stdout is piped.Verified locally
npm run cli:buildcleanapps/clitypecheck: no new errorsFORCE_COLOR=3+NO_COLOR=1behavior matches chalkPre-merge Checklist