ImportBatchSpecifier vs ImportNamespaceSpecifier

[sebmck]

Currently there's a discrepancy between Acorn and Esprima where Acorn has a ImportBatchSpecifier node which has an name property while Esprima uses ImportNamespaceSpecifier with an id property. How are node types standardised (if at all) and which is correct?

[RReverser]

@sebmck Yes, I also figured we have got this problem after ariya/esprima#287 (and tried to discuss there, no luck with results so far).

Our node types were implemented few months earlier, discussed with guys from Mozilla and submitted to Parser API as tasks, but few months after esprima decided to invent own types instead of using existing implementations. That doesn't feel good, especially given that there exist only three of those (Reflect.parse, acorn and esprima) so it wouldn't be hard to check existing solutions and use them or discuss with their authors in corresponding trackers.

[sebmck]

@RReverser Ah okay. This issue was more to see whose behaviour is correct and fix it if possible. Closing this since esprima is the one deviating from the spec, at least a proposed one.

[caridy]

@RReverser just copying a couple of notes from ast-types PR here:

• semantically, import * as name from ... is not a batch operation, it is not doing any repetitive operation, instead it is declaring a new identifier that points to the live bindings of a module, this is completely different from export * from ... in which case you will iterate over each binding (based on the spec'd algorithm), apply a logic to see if it should be defined as a named export identifier. the fact they use the same character * does not means they are the same thing.
• using name instead of id here means tools like es6-module-transpiler will have trouble dealing with the AST. semantically, the id defines the identifier to be defined locally, while name defines the binding name from the module identifier. you can see how namespace and default specifiers will only use id.

the second one is the most concerning one.

[RReverser]

@caridy

• AFAIR both import * as name from ... and export * from ... do the same - it's import for all live bindings defined within module and since module can't export anything dynamically, so it's semantically same batch operation for all statically found exports. The only difference is that import * collects those found "properties" into local object white export * re-introduces them outside as part of own exports. But that doesn't change the fact that they both do the same batch operation for all the statically found exports of given module.
• Arguments regarding changing name to id look pretty reasonable, I think we can make this change and send update to guys from Mozilla. Do you want to send PR on your own?

[caridy]

@RReverser I hate to go in circles about this, but the reality is that in the loader specs -- which we are still working on -- the import * as operation is not the same as export *, the fact that import * as does not apply any algorithm, and the instance of Reflect.Loader.Module is what is passed around (shared for all modules who imports the same bindings thru the import * as syntax, is a testament that this is NOT a batch operation. We have a meeting next week about this, I will clarify with the other editors, and try to make that more explicit is the specs to avoid this sort of confusion.

[RReverser]

@caridy That feels like passing same instance or batch-loop is more implementation details than semantic difference.

In both cases user 1) gets access to all the exports of module and 2) either can access them in own code through local var or can re-export to outer consumer. But import/export * part is more about (1) part where both have same semantic meaning - "get 'em all", which is perfectly reflected by Batch suffix if not digging into implementation/algorithm details which, I believe, is too specific as for AST representation (which is more about code structure than about under-the-hood implementation).

[caridy]

Ok, agree to disagree then. Ultimately, it is your call to produce a different AST.

[RReverser]

Ultimately, it is your call to produce a different AST.

Um, it's not. There were types for this syntax feature much earlier than Esprima's PR appeared and I believe such a constructive conversation on features and names should be done before implementing them differently in another library. I believe that even now it's not too late to discuss and fix things up so everybody would be happy.

[dead-claudia]

It does come across as inconsistent for ImportBatchSpecifier to use "name" instead of "id" like the rest of the ImportSpecifiers, which are used for both default and single namespace imports.

Side note, and I know I'm probably not helping the bikeshedding here by saying this, but why can't we do anything like this? This is very similar to how the ECMAScript specification handles it. (1, 2, see the second table at each anchor for examples) I've got a more complete spec in this repo, complete with an issue tracker/etc.