v2.8.8 breaks React Native

[brentvatne]

React Native depends on on uglify-js@^2.6.2 and as per facebook/react-native#12772 we have run into this error:

[email protected]:

e.runInContext=n

[email protected]:

r.runInContext=runInContext

runInContext is undefined in JSC, where react-native runs.

I know very little about uglify-js but posting this here in hopes that you folks may have a better idea of what the problem is.

[alexlamsl]

@brentvatne can you share/specify the input file to uglify-js so I can investigate further?

[brentvatne]

I'll try to get that for you now. Until this is resolved could you possibly mark 2.8.7 as the latest on npm? It will avoid frustration for many people

[alexlamsl]

@brentvatne not exactly familiar with how to do that on npm - any pointers? 😓

[brentvatne]

npm dist-tag add [email protected] latest

[alexlamsl]

@brentvatne thanks. I'll be releasing 2.8.9 with a couple of fixes soon, but I've run your command for now anyway.

[brentvatne]

@alexlamsl - can you let me know before you do so that I can test it?

[alexlamsl]

You can test the current master now if you want. The only thing pending to merge are test script changes which doesn't affect the release.

[brentvatne]

Same error on master. I'll get you that pre-uglify JS

[kzc]

@alexlamsl Release 2.8.9 anyway. Someone will have to provide a proper repro so we can fix it.

[brentvatne]

@alexlamsl @kzc - here is a (fairly large) file pre-uglify: https://exp-bundles.s3.amazonaws.com/%40community/rediscover/1.2.1/d0eb4b3f6483f41e32b9db0f116c6cda-14.0.0-ios.js

this is how react-native uses uglify-js: https://github.com/facebook/react-native/blob/a2c84d14ce80baeffc50d5f576a07efb6a7230ef/packager/src/JSTransformer/worker/minify.js (don't worry about the source map)

as mentioned above, the minified result should include a string like e.runInContext=n rather than r.runInContext=runInContext

[kzc]

@brentvatne Could you try out the 2.8.8 workaround in #1575 and tell us if that works?

[brentvatne]

sure, trying it.

[brentvatne]

that fixes the problem @kzc! it looks like reduce_vars is supposed to be disabled by default, right? if that's the case and it's not working as expected, that sounds like a very pleasant easy fix

[kzc]

It's supposed to be enabled by default. Just a matter of fixing it.

[brentvatne]

Docs seem inconsistent about that, maybe I'm missing something:

reduce_vars -- default false. Improve optimization on variables assigned with and used as constant values.

reduce_vars (enabled by default) should suffice.

https://github.com/mishoo/UglifyJS2/blob/1f0333e9f146311e0e412fbd0783c0e1e63c7802/README.md

[kzc]

Thanks for pointing that out. The docs will be updated.

[brentvatne]

OK thanks. I just wanted to point out that while this is an outstanding issue, anybody who doesn't use a yarn lockfile or npm shrinkwrap or tries to update those will have issues with React Native minified bundles. It's unfortunate that this has changed in a patch version (although it would have impacted us the same in a minor version) and not been reverted :( Thank you for the quick responses though.

[kzc]

repro:

$ cat ric.js

function f() {
    var runInContext = function runInContext(context) {
        function lodash(value) {}
        lodash.runInContext = runInContext;
        return lodash;
    }
    var _ = runInContext();
}

correct:

$ bin/uglifyjs ric.js -b -c reduce_vars=false

function f() {
    var runInContext = function runInContext(context) {
        function lodash(value) {}
        return lodash.runInContext = runInContext, lodash;
    };
    runInContext();
}

incorrect:

$ bin/uglifyjs ric.js -b -c reduce_vars=true

function f() {
    !function(context) {
        function lodash(value) {}
        lodash.runInContext = runInContext, lodash;
    }();
}

-c keep_fnames=true also appears to fix it...

correct:

$ bin/uglifyjs ric.js -b -c reduce_vars=true,keep_fnames=true

function f() {
    !function runInContext(context) {
        function lodash(value) {}
        return lodash.runInContext = runInContext, lodash;
    }();
}

[kzc]

anybody who doesn't use a yarn lockfile or npm shrinkwrap

They should.

Alternatively, disable reduce_vars or use [email protected] until it is fixed.

[kzc]

Duplicate of #1575.

Possible fix: #1575 (comment)

[brentvatne]

Alternatively, disable reduce_vars or use [email protected] until it is fixed.

The only problem is that this is difficult to communicate to many thousands of users :\

I'll test that fix by modifying uglify-js in-place in node_modules and let you know

[kzc]

The only problem is that this is difficult to communicate to many thousands of users :\

When you don't freeze your module versions you take your chances. Nothing new there.

[ide]

Even if you use a lockfile, people setting up new projects will get the latest version because they don't have a lockfile in their app yet. That's just how npm shrinkwrap and Yarn's lock work.

Even if React Native (or any other library) were to forgo semver and specify precise versions in its package.json, it depends on other packages who use semver in their own package.json files and indirectly depend on Uglify ^2.x.x. So the only practical solutions I see are either to roll back the npm release -- the least disruptive way to everyone being npm dist-tag add [email protected] latest -- or for dependents who rely on semver to move to alternatives that honor semver e.g. babel-minify.

[alexlamsl]

Fix is at #1576

[kzc]

other packages who use semver in their own package.json files and indirectly depend on Uglify ^2.x.x

Then those packages should be changed to lock down their version.

Semver assumes there are no maintenance bugs. Even the act of fixing a bug can create others. The only stable software version is the one you tested with.

[jkomyno]

@alexlamsl Thanks man