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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
1 task
BrooklynKing opened this issue Dec 5, 2023 · 11 comments 路 Fixed by #16181
Closed
1 task

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

BrooklynKing opened this issue Dec 5, 2023 · 11 comments 路 Fixed by #16181
Labels
i: bug outdated A closed issue/PR that is archived due to age. Recommended to make a new issue pkg: preset-env

Comments

@BrooklynKing
Copy link

馃捇

  • Would you like to work on a fix?

How are you using Babel?

Programmatic API (babel.transform, babel.parse)

Input code

Babel v7.23.2 - Link
Babel v7.23.5 - Link

Configuration file name

No response

Configuration

No response

Current and expected behavior

After 7.23.3 version plugin transform-export-namespace-from is not included in config, provided by @babel/preset-env.

I belive it's because of changes in #15988
Explicitly this line
before that plugin included if module !== "auto", but now it require module === "auto" to be included. Looks like a typo, since other part of that condition are changes properly (api.caller?.(supportsExportNamespaceFrom) into !api.caller(supportsExportNamespaceFrom) )

Environment

Babel REPL

Possible solution

No response

Additional context

No response

@babel-bot
Copy link
Collaborator

Hey @BrooklynKing! We really appreciate you taking the time to report an issue. The collaborators on this project attempt to help as many people as possible, but we're a limited number of volunteers, so it's possible this won't be addressed swiftly.

If you need any help, or just have general Babel or JavaScript questions, we have a vibrant Slack community that typically always has someone willing to help. You can sign-up here for an invite.

@JLHwung
Copy link
Contributor

JLHwung commented Dec 5, 2023

The change is expected, according to our compat data

"transform-export-namespace-from": {
"chrome": "72",
"deno": "1.0",
"edge": "79",
"firefox": "80",
"node": "13.2",
"opera": "60",
"opera_mobile": "51",
"safari": "14.1",
"ios": "14.5",
"samsung": "11.0",
"android": "72",
"electron": "5.0"
},

The namespace export has been supported natively since Node.js 13.2. So this plugin is not enabled giving Node.js 14 as a target. Previously this procedure is only applied when modules is false. After 7.23.3, it is applied to other modules options except auto, which uses the caller support data instead of the compat data.

Is this change breaking your website?

@BrooklynKing
Copy link
Author

Correct me if I'm wrong, but according to current codebase, this plugin is applied only if modules === "auto".

In our case after we upgraded to 7.23.3 we do not get this plugin as part of preset. So now we need to explicitly provide it in our config. It's not something we don't like, but this type of changes are not expected in patch version.

@JLHwung
Copy link
Contributor

JLHwung commented Dec 5, 2023

If you specify modules: false with Node.js 13.1 as a target, the namespace export will be enabled because Node.js 13.1 does not support namespace export: Babel 7.23.5 Link

Thanks for digging the codebase,

// 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");
}

specifies the only exception case where we should load transform-export-namespace-from when the compat data says otherwise.

In Babel 7.23.3, the export-namespace-from, like any other normal plugins, is also determined by the filterItems utility:

export default function filterItems(
list: { [feature: string]: Targets },
includes: Set<string>,
excludes: Set<string>,
targets: Targets,
defaultIncludes: Array<string> | null,
defaultExcludes?: Array<string> | null,
pluginSyntaxMap?: Map<string, string | null>,
) {
const result = new Set<string>();
const options = { compatData: list, includes, excludes };
for (const item in list) {
if (isRequired(item, targets, options)) {
result.add(item);
} else if (pluginSyntaxMap) {
const shippedProposalsSyntax = pluginSyntaxMap.get(item);
if (shippedProposalsSyntax) {
result.add(shippedProposalsSyntax);
}
}
}
defaultIncludes?.forEach(item => !excludes.has(item) && result.add(item));
defaultExcludes?.forEach(item => !includes.has(item) && result.delete(item));
return result;
}

which goes through the compat-data (it contains export-namespace-from) and filter a list of plugins that should be enabled by preset-env.

We consider this change a patch because preset-env is now better at excluding plugins users don't actually need as it is supported natively.

@BrooklynKing
Copy link
Author

Thank you for explanation.

So if it's expected that for node >13.1 I should explicitly add that plugin if I'm not using modules: "auto" - we will do it.

@JLHwung
Copy link
Contributor

JLHwung commented Dec 5, 2023

@BrooklynKing

So if it's expected that for node >13.1 I should explicitly add that plugin

Actually quite the opposite, Node >= 13.2 already supports export * as ns from "./module", it is expected that this plugin should not be enabled.

If you want to force enabling this plugin, please specify the include option

{
  "presets": [["@babel/preset-env", { "include": ["transform-export-namespace-from"] }]]
}

So you don't have to rely on the modules implementation details.

@BrooklynKing
Copy link
Author

Yeah, thanks again for the explanation, I think now I understand better in what part of our flow we don't have a proper sequence. I will close issue.

@mareolan
Copy link

mareolan commented Dec 13, 2023

@JLHwung
I might be misinterpreting something here, but the doc states that modules option is "Enable transformation of ES module syntax to another module type.". So if I have e.g. modules="commonjs", I assumed that the end result should use commonjs syntax (and the source code could still be using ES module syntax) - shouldn't the plugin transform-export-namespace-from still be automatically used too so that export * as Xyz from ... syntax is still parsable on the input side? In 7.23.3 it isn't so transformation fails.

Your suggestion of force-including the plugin works in my case too, though I kind of expected it to work without having to do that.

@nicolo-ribaudo
Copy link
Member

Babel parses export * as Xyz from ... by default, it doesn't need a plugin for that. @babel/transform-export-namespace-from is only necessary when not compiling to CommonJS and targeting some platform that supports ESM but not export * as ....

Do you have a complete stack trace for the error you are seeing?

@mareolan
Copy link

The stack trace is:

mareolan@LAPTOP:~/tmp/tst-babel$ ./node_modules/.bin/babel src --out-dir lib
SyntaxError: /home/mareolan/tmp/tst-babel/src/index.js: Export namespace should be first transformed by `@babel/plugin-transform-export-namespace-from`.
> 1 | export * as Extra from "./extra.js";
    |        ^^^^^^^^^^
  2 |
    at File.buildCodeFrameError (/home/mareolan/tmp/tst-babel/node_modules/@babel/core/lib/transformation/file/file.js:205:12)
    at NodePath.buildCodeFrameError (/home/mareolan/tmp/tst-babel/node_modules/@babel/traverse/lib/path/index.js:98:21)
    at assertExportSpecifier (/home/mareolan/tmp/tst-babel/node_modules/@babel/helper-module-transforms/lib/normalize-and-load-metadata.js:98:16)
    at /home/mareolan/tmp/tst-babel/node_modules/@babel/helper-module-transforms/lib/normalize-and-load-metadata.js:191:9
    at Array.forEach (<anonymous>)
    at /home/mareolan/tmp/tst-babel/node_modules/@babel/helper-module-transforms/lib/normalize-and-load-metadata.js:190:31
    at Array.forEach (<anonymous>)
    at getModuleMetadata (/home/mareolan/tmp/tst-babel/node_modules/@babel/helper-module-transforms/lib/normalize-and-load-metadata.js:137:27)
    at normalizeModuleAndLoadMetadata (/home/mareolan/tmp/tst-babel/node_modules/@babel/helper-module-transforms/lib/normalize-and-load-metadata.js:47:7)
    at rewriteModuleStatementsAndPrepareHeader (/home/mareolan/tmp/tst-babel/node_modules/@babel/helper-module-transforms/lib/index.js:92:54) {
  code: 'BABEL_TRANSFORM_ERROR'
}

It can be simulated by creating simple project in an empty folder:

npm init -y
npm i -D @babel/core @babel/cli @babel/preset-env
mkdir src
echo 'export * as Extra from "./extra.js";' > src/index.js
echo 'export const A = 10;' > src/extra.js
cat > babel.config.js <<EOF
module.exports = {
  sourceType: "unambiguous",
  presets: [
    [
      "@babel/preset-env",
      {
        modules: "commonjs",
        targets: "node 16",
        bugfixes: true,
      },
    ],
  ],
};
EOF

And then just run

./node_modules/.bin/babel src --out-dir lib

There seems to be an issue with targets - if it's left out, it works, and output looks like ES5. If it's there then it doesn't work (and the value of targets doesn't seem to matter).

Adding include: ["transform-export-namespace-from"] makes it work even with targets.

@JLHwung
Copy link
Contributor

JLHwung commented Dec 14, 2023

@mareolan Thanks for the reproduction, reopen the issue.

@JLHwung JLHwung reopened this Dec 14, 2023
@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 9, 2024
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 9, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
i: bug outdated A closed issue/PR that is archived due to age. Recommended to make a new issue pkg: preset-env
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants