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

Trying to parse JS file with return statement: 'return' outside of function #288

Closed
walling opened this issue Sep 2, 2013 · 14 comments
Closed

Comments

@walling
Copy link

walling commented Sep 2, 2013

I'm trying to parse a JS file that looks a bit like this:

if (isSomeEnvironment) return;

console.log('Loading stuff.');

However, I get an error: 'return' outside of function [test.js:1,29]

What I want to do is parse the above JS file and then run my custom TreeTransformer. Can I tell the parser to ignore this error, as I'm already fixing it by wrapping it in a lambda function afterwards (and then doing some more transformations).

@stephenmathieson
Copy link

Please fix this. Returning at the global scope is allowed.

@michaelficarra
Copy link
Contributor

@stephenmathieson: Not according to ES5 it's not. Some interpreters like node wrap your program in a function, changing its behaviour. See nodejs/node-v0.x-archive#6254

@stephenmathieson
Copy link

<!doctype html>
<script>return;</script>
<script>alert("it works");</script>

@michaelficarra
Copy link
Contributor

Yes, I said that. It's still not valid JavaScript.

@walling
Copy link
Author

walling commented Mar 28, 2014

It could be nice with an option to allow global scope returns.

@Marshallx
Copy link

I'm trying to uglify a commonJS module in which it is perfectly valid to return from, but I cannot get past this error.

@michaelficarra
Copy link
Contributor

I'll bite. Who/what told you it was valid?

@Marshallx
Copy link

CommonJS modules are wrapped in functions. The following example is valid and works fine:

exports.a = function() {...};

if (typeof Object.keys === 'function') {
    return; // uglify won't allow this
}

exports.keysShim = function() {...};

We use return from within modules in many many places. The issue is that uglify assumes that the contents of a JS file are going to be in global scope, but dynamically loaded modules are not in the global scope.

@michaelficarra
Copy link
Contributor

This is the CommonJS spec: http://wiki.commonjs.org/wiki/Modules/1.1

Where does it say "CommonJS modules are wrapped in functions"? It says "These modules are offered privacy of their top scope", which some implementations (node) choose to implement as a function body. This also gives them a convenient way to introduce the CommonJS module context. But it unintentionally (and incorrectly) adds support for top-level return. Don't take advantage of this feature.

@Marshallx
Copy link

That has nothing to do with uglify. But fair enough mine was a bad example, so forget commonJS and consider a situation where one is building a massive JS-based application with hundreds of Javascript files in pieces that get concatenated together and wrapped in functions (or not) depending on a complex build script. Why should I not be able to uglify my individual files before putting them together, so that I know what has been uglified and what goes where? Scope needn't be relevant to uglify.

"Don't take advantage of this feature." So will uglify also tell me that I am not allowed to use eval or non-strict equals?

@mishoo
Copy link
Owner

mishoo commented Oct 20, 2014

So will uglify also tell me that I am not allowed to use eval or non-strict equals?

No way. :-)

In any case, the idea is that the parser tries to be spec-compliant, and by the JS spec the return keyword is only allowed inside a function. We could add an option to just ignore this fact, though. I'll do that.

@Marshallx
Copy link

Oh I certainly wasn't suggesting for a moment that ignoring return outside function should be standard behavior, clearly that would be inappropriate for most situations (even more now that I have been enlightened about the CJS spec). This is clearly a specialist case but it seems at least a couple of people want it for one reason or another. Awesomes.

@mishoo mishoo closed this as completed in f36a1ea Oct 20, 2014
@mishoo
Copy link
Owner

mishoo commented Oct 20, 2014

There you go. Pass --bare-returns. The original behavior is still on by default.

@stephenmathieson
Copy link

thanks!!

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

No branches or pull requests

5 participants