Update uglify-es dependency to use maintained terser module #296

fabiosantoscode · 2018-05-17T12:43:04Z

I'm updating the uglify-es dependency to the new fork, which is now named terser

jsf-clabot · 2018-05-17T12:43:05Z

All committers have signed the CLA.

alexander-akait · 2018-05-17T13:11:16Z

@fabiosantoscode thanks for PR. Great!

alexander-akait · 2018-05-17T13:12:23Z

@fabiosantoscode any breaking changes between abandoned uglify-es, should we do major release?

fabiosantoscode · 2018-05-17T13:13:57Z

There are no breaking changes!

kzc · 2018-05-17T23:57:44Z

For security reasons it is not recommended to use terser until the npm package ownership issue is resolved:

terser/terser#2 (comment)

All changes to CHANGELOG.md should be reverted - shouldn't change historical references of uglify-es versions to terser retroactively.

alexander-akait · 2018-05-18T10:25:00Z

@kzc thanks for feedback 👍

fabiosantoscode · 2018-06-01T22:01:13Z

The issue has been resolved.

alexander-akait · 2018-06-01T22:18:15Z

@fabiosantoscode what issue? BTW great 👍

kzc · 2018-06-02T17:06:48Z

CHANGELOG.md

@@ -122,7 +122,7 @@ All notable changes to this project will be documented in this file. See [standa

 ### Bug Fixes

-* **package:** use exact `uglify-es` version (`dependencies`) ([#199](https://github.com/webpack-contrib/uglifyjs-webpack-plugin/issues/199)) ([2e2ed36](https://github.com/webpack-contrib/uglifyjs-webpack-plugin/commit/2e2ed36))
+* **package:** use exact `terser` version (`dependencies`) ([#199](https://github.com/webpack-contrib/uglifyjs-webpack-plugin/issues/199)) ([2e2ed36](https://github.com/webpack-contrib/uglifyjs-webpack-plugin/commit/2e2ed36))


Shouldn't change historical references of uglify-es versions to terser retroactively.

kzc · 2018-06-02T17:07:03Z

CHANGELOG.md

@@ -132,7 +132,7 @@ All notable changes to this project will be documented in this file. See [standa

 ### Chores

-* update `uglify-es` v3.2.0...v3.2.1 ([#190](https://github.com/webpack-contrib/uglifyjs-webpack-plugin/issues/190)) ([b356f74](https://github.com/webpack-contrib/uglifyjs-webpack-plugin/commit/b356f74))
+* update `terser` v3.2.0...v3.2.1 ([#190](https://github.com/webpack-contrib/uglifyjs-webpack-plugin/issues/190)) ([b356f74](https://github.com/webpack-contrib/uglifyjs-webpack-plugin/commit/b356f74))


kzc · 2018-06-02T17:07:14Z

CHANGELOG.md

@@ -152,7 +152,7 @@ All notable changes to this project will be documented in this file. See [standa

 ### Chores

-* update `uglify-es` v3.1.3...3.2.0 ([#176](https://github.com/webpack-contrib/uglifyjs-webpack-plugin/issues/176)) ([3be7f62](https://github.com/webpack-contrib/uglifyjs-webpack-plugin/commit/3be7f62))
+* update `terser` v3.1.3...3.2.0 ([#176](https://github.com/webpack-contrib/uglifyjs-webpack-plugin/issues/176)) ([3be7f62](https://github.com/webpack-contrib/uglifyjs-webpack-plugin/commit/3be7f62))


kzc · 2018-06-02T17:07:44Z

CHANGELOG.md

@@ -200,7 +200,7 @@ All notable changes to this project will be documented in this file. See [standa

 ### Features

-* update to `uglify-es` ([#63](https://github.com/webpack-contrib/uglifyjs-webpack-plugin/issues/63)) ([1d62560](https://github.com/webpack-contrib/uglifyjs-webpack-plugin/commit/1d62560))
+* update to `terser` ([#63](https://github.com/webpack-contrib/uglifyjs-webpack-plugin/issues/63)) ([1d62560](https://github.com/webpack-contrib/uglifyjs-webpack-plugin/commit/1d62560))


kzc · 2018-06-02T17:08:00Z

CHANGELOG.md

@@ -253,7 +253,7 @@ All notable changes to this project will be documented in this file. See [standa

 ### Features

-* update to `uglify-es` ([#63](https://github.com/webpack-contrib/uglifyjs-webpack-plugin/issues/63)) ([1d62560](https://github.com/webpack-contrib/uglifyjs-webpack-plugin/commit/1d62560))
+* update to `terser` ([#63](https://github.com/webpack-contrib/uglifyjs-webpack-plugin/issues/63)) ([1d62560](https://github.com/webpack-contrib/uglifyjs-webpack-plugin/commit/1d62560))


kzc · 2018-06-02T17:08:54Z

CHANGELOG.md

@@ -112,7 +112,7 @@ All notable changes to this project will be documented in this file. See [standa

 ### Reverts

-* **package:** use exact `uglify-es` version (`dependencies`) ([#199](https://github.com/webpack-contrib/uglifyjs-webpack-plugin/issues/199)) ([#202](https://github.com/webpack-contrib/uglifyjs-webpack-plugin/issues/202)) ([426bafd](https://github.com/webpack-contrib/uglifyjs-webpack-plugin/commit/426bafd))
+* **package:** use exact `terser` version (`dependencies`) ([#199](https://github.com/webpack-contrib/uglifyjs-webpack-plugin/issues/199)) ([#202](https://github.com/webpack-contrib/uglifyjs-webpack-plugin/issues/202)) ([426bafd](https://github.com/webpack-contrib/uglifyjs-webpack-plugin/commit/426bafd))


Shouldn't change historical references of uglify-es versions to terser retroactively.

Thanks for these! I wasn't being careful enough :/

alexander-akait · 2018-06-04T09:30:07Z

/cc @fabiosantoscode need do changes above

irae · 2018-06-05T18:52:09Z

This feels like a major change community wise. Should the PR be named something like "Move UglifyJS-ES to Terser fork". I don't feel quite comfortable calling it just an "update".

Another approach I would suggest and feel more comfortable with would be a multi step phased approach: First create terser-webpack-plugin (bonus points for having your own readme, goals, community model, website, etc.). Then make a new PR here adding a deprecation notice in favor of terser. Lastly, if adoption is too slow, just require the new terser-webpack-plugin from this one, leaving the warning behind.

Security awareness is growing strong in the community, and replacing core dependencies without user knowledge feels a bit against the current trend.

alexander-akait · 2018-06-05T19:13:35Z

@irae we don't have time on creating new plugin, also terser it is fork uglify-es without breaking change (i.e. it is safe).

Also for webpack@5 we release MinimizerWebpackPlugin support out of box cache/parallel/source maps with support build-in uglify-js, babel-minify and terser minimizers, also allow to using own minimizer, example for css, images and etc.

Creating new plugin will only result in a loss of time. I have been helping a lot of times for webpack/webpack-contrib and i know that no one will create this plugin.

irae · 2018-06-05T20:45:20Z

I was under the impression that creating a new plugin could just be copy/paste (or cp -r ./old ./new) of the existing plugin and renaming it in some places and documenting it in others. The changelog could start fresh with the first changes and a link to the old changelog.

I trust you when you say there are no breaking changes. Tests will pass, everything is fine in the automation part. But when auditing code, thats only part of the job. People will see the core dependency changing and will be more skeptical than I am. Some people will notice, some other people will get warns from security automation tools.

I used to work at a large corporation and this kind of changes happens to go through a lot of scrutiny. The usual outcome of this kind of changes is to pin to an older minor release and move on, because it is much more work to whitelist new packages with security teams and open source legal team.

My point is not about all the work work it takes or making it harder for no reason. It is about how much people trust on the work you've done and about being honest with the community. If I search for UglifyJS on the web I won't find terser website or the terser fork of uglify. I will find the actual uglify-js, old school and es5 that don't mention terser at all.

It was quite confusing to to me to find out that uglifyjs-es was a branch of uglify-js and later find a fork under a different username/organization was actually working on more features. It gave me a lot of trouble to understand this was not a security breach and I am only 85% certain at this point.

I might be wrong here, or overreacting. Feel free to move on and change the dependency.
I pinned all my versions and at some point I'll re-evaluate my decisions.

alexander-akait · 2018-06-05T20:58:39Z

@irae even copy/paste require time, I understand perfectly what you are talking here, but unfortunately at the moment only two developers works in webpack-contrib constantly and creating new plugins require time, also it is require maintenance time, many people constantly using this plugin.

We constantly receive issue about problem with uglify and inline compression in webpack repo. Creating new plugin require change default minimizer plugin (now it is uglify) in webpack (i.e. breaking change for webpack).

We have a choice:

Leave all as is and leave all problems in many projects with minification (i.e. just tell people that nothing is going to fix).
Create new plugin, change this in webpack and release major version (require around 2-3 month).
Merge this and fix many problems in many projects (as i said above terser doesn't have incompatibilities with uglify-es, just fixes).

kzc · 2018-06-15T16:27:08Z

When #308 lands this terser PR should be rebased and disabling the inline compress option should be reverted, as bugs related to it have been fixed in terser.

kzc · 2018-06-21T12:06:27Z

@fabiosantoscode The inline compress option override from #308 should be reverted as there are no known bugs associated with the default inline compress setting in terser:

uglifyjs-webpack-plugin/src/index.js

Lines 45 to 47 in 4f50156

    
           compress: { 
        
             inline: 1, 
        
           },

fabiosantoscode · 2018-06-22T14:06:57Z

@kzc I would much rather let someone else do that, as that commit's changes also touch the tests, docs and other parts of the codebase, not all of those changes are undesirable, and it is a bit out of scope for this PR.

Due to a bug in UglifyJS (mishoo/UglifyJS#2842) we should disable function inlining to avoid falling into this issue. the uglifyjs-webpack-plugin module is considering to move to the maintained fork of UglifyJS (terser: https://github.com/fabiosantoscode/terser): - webpack-contrib/uglifyjs-webpack-plugin#264 - webpack-contrib/uglifyjs-webpack-plugin#296 But until that happens I'd propose to disable function inlining.

timdorr · 2018-07-11T19:38:01Z

BTW, since I'm running into issues with this and am impatient, I published this PR as-is in a scoped package on npm for others to use: https://www.npmjs.com/package/@salesloft/terser-webpack-plugin

DeTeam · 2018-07-19T16:29:30Z

@fabiosantoscode do you think there's any chance getting these changes out there in webpack npm package? :)

fabiosantoscode · 2018-07-19T19:06:20Z

@DeTeam I can't see why not use @timdorr's version of the package in the meantime.

alexander-akait · 2018-08-01T20:00:59Z

Close in favor https://github.com/webpack-contrib/terser-webpack-plugin, feel free to feedback and create new issues

alexander-akait mentioned this pull request May 17, 2018

chore(package): switch to @fabiosantoscode/uglify-es #293

Closed

alexander-akait requested a review from joshwiens May 17, 2018 13:11

alexander-akait added type: Bug type: Chore severity: 4 (inconvenient) semver: Patch priority: 4 (important) labels May 17, 2018

kzc reviewed Jun 2, 2018

View reviewed changes

fabiosantoscode changed the title ~~Update package.json~~ Update uglify-es dependency to use maintained terser module Jun 11, 2018

fabiosantoscode mentioned this pull request Jun 11, 2018

New dependency name: uglify-es => terser hustxiaoc/gulp-minify#43

Closed

This was referenced Jun 13, 2018

Uglify errors with jarallax #310

Closed

Bug report: Minification causes runtime error for a special variable name #311

Closed

olsonpm mentioned this pull request Jun 19, 2018

New dependency name: uglify-es => terser #1 #298

Closed

fabiosantoscode force-pushed the patch-1 branch from 98500ba to 35c3b8d Compare June 20, 2018 12:51

limonte mentioned this pull request Jun 24, 2018

Super expression must either be null or a function, not undefined sweetalert2/sweetalert2#1127

Closed

ZauberNerd mentioned this pull request Jun 28, 2018

fix(webpack): turn off function inlining to avoid const reassignment untool/untool#60

Merged

Cherry mentioned this pull request Jul 11, 2018

Switch to maintained uglify-es fork, Terser gdborton/webpack-parallel-uglify-plugin#54

Closed

fabiosantoscode added 5 commits July 19, 2018 20:10

replace mentions of uglify-es with terser

64c86f2

lint

9b91a16

update with @kzc's proposed fixes

fa744fe

fix snapshots

e217332

update package.json

fca19fe

fabiosantoscode force-pushed the patch-1 branch from 11283a9 to fca19fe Compare July 19, 2018 19:12

fabiosantoscode added 2 commits July 19, 2018 20:27

increase versions and update snapshots

e67f9ed

update package-lock.json

726ab5b

appden mentioned this pull request Jul 22, 2018

Fix minimization of bundles callstack/haul#459

Merged

alexander-akait closed this Aug 1, 2018

DorianGrey mentioned this pull request Aug 16, 2018

Replace uglifyjs-webpack-plugin with terser-webpack-plugin vuejs/vue-cli#2245

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update uglify-es dependency to use maintained terser module #296

Update uglify-es dependency to use maintained terser module #296

fabiosantoscode commented May 17, 2018

jsf-clabot commented May 17, 2018 •

edited

Loading

alexander-akait commented May 17, 2018

alexander-akait commented May 17, 2018

fabiosantoscode commented May 17, 2018

kzc commented May 17, 2018

alexander-akait commented May 18, 2018

fabiosantoscode commented Jun 1, 2018

alexander-akait commented Jun 1, 2018

kzc Jun 2, 2018

kzc Jun 2, 2018

kzc Jun 2, 2018

kzc Jun 2, 2018

kzc Jun 2, 2018

kzc Jun 2, 2018

fabiosantoscode Jun 5, 2018

alexander-akait commented Jun 4, 2018

irae commented Jun 5, 2018

alexander-akait commented Jun 5, 2018 •

edited

Loading

irae commented Jun 5, 2018

alexander-akait commented Jun 5, 2018 •

edited

Loading

kzc commented Jun 15, 2018 •

edited

Loading

kzc commented Jun 21, 2018

fabiosantoscode commented Jun 22, 2018

timdorr commented Jul 11, 2018 •

edited

Loading

DeTeam commented Jul 19, 2018

fabiosantoscode commented Jul 19, 2018

alexander-akait commented Aug 1, 2018 •

edited

Loading

Update uglify-es dependency to use maintained terser module #296

Update uglify-es dependency to use maintained terser module #296

Conversation

fabiosantoscode commented May 17, 2018

jsf-clabot commented May 17, 2018 • edited Loading

alexander-akait commented May 17, 2018

alexander-akait commented May 17, 2018

fabiosantoscode commented May 17, 2018

kzc commented May 17, 2018

alexander-akait commented May 18, 2018

fabiosantoscode commented Jun 1, 2018

alexander-akait commented Jun 1, 2018

kzc Jun 2, 2018

Choose a reason for hiding this comment

kzc Jun 2, 2018

Choose a reason for hiding this comment

kzc Jun 2, 2018

Choose a reason for hiding this comment

kzc Jun 2, 2018

Choose a reason for hiding this comment

kzc Jun 2, 2018

Choose a reason for hiding this comment

kzc Jun 2, 2018

Choose a reason for hiding this comment

fabiosantoscode Jun 5, 2018

Choose a reason for hiding this comment

alexander-akait commented Jun 4, 2018

irae commented Jun 5, 2018

alexander-akait commented Jun 5, 2018 • edited Loading

irae commented Jun 5, 2018

alexander-akait commented Jun 5, 2018 • edited Loading

kzc commented Jun 15, 2018 • edited Loading

kzc commented Jun 21, 2018

fabiosantoscode commented Jun 22, 2018

timdorr commented Jul 11, 2018 • edited Loading

DeTeam commented Jul 19, 2018

fabiosantoscode commented Jul 19, 2018

alexander-akait commented Aug 1, 2018 • edited Loading

jsf-clabot commented May 17, 2018 •

edited

Loading

alexander-akait commented Jun 5, 2018 •

edited

Loading

alexander-akait commented Jun 5, 2018 •

edited

Loading

kzc commented Jun 15, 2018 •

edited

Loading

timdorr commented Jul 11, 2018 •

edited

Loading

alexander-akait commented Aug 1, 2018 •

edited

Loading