bemusic / bemuse

⬤▗▚▚▚ Web-based online rhythm action game. Based on HTML5 technologies, React, Redux and Pixi.js.
https://bemuse.ninja/
GNU Affero General Public License v3.0
1.15k stars 147 forks source link

:building_construction: Don't register service worker in localhost #504

Closed resir014 closed 6 years ago

resir014 commented 6 years ago

This prevents the inevitable fact that any other local apps that use the same port as ours will be overriden by Bemuse's service worker.

codecov-io commented 6 years ago

Codecov Report

Merging #504 into master will not change coverage. The diff coverage is n/a.

Impacted file tree graph

@@          Coverage Diff           @@
##           master    #504   +/-   ##
======================================
  Coverage    84.1%   84.1%           
======================================
  Files         171     171           
  Lines        5387    5387           
  Branches        1       1           
======================================
  Hits         4531    4531           
  Misses        856     856

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 6941f7f...9bc7bf2. Read the comment docs.

resir014 commented 6 years ago

We do need to find a way to test our service workers, still. Maybe moving it to our staging URL also works. What do you think?

dtinth commented 6 years ago

@resir014 Yes, I would prefer that we can test our service workers locally.

Solutions that came up to my mind is:

  1. Make service worker self-destruct itself if it has determined that the server is no longer running Bemuse. Here’s how to self-destruct a service worker: https://github.com/NekR/self-destroying-sw

  2. On localhost make service worker registration opt-in.

  3. Move Bemuse server to a different port so that it less likely clash with other apps.

These solutions can be combined.

I would prefer doing option 1 as it would keep service worker on by default, but makes sure it only stays for Bemuse development.

resir014 commented 6 years ago

@dtinth Hmm, I'm not sure I'm understanding the code examples for the first approach. Sure it self-destructs, but I don't see where we can put our service worker + its caching strategies.

Also, I had a (now-deleted) local branch to rewrite most of our service worker code with Workbox. I initially deleted it because I didn't go far with it, but now that we're working on this PR, I could look into it again.

dtinth commented 6 years ago

@resir014 Here’s an example strategy:

When a service worker is activated, make a fetch request to, e.g. /bemuse.txt or some special file that identifies the Bemuse project. If server returns 404, it means that the current server is no longer running Bemuse. In this case service worker self-destructs.

(Note that I look specifically for 404, since we want Bemuse to continue working offline if no server is running, so we make SW self-destruct only if the server runs a different project.)

resir014 commented 6 years ago

@dtinth Alright, I'll close this PR and redo it with the method mentioned above.