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

(#5304) - Service workers instead of Appcache for the website #5304

Merged
merged 1 commit into from Jun 18, 2016

Conversation

Projects
None yet
3 participants
@nickcolley
Member

nickcolley commented Jun 7, 2016

Continuation of #4846

@nickcolley

This comment has been minimized.

Member

nickcolley commented Jun 7, 2016

Okay so this is working pretty well as is but I'm trying to think of a good way to let the user update the page like we have with appcache.

I'm not sure this strategy is the right one with this in mind since a user needs to exit the page entirely for the newly generated worker to take over..

@nickcolley nickcolley changed the title from WIP: (#4480) - Service workers instead of Appcache for the website to (#5304) - Service workers instead of Appcache for the website Jun 11, 2016

nickcolley added a commit that referenced this pull request Jun 11, 2016

(#5304) - Add ServiceWorker support to the website
- Critical assets that must be cached to install the service worker.
- Non critical assets such as the guide etc are optional
- If a page isn't cached it can be added if you visit it while online
- If you try to visit a page that is not cached while offline you'll see an offline message.

nickcolley added a commit that referenced this pull request Jun 11, 2016

(#5304) - Add ServiceWorker support to the website
- Critical assets that must be cached to install the service worker.
- Non critical assets such as the guide etc are optional
- If a page isn't cached it can be added if you visit it while online
- If you try to visit a page that is not cached while offline you'll see an offline message.

[skip ci]
@nickcolley

This comment has been minimized.

Member

nickcolley commented Jun 11, 2016

Okay this is ready to review, one con to this approach is that for people to see updates to pages they already have cached they need to leave the site and come back.

If we think that's important enough I can look at trying to handle that case better so let me know your thoughts. 👍

@nolanlawson

This comment has been minimized.

Member

nolanlawson commented Jun 12, 2016

Hm, I would really rather not lose that "content updated, reload?" message if at all possible. I know there are ways to detect that the content changed; e.g. check out SVGOMG's method: https://github.com/jakearchibald/svgomg/blob/0f2ce1ace903d569337b72f57b8eb255b65177df/src/js/page/main-controller.js#L123-L127

@nickcolley

This comment has been minimized.

Member

nickcolley commented Jun 12, 2016

@nolanlawson unless I'm mistaken the user needs to close the tab and then return for the service worker to become obsolete and be overridden?

Looking at that snippet it does suggest a reload on it's own will work though, couldn't get that to happen in my testing though..

@nolanlawson

This comment has been minimized.

Member

nolanlawson commented Jun 12, 2016

@nickcolley, no, you can update a service worker on-the-fly using clients.claim() and skipWaiting(). I admit my memory is fuzzy on which one exactly does what, but using one or both of those two, you definitely can.

(#5304) - Add ServiceWorker support to the website
- Critical assets that must be cached to install the service worker.
- Non critical assets such as the guide etc are optional
- If a page isn't cached it can be added if you visit it while online
- If you try to visit a page that is not cached while offline you'll see an offline message.

[skip ci]
@nickcolley

This comment has been minimized.

Member

nickcolley commented Jun 17, 2016

Okay updated the PR with your feedback, didn't know you could skip the waiting!

@daleharvey

This comment has been minimized.

Member

daleharvey commented Jun 18, 2016

Woot this is looking great, so having both appcache and serviceworkers on the page doesnt seem to conflict, w3c/ServiceWorker#697 suggests so too. Updates are working and seeing it working in both firefox and chrome, 👍 :)

@daleharvey daleharvey merged commit be40f03 into master Jun 18, 2016

@daleharvey

This comment has been minimized.

Member

daleharvey commented Jun 18, 2016

Given the way appcache went I expect we will probably still see a few minor issues, but I think those will get found and fixed once its released, I tried checking as much as I can and all worked great for me so gonna push this then see how it goes :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment