RFC 0004: Add ImportExpression AST type

[JLHwung]

View Rendered Text

This RFC proposes to parse import() as a new AST node type ImportExpression instead of CallExpression.

Pinging a few people who may be interested in this:

@axetroy - vscode-npm-import-package-version
@devongovett - parcel
@flytam - babel-plugin-resolve-config-json
@kevinbarabash - eslint-plugin-khan
@ljharb - babel-plugin-dynamic-import-node, eslint-plugin-import
@suchipi - transform-imports
@swannodette - closurescript

Please share this RFC to anyone who can contribute to the discussion.

[ljharb]

Note this will also likely impact eslint-plugin-import, which I also maintain.

[JLHwung]

@ljharb I think any ESLint plugins that relies on CallExpression[callee=Import] will have to adapt to the new ImportExpression regardless of whether this RFC is accepted, because it has been included in ESTree and eslint should follow this path, too. In this way the RFC also aligns babel parser behaviour to ESTree.

[nicolo-ribaudo]

I like this proposal. Note that I don't think that aligning with ESTree should be a goal for us, but this AST shape is definitely better than our current one since it "automatically" disallows invalid states.

[nicolo-ribaudo]

@ljharb Currently, the @babel/plugin-proposal-dynamic-import when targeting CommonJS is just a proxy to babel-plugin-dynamic-import.

Since this AST change makes it harder to have a plugin working with Babel 6-7 and with Babel 8, I was thinking that it would be easier to only support Babel 6-7 in babel-plugin-dynamic-import and write the new logic for Babel 8 in our monorepo.

Does that sound good to you?

[ljharb]

@nicolo-ribaudo that doesn't make sense to me; how does it make it harder? It seems like I just have to add an extra visitor.

If babel wants to inline the dependency, that's one thing, but otherwise it seems pretty trivial to support 6, 7, and 8?

More to the point, if it's not easy to support both babel 7 and 8 in the same transform, then imo the change is poorly designed and shouldn't happen at all.

[jridgewell]

There's a bit of discussion in tc39/proposal-import-attributes#52. Honestly, I'd rather we treat it as a regular function call, exactly the same as super().

[ljharb]

I very much doubt that import() will ever get consensus to be treated as a normal function call; there was a lot of discussion about that during its advancement. It’s syntax, and will likely always be special in a number of ways.

[JLHwung]

Note that the ImportExpression is also created to be consistent with ImportDeclaration.

interface ImportDeclaration <: ModuleDeclaration {
  type: "ImportDeclaration";
  source: Literal;
}

interface ImportExpression <: Expression {
  type: "ImportExpression";
  source: Expression;
}

Although on the CST level an import expression shares similar syntax features with a function call, but on the AST level I think we should abstract them to common import interface.

[kliukovkin]

@JLHwung is there anything left before merging this PR?

[JLHwung]

@ACSAIS

An RFC is considered as accepted if it receives at least 4 approving reviews, and no rejection from core team members. If the RFC is proposed by a core team member, this number is lowered to 3.

https://github.com/babel/rfcs#reviewing-rfcs

[kliukovkin]

Didn't know it, thanks for your answer @JLHwung !

[fisker]

Progress?

[nicolo-ribaudo]

This AST change would also make it easier to support https://github.com/tc39/proposal-import-reflection/. I wonder if we can start implementing it in Babel 7 as opt-in with a parser option.

[fisker]

Though estree uses ImportExpression, and I hope babel aligh with them.

I wounder what the ast will look like for import.source("<specifier>").

{
  "type": "ImportExpression", 
  "phase": "source"
}

?

CallExpression seems easier to extend

{
  "type": "CallExpression", 
  "callee": {
    "type": "MetaProperty"
  }
}

[JLHwung]

@fisker Good question. Currently the MetaProperty represents member-expression-like productions: function.sent and import.meta and they are expressions. However import.source() is different as import.source does not exist. If it were a MetaProperty, it will be inconsistent with other meta properties. If we extend Babel's current CallExpression approach for import.source(), we could use

{
  "type": "CallExpression", 
  "callee": {
    "type": "ImportSource"
  }
}

but popular matchers like CallExpression[callee.type = Import] now have to cover a new AST type. In this case, since either way we will need new AST type for import.source(), I think the ImportExpression approach is more compelling than before.

[fisker]

That's reasonable. 👍

Introduce ImportSource for import.source() seems not a good idea to me, we'll need a new one for import.asset()...

What do you think {ImportExpression,Import}[phase="source" | "asset"]？

[JLHwung]

What do you think {ImportExpression,Import}[phase="source" | "asset"]?

@fisker The Babel AST of import phase will follow the ESTree proposal here. It will be built on top of the ImportExpression AST. Eventually we will phase out the current CallExpression(Import) model.

[nicolo-ribaudo]

@JLHwung Can we enable the new mode by default for Babel 8, and then merge this RFC?

[nicolo-ribaudo]

This is enabled by default in Babel 8 🎉

[suchipi]

Woo! :D