While working on #2983935: serviceworkers.js - Response not cacheable I came across something in the install event listener which I found curious. Originally the code would install even when the list of assets couldn't be cached.
I personally think it should err on the side of caution and avoid signaling successful installation unless everything gets cached. This is the general behavior expected when the very commonly-used cache.addAll() method is used. We're not using it in order to handle potential third-party requests such as Google Fonts and the like.
However if we fail to cache stuff, the PWA (and the module) fails at one of its basic missions: offline support. Since this is the primary feature we advertise on the module's page, I have a hunch that people will find half-installations unsatisfying. I also feel like site admins would rather field support requests for the PWA not installing, rather than fielding requests for offline support that only entails black-on-white HTML files that lack styles.
For now I am filing this issue and doing the conservative thing: to comment out this line of code and fail the installation unless everything in the pre-cache arrays is successfully cached:
CACHE_URLSCACHE_URLS_ASSETSCACHE_OFFLINECACHE_OFFLINE_IMAGEIs there anybody out there evaluating this module that has an opinion?
Comments
Comment #3
mustanggb commentedI think it's fine to fail if pre-caching is unsuccessful, as it points a developer that something needs sorting.
---
On a possibly unrelated note, just thinking out load a bit, if #3032124: delivery callback not found and #3032128: Handle menu status codes in pwa_deliver_js_file() make it in it might be useful to do something with status codes.
Comment #4
nod_now it's dealt with by workbox, so their default is our default seems to do the job.