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

Allow require() of runtime helpers in Node.js 13.2-13.6 #12893

Merged
merged 1 commit into from Feb 25, 2021

Conversation

nicolo-ribaudo
Copy link
Member

@nicolo-ribaudo nicolo-ribaudo commented Feb 24, 2021

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

This PR restores compatibility with Node.js 13.2-13.6.

We now have tests for every Node.js versions where there were ESM-related changes and for Webpack/Rollup, so I'm confident that these changes don't break anything.

The fix is based on https://twitter.com/ljharb/status/1364324634540142596.

@nicolo-ribaudo nicolo-ribaudo added the PR: Bug Fix 🐛 A type of pull request used for our changelog categories label Feb 24, 2021
@codesandbox-ci
Copy link

codesandbox-ci bot commented Feb 24, 2021

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit 59a87a9:

Sandbox Source
babel-repl-custom-plugin Configuration
babel-plugin-multi-config Configuration

@babel-bot
Copy link
Collaborator

babel-bot commented Feb 24, 2021

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

@nicolo-ribaudo nicolo-ribaudo changed the base branch from nicolo-ribaudo/better-runtime to main February 24, 2021 19:14
@nicolo-ribaudo
Copy link
Member Author

CI is fixed on main

@nicolo-ribaudo nicolo-ribaudo merged commit bb558aa into babel:main Feb 25, 2021
@nicolo-ribaudo nicolo-ribaudo deleted the fix-node-13.6 branch February 25, 2021 11:28
@nicolo-ribaudo
Copy link
Member Author

@larixer Are self-references (documented in the first tip here) supposed to work with the node_modules linker?

After merging this PR to main (bb558aa) the tests that here was passing started failing. I can consistently reproduce it with Node.js 13.6 (which doesn't natively implement self-references), running make bootstrap and node test/runtime-integration/src/main-cjs.cjs.

The problem is that packages/babel-runtime has require() calls like

var objectWithoutPropertiesLoose = require("@babel/runtime/helpers/objectWithoutPropertiesLoose");

and this ends up requiring node_modules/@babel/runtime (which is the version downloaded from the registry) rather than packages/babel-runtime.

@larixer
Copy link

larixer commented Feb 25, 2021

@nicolo-ribaudo No, they don't fully work. The node-modules linker is meant to be 100% compatible with yarn classic, only adding features is allowed, breaking compatibility is not allowed. Because of this, self-references are supported only in a limited way, basically the same way as supported by yarn classic. Better self-references support can be theoretically implemented for nm linker, but the support should be enabled via some config setting. For this case this would basically mean creating symlink packages/babel-runtime/node_modules/@babel/runtime -> ../../../. If nm linker will always create such symlinks this will lead to problems with various packages, for example with enhanced-resolve and the likes, so, it can be implemented, but only behind a settting
CC: @arcanis, for ideas.

@nicolo-ribaudo
Copy link
Member Author

Oh ok thanks for the reply 👍

For now I'm adding @babel/runtime@workspace:* as a devDependency so that it works.

@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 28, 2021
@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 28, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
i: regression 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.

Error [ERR_REQUIRE_ESM]: Must use import to load ES Module on @babel/runtime 7.13.*
5 participants