UglifyJS broken on node master

[isaacs]

Here's what I'm seeing:

$ make clean all
rm -f semver.browser.js semver.min.js semver.browser.js.gz semver.min.js.gz
( cat head.js; \
        cat semver.js | \
            egrep -v '^ *\/\* nomin \*\/' | \
            perl -pi -e 's/debug\([^\)]+\)//g'; \
        cat foot.js ) > semver.browser.js
uglifyjs -m <semver.browser.js >semver.min.js

/Users/isaacs/dev/js/uglify-js/bin/uglifyjs:221
    if (ex instanceof UglifyJS.DefaultsError) {
                              ^
TypeError: Expecting a function in instanceof check, but got TypeError: Object #<Object> has no method 'OutputStream'
    at Object.<anonymous> (/Users/isaacs/dev/js/uglify-js/bin/uglifyjs:221:31)
    at Module._compile (module.js:449:26)
    at Object.Module._extensions..js (module.js:467:10)
    at Module.load (module.js:349:32)
    at Function.Module._load (module.js:305:12)
    at Function.Module.runMain (module.js:490:10)
    at startup (node.js:121:16)
    at node.js:764:3
make: *** [semver.min.js] Error 1

It works fine on Node v0.10. I'd happily accept that this is a bug in Node (even if it's just "something internal/undocumented changed in a subtle way"), but I'm a bit confused by the UglifyJS source code, and can't seem to find where UglifyJS.DefaultsError is even being assigned.

[isaacs]

Aha, appears to be a V8 bug:

> var o = vm.createContext({}); vm.runInContext('function f(){}', o, 'f.js'); o.f
undefined
> var o = vm.createContext({}); vm.runInContext('var f = function f(){}', o, 'f.js'); o.f
[Function: f]

Function declarations should create globals, but apparently are not doing that.

[michaelficarra]

Wow, that's a pretty serious v8 bug. The test suite must include something as simple as that, no?

[isaacs]

Nope, this is not a V8 bug. It's a Node bug.

The issue is that function declarations are not "set" operations, they're "configure" operations, and we're not catching those and handling them.

https://gist.github.com/isaacs/7163071

[isaacs]

Still, though, seriously... Uglify should be using regular modules and explicit exports, rather than this homegrown vm stuff. What percentage of users of uglify are not running it on the command line with Node?

[mishoo]

Uglify should be using regular modules and explicit exports, rather than this homegrown vm stuff.

I'm not a big fan of CommonJS… Essentially, I'd like to have a way to split a module across multiple files, so that I can access stuff which is internal to my library unprefixed and without explicit exports/imports. Because there was no way to do that, I invented one that works for me.

Since I did not use undocumented APIs, I'd expect things to continue to work in new versions of Node (and if such API is to be deprecated, I'd expect a warning and sufficient time to fix my stuff before the API is dropped completely).

On the other hand, I'm glad it helped exposing such serious bug early. ;-)

[iolloyd]

so where is the fix for this?

[mishoo]

@iolloyd in NodeJS, hopefully.

[isaacs]

Yes, it's been fixed in node master.