Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

FunctionDeclaration should be defined in a variable scope #31

Closed
wants to merge 2 commits into from

Conversation

nkzawa
Copy link
Contributor

@nkzawa nkzawa commented Feb 16, 2014

I had to update the version of gulp to pass tests.

@Constellation
Copy link
Member

Thank you for filing this issue.

What do you mean?
FunctionDeclaration should not be defined in a variable scope in ES6.
For example,

{
  // block scope
  function test() { }
}

In this example, function name test is defined in the block scope.

And in ES5 side, function declaration is not allowed inside a block. So above script should be treated as SyntaxError in ES5 side.

@nkzawa
Copy link
Contributor Author

nkzawa commented Feb 16, 2014

How about the following case? It actually works in node.js and browsers.

try {} catch (e) {
  function test() {}
} 

@Constellation
Copy link
Member

I've read spec and got it.

When

try {} catch (e) {
  function test() {}
} 

is executed, catch (e) creates new scope for e and after that, block

{
  function test() {}
} 

creates innser scope and test is defined to this block scope.

To support it completely, we need to refine escope for ES6. But currently escope only supports ES5 script and anyway function declaration doesn't create variable to VariableEnvironment.

Could you file the issue support ES6?

@Constellation
Copy link
Member

And I appreciate it if you would create PR for updating gulp :)

@nkzawa
Copy link
Contributor Author

nkzawa commented Feb 16, 2014

Ah, got that you respect spec than how it works in actual environments.

I sent PR for updating gulp anyway.
Regarding support ES6, I can't create the issue since I don't know ES6 so much, sorry for that :(

@Constellation
Copy link
Member

I've filed the tracking issue. #33 :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants