Skip to content

Commit

Permalink
fix(gatsby-plugin-offline): delay adding resources for paths until we…
Browse files Browse the repository at this point in the history
… have urls (#8613)

* Fix fetching resources before prefetch finishes

* Pass onPostPrefetchPathname into onPrefetchPathname so plugins can call this when ready

* Improve docs

* Move onPostPrefetchPathname to the end

* Refactor Guess.js plugin to use getResourcesForPathname

Also make it easier to read by removing some unnecessary lines and
changing indentation

* Remove unnecessary lines

* Undo accidental change

* Call onPostPrefetchPathname from getResourcesForPathname

* Refactor prefetching APIs

* Remove comment + minor refactor

* Remove unnecessary params

* Refactor for consistency

* Move API call outside the if-block

* set api runner for loader earlier
  • Loading branch information
vtenfys authored and pieh committed Oct 9, 2018
1 parent 2ba57ea commit 2605aa0
Show file tree
Hide file tree
Showing 7 changed files with 52 additions and 74 deletions.
75 changes: 14 additions & 61 deletions packages/gatsby-plugin-guess-js/src/gatsby-browser.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,65 +12,18 @@ exports.onRouteUpdate = ({ location }) => {
initialPath = location.pathname
}

let chunksPromise
const chunks = () => {
if (!chunksPromise) {
chunksPromise = fetch(`${window.location.origin}/webpack.stats.json`).then(
res => res.json()
)
}

return chunksPromise
}

let hasPrefetched = {}
const prefetch = url => {
if (hasPrefetched[url]) {
return
}
hasPrefetched[url] = true
const link = document.createElement(`link`)
link.setAttribute(`rel`, `prefetch`)
link.setAttribute(`href`, url)
const parentElement =
document.getElementsByTagName(`head`)[0] ||
document.getElementsByName(`script`)[0].parentNode
parentElement.appendChild(link)
}

exports.onPrefetchPathname = ({ pathPrefix }, pluginOptions) => {
if (process.env.NODE_ENV === `production`) {
const matchedPaths = Object.keys(
guess({
path: window.location.pathname,
threshold: pluginOptions.minimumThreshold,
})
)

// Don't prefetch from client for the initial path as we did that
// during SSR
if (notNavigated && initialPath === window.location.pathname) {
return
}

if (matchedPaths.length > 0) {
matchedPaths.forEach(p => {
chunks(p).then(chunk => {
// eslint-disable-next-line
const page = ___loader.getPage(p)
if (!page) return
let resources = []
if (chunk.assetsByChunkName[page.componentChunkName]) {
resources = resources.concat(
chunk.assetsByChunkName[page.componentChunkName]
)
}
// eslint-disable-next-line
resources.push(`static/d/${___dataPaths[page.jsonName]}.json`)
// TODO add support for pathPrefix
resources.forEach(r => prefetch(`/${r}`))
})
})
}
}
exports.onPrefetchPathname = ({ getResourcesForPathname }, pluginOptions) => {
if (process.env.NODE_ENV !== `production`) return

const matchedPaths = Object.keys(
guess({
path: window.location.pathname,
threshold: pluginOptions.minimumThreshold,
})
)

// Don't prefetch from client for the initial path as we did that
// during SSR
if (!(notNavigated && initialPath === window.location.pathname))
matchedPaths.forEach(getResourcesForPathname)
}
2 changes: 1 addition & 1 deletion packages/gatsby-plugin-offline/src/gatsby-browser.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ exports.registerServiceWorker = () => true
let swNotInstalled = true
const prefetchedPathnames = []

exports.onPrefetchPathname = ({ pathname }) => {
exports.onPostPrefetchPathname = ({ pathname }) => {
// 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 (swNotInstalled && `serviceWorker` in navigator) {
Expand Down
3 changes: 2 additions & 1 deletion packages/gatsby/cache-dir/app.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,12 @@ import { hot } from "react-hot-loader"
import socketIo from "./socketIo"
import emitter from "./emitter"
import { apiRunner, apiRunnerAsync } from "./api-runner-browser"
import loader from "./loader"
import loader, { setApiRunnerForLoader } from "./loader"
import syncRequires from "./sync-requires"
import pages from "./pages.json"

window.___emitter = emitter
setApiRunnerForLoader(apiRunner)

// Let the site/plugins run code very early.
apiRunnerAsync(`onClientEntry`).then(() => {
Expand Down
29 changes: 22 additions & 7 deletions packages/gatsby/cache-dir/loader.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ import prefetchHelper from "./prefetch"

const preferDefault = m => (m && m.default) || m

let prefetcher
let devGetPageData
let inInitialRender = true
let hasFetched = Object.create(null)
Expand Down Expand Up @@ -147,6 +146,20 @@ const handleResourceLoadError = (path, message) => {
}
}

const onPrefetchPathname = pathname => {
if (!prefetchTriggered[pathname]) {
apiRunner(`onPrefetchPathname`, { pathname: pathname })
prefetchTriggered[pathname] = true
}
}

const onPostPrefetchPathname = pathname => {
if (!prefetchCompleted[pathname]) {
apiRunner(`onPostPrefetchPathname`, { pathname: pathname })
prefetchCompleted[pathname] = true
}
}

// Note we're not actively using the path data atm. There
// could be future optimizations however around trying to ensure
// we load all resources for likely-to-be-visited paths.
Expand All @@ -163,6 +176,7 @@ const sortResourcesByCount = (a, b) => {
let findPage
let pathScriptsCache = {}
let prefetchTriggered = {}
let prefetchCompleted = {}
let disableCorePrefetching = false

const queue = {
Expand Down Expand Up @@ -192,12 +206,7 @@ const queue = {

// Tell plugins with custom prefetching logic that they should start
// prefetching this path.
if (!prefetchTriggered[path]) {
apiRunner(`onPrefetchPathname`, {
pathname: path,
})
prefetchTriggered[path] = true
}
onPrefetchPathname(path)

// If a plugin has disabled core prefetching, stop now.
if (disableCorePrefetching.some(a => a)) {
Expand Down Expand Up @@ -236,6 +245,9 @@ const queue = {
prefetchResource(page.componentChunkName)
}

// Tell plugins the path has been successfully prefetched
onPostPrefetchPathname(path)

return true
},

Expand Down Expand Up @@ -375,6 +387,9 @@ const queue = {
}
})
}

// Tell plugins the path has been successfully prefetched
onPostPrefetchPathname(path)
}),
}

Expand Down
1 change: 0 additions & 1 deletion packages/gatsby/cache-dir/navigation.js
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,6 @@ function init() {
// Temp hack while awaiting https://github.com/reach/router/issues/119
window.__navigatingToLink = false

setApiRunnerForLoader(apiRunner)
window.___loader = loader
window.___push = to => navigate(to, { replace: false })
window.___replace = to => navigate(to, { replace: true })
Expand Down
3 changes: 2 additions & 1 deletion packages/gatsby/cache-dir/production-app.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import emitter from "./emitter"
window.___emitter = emitter
import PageRenderer from "./page-renderer"
import asyncRequires from "./async-requires"
import loader from "./loader"
import loader, { setApiRunnerForLoader } from "./loader"
import loadDirectlyOr404 from "./load-directly-or-404"
import EnsureResources from "./ensure-resources"

Expand All @@ -25,6 +25,7 @@ window.___loader = loader
loader.addPagesArray([window.page])
loader.addDataPaths({ [window.page.jsonName]: window.dataPath })
loader.addProdRequires(asyncRequires)
setApiRunnerForLoader(apiRunner)

navigationInit()

Expand Down
13 changes: 11 additions & 2 deletions packages/gatsby/src/utils/api-browser-docs.js
Original file line number Diff line number Diff line change
Expand Up @@ -159,11 +159,20 @@ exports.wrapRootElement = true
* Called when prefetching for a pathname is triggered. Allows
* for plugins with custom prefetching logic.
* @param {object} $0
* @param {object} $0.pathname The pathname whose resources should now be prefetched
* @param {object} $0.getResourcesForPathname Function for fetching resources related to pathname
* @param {string} $0.pathname The pathname whose resources should now be prefetched
* @param {function} $0.getResourcesForPathname Function for fetching resources related to pathname
*/
exports.onPrefetchPathname = true

/**
* Called when prefetching for a pathname is successful. Allows
* for plugins with custom prefetching logic.
* @param {object} $0
* @param {string} $0.pathname The pathname whose resources have now been prefetched
* @param {function} $0.getResourceURLsForPathname Function for fetching URLs for resources related to the pathname
*/
exports.onPostPrefetchPathname = true

/**
* Plugins can take over prefetching logic. If they do, they should call this
* to disable the now duplicate core prefetching logic.
Expand Down

0 comments on commit 2605aa0

Please sign in to comment.