[babel 8] Use ESM-based `chalk@5` #15792

nicolo-ribaudo · 2023-07-17T17:58:50Z

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

I updated yarn-plugin-conditions to allow defining ESM dependencies, so that we can start updating other deps for Babel 8.

~~Would this be ok for you other maintainers? It means that you can only run with BABEL_8_BREAKING=true if you are developing in the monorepo in ESM mode (it's enough to run make use-esm once).~~

~~The commits marked as [15816] are from #15816, please review that PR first.~~

babel-bot · 2023-07-24T16:47:19Z

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

liuxingbaoyu

To be honest, because of these special handling of chalk@5, I would rather stay with the old version unless there is a security problem.🤦‍♂️

liuxingbaoyu · 2023-08-25T05:45:19Z

packages/babel-standalone/test/helpers/env.js

+export function setup() {
+ // chalk, which we bundle in @babel/standalone, relies on `window.navigator`
+ // for the browser build:
+ // https://github.com/chalk/chalk/blob/f399cd0ff69841e88cca89d43a49f1cc9ba2efd5/source/vendor/supports-color/browser.js#L4
+ // We only bundle Chalk 5 in local dev and in Babel 8, so we avoid this
+ // "polyfill" when releasing Babel 7 to make sure that we do not accidentally
+ // bundle Chalk 5.
+
+ if (process.env.BABEL_8_BREAKING) {
+ globalThis.navigator = {};
+ } else if (!process.env.IS_PUBLISH) {
+ global.navigator = {};
+ }
+}


Can we replace it with navigator || {} in rollup?
I remember someone seems to be using @babel/standalone in node, maybe it's not worth breaking them.
If you agree I can try to do this. :)

Yes please!

nicolo-ribaudo · 2023-08-25T09:23:04Z

To be honest, because of these special handling of chalk@5, I would rather stay with the old version unless there is a security problem.🤦‍♂️

The complexity is only there because we are using both chalk 2 and chalk 5 at the same time, if it was only chalk 5 it would be as complex as any other version. I would rather not have Babel 7 holding back Babel 8 😬

The main advantages of using Chalk 5 are that:

when using ESM, yarn-plugin-conditions works much better and thus it can properly load the TS definitions. Additionally, it exports the Instance type that it needs: this makes it possible for us to avoid this ugly import form:

babel/packages/babel-code-frame/src/index.ts

Lines 3 to 8 in 78e5041

 import _chalk from "chalk"; 

 const chalk = _chalk as unknown as typeof import("chalk-BABEL_8_BREAKING-true"); 

 type Chalk = 

 typeof import("chalk-BABEL_8_BREAKING-true").Instance extends new () => infer R 

 ? R 

 : never;

it's much smaller than chalk 4 (75% smaller)

liuxingbaoyu · 2023-08-25T09:45:29Z

it's much smaller than chalk 4 (75% smaller)

Amazing! Now i like it! :)

nicolo-ribaudo · 2023-08-25T10:13:23Z

Good :)

Do you want to do #15792 (comment) here on in a separate PR?

liuxingbaoyu · 2023-08-25T11:55:02Z

It didn't look complicated, so I pushed it directly in this PR. :)

liuxingbaoyu · 2023-08-25T12:05:19Z

I ran into a little strange thing, even if I just build babel7, this code is still there.🤔
$2{ RGC%TBH0OSS6__R_%Z4$

nicolo-ribaudo · 2023-08-25T12:11:52Z

Unless you specify STRIP_BABEL_8_BREAKNG, the bundle will include both Babel 7 and Babel 8 code. Thus, I would also prefer to avoid the BABEL_8_BREAKING guard in the rollup config ans always replace navigator.

nicolo-ribaudo · 2023-08-25T13:05:00Z

@liuxingbaoyu I pushed a commit to use a Babel plugin instead. Rollup's replace plugin had some problems, because:

by default, it does not replace things in member expressions
it was also replacing navigator in a JSON file one of our dependencies was importing

liuxingbaoyu · 2023-08-25T13:06:27Z

it was also replacing navigator in a JSON file one of our dependencies was importing

Yes, I found this problem too.
Thanks!

liuxingbaoyu · 2023-08-25T13:13:45Z

chalk/chalk#598 (comment)
I think there is still a problem.
Currently its color detection is completely broken in non-browsers for this reason.🤦‍♂️

.

nicolo-ribaudo · 2023-08-25T13:20:17Z

Well, Node.js will get the non-highlighted version but the result will still be ok. @babel/standalone is mostly meant to be used in browser, and while I agree that we should not break people using it in Node.js we should prioritize the in-browser experience.

This is already the current behavior for Babel 7: search for var stdoutColor = requireBrowser().stdout; and function requireBrowser() { in https://unpkg.com/@babel/[email protected]/babel.js, and you will see that it always return false. In Babel 8 / Chalk 5, it would at least return true in browsers that support colors.

liuxingbaoyu

Sorry, I made a mistake. 😅
I thought code-frame and highlight would be affected too, now it looks fine.

nicolo-ribaudo force-pushed the up-chalk-5 branch from 84703d8 to 8524e1c Compare July 24, 2023 14:12

This was referenced Jul 24, 2023

[babel 8] Use chalk@4 #15814

Merged

Only allow developing Babel 8 in ESM mode #15816

Merged

nicolo-ribaudo force-pushed the up-chalk-5 branch 2 times, most recently from 8187ff5 to 2f30e87 Compare August 2, 2023 13:21

nicolo-ribaudo mentioned this pull request Aug 3, 2023

Remove Babel 7 e2e ESM tests #15837

Merged

nicolo-ribaudo force-pushed the up-chalk-5 branch from 176ffcb to 330d883 Compare August 3, 2023 15:09

nicolo-ribaudo added the babel 8 label Aug 3, 2023

nicolo-ribaudo force-pushed the up-chalk-5 branch from f33c2f1 to eba0252 Compare August 5, 2023 16:05

nicolo-ribaudo marked this pull request as ready for review August 5, 2023 16:09

nicolo-ribaudo mentioned this pull request Aug 5, 2023

Materialize conditional dependencies when bundling #15842

Merged

nicolo-ribaudo force-pushed the up-chalk-5 branch from eba0252 to 61dd3d3 Compare August 7, 2023 17:21

JLHwung added PR: Dependency ⬆️ PR: Internal (next major) 🏠 A type of pull request used for our changelog categories for next major release and removed babel 8 labels Aug 8, 2023

JLHwung added this to the Babel 8.0 milestone Aug 8, 2023

nicolo-ribaudo added 6 commits August 24, 2023 17:12

[babel 8] Use chalk@5

8fb54f4

Fix yarn checks

d37712a

Fix standalone tests for Babel 8

6485afa

Fix standalone tests in old node

0c8aca5

Compile dynamic import in tests for old node versions

477e2f0

Remove supportsColor from specifier

abeaafc

nicolo-ribaudo force-pushed the up-chalk-5 branch from 61dd3d3 to abeaafc Compare August 24, 2023 15:14

JLHwung approved these changes Aug 24, 2023

View reviewed changes

liuxingbaoyu previously approved these changes Aug 25, 2023

View reviewed changes

liuxingbaoyu reviewed Aug 25, 2023

View reviewed changes

review

8fcb970

liuxingbaoyu force-pushed the up-chalk-5 branch from e7d7842 to 8fcb970 Compare August 25, 2023 12:00

Use Babel plugin instead

136c337

liuxingbaoyu approved these changes Aug 25, 2023

View reviewed changes

nicolo-ribaudo merged commit e43d8e7 into babel:main Aug 25, 2023
47 checks passed

nicolo-ribaudo deleted the up-chalk-5 branch August 25, 2023 13:26

github-actions bot added the outdated A closed issue/PR that is archived due to age. Recommended to make a new issue label Nov 25, 2023

github-actions bot locked as resolved and limited conversation to collaborators Nov 25, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[babel 8] Use ESM-based `chalk@5` #15792

[babel 8] Use ESM-based `chalk@5` #15792

nicolo-ribaudo commented Jul 17, 2023 •

edited

Loading

babel-bot commented Jul 24, 2023 •

edited

Loading

liuxingbaoyu left a comment

liuxingbaoyu Aug 25, 2023

nicolo-ribaudo Aug 25, 2023

nicolo-ribaudo commented Aug 25, 2023

liuxingbaoyu commented Aug 25, 2023

nicolo-ribaudo commented Aug 25, 2023

liuxingbaoyu commented Aug 25, 2023

liuxingbaoyu commented Aug 25, 2023

nicolo-ribaudo commented Aug 25, 2023

nicolo-ribaudo commented Aug 25, 2023

liuxingbaoyu commented Aug 25, 2023

liuxingbaoyu commented Aug 25, 2023

nicolo-ribaudo commented Aug 25, 2023 •

edited

Loading

liuxingbaoyu left a comment

[babel 8] Use ESM-based chalk@5 #15792

[babel 8] Use ESM-based chalk@5 #15792

Conversation

nicolo-ribaudo commented Jul 17, 2023 • edited Loading

babel-bot commented Jul 24, 2023 • edited Loading

liuxingbaoyu left a comment

Choose a reason for hiding this comment

liuxingbaoyu Aug 25, 2023

Choose a reason for hiding this comment

nicolo-ribaudo Aug 25, 2023

Choose a reason for hiding this comment

nicolo-ribaudo commented Aug 25, 2023

liuxingbaoyu commented Aug 25, 2023

nicolo-ribaudo commented Aug 25, 2023

liuxingbaoyu commented Aug 25, 2023

liuxingbaoyu commented Aug 25, 2023

nicolo-ribaudo commented Aug 25, 2023

nicolo-ribaudo commented Aug 25, 2023

liuxingbaoyu commented Aug 25, 2023

liuxingbaoyu commented Aug 25, 2023

nicolo-ribaudo commented Aug 25, 2023 • edited Loading

liuxingbaoyu left a comment

Choose a reason for hiding this comment

[babel 8] Use ESM-based `chalk@5` #15792

[babel 8] Use ESM-based `chalk@5` #15792

nicolo-ribaudo commented Jul 17, 2023 •

edited

Loading

babel-bot commented Jul 24, 2023 •

edited

Loading

nicolo-ribaudo commented Aug 25, 2023 •

edited

Loading