fix(index): deep cloning options (options.uglifyOptions)

[scott-thrillist]

Proper deep clone, as recommended by MDN:
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Object/assign#Deep_Clone

Use case: I found my config.uglifyOptions object being altered when I passed it into uglifyjs-webpack-plugin. The properties "test" and "warningsFilter" were being added. I also have processes outside of webpack that use this object and it fails as these properties are not recognized by uglify-es/js.

[jsf-clabot]

All committers have signed the CLA.

[michael-ciniawsky]

Please share your webpack.config.js with the UglifyJSPlugin causing this aswell :)

[hulkish]

@scott-thrillist @michael-ciniawsky Pretty sure this is already covered in #80

[scott-thrillist]

@hulkish interesting, this schema method is new to me. I'll test 😄

[scott-thrillist]

Not sure why this isn't installing the /dist directory but can't seem to test it.

"uglifyjs-webpack-plugin": "git+ssh://[email protected]:webpack-contrib/uglifyjs-webpack-plugin.git#hulkish-OptimizeAndValidateOptions"

[hulkish]

@scott-thrillist try commenting this line on your local: https://github.com/webpack-contrib/uglifyjs-webpack-plugin/blob/master/test/helpers.js#L51

[scott-thrillist]

I think my install process may be wrong. I add that dependency line in my package.json, run an npm install and I only get these files in node_modules/uglifyjs-webpack-plugin:

CHANGELOG.md
LICENSE
options.json
package.json
README.md

No subdirectories for some reason.

[hulkish]

@scott-thrillist make sure you're using npm >= 5

[scott-thrillist]

Yea totally:

npm -v
5.3.0

[hulkish]

whats the full extent of the commands youre trying and whats the error you receive? Also, whats your node version?

[scott-thrillist]

Node v8.1.4

• add dependency line to package.json
• delete package-lock.json and node_modules folder to be safe :)
• npm install
• npm run build

Here's the error I get: Error: Cannot find module 'uglifyjs-webpack-plugin'

[hulkish]

dont do this:

add dependency line to package.json
delete package-lock.json and node_modules folder to be safe :)

By doing this, you're essentially side-stepping the package-lock behavior.

do this instead:

git checkout package-lock.json
rm -rf node_modules
npm install
npm install <my dependency>

[scott-thrillist]

Really appreciate the help, though doing all of the above gives me the same result- no subdirectories in node_modules/uglifyjs-webpack-plugin. Here's my dependency line:

npm install uglifyjs-webpack-plugin@git://github.com/webpack-contrib/uglifyjs-webpack-plugin.git#hulkish-OptimizeAndValidateOptions

[hulkish]

Yea dont do that, it won't really work correctly. Instead:

git clone https://github.com/webpack-contrib/uglifyjs-webpack-plugjn.git
cd uglifyjs-webpack-plugin
npm install
npm link
cd <path to my test project>
npm link uglifyjs-webpack-plugin

[scott-thrillist]

That seems right. All my Googling kept referencing install by URL :p Apologies for taking up so much of your time. Will try it this way

[hulkish]

No worries, the internet has a lot of misinformation on the topic.

[hulkish]

Dont forget to npm run build inside uglifyjs-webpack-plugin dir

[hulkish]

That said.. let me know if your issue is resolved in that PR and ill close this.

[scott-thrillist]

Will do! I'm guessing passing this to UglifyJS() won't work anymore:

{
    warnings: false,
    mangle: {
        reserved: ['$super', '$', 'exports', 'require']
    }
}

Getting this: throw new _ValidationError2.default(ajv.errors, name);

Based on the schema file it seems like it should look like this correct?

    let ugOpts = {
        "properties": {
            "uglifyOptions": {
                "properties": {
                    warnings: false,
                    mangle: {
                      reserved: ['$super', '$', 'exports', 'require']
                    }
                }
            }
        }
    };


new UglifyJSPlugin(ugOpts);

[hulkish]

@scott-thrillist try it:

node
> console.log(require('uglify-es').minify{({ foo: `const $super = 5 + 5; exports.foo = require('foo');`,
    warnings: false,
    mangle: {
        reserved: ['$super', '$', 'exports', 'require']
    }
}).code);

But, yea you will need to renest those options when you create the plugin instance.

[scott-thrillist]

Sorry, I meant the new schema for uglifyjs-webpack-plugin. Trying to match my options with this file:

https://github.com/webpack-contrib/uglifyjs-webpack-plugin/blob/hulkish-OptimizeAndValidateOptions/options.json

[hulkish]

I see, yes the schema is a breaking change in that regard. Have you confirmed yet if that pr solves your issue?

[scott-thrillist]

Unfortunately no. :(

const UglifyJSPlugin = require('uglifyjs-webpack-plugin');

let ugOpts = {
    warnings: false,
    mangle: {
        reserved: ['$super', '$', 'exports', 'require']
    }  
};

let formattedUgOpts = {
    "uglifyOptions": ugOpts
};

module.exports.plugins = [
    
    new UglifyJSPlugin(formattedUgOpts),

    function(){
        this.plugin("compile", function(){
            console.log(ugOpts);  // ok
        });
        this.plugin("done", function(){
            console.log(ugOpts);  // has a bunch of new properties inserted (see below)
        });
    }
];



{ warnings: false,
  mangle:
   { reserved:
      [ '$super',
        '$',
        'exports',
        'require',
        'arguments',
        'arguments',
        'arguments',
        'arguments',
        'arguments',
        'arguments',
        'arguments',
        'arguments',
        'arguments',
        'arguments',
        'arguments',
        'arguments',
        'arguments',
        'arguments',
        'arguments',
        'arguments',
        'arguments',
        'arguments',
        'arguments',
        'arguments',
        'arguments',
        'arguments',
        'arguments',
        'arguments',
        'arguments',
        'arguments',
        'arguments',
        'arguments',
        'arguments',
        'arguments',
        'arguments',
        'arguments',
        'arguments',
        'arguments',
        'arguments',
        'arguments',
        'arguments',
        'arguments',
        'arguments',
        'arguments',
        'arguments',
        'arguments',
        'arguments',
        'arguments',
        'arguments',
        'arguments',
        'arguments' ],
     cache: null,
     eval: false,
     ie8: false,
     keep_classnames: false,
     keep_fnames: false,
     properties: false,
     safari10: false,
     toplevel: false } }

I got back and forth on this, but in the end I think best practice would be for me to deep clone my own data before sending it to a plugin, mainly because the nature of open source plugins can do anything they want with the data you provide.

If you don't have any other feedback on this issue feel free to close this PR. And thanks again for being patient with my testing.

[hulkish]

I think handling the deep clone in your implementation is probably best bet. Mainly because this is not an issue which is exclusive to this plugin.

For ex... if you were to do the same test with your actual webpack config.. you will see your original webpack config object be mutated. Which I realize is generally not a good idea from a purist perspective... but the reality is that by solving this here wouldn't really be solving the problem as a whole imo.

[scott-thrillist]

Oh I forgot to mention the best part. It now builds fine with my vanilla uglify-es processes in the done callback. So at least the newly added properties play nicely now. 👍

[hulkish]

Awesome... I'm thinking we can close this now considering previous convo... agreed?

[scott-thrillist]

Yes, done