Update for webpack 4

[Sebazzz]

You need to remove the webpack var usage, then it should probably be OK :)

[oliviertassinari]

@fnlctrl I will review this pull request this evening :)

[fnlctrl]

@woutervanvliet Added a minimize option, but I think warnings wouldn't be necessary since it's not required.

[woutervanvliet]

@fnlctrl Looks good!

[woutervanvliet]

@fnlctrl You have some code style errors though, if you can fix those then hopefully @oliviertassinari can merge soon after :)

[woutervanvliet]

@fnlctrl Looks like you need another "fis lint" commit. Perhaps you should run npm run prettier once before you commit ;).

[fnlctrl]

Hmm running prettier isn't changing anything here locally. Not sure why it fails on CI. And that failing line isn't introduced by this PR.

[woutervanvliet]

@fnlctrl It is, according to the diff.

Running prettier shouldn't change anything, unless you call it with --fix, but it should give you the same error as Travis CI does.

[fnlctrl]

@woutervanvliet I meant --fix. It's not giving errors or making fixes locally..

[woutervanvliet]

Maybe it's not something that can be fixed automatically?

Just checked locally, and it's this change it wants you to make:

diff --git a/docs/src/sw.js b/docs/src/sw.js
index 4db6962..6339cea 100644
--- a/docs/src/sw.js
+++ b/docs/src/sw.js
@@ -116,9 +116,7 @@ self.addEventListener('fetch', event => {
         if (!responseNetwork || !responseNetwork.ok) {
           if (DEBUG) {
             console.log(
-              `[SW] URL [${requestUrl.toString()}] wrong responseNetwork: ${
-                responseNetwork.status
-              } ${responseNetwork.type}`
+              `[SW] URL [${requestUrl.toString()}] wrong responseNetwork: ${responseNetwork.status} ${responseNetwork.type}`
             )
           }

Which brings it back to the same as what's on the current master.

And you know what they say, you gotta do as the linter says you should ;-) Maybe your line length is set to something else then the projects prettier config?

Looking at package.json btw, I noticed that peerDependencies still specify support for webpack 1, 2 and 3 (not 4). Does this change not break anything for webpack 1, 2, or 3? Either way, webpack 4 should probably be in the allowed list of versions.

[oliviertassinari]

I'm sorry, I couldn't find the time to look into this pull request. While beeing an early adopter of webpack, I have lost interest in this serviceworker project. I'm no longer using it. My top priorities right now are with Material-UI.

If anyone is interested in pushing the projet forward, please raise your voice. I would be very happy to give you admin access to GitHub and npm.

[woutervanvliet]

I might be interested in at least bringing the plugin further to a webpack 4 compatible release. Have no previous experience though with webpack plugins, other than using them...

On Sat, 10 Mar 2018, 14:31 Olivier Tassinari, notifications@github.com wrote:

I'm sorry, I couldn't find the time to look into this pull request. While beeing an early adopter of webpack, I have lost interest in this serviceworker project. I'm no longer using it. My top priorities right now are with Material-UI.

If anyone is interested in pushing the projet forward, please raise your voice. I would be very happy to give you admin access to GitHub and npm.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/oliviertassinari/serviceworker-webpack-plugin/pull/55#issuecomment-372030207, or mute the thread https://github.com/notifications/unsubscribe-auth/AAHh2jB9fMI7JgUokCr5BQauEt7ATAQPks5tc9XKgaJpZM4STop- .

[devCrossNet]

@woutervanvliet I can support you, I have also no previous experience but we can figure it out. I think this plugin should stay alive because it's more flexible than the offline-plugin. 👍

[oliviertassinari]

Sounds great! I have added you as collaborators :).

[devCrossNet]

@fnlctrl this error: 1 error, 0 warnings potentially fixable with the--fixoption. comes from eslint. you can run ./node_modules/.bin/eslint . --fix to fix it. Or you enable eslint in your IDE to see the error in ./docs/src/sw.js and reformat it manually.

[fnlctrl]

@woutervanvliet @devCrossNet I did run ./node_modules/.bin/eslint . --fix but nothing happened. So I updated eslint related deps and it doesn't complain now. Though on newer versions of eslint react plugin and flow there are new errors. Leaving them as is as it's out of scope of this PR.

[devCrossNet]

@woutervanvliet the PR LGTM, in my opinion, we could merge it. @oliviertassinari I think you are the only one who can publish a new version, this should be a new major version release because it has a breaking change (just works with webpack 4). @fnlctrl thanks for the PR 👍

[oliviertassinari]

@devCrossNet Awesome! What's your npm username? I can add you too.

[devCrossNet]

@oliviertassinari it's devcrossnet (https://www.npmjs.com/~devcrossnet) :-)

[woutervanvliet]

@oliviertassinari Thanks for making me a contributor, looking forward to ... contributing ;-). My NPM username is woutervanvliet

I've created a new branch next, from master. Think I'd prefer to merge this work there so we can push out a webpack 4 compatible beta containing the current work, ahead of a stable release that's undergone a bit more testing.

@fnlctrl Could you change the target branch of your pull request?

[fnlctrl]

@woutervanvliet Done https://github.com/oliviertassinari/serviceworker-webpack-plugin/pull/56 Closing this then.