Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Mangling named functions in strict mode can cause syntax errors in Safari #179

Closed
markmevans opened this Issue · 14 comments

6 participants

@markmevans

In strict mode, Safari 6.0.3 on OS X 10.8.3 raises an exception if a named function's name is the same as one of the function's argument names (at least in some scenarios). For example:

$ cat <<EOF | ./bin/uglifyjs -  --mangle
> "use strict";
> 
> function f1() {
>   return function f2(a,b,c) {
>   }
> }
> EOF

results in the following code:

"use strict";function f1(){return function n(n,t,u){}}

Safari parses the original code without errors, but the generated code causes the following error:

SyntaxError: Cannot declare a parameter named 'n'  in strict mode

Manually converting the inner function to be anonymous or renaming to something not matching an argument's name resolves the problem.

Also, the generated code runs fine in Chrome and Firefox and Safari does not raise a syntax error for functions declared globally.

@mishoo
Owner

Looks like a bug in Safari, isn't it?

@markmevans

@mishoo Seems like it. I haven't found anything in the ECMA-262 5.1 that justifies Safari's behavior, but I am a JavaScript noob so I thought maybe it was a matter of interpretation of the spec. Anyway, I wasn't sure and I thought it might be an issue that others would run into. I ran into this "minimizing" an AngularJS project. The code that's tripping up Safari is in AngularJS so I can't modify it. But it looks like I can work around this by using angular.min.js.

In case you're interested, that file is generated using Google's closure compiler which apparently converts the nested function into an anonymous function since there are no references to the function's name. Something like:

"use strict";function f1(){return function(n,t,u){}}

Maybe that's a worthwhile enhancement?

@mishoo
Owner

It does that if you pass -c (compress).

@mishoo mishoo closed this
@markmevans

Facepalm. Thanks.

@markmevans
@mgcrea

Encountering this as well, options: { mangle: true, compress : true } did not work for me, I still get a function r() that breaks safari 6.

This can break many minified scripts, shouldn't this be handled by uglify in some way (through an option maybe), even it the behavior is safari-only?

How can I minify & avoid name collision breaking safari otherwise?

@ashclarke

@mgcrea: On a side note (sorry it's not contributing anything):

Thanks for finding this! I found an issue with my own code whereby I was encountering a strict mode exception because I had the following:

Row { // Object 
    show: function show(show) { //code }
}

The parameter name show was conflicting with the named function, but only in Safari (My version happens to be 5.1.7 on Windows).

This bug report saved me a lot of searching time!

@mgcrea

@ashclarke Thanks! Found that this error was indeed triggered by this code:

var CountdownFactory = function CountdownFactory(options) {
@rvanvelzen rvanvelzen referenced this issue from a commit in rvanvelzen/UglifyJS2
@rvanvelzen rvanvelzen Fix mangling issues with safari
Fixes #326 and #179

It isn't a really clean solution, but it works. The idea is to
"virtually" reference the function name inside the scope. But to be able
know it isn't *really* referenced, we add a `virtual` flag to
`AST_SymbolRef.reference`.
360628d
@steeeveb

I think I have the same issue using uglify-js 2.4.16 via grunt-contrib-uglify 0.7.0. In my case, the error is on something like

c.prototype.key=function k(k){var a=this._baseState;return f(null===a.key),a.key=k,this}

tested on safari 7 and 8

@steeeveb

ok, I found the code that produce this result:

Node.prototype.key = function key(key) {
      var state = this._baseState;

        assert(state.key === null);
          state.key = key;

            return this;
};

It's part of the library added by browserify to my node modules.
Am I doing something wrong?

@steeeveb

I tried reserving the word 'key', but the result is the same, because key is also a reserved word in ES5.
So I don't really know if the issue has to resolved with Uglifyjs.
Can I map reserved words with mangle?

@avdg

I know it's safari bug, but lets try to get the total perspective of ecmascript's keywords

@steeveb where did you get the claim of key being a reserved word in es5?

Looking up the ecmascript standard: (lets assume the input is always an identifier)

Identifier ::
    IdentifierName **but not** ReservedWord

ReservedWord ::
    Keyword
    FutureReservedWord
    NullLiteral
    BooleanLiteral

Keyword :: **one of**
    break        do          instanceof  typeof
    case         else        new         var
    catch        finally     return      void
    continue     for         switch      while
    debugger     function    this        with
    default      if          throw
    delete       in          try

FutureReservedWord :: **one of**
    class   enum    extends  super
    const   export  import

NullLiteral ::
    **null**

BooleanLiteral ::
    **true**
    **false**

(see http://www.ecma-international.org/ecma-262/5.1/#sec-A.1)

My current guess is that the safari implementation believes that key is a reserved keyword and by that does not agree with what the ecmascript 5 standard says.

Though I will not rule out that previous versions of ecmascript may had it as keyword, but I don't have time yet to find it out.

@ashclarke

@steeeveb The problem is not the word "key" as such. As per my comment, #179 (comment), the problem is that your key function has a parameter named key also.

If you had something like Node.prototype.key = function key(theKey) { instead then it would work.

@steeeveb

Yes, I confirm. Now I'm online with the minified version modified by hand in that point(a little bit ugly :) )
After looking better, I found that the function "key" is imported by Mathjs(I think inside a dependency)
So searching for a solution with Uglifyjs options is not the right thing. I'll try elsewhere!

thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.