-
Notifications
You must be signed in to change notification settings - Fork 173
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
feat(workbox): workbox 3 + offlinePage #60
Conversation
+ Use global variable instead of class instance + Update methods + Add cleanUrls property
Unfortunately I don't know how to proceed further :( Hope if someone could give it a go and take over from here. |
Just doing some preliminary research currently, but looks like vue-cli has a work in progress PWA with workbox 3. May be worth looking at the implementation there: https://github.com/vuejs/vue-cli/tree/dev/packages/%40vue/cli-plugin-pwa |
|
||
<% options.runtimeCaching.forEach(r => { | ||
const strategy = JSON.stringify(r.strategyOptions || {}) | ||
%> | ||
workboxSW.router.registerRoute(new RegExp('<%= r.urlPattern %>'), workboxSW.strategies.<%= r.handler %>(<%= strategy %>), '<%= r.method %>') | ||
workbox.routing.registerRoute(new RegExp('<%= r.urlPattern %>'), workboxSW.strategies.<%= r.handler %>(<%= strategy %>), '<%= r.method %>') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should workboxSW.strategies
be workbox.strategies
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, will fix!
I think it might be related to the path of the jsdelivr reflects the path for v2:
and for v3:
|
Thanks to @XanderLuciano and @monotykamary for helping out Looks like the ball is rolling again 🙊 |
This looks somewhat ready. Will do some tests with a real-world project of mine and report back and soon as I have the results |
LGTM so far. cc @pi0 for cross-validation and review :) |
@XanderLuciano Happy to hear that! The only thing I'm experiencing is that workbox is still used in dev mode/ on localhost, which results in an endless loop where workbox and the webpack HMR are fighting and reloading the page every few seconds. A hard refresh (CTRL + F5) does stop it though.
|
@manniL I swear I had not run into this issue all night, but as soon as I saw your comment it happened. It seems a bit like an edge case for me though because in dev mode if I remove the service worker a new one isn't registered. So I was only able to reproduce by building and starting the app, then stopping the server and starting a dev instance, and then updating some code that triggers the HMR. Like you said though, a quick ctrl+shift+r / ctrl + F5 fixed it. I'm gunna continue development using this version so if I run into additional issues (or a solution to the HMR reloading) I'll let you know. |
Struggled to get offline support functional, but eventually figured it out. workbox: {
runtimeCaching: [
// Needed to cache the homepage and manifest file (not 100% sure on that though)
{
urlPattern: '/.*',
handler: 'networkFirst',
method: 'GET',
},
// Cache our CDN libraries
{
urlPattern: 'https://cdnjs.cloudflare.com/ajax/libs/.*',
handler: 'cacheFirst',
method: 'GET',
},
// Cache our fonts
{
urlPattern: 'https://fonts.(?:googleapis|gstatic).com/(.*)',
strategyOptions: {
cacheName: 'google-fonts',
},
},
],
}, Would it make sense to add that pattern to the template also? We already cache everything in Update: Looking good ✨ |
I've investigated the "update issue" and I've likely tracked it down. It is, as I assumed, a caching issue, so #75 should definitely be merged in before updating. After clearing my cache for localhost, everything worked well :) |
There are some caveats left when specifying own precaching but it shouldn't be hard to fix Really want to add some tests to it too later. |
Well it took me 2 weeks to figure out how to create my own personal build of this package with all the PR's I wanted, but now I've got that all setup under my own personal scope (since it's taking so long to get any PR's to this repo). That said, I included #75 in that build and still get stuck in a reload loop when going from a built instance to a dev instance with hot reloading. So as you said, there are still some caveats to work out. For example, going from built version to dev, upon triggering an HMR the server console will output a loop of errors such as:
And upon going from a dev instance to a built server instance, there appears to be an HMR request that continues to try and update, even though it's a production build: So it seems to me that the overly broad URL pattern of Maybe another solution to this would be to disable (or change) precaching on development builds. That way when you you run I'm still learning a lot about workbox, but so far I've had no issues running this in a production environment. Add to homescreen banners appearing properly too. Edit: Now workbox isn't loading at all under dev, which is great because now I don't have the HMR issue, but not sure what triggered the change. |
@manniL @XanderLuciano anything else you'd like to get in before I merge this? I'll give it a round of testing tonight. |
@galvez I’d wait until we fixed the plugin thing I mentioned above. Otherwise custom strategies are hard to accomplish. Testing is happily seen! 😋 |
@manniL I'm gonna dabble at fixing that problem, do you have any WIP updates to this branch? |
@galvez Nope, have no WIP there yet :/ |
@manniL no worries -- picking this up on the |
We should adopt the changes for |
As I have pointed out over Discord, we're not using any of the features that require updating. |
Co-authored-by: manniL <hello@lichter.io> Co-authored-by: galvez <jonasgalvez@gmail.com>"
Co-authored-by: manniL <hello@lichter.io> Co-authored-by: galvez <jonasgalvez@gmail.com>"
@galvez Well, we do use the feature as we enable customization of strategies (fka. as runtime-caching) by the user. pwa-module/packages/workbox/templates/sw.template.js Lines 11 to 15 in ef670b8
Formerly you could pass the options as a simple object {
cacheExpiration: {maxEntries: 50}
} but now you have to use plugins: [
new workbox.expiration.Plugin({maxEntries: 50})
] (from the example linked above) |
Workbox has released version 3, so we could upgrade to v3 as well. But there are some obstacles.
In version 3, there is no
WorkboxSW
class anymore.workbox
is exposed as a global variable instead (see the Migration Guide). The problem is thatworkbox
is alwaysundefined
when compiling thesw.template.js
lodash template.