yyx990803 / register-service-worker

A script to simplify service worker registration with hooks for common events.
MIT License
638 stars 58 forks source link

ready event is only triggered on localhost #20

Closed yennor closed 4 years ago

yennor commented 5 years ago

as it seems. The ready event, will only be triggered if the service-worker runs on localhost. That seems to be quite confusing (at least to me). If that is intentionally then it should be documented.

assemblethis commented 4 years ago

Yes, I'm just discovering the same thing.

It might be intentional given this comment:

https://github.com/yyx990803/register-service-worker/blob/cedb35fdb41ef626e948211000fa3938fcf1e682/src/index.js#L37

Until the service worker is ready, you don't seem to get a ServiceWorkerRegistration instance.

assemblethis commented 4 years ago

I've copied the following lines...

https://github.com/yyx990803/register-service-worker/blob/cedb35fdb41ef626e948211000fa3938fcf1e682/src/index.js#L33-L34

... and placed them right after the call to register the SW when isLocalhost returns false and now the ready hook fires as I expected:

https://github.com/yyx990803/register-service-worker/blob/cedb35fdb41ef626e948211000fa3938fcf1e682/src/index.js#L38

This seems to be the behavior you'd expect, that the ready hook would fire whether or not isLocalhost returns true or false. But I may be wrong. I also opened another issue #36 that is possibly related.

bseib commented 4 years ago

I was also surprised by this behavior. I think there should be a consistent ready event that you can expect in both dev and production environments.

In my use case I have a kiosk where I need my service worker to load/update some mp4 video files into the local cache before I instantiate the root Vue component (whose children refer to these videos). So I need an event that says a service worker is "ready" (regardless of which version of service worker it is).

From the code:

https://github.com/yyx990803/register-service-worker/blob/cedb35fdb41ef626e948211000fa3938fcf1e682/src/index.js#L29-L40

I think the code change suggested by @assemblethis addresses this issue. That means inserting the following three lines after line 38:

     navigator.serviceWorker.ready.then(registration => { 
       emit('ready', registration) 
     }) 
assemblethis commented 4 years ago

I still can't see why you wouldn't fire the 'ready' event for non-localhost. bseib's PR seems good and is what I've been using.

assemblethis commented 4 years ago

Ok, I'm just using register-service-worker as is (v1.6.2) since PR #37 hasn't been approved yet. So, instead, after registering the service-worker I then have my own ready hook placed right after. This is because, as far as I understand it, the ready hook will only fire in a DEV environment since it requires localhost.

The code in this project seems to have been taken from the create-react-app template as far as I can tell.

In keeping with the name of the project it will register the service worker reliably but will not tell you when it's ready.

So it seems to me the 'ready' hook from this project should be ignored and you should just have your own 'ready' hook placed right after the register function returns.

register(`${process.env.BASE_URL}service-worker.js`, { 'code for hooks minus ready hook' });
navigator.serviceWorker.ready.then(swr => { 'ready hook code' });

My worry is that there is some foundational technical piece of information that I'm missing about service workers, such as when they're running properly they always run as localhost since essentially they're running on the device. But I haven't experienced that.

bseib commented 4 years ago

For what it's worth, I ended up writing my own ServiceWorkerManager so that I could have a solid understanding of when the service worker related events occur, and so I'd really know what the state of "current" and "waiting" services workers are at those moments.

If anyone is interested, here is what I did: https://gist.github.com/bseib/06164a973552fe9f814df97bb4d30305

assemblethis commented 4 years ago

Thanks, very interesting. I need to take a closer look at what you did.

frostfire64 commented 4 years ago

I join the reports of this behavior being bizarre, such unexpected behavior should be at least clearly documented. I wish I searched issues with the package before I spend many hours trying to debug my code.