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

Add support for native esm to @babel/runtime #10748

Merged
merged 4 commits into from Nov 22, 2019

Conversation

nicolo-ribaudo
Copy link
Member

Q                       A
Fixed Issues? Fixes #8462
Patch: Bug Fix? Yes?
Major: Breaking Change?
Minor: New Feature?
Tests Added + Pass? Yes
Documentation PR Link
Any Dependency Changes?
License MIT

Currently esm/ helpers are broken: when you try to natively import them, they are parsed as scripts and thus throw an error.

Assing type: module is a better fix than renaming the files to *.mjs, because this isn't a breaking change.

I only tested this PR manually because:

  1. Babel can't be built on Node 13.2 which supports unflagged modules ([13.2.0] Assertion `(new_handler_count) >= (0)' failed nodejs/node#30581)
  2. Jest doesn't support native modules

@nicolo-ribaudo nicolo-ribaudo added PR: Bug Fix 🐛 A type of pull request used for our changelog categories area: node area: helpers labels Nov 21, 2019
rm -rf packages/babel-runtime-corejs2/helpers
rm -f packages/babel-runtime/helpers/**/*.js
rm -f packages/babel-runtime-corejs2/helpers/**/*.js
rm -f packages/babel-runtime-corejs3/helpers/**/*.js
rm -rf packages/babel-runtime-corejs2/core-js
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: I think we should also clean packages/babel-runtime-corejs3/core-js(-stable)?.

Copy link
Contributor

@JLHwung JLHwung left a comment

Choose a reason for hiding this comment

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

In order to fix #8462, we still need #10548 which generates explicit extension for ESM imports.

@nicolo-ribaudo
Copy link
Member Author

@JLHwung Why isn't the package.json enough?

@nicolo-ribaudo nicolo-ribaudo merged commit 1b4cfc2 into babel:master Nov 22, 2019
@nicolo-ribaudo nicolo-ribaudo deleted the helpers-native-esm branch November 22, 2019 22:59
@jaydenseric
Copy link

jaydenseric commented Nov 23, 2019

@nicolo-ribaudo native ESM in Node.js requires the file extension to be in the import specifier:

https://nodejs.org/api/esm.html#esm_mandatory_file_extensions

@nicolo-ribaudo
Copy link
Member Author

Thanks, I reopened the issue

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area: helpers area: node outdated A closed issue/PR that is archived due to age. Recommended to make a new issue PR: Bug Fix 🐛 A type of pull request used for our changelog categories
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ESM runtime helper files should have the .mjs extension
4 participants