Parse JS Module Blocks proposal #12469

sosukesuzuki · 2020-12-09T20:29:35Z

Q	A
Fixed Issues?	N/A
Patch: Bug Fix?	N
Major: Breaking Change?	N
Minor: New Feature?	Y
Tests Added + Pass?	Yes
License	MIT

Add parsing support for JS Module Blocks proposal.

/cc @littledan @surma

babel-bot · 2020-12-09T20:33:06Z

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

JLHwung · 2020-12-09T20:35:12Z

Can you submit a PR to ESTree on the AST shapes? See also https://github.com/babel/babel/blob/main/CONTRIBUTING.md#creating-a-new-plugin-spec-new

codesandbox-ci · 2020-12-09T20:35:31Z

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit e53f5fe:

Sandbox	Source
babel-repl-custom-plugin	Configuration
babel-plugin-multi-config	Configuration

JLHwung · 2020-12-09T20:44:00Z

packages/babel-parser/src/parser/expression.js

+    const oldExportedIdentifiers = this.state.exportedIdentifiers;
+    this.state.exportedIdentifiers = [];
+    this.eat(tt.braceL);
+    this.scope.enter(SCOPE_OTHER);


We should also enter a new production parameter scope with [Await] parameter when topLevelAwait is enabled, e.g.

module { await 42 }

See

babel/packages/babel-parser/src/parser/statement.js

Line 1537 in 0e27105

this.prodParam.enter(PARAM);

for an example.

189bc49 👍

JLHwung · 2020-12-09T21:40:12Z

packages/babel-parser/src/parser/expression.js

+
+  // https://github.com/tc39/proposal-js-module-blocks
+  parseModuleExpression(): N.ModuleExpression {
+    this.expectPlugin("jsModuleBlocks");


I personally prefer moduleBlocks for brevity.

5dd532b 👍

JLHwung · 2020-12-09T22:00:17Z

packages/babel-types/src/definitions/experimental.js

+defineType("ModuleExpression", {
+  visitor: ["body"],
+  fields: {
+    body: {


I think we should wrap the module blocks within a BlockStatement. Like we did for DoExpression: https://github.com/babel/babel/blob/main/packages/babel-parser/ast/spec.md#doexpression

The BlockStatement is of Scopeable alias, and many more. So when traversing the module body statements in a module block, path.scope can be well defined, currently it points to the scope when the ModuleExpression defines since it is not Scopeable.

We could also consider Program instead of BlockStatement.

Program allows to include Directive in its body. But Module Blocks doesn't seem to allow that. Also current BlockStatement body cannot have ModuleDeclaration.

Should we add new node type? or extends BlockStatement?

I would be really surprised if module blocks don't allow directives, since they are currently also allowed in blocks and functions.

Oh sorry, I misunderstood about directives in ECMAScript spec. Module blocks allows directives. As you said, we can use Program.

Btw, the original reason I proposed Program is that in many transforms we need to inject things to the top-level.
We find the top-level with path.findParent(p => p.isProgram()), and this would nicely work out of the box with module blocks.

Fixed at e381be0

JLHwung · 2020-12-09T22:07:50Z

packages/babel-parser/src/parser/expression.js

+    this.state.exportedIdentifiers = [];
+    this.eat(tt.braceL);
+    this.scope.enter(SCOPE_OTHER);
+    this.parseBlockOrModuleBlockBody(node.body, undefined, true, tt.braceR);


Can you a test case about the locality of this.inModule? We should throw for

// sourceType: "script" module { await: 1 }

because "await" as a label identifier is forbidden when the goal syntax is Module: https://tc39.es/ecma262/#sec-identifiers-static-semantics-early-errors

b4adac3 👍

JLHwung · 2021-01-20T20:56:26Z

packages/babel-parser/src/parser/expression.js

+    const oldInModule = this.inModule;
+    this.inModule = true;
+    const program = this.startNode<N.Program>();
+    node.body = this.parseProgram(program, tt.braceR, "module");


We should also construct new classScope like we did in

babel/packages/babel-parser/src/parser/index.js

Line 37 in 10978bb

this.classScope = new ClassScopeHandler(this.raise.bind(this));

otherwise it will mess up with upper class definition:

class B { #p() { module { class C { [this.#p]; } // it should not have access to `#p` defined across the module block } } }

I wonder if it would be better to introduce a new initializeScopes function, so htat if we have to initialize more things we only have to update one place.

Maybe something like

const oldScopes = this.initializeScopes(); node.body = this.parseProgram(program, tt.braceR, "module")); Object.assign(this, oldScopes);

and then we re-use initializeScopes in Parser's constructor.

nicolo-ribaudo · 2021-01-27T17:57:27Z

Update: Stage 2!

nicolo-ribaudo · 2021-01-27T17:59:13Z

packages/babel-parser/src/parser/expression.js

+/*::
+import type { SourceType } from "../options";
+*/


Nit: no need to comment this. It was only needed for class fields because Flow didn't support the declare keyword.

Suggested change

/*::

import type { SourceType } from "../options";

*/

import type { SourceType } from "../options";

If remove this comment, ESLint warns:

error: 'SourceType' is defined but never used (no-unused-vars) at packages/babel-parser/src/parser/expression.js:58:15: 56 | } from "../util/expression-scope"; 57 | import { Errors } from "./error"; > 58 | import type { SourceType } from "../options"; | ^ 59 | 60 | export default class ExpressionParser extends LValParser { 61 | // Forward-declaration: defined in statement.js

Sorry I'm not familiar with Flow, what should we do?

It's ok like this then.

nicolo-ribaudo · 2021-01-27T18:00:39Z

packages/babel-parser/src/parser/expression.js

+    if (this.isContextual("module")) {
+      const lookahead = this.lookahead();
+      if (
+        lookahead.type === tt.braceL &&


We can use this.lookaheadCharCode() === charCodes.rightCurlyBrace and avoid the expensive this.lokahead().

Also we can avoid lookahead via using new hasFollowingLineBreak, thank you. 370953a

packages/babel-parser/src/parser/expression.js

nicolo-ribaudo · 2021-01-27T18:07:29Z

packages/babel-parser/src/parser/expression.js

+    const oldInModule = this.inModule;
+    this.inModule = true;
+    const program = this.startNode<N.Program>();
+    node.body = this.parseProgram(program, tt.braceR, "module");


I wonder if it would be better to introduce a new initializeScopes function, so htat if we have to initialize more things we only have to update one place.

Maybe something like

const oldScopes = this.initializeScopes(); node.body = this.parseProgram(program, tt.braceR, "module")); Object.assign(this, oldScopes);

and then we re-use initializeScopes in Parser's constructor.

nicolo-ribaudo · 2021-01-27T18:07:50Z

packages/babel-parser/src/util/scope.js

@@ -170,13 +170,16 @@ export default class ScopeHandler<IScope: Scope = Scope> {
  }

  checkLocalExport(id: N.Identifier) {
+    const currentScope = this.currentScope();
+    const scope =
+      currentScope.flags & SCOPE_PROGRAM ? currentScope : this.scopeStack[0];


Can currentScope.flags & SCOPE_PROGRAM ever be false?

I seem to have made a mistake when I added this... 15c8306

nicolo-ribaudo · 2021-01-27T18:08:56Z

.../test/fixtures/experimental/module-blocks/invalid-undefined-export-in-module-blocks/input.js

+module {
+  export { foo }
+}


Another test:

let foo; module { export { foo }; }

sosukesuzuki · 2021-02-12T23:10:15Z

@nicolo-ribaudo

I wonder if it would be better to introduce a new initializeScopes function, so htat if we have to initialize more things we only have to update one place.

What do you think about the interface as below? f651952

const revertScopnes = this.initializeScopes();
node.body = this.parseProgram(program, tt.braceR, "module"));
revertScopes();

nicolo-ribaudo · 2021-02-12T23:27:05Z

We should use try/finally around this.parseProgram (since sometimes we throw errors to "revert" part of the parsing process), but it looks good 👍

EDIT: I was suggesting to use initializeScopes also in

babel/packages/babel-parser/src/parser/index.js

Lines 31 to 38 in 10978bb

    
           const ScopeHandler = this.getScopeHandler(); 
        
           this.options = options; 
        
           this.inModule = this.options.sourceType === "module"; 
        
           this.scope = new ScopeHandler(this.raise.bind(this), this.inModule); 
        
           this.prodParam = new ProductionParameterHandler(); 
        
           this.classScope = new ClassScopeHandler(this.raise.bind(this)); 
        
           this.expressionScope = new ExpressionScopeHandler(this.raise.bind(this));

, so that we don't have to maintain two places where we initialize the same things.

surma · 2021-02-13T12:50:50Z

Just wanted to thank y’all for being on top of my proposal! This is amazing to see. Thanks to this PR, I was able to prototype a babel transform to shim Module Blocks.

❤️

nicolo-ribaudo · 2021-02-13T13:07:09Z

@sosukesuzuki Can we mark this PR as ready?

littledan

Parsing algorithm and AST LGTM.

Nit: Consider calling the AST node "ModuleBlock" instead of "ModuleExpression"? (and ditto parseModuleBlock, etc)

nicolo-ribaudo · 2021-02-13T16:53:41Z

Nit: Consider calling the AST node "ModuleBlock" instead of "ModuleExpression"?

Currently all the expression node names end with "Expression" (both in Babel and in the spec), so I'd prefer to keep it as ModuleExpression even if the proposal is called "module blocks" (I also think that the final production in the spec should/will be called ModuleExpression or something similar).

JLHwung self-requested a review December 9, 2020 20:33

JLHwung reviewed Dec 9, 2020

View reviewed changes

JLHwung added PR: New Feature 🚀 A type of pull request used for our changelog categories pkg: generator pkg: parser Spec: Module Expressions labels Dec 9, 2020

JLHwung reviewed Dec 9, 2020

View reviewed changes

nicolo-ribaudo added the pkg: types label Dec 9, 2020

sosukesuzuki force-pushed the parse-module-blocks branch from aebc37b to b4adac3 Compare December 12, 2020 11:49

sosukesuzuki mentioned this pull request Dec 15, 2020

Add module-expression estree/estree#238

Merged

JLHwung reviewed Jan 20, 2021

View reviewed changes

sosukesuzuki force-pushed the parse-module-blocks branch 2 times, most recently from 8645fbc to fa763c2 Compare January 27, 2021 09:06

nicolo-ribaudo reviewed Jan 27, 2021

View reviewed changes

sosukesuzuki force-pushed the parse-module-blocks branch from fa763c2 to 15c8306 Compare February 12, 2021 23:05

sosukesuzuki force-pushed the parse-module-blocks branch from 0ebce22 to 96c07c5 Compare February 13, 2021 02:01

nicolo-ribaudo approved these changes Feb 13, 2021

View reviewed changes

sosukesuzuki marked this pull request as ready for review February 13, 2021 13:08

littledan approved these changes Feb 13, 2021

View reviewed changes

github-actions bot added the outdated A closed issue/PR that is archived due to age. Recommended to make a new issue label May 28, 2021

github-actions bot locked as resolved and limited conversation to collaborators May 28, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Parse JS Module Blocks proposal #12469

Parse JS Module Blocks proposal #12469

sosukesuzuki commented Dec 9, 2020 •

edited by gitpod-io bot

babel-bot commented Dec 9, 2020 •

edited

JLHwung commented Dec 9, 2020

codesandbox-ci bot commented Dec 9, 2020 •

edited

JLHwung Dec 9, 2020 •

edited

sosukesuzuki Dec 12, 2020

JLHwung Dec 9, 2020

sosukesuzuki Dec 12, 2020

JLHwung Dec 9, 2020

nicolo-ribaudo Dec 9, 2020

sosukesuzuki Dec 12, 2020 •

edited

nicolo-ribaudo Dec 12, 2020

sosukesuzuki Dec 12, 2020

nicolo-ribaudo Dec 12, 2020

sosukesuzuki Dec 12, 2020

JLHwung Dec 9, 2020

sosukesuzuki Dec 12, 2020

JLHwung Jan 20, 2021

nicolo-ribaudo Jan 27, 2021

nicolo-ribaudo commented Jan 27, 2021

nicolo-ribaudo Jan 27, 2021

sosukesuzuki Feb 12, 2021

nicolo-ribaudo Feb 13, 2021

nicolo-ribaudo Jan 27, 2021

sosukesuzuki Feb 12, 2021

nicolo-ribaudo Jan 27, 2021

nicolo-ribaudo Jan 27, 2021

sosukesuzuki Feb 12, 2021

nicolo-ribaudo Jan 27, 2021

sosukesuzuki commented Feb 12, 2021

nicolo-ribaudo commented Feb 12, 2021 •

edited

surma commented Feb 13, 2021 •

edited

nicolo-ribaudo commented Feb 13, 2021

littledan left a comment

nicolo-ribaudo commented Feb 13, 2021 •

edited

Parse JS Module Blocks proposal #12469

Parse JS Module Blocks proposal #12469

Conversation

sosukesuzuki commented Dec 9, 2020 • edited by gitpod-io bot

babel-bot commented Dec 9, 2020 • edited

JLHwung commented Dec 9, 2020

codesandbox-ci bot commented Dec 9, 2020 • edited

JLHwung Dec 9, 2020 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sosukesuzuki Dec 12, 2020 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nicolo-ribaudo commented Jan 27, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sosukesuzuki commented Feb 12, 2021

nicolo-ribaudo commented Feb 12, 2021 • edited

surma commented Feb 13, 2021 • edited

nicolo-ribaudo commented Feb 13, 2021

littledan left a comment

Choose a reason for hiding this comment

nicolo-ribaudo commented Feb 13, 2021 • edited

sosukesuzuki commented Dec 9, 2020 •

edited by gitpod-io bot

babel-bot commented Dec 9, 2020 •

edited

codesandbox-ci bot commented Dec 9, 2020 •

edited

JLHwung Dec 9, 2020 •

edited

sosukesuzuki Dec 12, 2020 •

edited

nicolo-ribaudo commented Feb 12, 2021 •

edited

surma commented Feb 13, 2021 •

edited

nicolo-ribaudo commented Feb 13, 2021 •

edited