Register service worker if no one else does

apache / cordova-browser

Apache Cordova

Apache License 2.0

170 stars 85 forks source link

Register service worker if no one else does #43

Closed purplecabbage closed 6 years ago

purplecabbage commented 6 years ago

Platforms affected

This one ...

What does this PR do?

Adds service worker workflow at runtime

What testing has been done on this change?

manual testing in a previous branch. This code was overwritten by coho.

Checklist

[ ] Reported an issue in the JIRA database
[ ] Commit message follows the format: "CB-3232: (android) Fix bug with resolving file paths", where CB-xxxx is the JIRA ID & "android" is the platform affected.
[ ] Added automated test coverage as appropriate for this change.

macdonst commented 6 years ago

@purplecabbage I don't recommend merging this PR in. The way our service-worker is written the first time the service-worker is loaded it will cache all the files and then that cache will always be used until the end of time. Until that gets fixed we are better off not registering the service-worker.

See my comment: https://github.com/apache/cordova-browser/commit/1f7bda8e27731c9e6acd031e644edea9cbede969#commitcomment-24013770

We should all chat on whether or not we want service-workers to land in browser.

stevengill commented 6 years ago

Sweet. Thanks for chiming in @macdonst! So would you suggest we make the changes you suggest in your comment to cordova-sw.js. After that, are the changes in this PR good? This would still essentially add service workers to the development process. But it sounds like with your changes to cordova-sw.js the cache would properly update and show the latest changes. So it isn't that big of a deal. Am I understanding this correctly?

macdonst commented 6 years ago

@stevengill let's talk more tomorrow but in short the answer is yes and no. It will enable the user to see updated changes but it won't happen by default thanks to the service worker lifecycle.