Skip to content

Commit 504b077

Browse files
vtenfysGatsbyJS Bot
authored and
GatsbyJS Bot
committed
fix(gatsby-plugin-offline): Change navigation handler logic (#13502)
* Refactor offline plugin: check for resources rather than checking against a whitelist * Fix broken tests * Fix tests, for real (fix illegal invocations) * Upgrade CircleCI Chrome to 74 * Chrome 73 (74 isn't available yet) * Specify correct image version * Catch failed resource errors * Remove waitForAPIorTimeout in favor of smart wait skipping * Fix function name * Fix skipping wrong path * Add comment explaining navigation handler logic * Fix code style issues * Fix Markdown formatting * Improve Cypress logging; temporary logging for testing * Revert some temporary changes * Clean up * Clean up some more * Remove testing line * Undo React lifecycle function rename * bump timeout back to 10sec * Revert timeout to 5s and Chrome version * Bump timeout back to 10 seconds * fm,l
1 parent 67967a1 commit 504b077

File tree

6 files changed

+68
-132
lines changed

6 files changed

+68
-132
lines changed

e2e-tests/production-runtime/cypress/integration/accessibility.js

+9-3
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
describe(`focus management`, () => {
22
it(`Focus router wrapper after navigation to regular page (from index)`, () => {
3-
cy.visit(`/`).waitForRouteChange()
3+
cy.visit(`/`).waitForAPIorTimeout(`onRouteUpdate`, { timeout: 5000 })
44

55
cy.changeFocus()
66
cy.assertRouterWrapperFocus(false)
@@ -27,7 +27,10 @@ describe(`focus management`, () => {
2727
})
2828

2929
it(`Focus router wrapper after navigation from 404`, () => {
30-
cy.visit(`/broken-path`, { failOnStatusCode: false }).waitForRouteChange()
30+
cy.visit(`/broken-path`, { failOnStatusCode: false }).waitForAPIorTimeout(
31+
`onRouteUpdate`,
32+
{ timeout: 5000 }
33+
)
3134

3235
cy.changeFocus()
3336
cy.assertRouterWrapperFocus(false)
@@ -36,7 +39,10 @@ describe(`focus management`, () => {
3639
})
3740

3841
it(`Focus router wrapper after navigation from one 404 path to another 404 path`, () => {
39-
cy.visit(`/broken-path`, { failOnStatusCode: false }).waitForRouteChange()
42+
cy.visit(`/broken-path`, { failOnStatusCode: false }).waitForAPIorTimeout(
43+
`onRouteUpdate`,
44+
{ timeout: 5000 }
45+
)
4046

4147
// navigating to different not existing page should also trigger
4248
// router wrapper focus as this is different page

packages/gatsby-cypress/src/commands.js

+1-1
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ Cypress.Commands.add(`getTestElement`, (selector, options = {}) =>
66
cy.get(`[data-testid="${selector}"]`, options)
77
)
88

9-
const TIMEOUT = 5000
9+
const TIMEOUT = 10000
1010

1111
Cypress.Commands.add(
1212
`waitForAPI`,
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,15 @@
11
exports.registerServiceWorker = () => true
22

33
const prefetchedPathnames = []
4-
const whitelistedPathnames = []
54

65
exports.onServiceWorkerActive = ({
76
getResourceURLsForPathname,
87
serviceWorker,
98
}) => {
10-
// if the SW has just updated then reset whitelisted paths and don't cache
9+
// if the SW has just updated then clear the path dependencies and don't cache
1110
// stuff, since we're on the old revision until we navigate to another page
1211
if (window.___swUpdated) {
13-
serviceWorker.active.postMessage({ gatsbyApi: `resetWhitelist` })
12+
serviceWorker.active.postMessage({ gatsbyApi: `clearPathResources` })
1413
return
1514
}
1615

@@ -26,15 +25,22 @@ exports.onServiceWorkerActive = ({
2625
.call(nodes)
2726
.map(node => node.src || node.href || node.getAttribute(`data-href`))
2827

29-
// Loop over all resources and fetch the page component and JSON
30-
// to add it to the sw cache.
28+
// Loop over prefetched pages and add their resources to an array,
29+
// plus specify which resources are required for those paths.
3130
const prefetchedResources = []
32-
prefetchedPathnames.forEach(path =>
33-
getResourceURLsForPathname(path).forEach(resource =>
34-
prefetchedResources.push(resource)
35-
)
36-
)
31+
prefetchedPathnames.forEach(path => {
32+
const resources = getResourceURLsForPathname(path)
33+
prefetchedResources.push(...resources)
3734

35+
serviceWorker.active.postMessage({
36+
gatsbyApi: `setPathResources`,
37+
path,
38+
resources,
39+
})
40+
})
41+
42+
// Loop over all resources and fetch the page component + JSON data
43+
// to add it to the SW cache.
3844
const resources = [...headerResources, ...prefetchedResources]
3945
resources.forEach(resource => {
4046
// Create a prefetch link for each resource, so Workbox runtime-caches them
@@ -47,44 +53,26 @@ exports.onServiceWorkerActive = ({
4753

4854
document.head.appendChild(link)
4955
})
50-
51-
serviceWorker.active.postMessage({
52-
gatsbyApi: `whitelistPathnames`,
53-
pathnames: whitelistedPathnames,
54-
})
5556
}
5657

57-
function whitelistPathname(pathname, includesPrefix) {
58+
exports.onPostPrefetchPathname = ({ pathname, getResourceURLsForPathname }) => {
59+
// do nothing if the SW has just updated, since we still have old pages in
60+
// memory which we don't want to be whitelisted
61+
if (window.___swUpdated) return
62+
5863
if (`serviceWorker` in navigator) {
5964
const { serviceWorker } = navigator
6065

61-
if (serviceWorker.controller !== null) {
66+
if (serviceWorker.controller === null) {
67+
// if SW is not installed, we need to record any prefetches
68+
// that happen so we can then add them to SW cache once installed
69+
prefetchedPathnames.push(pathname)
70+
} else {
6271
serviceWorker.controller.postMessage({
63-
gatsbyApi: `whitelistPathnames`,
64-
pathnames: [{ pathname, includesPrefix }],
72+
gatsbyApi: `setPathResources`,
73+
path: pathname,
74+
resources: getResourceURLsForPathname(pathname),
6575
})
66-
} else {
67-
whitelistedPathnames.push({ pathname, includesPrefix })
6876
}
6977
}
7078
}
71-
72-
exports.onPostPrefetchPathname = ({ pathname }) => {
73-
// do nothing if the SW has just updated, since we still have old pages in
74-
// memory which we don't want to be whitelisted
75-
if (window.___swUpdated) return
76-
77-
whitelistPathname(pathname, false)
78-
79-
// if SW is not installed, we need to record any prefetches
80-
// that happen so we can then add them to SW cache once installed
81-
if (
82-
`serviceWorker` in navigator &&
83-
!(
84-
navigator.serviceWorker.controller !== null &&
85-
navigator.serviceWorker.controller.state === `activated`
86-
)
87-
) {
88-
prefetchedPathnames.push(pathname)
89-
}
90-
}

packages/gatsby-plugin-offline/src/gatsby-node.js

+4-7
Original file line numberDiff line numberDiff line change
@@ -45,19 +45,15 @@ exports.onPostBuild = (args, pluginOptions) => {
4545
`webpack-runtime`,
4646
`component---node-modules-gatsby-plugin-offline-app-shell-js`,
4747
])
48+
const appFile = files.find(file => file.startsWith(`app-`))
4849

4950
// Remove the custom prefix (if any) so Workbox can find the files.
5051
// This is added back at runtime (see modifyUrlPrefix) in order to serve
5152
// from the correct location.
5253
const omitPrefix = path => path.slice(pathPrefix.length)
5354

54-
const criticalFilePaths = _.uniq(
55-
_.concat(
56-
getResourcesFromHTML(`${process.cwd()}/${rootDir}/404.html`),
57-
getResourcesFromHTML(
58-
`${process.cwd()}/${rootDir}/offline-plugin-app-shell-fallback/index.html`
59-
)
60-
)
55+
const criticalFilePaths = getResourcesFromHTML(
56+
`${process.cwd()}/${rootDir}/offline-plugin-app-shell-fallback/index.html`
6157
).map(omitPrefix)
6258

6359
const globPatterns = files.concat([
@@ -130,6 +126,7 @@ exports.onPostBuild = (args, pluginOptions) => {
130126
const swAppend = fs
131127
.readFileSync(`${__dirname}/sw-append.js`, `utf8`)
132128
.replace(/%pathPrefix%/g, pathPrefix)
129+
.replace(/%appFile%/g, appFile)
133130

134131
fs.appendFileSync(`public/sw.js`, `\n` + swAppend)
135132
console.log(
Original file line numberDiff line numberDiff line change
@@ -1,101 +1,46 @@
11
/* global importScripts, workbox, idbKeyval */
22

33
importScripts(`idb-keyval-iife.min.js`)
4-
const WHITELIST_KEY = `custom-navigation-whitelist`
54

6-
const navigationRoute = new workbox.routing.NavigationRoute(({ event }) => {
7-
const { pathname } = new URL(event.request.url)
5+
const { NavigationRoute } = workbox.routing
86

9-
return idbKeyval.get(WHITELIST_KEY).then((customWhitelist = []) => {
10-
// Respond with the offline shell if we match the custom whitelist
11-
if (customWhitelist.includes(pathname)) {
12-
const offlineShell = `%pathPrefix%/offline-plugin-app-shell-fallback/index.html`
13-
const cacheName = workbox.core.cacheNames.precache
7+
const navigationRoute = new NavigationRoute(async ({ event }) => {
8+
let { pathname } = new URL(event.request.url)
9+
pathname = pathname.replace(new RegExp(`^%pathPrefix%`), ``)
1410

15-
return caches.match(offlineShell, { cacheName }).then(cachedResponse => {
16-
if (cachedResponse) return cachedResponse
17-
18-
console.error(
19-
`The offline shell (${offlineShell}) was not found ` +
20-
`while attempting to serve a response for ${pathname}`
21-
)
11+
// Check for resources + the app bundle
12+
// The latter may not exist if the SW is updating to a new version
13+
const resources = await idbKeyval.get(`resources:${pathname}`)
14+
if (!resources || !(await caches.match(`%pathPrefix%/%appFile%`))) {
15+
return await fetch(event.request)
16+
}
2217

23-
return fetch(offlineShell).then(response => {
24-
if (response.ok) {
25-
return caches.open(cacheName).then(cache =>
26-
// Clone is needed because put() consumes the response body.
27-
cache.put(offlineShell, response.clone()).then(() => response)
28-
)
29-
} else {
30-
return fetch(event.request)
31-
}
32-
})
33-
})
18+
for (const resource of resources) {
19+
// As soon as we detect a failed resource, fetch the entire page from
20+
// network - that way we won't risk being in an inconsistent state with
21+
// some parts of the page failing.
22+
if (!(await caches.match(resource))) {
23+
return await fetch(event.request)
3424
}
25+
}
3526

36-
return fetch(event.request)
37-
})
27+
const offlineShell = `%pathPrefix%/offline-plugin-app-shell-fallback/index.html`
28+
return await caches.match(offlineShell)
3829
})
3930

4031
workbox.routing.registerRoute(navigationRoute)
4132

42-
let updatingWhitelist = null
43-
44-
function rawWhitelistPathnames(pathnames) {
45-
if (updatingWhitelist !== null) {
46-
// Prevent the whitelist from being updated twice at the same time
47-
return updatingWhitelist.then(() => rawWhitelistPathnames(pathnames))
48-
}
49-
50-
updatingWhitelist = idbKeyval
51-
.get(WHITELIST_KEY)
52-
.then((customWhitelist = []) => {
53-
pathnames.forEach(pathname => {
54-
if (!customWhitelist.includes(pathname)) customWhitelist.push(pathname)
55-
})
56-
57-
return idbKeyval.set(WHITELIST_KEY, customWhitelist)
58-
})
59-
.then(() => {
60-
updatingWhitelist = null
61-
})
62-
63-
return updatingWhitelist
64-
}
65-
66-
function rawResetWhitelist() {
67-
if (updatingWhitelist !== null) {
68-
return updatingWhitelist.then(() => rawResetWhitelist())
69-
}
70-
71-
updatingWhitelist = idbKeyval.set(WHITELIST_KEY, []).then(() => {
72-
updatingWhitelist = null
73-
})
74-
75-
return updatingWhitelist
76-
}
77-
7833
const messageApi = {
79-
whitelistPathnames(event) {
80-
let { pathnames } = event.data
81-
82-
pathnames = pathnames.map(({ pathname, includesPrefix }) => {
83-
if (!includesPrefix) {
84-
return `%pathPrefix%${pathname}`
85-
} else {
86-
return pathname
87-
}
88-
})
89-
90-
event.waitUntil(rawWhitelistPathnames(pathnames))
34+
setPathResources(event, { path, resources }) {
35+
event.waitUntil(idbKeyval.set(`resources:${path}`, resources))
9136
},
9237

93-
resetWhitelist(event) {
94-
event.waitUntil(rawResetWhitelist())
38+
clearPathResources(event) {
39+
event.waitUntil(idbKeyval.clear())
9540
},
9641
}
9742

9843
self.addEventListener(`message`, event => {
9944
const { gatsbyApi } = event.data
100-
if (gatsbyApi) messageApi[gatsbyApi](event)
45+
if (gatsbyApi) messageApi[gatsbyApi](event, event.data)
10146
})

packages/gatsby/cache-dir/production-app.js

+1-1
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@ apiRunnerAsync(`onClientEntry`).then(() => {
5858

5959
class LocationHandler extends React.Component {
6060
render() {
61-
let { location } = this.props
61+
const { location } = this.props
6262
return (
6363
<EnsureResources location={location}>
6464
{({ pageResources, location }) => (

0 commit comments

Comments
 (0)