Skip to content

Replace picocolors with node:util.styleText() in @babel/code-frame#17678

Merged
nicolo-ribaudo merged 10 commits intobabel:mainfrom
fisker:picocolors
Jan 31, 2026
Merged

Replace picocolors with node:util.styleText() in @babel/code-frame#17678
nicolo-ribaudo merged 10 commits intobabel:mainfrom
fisker:picocolors

Conversation

@fisker
Copy link
Contributor

@fisker fisker commented Dec 21, 2025

Q                       A
Fixed Issues? Fixes #1, Fixes #2
Patch: Bug Fix?
Major: Breaking Change?
Minor: New Feature?
Tests Added + Pass? Yes
Documentation PR Link
Any Dependency Changes?
License MIT
  1. Do we want support for colors in the browser?
  2. We'll need another entry for browser, since styleText is available in browser, how do we process?

Fixes #16945
Closes #14555

Copy link
Member

@nicolo-ribaudo nicolo-ribaudo left a comment

Choose a reason for hiding this comment

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

I think it's fine if we make it so in the browser it throws when trying to force colors.

@fisker
Copy link
Contributor Author

fisker commented Jan 9, 2026

  1. Use export condition? Do we support bundling two different files?
  2. We should also drop js-tokens in browser, right?

@liuxingbaoyu
Copy link
Member

Unfortunately, we do not support conditional export of browser.

@babel-bot
Copy link
Collaborator

babel-bot commented Jan 29, 2026

Build successful! You can test your changes in the REPL here: https://babeljs.io/repl/build/60821

@liuxingbaoyu
Copy link
Member

The CI error is relevant, but it appears to have existed before.
It seems Rollup is always using the browser version for index.ts.
I will continue the investigation after merging #17756.

@nicolo-ribaudo
Copy link
Member

Other PR merged

@pkg-pr-new
Copy link

pkg-pr-new bot commented Jan 30, 2026

Open in StackBlitz

commit: 7cd3cfc

@liuxingbaoyu liuxingbaoyu marked this pull request as ready for review January 30, 2026 23:13
@fisker
Copy link
Contributor Author

fisker commented Jan 30, 2026

Thanks for your help!


export function highlight(code: string) {
return code;
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I remember I shared some code between two versions.
Will this make it harder to maintain having duplicate logic?

Copy link
Member

Choose a reason for hiding this comment

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

I've forgotten why I restored them...
Now I've made them shared again.

if (process.emitWarning) {
// A string is directly supplied to emitWarning, because when supplying an
// Error object node throws in the tests because of different contexts
process.emitWarning(message, "DeprecationWarning");
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This should be removed from browser version.

return ` ${gutter}${line.length > 0 ? ` ${line}` : ""}`;
}
})
.join("\n");
Copy link
Contributor Author

@fisker fisker Jan 31, 2026

Choose a reason for hiding this comment

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

If we pass {gutter, marker, message} as argument, we should be able to share these 40 lines?

Copy link
Member

Choose a reason for hiding this comment

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

I've removed the coloring logic, it's actually the content. :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I know, but we can still pass these 3 empty functions.

"exports": {
".": {
"types": "./lib/index.d.ts",
"browser": "./lib/browser.cjs",
Copy link
Member

Choose a reason for hiding this comment

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

Let's do this in a separate PR

Copy link
Member

Choose a reason for hiding this comment

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

This has already been removed from #17229. :)

Copy link
Member

@nicolo-ribaudo nicolo-ribaudo left a comment

Choose a reason for hiding this comment

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

Thank you both!

@nicolo-ribaudo nicolo-ribaudo added the PR: Internal (next major) 🏠 A type of pull request used for our changelog categories for next major release label Jan 31, 2026
@nicolo-ribaudo nicolo-ribaudo merged commit 153ca37 into babel:main Jan 31, 2026
55 checks passed
@fisker fisker deleted the picocolors branch January 31, 2026 13:41
@nicolo-ribaudo nicolo-ribaudo added the PR: Internal 🏠 A type of pull request used for our changelog categories label Mar 7, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

PR: Internal 🏠 A type of pull request used for our changelog categories PR: Internal (next major) 🏠 A type of pull request used for our changelog categories for next major release

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Provide "browser" entry for @babel/code-frame package

4 participants