Skip to content

Commit

Permalink
fix(gatsby-plugin-offline): Change navigation handler logic (#13502)
Browse files Browse the repository at this point in the history
* 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
  • Loading branch information
valin4tor authored and GatsbyJS Bot committed Aug 8, 2019
1 parent 67967a1 commit 504b077
Show file tree
Hide file tree
Showing 6 changed files with 68 additions and 132 deletions.
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
describe(`focus management`, () => {
it(`Focus router wrapper after navigation to regular page (from index)`, () => {
cy.visit(`/`).waitForRouteChange()
cy.visit(`/`).waitForAPIorTimeout(`onRouteUpdate`, { timeout: 5000 })

cy.changeFocus()
cy.assertRouterWrapperFocus(false)
Expand All @@ -27,7 +27,10 @@ describe(`focus management`, () => {
})

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

cy.changeFocus()
cy.assertRouterWrapperFocus(false)
Expand All @@ -36,7 +39,10 @@ describe(`focus management`, () => {
})

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

// navigating to different not existing page should also trigger
// router wrapper focus as this is different page
Expand Down
2 changes: 1 addition & 1 deletion packages/gatsby-cypress/src/commands.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ Cypress.Commands.add(`getTestElement`, (selector, options = {}) =>
cy.get(`[data-testid="${selector}"]`, options)
)

const TIMEOUT = 5000
const TIMEOUT = 10000

Cypress.Commands.add(
`waitForAPI`,
Expand Down
70 changes: 29 additions & 41 deletions packages/gatsby-plugin-offline/src/gatsby-browser.js
Original file line number Diff line number Diff line change
@@ -1,16 +1,15 @@
exports.registerServiceWorker = () => true

const prefetchedPathnames = []
const whitelistedPathnames = []

exports.onServiceWorkerActive = ({
getResourceURLsForPathname,
serviceWorker,
}) => {
// if the SW has just updated then reset whitelisted paths and don't cache
// if the SW has just updated then clear the path dependencies and don't cache
// stuff, since we're on the old revision until we navigate to another page
if (window.___swUpdated) {
serviceWorker.active.postMessage({ gatsbyApi: `resetWhitelist` })
serviceWorker.active.postMessage({ gatsbyApi: `clearPathResources` })
return
}

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

// Loop over all resources and fetch the page component and JSON
// to add it to the sw cache.
// Loop over prefetched pages and add their resources to an array,
// plus specify which resources are required for those paths.
const prefetchedResources = []
prefetchedPathnames.forEach(path =>
getResourceURLsForPathname(path).forEach(resource =>
prefetchedResources.push(resource)
)
)
prefetchedPathnames.forEach(path => {
const resources = getResourceURLsForPathname(path)
prefetchedResources.push(...resources)

serviceWorker.active.postMessage({
gatsbyApi: `setPathResources`,
path,
resources,
})
})

// Loop over all resources and fetch the page component + JSON data
// to add it to the SW cache.
const resources = [...headerResources, ...prefetchedResources]
resources.forEach(resource => {
// Create a prefetch link for each resource, so Workbox runtime-caches them
Expand All @@ -47,44 +53,26 @@ exports.onServiceWorkerActive = ({

document.head.appendChild(link)
})

serviceWorker.active.postMessage({
gatsbyApi: `whitelistPathnames`,
pathnames: whitelistedPathnames,
})
}

function whitelistPathname(pathname, includesPrefix) {
exports.onPostPrefetchPathname = ({ pathname, getResourceURLsForPathname }) => {
// do nothing if the SW has just updated, since we still have old pages in
// memory which we don't want to be whitelisted
if (window.___swUpdated) return

if (`serviceWorker` in navigator) {
const { serviceWorker } = navigator

if (serviceWorker.controller !== null) {
if (serviceWorker.controller === null) {
// if SW is not installed, we need to record any prefetches
// that happen so we can then add them to SW cache once installed
prefetchedPathnames.push(pathname)
} else {
serviceWorker.controller.postMessage({
gatsbyApi: `whitelistPathnames`,
pathnames: [{ pathname, includesPrefix }],
gatsbyApi: `setPathResources`,
path: pathname,
resources: getResourceURLsForPathname(pathname),
})
} else {
whitelistedPathnames.push({ pathname, includesPrefix })
}
}
}

exports.onPostPrefetchPathname = ({ pathname }) => {
// do nothing if the SW has just updated, since we still have old pages in
// memory which we don't want to be whitelisted
if (window.___swUpdated) return

whitelistPathname(pathname, false)

// if SW is not installed, we need to record any prefetches
// that happen so we can then add them to SW cache once installed
if (
`serviceWorker` in navigator &&
!(
navigator.serviceWorker.controller !== null &&
navigator.serviceWorker.controller.state === `activated`
)
) {
prefetchedPathnames.push(pathname)
}
}
11 changes: 4 additions & 7 deletions packages/gatsby-plugin-offline/src/gatsby-node.js
Original file line number Diff line number Diff line change
Expand Up @@ -45,19 +45,15 @@ exports.onPostBuild = (args, pluginOptions) => {
`webpack-runtime`,
`component---node-modules-gatsby-plugin-offline-app-shell-js`,
])
const appFile = files.find(file => file.startsWith(`app-`))

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

const criticalFilePaths = _.uniq(
_.concat(
getResourcesFromHTML(`${process.cwd()}/${rootDir}/404.html`),
getResourcesFromHTML(
`${process.cwd()}/${rootDir}/offline-plugin-app-shell-fallback/index.html`
)
)
const criticalFilePaths = getResourcesFromHTML(
`${process.cwd()}/${rootDir}/offline-plugin-app-shell-fallback/index.html`
).map(omitPrefix)

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

fs.appendFileSync(`public/sw.js`, `\n` + swAppend)
console.log(
Expand Down
103 changes: 24 additions & 79 deletions packages/gatsby-plugin-offline/src/sw-append.js
Original file line number Diff line number Diff line change
@@ -1,101 +1,46 @@
/* global importScripts, workbox, idbKeyval */

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

const navigationRoute = new workbox.routing.NavigationRoute(({ event }) => {
const { pathname } = new URL(event.request.url)
const { NavigationRoute } = workbox.routing

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

return caches.match(offlineShell, { cacheName }).then(cachedResponse => {
if (cachedResponse) return cachedResponse

console.error(
`The offline shell (${offlineShell}) was not found ` +
`while attempting to serve a response for ${pathname}`
)
// Check for resources + the app bundle
// The latter may not exist if the SW is updating to a new version
const resources = await idbKeyval.get(`resources:${pathname}`)
if (!resources || !(await caches.match(`%pathPrefix%/%appFile%`))) {
return await fetch(event.request)
}

return fetch(offlineShell).then(response => {
if (response.ok) {
return caches.open(cacheName).then(cache =>
// Clone is needed because put() consumes the response body.
cache.put(offlineShell, response.clone()).then(() => response)
)
} else {
return fetch(event.request)
}
})
})
for (const resource of resources) {
// As soon as we detect a failed resource, fetch the entire page from
// network - that way we won't risk being in an inconsistent state with
// some parts of the page failing.
if (!(await caches.match(resource))) {
return await fetch(event.request)
}
}

return fetch(event.request)
})
const offlineShell = `%pathPrefix%/offline-plugin-app-shell-fallback/index.html`
return await caches.match(offlineShell)
})

workbox.routing.registerRoute(navigationRoute)

let updatingWhitelist = null

function rawWhitelistPathnames(pathnames) {
if (updatingWhitelist !== null) {
// Prevent the whitelist from being updated twice at the same time
return updatingWhitelist.then(() => rawWhitelistPathnames(pathnames))
}

updatingWhitelist = idbKeyval
.get(WHITELIST_KEY)
.then((customWhitelist = []) => {
pathnames.forEach(pathname => {
if (!customWhitelist.includes(pathname)) customWhitelist.push(pathname)
})

return idbKeyval.set(WHITELIST_KEY, customWhitelist)
})
.then(() => {
updatingWhitelist = null
})

return updatingWhitelist
}

function rawResetWhitelist() {
if (updatingWhitelist !== null) {
return updatingWhitelist.then(() => rawResetWhitelist())
}

updatingWhitelist = idbKeyval.set(WHITELIST_KEY, []).then(() => {
updatingWhitelist = null
})

return updatingWhitelist
}

const messageApi = {
whitelistPathnames(event) {
let { pathnames } = event.data

pathnames = pathnames.map(({ pathname, includesPrefix }) => {
if (!includesPrefix) {
return `%pathPrefix%${pathname}`
} else {
return pathname
}
})

event.waitUntil(rawWhitelistPathnames(pathnames))
setPathResources(event, { path, resources }) {
event.waitUntil(idbKeyval.set(`resources:${path}`, resources))
},

resetWhitelist(event) {
event.waitUntil(rawResetWhitelist())
clearPathResources(event) {
event.waitUntil(idbKeyval.clear())
},
}

self.addEventListener(`message`, event => {
const { gatsbyApi } = event.data
if (gatsbyApi) messageApi[gatsbyApi](event)
if (gatsbyApi) messageApi[gatsbyApi](event, event.data)
})
2 changes: 1 addition & 1 deletion packages/gatsby/cache-dir/production-app.js
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ apiRunnerAsync(`onClientEntry`).then(() => {

class LocationHandler extends React.Component {
render() {
let { location } = this.props
const { location } = this.props
return (
<EnsureResources location={location}>
{({ pageResources, location }) => (
Expand Down

0 comments on commit 504b077

Please sign in to comment.