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

Make let generation the default. #91

Closed
wants to merge 1 commit into from
Closed

Make let generation the default. #91

wants to merge 1 commit into from

Conversation

jdalton
Copy link
Contributor

@jdalton jdalton commented Mar 24, 2017

Now that Node 4 is the min bar let by default would be rad. For the case of the REPL I'm guessing it could be configured false for REPL use. I'm missing the option toggle for the REPL use.

@jdalton
Copy link
Contributor Author

jdalton commented Mar 25, 2017

While working on this PR I noticed some nested import tests failing

switch (i) {
case 0:
  import { a as x } from "./abc";
  break;
case 1:
  import { b as x } from "./abc";
  break;
}

because:

SyntaxError: Identifier 'x' has already been declared

which makes sense because it's let. This is something to keep in mind as a toggle for nested import statements.

@@ -17,7 +17,7 @@ function wrap(name, optionsArgIndex) {
vm[name] = function (code) {
var options = arguments[optionsArgIndex];
var filename = options && options.filename;
var args = [compile(code, filename)];
var args = [compile(code, filename, { generateLetDeclarations: false })];
for (var i = 1; i < arguments.length; ++i) {
Copy link
Contributor Author

@jdalton jdalton Mar 25, 2017

Choose a reason for hiding this comment

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

Being able to set options for the REPL is probably a good thing despite the current nested import let issue.

@benjamn
Copy link
Owner

benjamn commented Mar 25, 2017

Re: switch case scope collisions, I'm tempted to wrap the case: bodies with braces, though I guess that would be an observable semantic change. Where should the let declarations go, if we wanted to make your example work?

@jdalton
Copy link
Contributor Author

jdalton commented Mar 26, 2017

Where should the let declarations go, if we wanted to make your example work?

If nested imports was configurable then in the case of it being disabled let could be used.
As it is, let shouldn't really be used while nested imports is enabled since enabling it can cause errors.

I'll hold off on this PR until a few other things are in place.

This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants