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

Global return statements fail #167

Closed
watson opened this issue Jun 19, 2015 · 11 comments

Comments

@watson
Copy link
Member

commented Jun 19, 2015

If I read #148 correctly, this should already be fixed and on npm, but given the following code in the file foo.js it fails anyway:

var n = Math.random()
if (n > 0.5) return console.log('You won the lottery!')
console.log('No luck - try again')

Standard will fail with the error "Illegal return statement":

$ standard --version
4.3.1
$ standard
standard: Use JavaScript Standard Style (https://github.com/feross/standard)
  /tmp/foo.js:2:20: Illegal return statement

So, help me out here :) What's going on?

watson added a commit to watson/git-att that referenced this issue Jun 20, 2015

Remove standard test
This is because of standard/standard#167
When that is fixed it should be added back
@feross

This comment has been minimized.

Copy link
Member

commented Jun 24, 2015

eslint (what standard uses under the hood for error checking) doesn't both support ES6 module syntax and global return statements at the same time. It's super annoying. We added module syntax support somewhere in the v3 series and lost global return support then. Right now, we've opted to support the module syntax instead of global return, as you can see in #148.

But, since we now have support for custom js parsers now, perhaps we can remove module support and add back global return support. Users can get module support by using the esprima-fb or babel-eslint custom parsers.

What do folks think?

@dcousens

This comment has been minimized.

Copy link
Member

commented Jun 24, 2015

Users can get module support by using the esprima-fb or babel-eslint custom parsers.

vs

No configuration. The easiest way to enforce consistent style in your project. Just drop it in.

and

No decisions to make. No .eslintrc, .jshintrc, or .jscsrc files to manage. It just works.

I'm undecided.

@watson

This comment has been minimized.

Copy link
Member Author

commented Jun 24, 2015

Just to clarify my use case: Some of my modules (like git-att referenced in this PR) ships with command line scripts (bin in package.json) and here it's really convenient to use global returns to abort under certain circumstances.

@feross

This comment has been minimized.

Copy link
Member

commented Jun 24, 2015

@watson Understood - I like to do that in my code too. Here's a workaround, if you're interested in one:

if (argv.version || argv.v) {
  version()
  process.exit(0)
}
@feross

This comment has been minimized.

Copy link
Member

commented Jun 24, 2015

@dcousens Yeah, I get what you're saying. Maybe we should just make standard use babel-eslint for broader ES6 and ES7 support? I wonder if that parser supports global return...

@dcousens

This comment has been minimized.

Copy link
Member

commented Jun 25, 2015

That could be worth looking into.

Out of interest, would the desired behaviour with ES6 modules be that if an export is detected, global return should give an error?

@watson

This comment has been minimized.

Copy link
Member Author

commented Jun 26, 2015

@feross Yeah, I usually also do that - but when calling an async function it doesn't help: https://github.com/watson/tweetcat/blob/4b287496b4a2d6dbcc22c495d1e071a9366242f3/bin.js#L25

@feross feross added the bug label Jun 28, 2015

@feross

This comment has been minimized.

Copy link
Member

commented Jun 28, 2015

Using babel-eslint won't solve the problem. babel doesn't support top-level return either.

babel/babel#1298

@feross

This comment has been minimized.

Copy link
Member

commented Jul 10, 2015

I think this issue is unlikely to be fixed since global returns aren't valid ecmascript (which is silly). The various teams that work on the parsers are unwilling to add a special rule for invalid ecmascript.

Unless anyone can think of something clever, I'm going to close this issue as WONTFIX.

@dcousens

This comment has been minimized.

Copy link
Member

commented Jul 11, 2015

I'm thinking WONTFIX is suitable. To clarify, the final position is that any globalReturn shall be invalid, ES5 or not?

@feross

This comment has been minimized.

Copy link
Member

commented Jul 13, 2015

@dcousens That's correct. standard doesn't distinguish between ES5 or ES6 right now so there's no easy way to allow it for just ES5.

@feross feross closed this Jul 13, 2015

perguth added a commit to perguth/signalhub that referenced this issue Jan 4, 2016

@lock lock bot locked as resolved and limited conversation to collaborators May 11, 2018

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
3 participants
You can’t perform that action at this time.