Skip to content
This repository has been archived by the owner on Apr 22, 2023. It is now read-only.

Node causes uglify-js package to throw ReferenceError on parse.js line 53 #6235

Closed
picomancer opened this issue Sep 17, 2013 · 18 comments
Closed

Comments

@picomancer
Copy link

This is not a bug in uglify, this is a bug in the node.js javascript interpreter which is triggered by an uglify source file. A test case would be nice, but I do not have one; the only way I know to reproduce it is to run the uglify package on an affected node version.

To reproduce this bug, try the following:

./configure --prefix=$HOME/opt/node
make
make install
$HOME/opt/node/bin/npm install uglify-js
echo "" | $HOME/opt/node/bin/node $HOME/opt/node/bin/uglifyjs

This results in the following error message:

DEBUG: ERROR in file: $HOME/opt/node/lib/node_modules/uglify-js/lib/parse.js / ReferenceError: KEYWORDS is not defined

Of course, $HOME in the above error message will be replaced with your actual home directory. The following code begins at the top of parse.js, immediately after the license comment:

"use strict";

var KEYWORDS = 'break case catch const continue debugger default delete do else finally for function if in instanceof new return switch throw try typeof var void while with';
var KEYWORDS_ATOM = 'false null true';
var RESERVED_WORDS = 'abstract boolean byte char class double enum export extends final float goto implements import int interface long native package private protected public short static super synchronized this throws transient volatile'
    + " " + KEYWORDS_ATOM + " " + KEYWORDS;
var KEYWORDS_BEFORE_EXPRESSION = 'return new delete throw else case';

KEYWORDS = makePredicate(KEYWORDS);        /* line 53 */

If KEYWORDS is undefined on line 53, that is clearly a bug in the Javascript engine, not in uglify!

Many other projects are affected by this bug, basically anything whose transitive dependencies use uglify. A few reports: mozilla/tincan#23 yeoman/generator-angular#360 pugjs/pug#1181

Several of those issue threads noted that this bug does not occur when the same version of uglify is used with an earlier version of node!

I did a git bisect, and I determined that the problematic commit is 7afdba6. Unfortunately this is a very complex patch with approximately 1000 inserts and deletions, and I am out of my depth, I know nothing about node's internals and learning them well enough to fix this would take a long time, so I'm filing this bug report instead.

As a node.js user, I think this bug should be high priority. That is, future numbered releases should either revert the problematic commit, or fix the problem at least well enough for uglify to work as it always has.

@Mithgol
Copy link

Mithgol commented Sep 17, 2013

Wait, what? The commit 59a075e (that you mentioned) is not so complex, it contains only 18 additions and 7 deletions.

@avalanchy
Copy link

What's the temporary solution?

@jo
Copy link

jo commented Sep 17, 2013

I can't uglify, too. Using node v0.11.8-pre

@pipobscure
Copy link

This seems to be a duplicate of #6208 and might be related to these as well:

#6201
#6228
#6232

@picomancer
Copy link
Author

Mithgol: Good catch! Sorry, I mislabeled the problematic commit, it's 7afdba6. I've edited the top post accordingly. I just grabbed a commit blindly to get the URL, and meant to copy-paste the bisect result from my shell. Good catch!

@bnoordhuis
Copy link
Member

Can you guys retest with the latest HEAD of master? I'm reasonably sure @domenic fixed that.

@bnoordhuis
Copy link
Member

Belay that, it's still broken with master. @domenic or @indutny, can one of you two look into this? Thanks.

@metakermit
Copy link

Affecting me too while trying to serve reveal.js - when I run grunt serve, it dies with:

Users/kermit/projekti/git/drugi/reveal.js/node_modules/grunt-contrib-uglify/node_modules/uglify-js/lib/parse.js:53
KEYWORDS = makePredicate(KEYWORDS);
         ^
util.debug: Use console.error instead
DEBUG: ERROR in file: /Users/kermit/projekti/git/drugi/reveal.js/node_modules/grunt-contrib-uglify/node_modules/uglify-js/lib/parse.js / ReferenceError: KEYWORDS is not defined

For now I worked around it by just removing uglify from the Gruntfile and using the unuglified (pretty?) reveal.js file.

metakermit added a commit to metakermit/reveal.js that referenced this issue Sep 18, 2013
@indutny
Copy link
Member

indutny commented Sep 18, 2013

Looking...

@tjfontaine
Copy link

btw @indutny and I worked out a much smaller reproducible for this

require('vm').runInNewContext('"use strict"; var v = 1; v = 2');

@indutny
Copy link
Member

indutny commented Sep 19, 2013

Works with this patch https://codereview.chromium.org/24272005/, waiting for v8 team to review

@sonewman
Copy link

👍 I am having the same problem with v0.11.8-pre

@indutny
Copy link
Member

indutny commented Sep 23, 2013

Fixed in 970bdcc.

@bnoordhuis
Copy link
Member

Reopening for now. I've rolled back the V8 upgrade in 14687eb due to stability issues.

@sc0ttwad3
Copy link

KEYWORDS is not defined ERROR (Details using node-v0.11.7-x86)

ERROR in file: PROJECT_DIR\node_modules\grunt-contrib-uglify\node_modules\uglify-js\lib\parse.js / ReferenceError: KEYWORDS is not defined

Functional Impact

Prevents the use of UglifyJS with Grunt while using the unstable node-v0.11.7-x86 build. Workaround: The obvious uninstall v0.11.x and re-install a stable v0.10.x version.

Minimal Repro Steps

Use grunt-contrib-uglify with Grunt, define a grunt task utilizing UglifyJS in a "normal" way, execute and the error will occur.

What would you expect to happen if there wasn't a bug

UglifyJS would execute and perform its operations without error (stable versions <= node-v0.10.21-x86 work without error).

Actual result

 MY_PROJECT_DIR\node_modules\grunt-contrib-uglify\node_modules\uglify-js\lib\parse.js:53
 KEYWORDS = makePredicate(KEYWORDS);
     ^
 util.debug: Use console.error instead
 DEBUG: ERROR in file: MY_PROJECT_DIR\node_modules\grunt-contrib-uglify\node_modules\uglify-js\lib\parse.js / ReferenceError: KEYWORDS is not defined

What is actually happening

Seems references to KEYWORDS fail with the not defined error.

Further technical details

No strange configurations, environments or otherwise on every machine I've encountered this error on.

@indutny
Copy link
Member

indutny commented Oct 28, 2013

@sc0ttwad3 thanks for detailed report, but it seems to be working on to-be-released node.js version 0.11.8-pre.

@sc0ttwad3
Copy link

@indutny Thanks! A couple of days after my bug report and day after your response, v0.11.8 (Unstable) became available. I installed and retested everything. Glad to report this issue no longer exists! Awesome!

@warpech
Copy link

warpech commented Nov 26, 2013

I was experiencing this issue with uglify-js in Node v0.11.7. The problem is gone in Node v0.11.9

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

No branches or pull requests