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.

Sign up for GitHub

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

fix: precached files are not updated #386

Merged
merged 6 commits into from
Nov 28, 2020
+33 −2
Merged

fix: precached files are not updated #386

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
c605134
fix: precached files are not updated
kedrzu Oct 21, 2020
bd04e0d
refactor: move randomString to utils
pi0 Oct 22, 2020
77a4464
refactor: apply default revision in options.js
pi0 Oct 22, 2020
ef76211
test: update snapshot
pi0 Oct 22, 2020
df107b5
Merge branch 'master' into master
pi0 Oct 22, 2020
801ff5c
test: fix revision
pi0 Oct 22, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions lib/workbox/module.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ module.exports = async function nuxtWorkbox (nuxt, pwa, moduleContainer) {
'importScripts',
'config',
'cacheNames',
'cacheOptions',
'clientsClaim',
'skipWaiting',
'cleanupOutdatedCaches',
Expand Down
17 changes: 17 additions & 0 deletions lib/workbox/options.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,11 @@ function getOptions (nuxt, pwa) {
options.publicPath = publicPath
}

// revision
if (!options.cacheOptions.revision) {
options.cacheOptions.revision = randomString(12)
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

If no revision is set, browser complains about precaching file without revision:
image

I think that adding some randomized revision as a default would be beneficial.
I tried to get random version from nuxt environmental variables, but couldn't find it.

Copy link
Member

Choose a reason for hiding this comment

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

I think we have to pass null instead for revision info of _nuxt/ assets because we always add hash to URL for long-term caching:

{url: '/scripts/app.0d5770.js', revision: null},
For the second and third object in the example above, the revision property is set to null. This is because the revisioning information is in the URL itself, which is generally a best practice for static assets.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure, for script assets that's true. But this case is about .html files, which do not have any hash in name.
Once they are precached, you cannot do anything to update them, other than unregistering and registering service worker.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The same applies to offline page (404.html) and start url for PWA (/?standalone=true by default).
Once they get precached, PWA stops being updated and offline page too.

This is a behavior I observed in my own application - when installed app as a standalone PWA application it refused to be updated even if completely new deploy was made.

Copy link
Member

Choose a reason for hiding this comment

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

I see you are right. But the problem is that generating revision on build-time only works for static generated websites. If response of a SSR page changes, workbox still refuses to update..

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If precache would work with NetworkFirst, everything would be fine.
But from what I've read, it always work as CacheFirst.

Maybe it's a good idea to disable precaching at all? Or replace it with manually fetching those pages in sw.js file and caching them with standard NetworkFirst cache, instead of depending on workbox standard precache?

Copy link
Member

Choose a reason for hiding this comment

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

@kedrzu This PR looks good after and partially fixing issue. But indeed, precaching is only CacheFirst (GoogleChrome/workbox#1767).

I will try to implement new offlineRoutes option for (NF or SWR) strategy :)

Copy link
Member

Choose a reason for hiding this comment

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

#406


// swTemplate
if (!options.swTemplate) {
options.swTemplate = path.resolve(__dirname, `templates/sw${options.enabled ? '' : '.unregister'}.js`)
Expand Down Expand Up @@ -122,6 +127,18 @@ function getOptions (nuxt, pwa) {
return options
}

function randomString (length) {
const result = []
const characters = 'ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789'

for (let i = 0; i < length; i++) {
const char = characters.charAt(Math.floor(Math.random() * characters.length))
result.push(char)
}

return result.join('')
}

module.exports = {
getOptions
}
9 changes: 8 additions & 1 deletion lib/workbox/templates/sw.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,10 +46,17 @@ function initWorkbox(workbox, options) {

function precacheAssets(workbox, options) {
if (options.preCaching.length) {
workbox.precaching.precacheAndRoute(options.preCaching, options.cacheOptions)
const cacheOptions = options.cacheOptions || {};
const precacheList = options.preCaching.map(url => ({
url: url,
revision: cacheOptions.revision
}))

workbox.precaching.precacheAndRoute(precacheList, options.cacheOptions)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

According to google:
https://developers.google.com/web/tools/workbox/modules/workbox-precaching#explanation_of_the_precache_list

Revision must be passed separately per each precached file and not in options object.

}
}


function runtimeCaching(workbox, options) {
for (const entry of options.runtimeCaching) {
const urlPattern = new RegExp(entry.urlPattern)
Expand Down