[Bug]: plugin-transform-modules-systemjs transformation is unsafe

[Rich-Harris]

💻

• Would you like to work on a fix?

How are you using Babel?

Programmatic API (babel.transform, babel.parse)

Input code

REPL.

import { x } from './x.js';

if (true) {
	const x = 1;
	console.log(x);
}

new (class extends x {})();

Configuration file name

No response

Configuration

Using @babel/plugin-transform-modules-systemjs, nothing else

Current and expected behavior

I would expect the x = 1 to declare a local constant, as it does in the original code. Instead, it assigns to a new module-level variable (which in this case has the same name as an import, causing an error).

Environment

  System:
    OS: macOS 10.15.7
  Binaries:
    Node: 16.5.0 - ~/.nvm/versions/node/v16.5.0/bin/node
    Yarn: 1.22.5 - /usr/local/bin/yarn
    npm: 7.19.1 - ~/.nvm/versions/node/v16.5.0/bin/npm
  npmPackages:
    @babel/core: ^7.15.8 => 7.15.8 
    @babel/plugin-proposal-dynamic-import: ^7.14.5 => 7.14.5 
    @babel/plugin-transform-modules-systemjs: ^7.15.4 => 7.15.4 

Possible solution

No response

Additional context

cc @guybedford :)

[babel-bot]

Hey @Rich-Harris! 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.

[guybedford]

Thanks for finding this @Rich-Harris. This is definitely a bug in the scope handling that it's collapsing all the local blocks without renaming in the hoisting operation.

//cc @joeldenning @nicolo-ribaudo

[The-x-Theorist]

I'd like to work on this issue?

[The-x-Theorist]

If we rename the variables in hoisting operation, babel-plugin-proposal-do-expressions/test/fixtures/do-expressions/variable-declaration-start-to-iife/exec.js: bar is not defined this test throws error.

[lightmare]

If we rename the variables in hoisting operation, babel-plugin-proposal-do-expressions/test/fixtures/do-expressions/variable-declaration-start-to-iife/exec.js: bar is not defined this test throws error.

Can you keep the name for var declarations, and only rename const/let ?

[The-x-Theorist]

Can you keep the name for var declarations, and only rename const/let ?

Yeah, I can do that.