Feature: helper-module-imports should not duplicate imports

[conartist6]

💻

• Would you like to work on this feature?

What problem are you trying to solve?

When I import the same name from the same source more than once I end up with copies of the name, as in this block which was created with @babel/helper-module-imports, specifically addNamed.

import {
  await_ as _await_4,
  wrapAsyncGenerator as _wrapAsyncGenerator2,
  await_ as _await_3,
  asyncIterator as _asyncIterator2,
  asyncGeneratorDelegate as _asyncGeneratorDelegate2,
  await_ as _await_2,
  wrapAsyncGenerator as _wrapAsyncGenerator,
  await_ as _await_,
  asyncIterator as _asyncIterator,
  asyncGeneratorDelegate as _asyncGeneratorDelegate,
} from '../internal/asyncish.js';

Describe the solution you'd like

Currently @babel/helper-module-imports exports addDefault, addNamed, addNamespace, and addSideEffect. I would like to see the addition of hasDefault, hasNamed, hasNamespace, and hasSideEffect. These has* methods could be used by the add* methods to avoid creating duplicate identifiers, or they could be used in other ways.

Describe alternatives you've considered

It would also probably be possible to simply bake the functionality into import-builder.ts by using some mechanism for the deferral of identifier binding so that the structure of an import could be completely built and then compared to existing declarations before choosing an appropriate local identifier.

Documentation, Adoption, Migration Strategy

While the change would affect output I would expect it to be backwards compatible.

[babel-bot]

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

[conartist6]

I've also created a stopgap in @babel/helper-ast-match pr which makes it easier to search for syntactic patterns which are represented by nested nodes in the AST.

[conartist6]

I'm going to talk this through with myself here, because that's what I do.

It's pretty obvious to me what needs to happen when you have a single import statement. You can freely add specifiers and you're done. But it's not always that simple. Sometimes you'll have to merge imports that are a multi-part construct such as

import _tmp from 'source';
var def = interopRequireDefault(_tmp).default;

[conartist6]

That specific construct is pretty easy to deal with conceptually. We can reuse the import _tmp line and we would need to create a new interopRequire line. For example to import a named specifier when we already have the default specifier:

import _tmp from 'source';
var def = interopRequireDefault(_tmp).default;
var named = _tmp.named;

In theory this basic approach works everywhere, but it's a little more complicated in practice since some requires are written on a single line for brevity, such as:

var def = interopRequireDefault(require('source')).default;

In order to allow freely adding specifiers that statement would need to become two separate statements similar to the import example:

var _tmp = require('source');
var def = interopRequireDefault(_tmp).default;

Once this is done it would again be trivial to locate the first line and add another name binding like var named = _tmp.named;.

The question is: should we just always defensively choose the two-line output format? Or should we write code that explicitly explodes the one-liner into two at the moment that it needs to create a third line...? Or should we just not worry about combining these one-liners at all?

[conartist6]

I guess the rock bottom for me is merging imports. Merging the requires would be a vaguely nice-to-have, but merging the imports is critical in my mind. Use the nice syntax, get the nice benefits.

Splitting one-line requires into two-line requires is a pretty good technique from the perspective of the quality of code generated and isn't too hard once you find the existing require, but it will be rather annoying to search out requires in the program body when they could wrapped in a bunch of other things. It would take a traversal of the module scope to find them.