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

fix: preset-env throws exception for export * as x #16181

Merged
merged 4 commits into from Jan 8, 2024

Conversation

liuxingbaoyu
Copy link
Member

@liuxingbaoyu liuxingbaoyu commented Dec 15, 2023

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

@babel-bot
Copy link
Collaborator

babel-bot commented Dec 15, 2023

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

@liuxingbaoyu
Copy link
Member Author

Because of its existence, it did not cause a more serious regression.
But I'm not sure if this PR is the best fix.

// If the caller does not support export-namespace-from, we forcefully add
// the plugin to `includes`.
// TODO(Babel 8): stop doing this, similarly to how we don't do this for any
// other plugin. We can consider adding bundlers as targets in the future,
// but we should not have a one-off special case for this plugin.
if (
optionsModules === "auto" &&
!api.caller(supportsExportNamespaceFrom) &&
!exclude.plugins.has("transform-export-namespace-from")
) {
include.plugins.add("transform-export-namespace-from");
}

@liuxingbaoyu liuxingbaoyu added PR: Bug Fix 🐛 A type of pull request used for our changelog categories i: regression labels Dec 15, 2023
Copy link
Member

@nicolo-ribaudo nicolo-ribaudo left a comment

Choose a reason for hiding this comment

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

The old error makes sense when one feature was standard and one a proposal. Now nobody even knows that export * as was not introduced together with ESM.

@JLHwung
Copy link
Contributor

JLHwung commented Dec 15, 2023

This would be not consistent with how we treat dynamic import() in the module transforms, the dynamic-import plugin is required if import() is seen in module transforms

if (!this.file.has("@babel/plugin-proposal-dynamic-import")) return;

I think the expected behaviour should be: let preset-env add module related transform plugins when modules is not false, and respect the compat-data only when module is false.

@liuxingbaoyu
Copy link
Member Author

actions/download-artifact#249
I'm surprised it's been two weeks and it still hasn't been fixed.

@nicolo-ribaudo
Copy link
Member

I'd like to release a patch once this is merged.

@nicolo-ribaudo nicolo-ribaudo merged commit 8666ee0 into babel:main Jan 8, 2024
48 checks passed
@preservance717
Copy link

Hi @liuxingbaoyu @JLHwung @nicolo-ribaudo
Could you help take a look the question when u are free,thx?

After the babel related dependencies in our project were upgraded from 7.23.7 to 7.23.8, the page could not work normally and the page prompted the error ES Modules may not assign module.exports or exports.*, Use ESM export syntax, instead: ./*/index.js.
Related babel configuration:

 presets: [
      [
        '@babel/preset-env',
        {
          useBuiltIns: 'usage',
          corejs: 3,
        },
      ],
      [
        '@babel/preset-react'
      ],
      ['@babel/preset-typescript'],
    ],
plugins: [***]

If I want the page to work fine, I need to use one of two following ways:

  • add configuration: sourceType: "unambiguous", and the page will work normally.
  • Or change the '@babel/preset-env' preset to
[
  '@babel/preset-env',
  {
    "targets": "defaults"
  },
]

What I'm curious about now is why 7.23.7 version doesn't need to add/update configurations, but 7.23.8 version needs to change these configurations. I feel that it is related to the three latest PR changes (#16181 #16201 #16199), but I can't find the specific point. Can you help me check it out? will appreciate it.

@liuxingbaoyu
Copy link
Member Author

Only this PR is relevant between 7.23.7 and 7.23.8, but I don't know how it breaks your build.
Can you provide a repository to reproduce?

@preservance717
Copy link

Thanks for your quick reply, I can't reproduce it on the practice demo, but I will try it on serval times, if reproduced successfully, I will provide it.

@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 Apr 23, 2024
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 23, 2024
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.

[Bug]: @babel/transform-export-namespace-from is not included after v7.23.3
5 participants