Fix transform of named import with shadowed namespace import

[dhlolo]

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

[babel-bot]

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

[nicolo-ribaudo]

Thank you! Could you add some tests in packages/babel-plugin-transform-modules-commonjs/test/fixtures to show the fixed behavior?

If it is the first time that you contribute to Babel, follow these steps: (you need to have make and yarn available on your machine)

1. Run yarn && make bootstrap to prepare the repository for running the tests
2. Wait ⏳
3. Run make watch (or make build whenever you change a file)
4. Add a test (only input.js; output.js will be automatically generated)
5. Update the code!
6. yarn jest modules to run the tests
   • If some test outputs don't match but the new results are correct, you can delete the bad output.js files and run the tests again
   • If you prefer, you can run OVERWRITE=true yarn jest code and they will be automatically updated.
7. If it is working, run make test to run all the tests

[dhlolo]

Thank you! Could you add some tests in packages/babel-plugin-transform-modules-commonjs/test/fixtures to show the fixed behavior?

If it is the first time that you contribute to Babel, follow these steps: (you need to have make and yarn available on your machine)

1. Run yarn && make bootstrap to prepare the repository for running the tests

2. Wait ⏳

3. Run make watch (or make build whenever you change a file)

4. Add a test (only input.js; output.js will be automatically generated)

5. Update the code!

6. yarn jest modules to run the tests

   • If some test outputs don't match but the new results are correct, you can delete the bad output.js files and run the tests again
   • If you prefer, you can run OVERWRITE=true yarn jest code and they will be automatically updated.

7. If it is working, run make test to run all the tests

Hi, nicola, I updated code and add a input.mjs for test. But expected output is to throw an Error for now. I don't know how to make jest know that the result of my input.mjs should be such an error in babel system.

[nicolo-ribaudo]

You can copy what https://github.com/babel/babel/blob/main/packages/babel-plugin-transform-typescript/test/fixtures/namespace/ambient-module-nested-exported/options.json does

[dhlolo]

You can copy what https://github.com/babel/babel/blob/main/packages/babel-plugin-transform-typescript/test/fixtures/namespace/ambient-module-nested-exported/options.json does

Can this temp PR be merged and pub a patch version for now ? As our team met this problem in production for twice.

[nicolo-ribaudo]

Instead of throwing an error, could you try fixing this issue by updating this check to be "if there is at least one namespace import and zero other imports"?

babel/packages/babel-helper-module-transforms/src/normalize-and-load-metadata.ts

Line 150 in 2c07574

if (metadata.importsNamespace.size > 0) {

Also, you could update the line below to use destructuring as described in the comment in the code, since we don't compile it as loose anymore :)

[dhlolo]

Instead of throwing an error, could you try fixing this issue by updating this check to be "if there is at least one namespace import and zero other imports"?

babel/packages/babel-helper-module-transforms/src/normalize-and-load-metadata.ts

Line 150 in 2c07574

if (metadata.importsNamespace.size > 0) {

Also, you could update the line below to use destructuring as described in the comment in the code, since we don't compile it as loose anymore :)

Thx for your advice. It is done. And to be on the safe side, I do not revert the changes of throwing error.

[nicolo-ribaudo]

And to be on the safe side, I do not revert the changes of throwing error.

Since we have now tests to make sure that it works, I would prefer to revert it :) It adds quite a bit of noise to the code, just for an error that is never expected to happen.

[dhlolo]

And to be on the safe side, I do not revert the changes of throwing error.

Since we have now tests to make sure that it works, I would prefer to revert it :) It adds quite a bit of noise to the code, just for an error that is never expected to happen.

It sounds reasonable. It is reverted.

[liuxingbaoyu]

Thanks!

[nicolo-ribaudo]

Thank you! :)