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 file extension when using absoluteRuntime #12827

Conversation

mbehzad
Copy link
Contributor

@mbehzad mbehzad commented Feb 19, 2021

es module imports need the file extension (e.g. import "@babel/runtime/helpers/jsx.js", Or the filenames being listed in the package.json's subpath exports (e.g. import "@babel/runtime/helpers/jsx" + pkg: "./helpers/jsx": "./helpers/jsx.js"). when the user passes a path via absoluteRuntime then the rendered require statement is not the module name + subpath which will be resolved via pkg.json but rather the absolute path to the file. for this case, add the file extension / index.js to prevent bundlers from raising a warning.

Q                       A
Fixed Issues? Fixes #12824
Patch: Bug Fix? Yes
Major: Breaking Change?
Minor: New Feature?
Tests Added + Pass? Yes (modified existing ones)
Documentation PR Link No
Any Dependency Changes? No
License MIT

@babel-bot
Copy link
Collaborator

babel-bot commented Feb 19, 2021

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

@codesandbox-ci
Copy link

codesandbox-ci bot commented Feb 19, 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 e67e44e:

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

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

@mbehzad Sorry for the long delay before reviewing this PR: there have been a lot of @babel/runtime changes between 7.13.0 and 7.13.7 so we have been waiting to review this one.

Could you rebase this PR, and add a test with absoluteRuntime: true + useESModules: true? 🙏

@mbehzad mbehzad force-pushed the fix-missing-extention-when-using-absoluteRuntime- branch from a5aaa13 to 38b64e5 Compare March 14, 2021 15:42
@mbehzad
Copy link
Contributor Author

mbehzad commented Mar 14, 2021

Hi @nicolo-ribaudo , I've added my changes to the current code, and added a test for absoluteRuntime + useESModules option.
But the regenerator import statement injection seems to have been moved to the babel-plugin-polyfill-regenerator module. It needs to have the /index.js at the end for the absoluteRuntime case. I'm wondering if i should add similar to the ext property in the runtime compat option, a flag/property to add that. or even use the ext prop for this and call /index.js the "extension" here. Let me know what is preferred. Thanx.

@charlessuh
Copy link
Contributor

@mbehzad Hi, I'm just another random person who also ran into this issue, but could you rebase this?

I'm wondering if perhaps the regenerator issue can be resolved later as a separate PR, as it may require a change to a package that lives in another repo (https://github.com/babel/babel-polyfills)

@nicolo-ribaudo
Copy link
Member

I have rebased babel/babel-polyfills#79. As soon as it's merged, I'll rebase this one.

mbehzad and others added 3 commits November 13, 2021 11:43
…re used (babel#12824)

the es module imports need the file extention (e.g. import "@babel/runtime/helpers/jsx.js", Or the filenames being listed in the package.json's subpath exports (e.g. "import "@babel/runtime/helpers/jsx" + pkg: "./helpers/jsx": "./helpers/jsx.js"). when the user passes a path via `absoluteRuntime` then the rendered require staemnts is not the module name + subpath which will be resolved via pkg.json but rather the absolute path to the file. for this case, add the file extention / index.js to prevent bundlers from raising a warning.
@nicolo-ribaudo nicolo-ribaudo force-pushed the fix-missing-extention-when-using-absoluteRuntime- branch from 38b64e5 to 4a31db7 Compare November 13, 2021 11:25
@nicolo-ribaudo nicolo-ribaudo changed the title add file extention when the absolute path to the runtime files are used (fix #12824) Add file extention when using absoluteRuntime with @babel/transform-runtime Nov 13, 2021
@nicolo-ribaudo nicolo-ribaudo changed the title Add file extention when using absoluteRuntime with @babel/transform-runtime Add file extension when using absoluteRuntime with @babel/transform-runtime Nov 13, 2021
@nicolo-ribaudo nicolo-ribaudo changed the title Add file extension when using absoluteRuntime with @babel/transform-runtime Add file extension when using absoluteRuntime Nov 13, 2021
@@ -0,0 +1 @@
// empty
Copy link
Member

Choose a reason for hiding this comment

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

This empty file was needed to be able to require.resolve it.

@nicolo-ribaudo nicolo-ribaudo merged commit d3fffd9 into babel:main Nov 14, 2021
@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 Feb 13, 2022
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 13, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
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
5 participants