Remove picocolors, add native util.styleText facade (src/style.ts) - #194
Merged
Conversation
- Add src/style.ts as the sole call site for node:util's styleText, mirroring the render/terminal.ts convention for Bun-native APIs. - Add src/style.d.ts ambient augmentation: @types/node 25.2.3 does not yet declare styleText, though Bun 1.4 implements it at runtime. - Migrate all 9 files importing picocolors directly (render.ts, render/highlight.ts, render/summary.ts, render/team-pick.ts, tui.ts, upgrade.ts, api.ts, github-code-search.ts) plus test fixtures. - Nested style compositions (e.g. pc.bold(pc.yellow(x))) are converted to style.style([...], x) using styleText's native array-of-formats support, verified byte-identical to picocolors via golden-master tests in src/style.test.ts (bold/dim share the same SGR reset code, so naive chaining is not always safe for composed styles). - Remove picocolors from package.json dependencies. - Update AGENTS.md and .github/skills/documentation.md conventions. Closes #193
|
Coverage after merging refactor/remove-picocolors into main will be
Coverage Report
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Contributor
There was a problem hiding this comment.
Pull request overview
This PR replaces direct picocolors usage with a centralized native node:util.styleText facade while preserving terminal styling behavior.
Changes:
- Adds the styling facade, typings, and parity tests.
- Migrates production and test styling call sites.
- Removes the direct dependency and updates documentation and lockfile metadata.
Reviewed changes
Copilot reviewed 17 out of 19 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Summary |
|---|---|
src/upgrade.ts |
Migrates upgrade-link styling. |
src/tui.ts |
Migrates TUI styling. |
src/test-setup.ts |
Updates color test setup. |
src/style.ts |
Adds the native styling facade. |
src/style.test.ts |
Adds ANSI parity and environment tests. |
src/style.d.ts |
Declares styleText; Critical (1 vote): make the file an external module so it augments rather than replaces node:util declarations. |
src/render/terminal.test.ts |
Migrates test styling. |
src/render/team-pick.ts |
Migrates team-picker styling. |
src/render/team-pick.test.ts |
Updates styling test references. |
src/render/summary.ts |
Migrates summary styling. |
src/render/highlight.ts |
Migrates syntax-highlighting styling. |
src/render/highlight.test.ts |
Updates styling references. |
src/render.ts |
Migrates rendering styles; Moderate (3 votes): split nested dim/bold spans to prevent reset collisions. |
src/api.ts |
Migrates progress styling. |
package.json |
Removes the direct dependency. |
github-code-search.ts |
Migrates CLI help and error styling. |
bun.lock |
Updates dependency metadata; Nit (2 votes): picocolors remains transitively and may conflict with the stated removal requirement. |
AGENTS.md |
Documents the styling convention. |
.github/skills/documentation.md |
Updates styling guidance. |
Suppressed comments (2)
AGENTS.md:273
- The list separator is malformed in this new documentation line:
`chalk`/`picocolors`/orshould use commas and a finalor.
- `src/style.ts` is the sole call site for `node:util`'s `styleText` and the only styling dependency-free abstraction in the codebase; do not import `styleText` directly elsewhere, and do not add `chalk`/`picocolors`/or similar external styling packages.
package.json:55
- Removing the workspace entry does not remove
picocolorsfrom the lockfile: it is still a transitive dependency of@babel/code-frameandpostcss(bun.lock:69 and bun.lock:1333). This leaves the stated DoD/acceptance criterion that it no longer appears inbun.lockunmet; either remove those dependency paths or narrow the claim to the direct production dependency.
"commander": "^15.0.0"
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> Signed-off-by: Sébastien HOUZÉ <sebastien.houze@fulll.fr>
shouze
force-pushed
the
refactor/remove-picocolors
branch
from
August 24, 2026 00:07
f1a489f to
cbdb668
Compare
|
Coverage after merging refactor/remove-picocolors into main will be
Coverage Report
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
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.
What does this PR do?
Removes the
picocolorsdependency and replaces it with a nativenode:utilstyleTextfacade (src/style.ts), the last remaining external styling dependency in the codebase (closes #193).src/style.tsas the sole call site fornode:util'sstyleText, mirroring therender/terminal.tsconvention already used for Bun-native APIs. Exposes one function per style actually in use (dim,bold,italic,underline,red,green,yellow,cyan,magenta,white,black,bgMagenta) plus astyle(names[], text)composer for multi-style call sites.src/style.d.ts: an ambient module augmentation, since@types/node(25.2.3, pulled transitively viabun-types) does not yet declarestyleText, even though Bun 1.4 implements it at runtime.picocolorsdirectly:src/render.ts,src/render/highlight.ts,src/render/summary.ts,src/render/team-pick.ts,src/tui.ts,src/upgrade.ts,src/api.ts,github-code-search.ts.pc.bold(pc.yellow(x))) tostyle.style([...], x), usingstyleText's native array-of-formats support instead of chaining. This matters becauseboldanddimshare the same SGR reset code (22); naive chained calls are not always safe for composed styles, while the array form is always correct.picocolorsfrompackage.jsondependencies and updatesbun.lock.AGENTS.mdand.github/skills/documentation.mdconventions to referencesrc/style.tsinstead ofpicocolors.Behavior-preserving refactor — no user-facing output changes.
How did you verify your code works?
src/style.test.ts: 23 golden-master tests comparingsrc/style.tsoutput byte-for-byte against the previouspicocolorsoutput, for every single style and every composed combination actually used in the codebase, plusNO_COLOR/FORCE_COLORbehavior tests.bun test: full suite green (644 tests).bun run lint,bun run format:check,bun run knip: all clean.bun run build.ts: binary compiles; manually smoke-tested--helpoutput (plain and withFORCE_COLOR=1) on the compiled binary.