Skip to content

Commit

Permalink
Treat function declarations in modules as lexical
Browse files Browse the repository at this point in the history
Closes #714
  • Loading branch information
marijnh committed Aug 6, 2018
1 parent a7dd5fa commit 2cd1d9a
Show file tree
Hide file tree
Showing 2 changed files with 3 additions and 1 deletion.
2 changes: 1 addition & 1 deletion src/statement.js
Original file line number Diff line number Diff line change
Expand Up @@ -498,7 +498,7 @@ pp.parseFunction = function(node, isStatement, allowExpressionBody, isAsync) {
if (isStatement) {
node.id = isStatement === "nullableID" && this.type !== tt.name ? null : this.parseIdent()
if (node.id) {
this.checkLVal(node.id, "var")
this.checkLVal(node.id, this.inModule ? "let" : "var")
}
}

Expand Down
2 changes: 2 additions & 0 deletions test/tests-harmony.js
Original file line number Diff line number Diff line change
Expand Up @@ -16237,3 +16237,5 @@ test('for ([...foo, bar].baz in qux);', {
],
"sourceType": "script"
}, {ecmaVersion: 6})

testFail("var f;\nfunction f() {}", "Identifier 'f' has already been declared (2:9)", {ecmaVersion: 6, sourceType: "module"});

3 comments on commit 2cd1d9a

@jdalton
Copy link
Contributor

@jdalton jdalton commented on 2cd1d9a Aug 6, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this only affects top-level functions.

<script type="module">
function a() {
  var f;
  function f() {}
}

a()
</script>

As that would not error.

@marijnh
Copy link
Member Author

@marijnh marijnh commented on 2cd1d9a Aug 6, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, interesting, I figured they'd taken this opportunity to fix that throughout, but I guess it's only to avoid strangeness around exports. See 3ffe903

@jdalton
Copy link
Contributor

@jdalton jdalton commented on 2cd1d9a Aug 6, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@marijnh Thank you!

Please sign in to comment.