vuejs / vue-hackernews-2.0

HackerNews clone built with Vue 2.0, vue-router & vuex, with server-side rendering
MIT License
10.97k stars 2.16k forks source link

Use service worker on localhost #268

Open Everettss opened 6 years ago

Everettss commented 6 years ago

Current Build Setup serve from http://localhost:8080/ but in entry-server.js is

// service worker
if ('https:' === location.protocol && navigator.serviceWorker) {
  navigator.serviceWorker.register('/service-worker.js')
}

There is problem with 'https:' === location.protocol on localhost. Is this check really needed? It introduce unnecessary complexity of need serving from https on localhost. If you remove this check and run on http not on localhost it will simple throw an error - for me this should be expected behaviour (developer will easily spot this problem) .

JounQin commented 6 years ago

Service workers only run over HTTPS, for security reasons.

Everettss commented 6 years ago

And on localhost without HTTPS, for development purposes.

JounQin commented 6 years ago

https://github.com/vuejs/vue-hackernews-2.0/commit/980ca3254c24e319b05bdffd38388948f4e320ef#r26021186

It seems browser will ignore localhost or 127.0.0.1 without https, so maybe we can add some check for that.

I add this code because we are only using service worker on production here, which will not only run on localhost.

Maybe check like following:

process.env.NODE_ENV === 'production' && ('https:' === location.protocol || ['localhost', '127.0.0.1'].indexOf(location.host) !== -1) && navigator.serviceWorker
Everettss commented 6 years ago

This one is working:

// service worker
if (
  process.env.NODE_ENV === 'production' &&
  ('https:' === location.protocol || location.host.match(/(localhost|127.0.0.1)/)) &&
  navigator.serviceWorker
) {
  navigator.serviceWorker.register('/service-worker.js');
}

but what is the purposes of process.env.NODE_ENV === 'production'? It looks like this NODE_ENV is in all builds set to production Why to stop people from debugging service worker in development environment?

Personally I think we over engineering this simple problem, just use the most popular option:

// service worker
if ('serviceWorker' in navigator) {
    navigator.serviceWorker.register('/service-worker.js');
}
JounQin commented 6 years ago

ServiceWorker is only enabled on production build. https://github.com/vuejs/vue-hackernews-2.0/blob/master/build/webpack.client.config.js#L44

And, if user publishes this demo onto his own server without https support, it will breaks, right?

Everettss commented 6 years ago

It will not break it will throw an error that you try to register service worker on not secure domain. But the application will work fine. OK - lets not change this process.env.NODE_ENV === 'production' workflow. Just use this:

// service worker
if (
  process.env.NODE_ENV === 'production' &&
  ('https:' === location.protocol || location.host.match(/(localhost|127.0.0.1)/)) &&
  navigator.serviceWorker
) {
  navigator.serviceWorker.register('/service-worker.js');
}
JounQin commented 6 years ago

Yes, it will throw an error, and just seems to be a break to me, so why not check it and avoid an error?

Everettss commented 6 years ago

Because it shows developer that he is trying to publish site with service worker on not secure domain. It is his fault that he doesn't care about HTTPS. Let's not nurse him.

rahul7p commented 5 years ago

@Everettss @JounQin

I have added same in webpack.client.config.js file but still getting error like

Messaging: We are unable to register the default service worker. Failed to register a ServiceWorker: The script has an unsupported MIME type ('text/html'). (messaging/failed-serviceworker-registration).

navigator.serviceWorker.register('firebase-messaging-sw.js') .then((registration) => { console.log('registration=>',registration); Vue.prototype.$messaging.useServiceWorker(registration) }).catch(err => { console.log("here....") console.log(err) })

Will you please guide me?