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

commonjs-dynamic-required modules are not transformed by plugin-babel #1085

Closed
JLHwung opened this issue Jan 8, 2022 · 5 comments
Closed

Comments

@JLHwung
Copy link
Contributor

JLHwung commented Jan 8, 2022

Reproduction steps:

pnpm i
pnpm test

Expected Behavior

It should contain the transformed module module.exports = { a: a };

Actual Behavior

It contains module.exports = { a };, in other words, the imported module is not transformed by plugin-babel

Additional Information

The dynamic-required-proxied modules have a wrapped id starting with \0. After they are loaded by the DynamicRequireProxy, they are not transformed by plugin-babel because any id containing \0 is rejected by pluginutils/createFilter.

The dynamic required targets are client files, I believe they should be transformed by plugin-babel and any other transformers. A potential solution is to change how the dynamic required targets id are determined.

This issue blocks babel/babel#14112. In that case, we use plugin-commonjs to load https://github.com/mathiasbynens/regenerate-unicode-properties. Each unicode properties script contains const set = regenerate() and they not transformed by the plugin-babel.

@shellscape
Copy link
Collaborator

Do we need to come up with more sophisticated means to tag virtual modules, perhaps with metadata?

@lukastaegert
Copy link
Member

I guess I need to finish #1038 as that will just get rid of all the dynamic register logic.

@lukastaegert
Copy link
Member

lukastaegert commented Jan 8, 2022

BTW as long as you do not rely on watch mode, the current @rollup/plugin-commonjs@beta should work well and likely solve all issues.

@JLHwung
Copy link
Contributor Author

JLHwung commented Jan 10, 2022

@lukastaegert Yes. I can confirm @beta work for Babel. babel/babel#14124

This issue can be closed once #1038 is merged. Can you update the PR info?

@stale
Copy link

stale bot commented Mar 17, 2022

Hey folks. This issue hasn't received any traction for 60 days, so we're going to close this for housekeeping. If this is still an ongoing issue, please do consider contributing a Pull Request to resolve it. Further discussion is always welcome even with the issue closed. If anything actionable is posted in the comments, we'll consider reopening it.

@stale stale bot closed this as completed Mar 17, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants