Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Service worker #87

Closed
wants to merge 47 commits into from
Closed

Conversation

dfabulich
Copy link
Contributor

Fixes #6

To test it, edit index.html and set useServiceWorker = true. Then, use the Application tab in Chrome to see the service worker register. You can then check the "Offline" button and refresh to see it working offline.

Note that as of right now, the master branch isn't very useful offline, but if you include my fix for #80, users could go to iplayif.com, click the browser's "install" button to install it locally, and use that to play locally downloaded games offline.

Also note that we're dropping serviceworker.js in the same directory as the index.html file, aka the root directory, because otherwise it doesn't work.

curiousdannii and others added 30 commits July 1, 2020 13:14
Tell GiLoad to look for the GLUL Blorb chunk
Package into a subfolder for Inform 7
Run VMs with autosaving if they support it
Add a Gulp target for Lectrote
Emglken files are smaller
Fixed a ZVM autosave bug
GlkOte styling improvements
index.html Outdated
<script>
var useServiceWorker = (location.protocol === 'https:' && 'serviceWorker' in navigator);
if (useServiceWorker) {
navigator.serviceWorker.register('serviceworker.js').then(function (registration) {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this need to be in index.html, or could it be moved to launcher?

Also, I think if you pass a scope then the serviceworker.js file could be located in dist/web/ too. And if you move this code to launcher, and run it after the options are loaded, then it could use the lib_path option just to be extra flexible. And maybe we should even add an option for registering the service worker - it's probably not useful for single file instances.

Copy link
Contributor Author

@dfabulich dfabulich Nov 29, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's not simply a matter of passing a scope. When I moved serviceworker.js to dist/web and passed in {scope: './'} Chrome spit out this error:

ServiceWorker registration failed: DOMException: Failed to register a ServiceWorker for scope ('http://127.0.0.1:8080/') with script ('http://127.0.0.1:8080/dist/web/serviceworker.js'): The path of the provided scope ('/') is not under the max scope allowed ('/dist/web/'). Adjust the scope, move the Service Worker script, or use the Service-Worker-Allowed HTTP header to allow the scope.

I guess I could try to investigate how to pass a custom HTTP header when using http-server, but it's wayyyy easier to just leave the service worker in the root directory.

A related approach would be to have a build script generate an index.html file in dist/web.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, yeah you can't move it without restricting the scope: https://stackoverflow.com/a/35780776/2854284

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Though this code should be able to be moved to launcher.js

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's not much code, and IMO it's a good practice to keep this code inline in index.html.

Users can directly refresh index.html, and even shift-refresh index.html if they need to, whereas they can't easily do that with the launcher script / main.js. If you make a mistake with caching the launcher script, you can get yourself stuck in a place where you can't easily force folks to update. I wrote a blog post about what can go wrong a few years ago: https://redfin.engineering/service-workers-break-the-browsers-refresh-button-by-default-here-s-why-56f9417694#988f

To be clear, you can always work your way out of the situation by replacing the service worker with a "nuke the service worker" script and waiting (gulp) 24 hours. But it's way easier to tell users "it will fix itself in the next 24 hours, or shift-refresh to fix it immediately"

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, because index.html won't be one of the cached URLs, is that the idea?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not exactly. index.html will be cached (if it weren't, Parchment wouldn't work offline) but there will be a user-visible way to circumvent the cache by shift-refreshing.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah. Well you can see my inexperience with these things ;)

Thank you for your work on this, I do appreciate it. I'll not merge this until I'm fully confident I understand what it's doing, but I will get to it.

index.html Outdated Show resolved Hide resolved
* Provide instructions to build and run Parchment
* Allow users to submit local files to play them. Fixes curiousdannii#80.

We're using a <label> in order to customize the appearance of <input type=file>
This forces us to add tabindex="0" role="button" and a keydown handler for accessibility
We hide the upload button initially and show it with JS (because it will require JS to work)

* Provide a comment hinting how to disable minification
* Watch and rebuild srcs while HTTP server is running
@dfabulich
Copy link
Contributor Author

Here's what I recommend to increase your confidence:

  1. Do Google's free Udacity web course. (You can skip the last bit about IndexedDB.) https://www.udacity.com/course/offline-web-applications--ud899
  2. Read my blog post https://redfin.engineering/service-workers-break-the-browsers-refresh-button-by-default-here-s-why-56f9417694 (linked above) and its corresponding part 2 https://redfin.engineering/how-to-fix-the-refresh-button-when-using-service-workers-a8e27af6df68
  3. Rewrite my service worker code from scratch, fixing bugs as you go (and feel free to ask me anything you like). It's only a few dozen lines, but there are a lot of weird failure cases (which you'll encounter while working on the Udacity course); by the time you could write this PR yourself, you'll be ready to merge it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support service workers
3 participants