NekR / offline-plugin

Offline plugin (ServiceWorker, AppCache) for webpack (https://webpack.js.org/)
MIT License
4.52k stars 295 forks source link

fix terser webpack plugin compatibility #436

Closed Bobgy closed 5 years ago

Bobgy commented 5 years ago

closes #426

Bobgy commented 5 years ago

I didn't update tests, but test is pulling in latest webpack by default. Seeing CIs passing should already prove the new version works with latest webpack without changing fixtures a little bit.

Bobgy commented 5 years ago

New behavior:

  1. when terser-webpack-plugin exists, it will use it instead of uglify-webpack-plugin
  2. when using terser-webpack-plugin, it will pass the same options from uglify-webpack-plugin to it
GGAlanSmithee commented 5 years ago

@NekR do you have time to check this? Will require a new version and npm deploy!

Bobgy commented 5 years ago

@GGAlanSmithee Thank you for the detailed CR. I will address the issues.

Bobgy commented 5 years ago

it seems CI is failing on the same code that passed before. I don't know why. I will debug soon

GGAlanSmithee commented 5 years ago

@Bobgy Thanks for your continued work. I see that you reverted your last commit, so I guess there is still something you want to address? Will await further comments from you.

Bobgy commented 5 years ago

@GGAlanSmithee I made the revert just to retry CI and it was still failing. It seemed that latest webpack was broken at that time. Now I just removed the revert and CI is passing again. Please review if you have time.

It seems I messed up a little bit when rebasing. Fixing it right now.

Bobgy commented 5 years ago

OK, all issues fixed and CI is passing again

Bobgy commented 5 years ago

@GGAlanSmithee Thank you for your thoughtful comments.

GGAlanSmithee commented 5 years ago

image

So green 💯