Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use native ESM for dev scripts #12137

Closed
2 tasks
nicolo-ribaudo opened this issue Oct 3, 2020 · 10 comments · Fixed by #12296
Closed
2 tasks

Use native ESM for dev scripts #12137

nicolo-ribaudo opened this issue Oct 3, 2020 · 10 comments · Fixed by #12296
Assignees
Labels
claimed good first issue i: enhancement outdated A closed issue/PR that is archived due to age. Recommended to make a new issue

Comments

@nicolo-ribaudo
Copy link
Member

Since we are now only building Babel on Node.js 14 (even if we still test it on Node.js 6), we can use native ECMAScript modules for the build scripts:

  • Top-level scripts
    There are a bunch of scripts/tests in the scripts folder: they can probably all be rewritten to ES modules.
    We can probably add "type": "module" to the top-level package.json, renaming config files (such as .eslintrc.js) to .cjs. Note that we cannot use ES modules for babel.config.js and jest.config.js, because those two files are used to run tests on Node.js 6.

  • babel-types/scripts can be rewritten to ES modules. These files should be renamed to .mjs, to keep the dist @babel/types files with the .js extension (they are CJS modules).

Please note that babel-types/scripts depends on the top-level scripts, so babel-types/scripts must be converted first (it cannot synchronously import an ESM file while still being CJS).

In order to test if your changes are still ok, running yarn (only when you clone the repo) and make build && yarn jest should be enough.

@JLHwung
Copy link
Contributor

JLHwung commented Oct 3, 2020

Note that we should also bump building requirements to ^12.16 || >= 14 in https://github.com/babel/babel/blob/main/CONTRIBUTING.md#developing

We can also update gulp to 4.2.0 which supports .mjs config.

@karansapolia
Copy link
Contributor

I would like to work on this 🙂

@sag1v
Copy link
Contributor

sag1v commented Oct 3, 2020

Can i have a go?

@nicolo-ribaudo
Copy link
Member Author

Uh... I'm assigning this to @karansapolia since he claimed this issue first, but if you two want to split the work in two parts (maybe the two points I wrote in the issue description? The second one is probably easier) please discuss about it 😄

@sag1v
Copy link
Contributor

sag1v commented Oct 3, 2020

Thats okay 🙂

@karansapolia Go for it! 💪

karansapolia added a commit to karansapolia/babel that referenced this issue Oct 7, 2020
@karansapolia
Copy link
Contributor

We can probably add "type": "module" to the top-level package.json, renaming config files (such as .eslintrc.js) to .cjs

Renaming Gulpfile.js to Gulpfile.cjs is not supported and neither does Gulp support esm.

Either we should not include "type": "module" in the top-level package.json and rename all esm files to .mjs from .js. Or we can do something else.

Currently, after following the recommendations in the issue description and putting "type": "module" in package.json, make build throws this error. Let me know what should be done or if I am missing something.

@nicolo-ribaudo

@nicolo-ribaudo
Copy link
Member Author

Oh that's unfortunate 🙁
We can use the .mjs extension instead of "type": "module" then.

@JLHwung
Copy link
Contributor

JLHwung commented Oct 8, 2020

neither does Gulp support esm

I don't think so. gulpjs/gulp-cli@c9e9125 ships in gulp-cli 2.3.0, which is a dependent of gulp. Gulpfile.mjs should work.

karansapolia added a commit to karansapolia/babel that referenced this issue Oct 13, 2020
karansapolia added a commit to karansapolia/babel that referenced this issue Oct 13, 2020
@nicolo-ribaudo
Copy link
Member Author

@karansapolia I see you pushed some commits! 💪 Do you need any help to move forward?

@karansapolia
Copy link
Contributor

@nicolo-ribaudo Yes, I was stuck on an error. Could use some help definitely. Let me create a draft PR so all can have a look.

karansapolia added a commit to karansapolia/babel that referenced this issue Nov 2, 2020
karansapolia added a commit to karansapolia/babel that referenced this issue Nov 2, 2020
karansapolia added a commit to karansapolia/babel that referenced this issue Nov 2, 2020
karansapolia added a commit to karansapolia/babel that referenced this issue Nov 2, 2020
karansapolia added a commit to karansapolia/babel that referenced this issue Nov 2, 2020
karansapolia added a commit to karansapolia/babel that referenced this issue Dec 3, 2020
karansapolia added a commit to karansapolia/babel that referenced this issue Dec 3, 2020
karansapolia added a commit to karansapolia/babel that referenced this issue Dec 3, 2020
karansapolia added a commit to karansapolia/babel that referenced this issue Dec 3, 2020
karansapolia added a commit to karansapolia/babel that referenced this issue Dec 3, 2020
@github-actions github-actions bot added the outdated A closed issue/PR that is archived due to age. Recommended to make a new issue label May 2, 2021
@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 2, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
claimed good first issue i: enhancement outdated A closed issue/PR that is archived due to age. Recommended to make a new issue
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants