JS minify failed on public/javascripts/app.js: TypeError: Cannot assign to read only property 'except' of [object Object] about terser-brunch HOT 20 CLOSED

does uglify works if you do it w/o brunch with these params?

from terser-brunch.

If I invoke uglifyjs with the following command line:

uglifyjs -m -c -r "require" public/javascripts/app.js

It works just fine.

from terser-brunch.

ah, indeed this is the issue, just checked code.

@es128 seems it’s from your changes

from terser-brunch.

I believe I'm just passing through the config parameters as-is. Perhaps these are malformed for what the ugiify API expects? You sure except is a configurable param and expects an array?

from terser-brunch.

If you look at the uglifyjs source, except is configurable and expected to be an array:

https://github.com/mishoo/UglifyJS2/blob/master/bin/uglifyjs#L130
https://github.com/mishoo/UglifyJS2/blob/master/lib/scope.js#L340

from terser-brunch.

The issue is definitely happening within Uglify, though, and you're trying to use the config options in an undocumented way. The only documented usage I can see via the node API is mangle: false. You might need to take your issue over there.

from terser-brunch.

It is not undocumented. Uglify's command line allows for reserved identifiers:

https://github.com/mishoo/UglifyJS2#usage (see -r/--reserved)

And grunt's uglify plugin uses the very same except array:

https://github.com/gruntjs/grunt-contrib-uglify/blob/master/README.md#reserved-identifiers

from terser-brunch.

Moreover, if I hack my own object into uglify-js-brunch/lib/index.js:

  optimized = uglify.minify(data, {
    fromString: true,
    mangle: {
      except: ["require"]
    }
  }).code;

That works just fine. For whatever reason, the config object as it is in the source is causing "TypeError: Cannot assign to read only property"

from terser-brunch.

It's undocumented in terms of the Uglify.minify API method, but yes you've proven your point. I'm glad you were able to isolate the issue further. I'll see if I can figure it out.

from terser-brunch.

Ok, looks like I found the source of the issue and a solution. PR on its way.

@paulmillr Brunch's freeze/deepFreeze stuff has turned out to have lots of side effects. This issue for one, but most notably the watching/changing of config.coffee. How important is it to keep doing that?

from terser-brunch.

Works fine, now. Thanks for the quick turnaround!

from terser-brunch.

whats up with plugins like that -- sass etc?

from terser-brunch.

I'll check, but for the most part I don't think they just copy the whole object from config like this one.

from terser-brunch.

Has this issue resurfaced? When using uglify-js-brunch 1.7.4, it works just fine. When I upgrade to 1.7.6, I get the same error I originally reported.

from terser-brunch.

Any points on how can this be reproduced?

from terser-brunch.

Looks like 7b1610a dropped the clone function in favor of a shallow extend. The config object went through deepFreeze, so extend doesn't cut it for solving the same problem as before of uglify attempting to mutate something in the config object that was passed in to it.

from terser-brunch.

extend right now basically copies all properties from config to a new empty object. What's wrong here?

We don't have any nested objects so it should not matter. clone(obj) === extend({}, obj).

from terser-brunch.

We don't have any nested objects

Sure we do. Isn't that the whole source of the problem?

from terser-brunch.

I see. There are arrays. Should be fixed now. @rtruong try master please

from terser-brunch.

Works fine, now. Thanks for the quick turnaround!

from terser-brunch.