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.

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: service worker failed to install when pathPrefix option was set #1912

Merged
merged 3 commits into from
Aug 25, 2017

Conversation

vincentbel
Copy link
Contributor

@vincentbel vincentbel commented Aug 25, 2017

Problem

I use the Vagr9K/gatsby-advanced-starter repo to illustrate the problem.

Service worker failed to install page caches when pathPrefix option was set.

demo:

gatsby-service-worker-error

What Causes the Problem

In the built sw.js of Vagr9K/gatsby-advanced-starter, each precachedConfig item is started with /(source):

var precacheConfig = [["/app-7ab9dc184b7d3049a294.js","3d59a3d5acf6826942d983aa505b95d5"],["/commons-5a25b4f277d17d73c441.js","25d10092faa49cd22c42f1e9fb69b902"],["/index.html","e17744bd031cc8a71fc8d8f2162e71c5"],["/manifest.json","53da04d3756b99d845311cf0dab7b6fa"],["/offline-plugin-app-shell-fallback/index.html","8475e6e45e61110e101c99888a1831b2"]];

and the assets url is generated by(source):

var absoluteUrl = new URL(relativeUrl, self.location);
// relativeUrl: "/app-7ab9dc184b7d3049a294.js"
// self.location: { href: "https://vagr9k.github.io/gatsby-advanced-starter/sw.js", ... }
// so absoluteUrl => { href: "https://vagr9k.github.io/app-7ab9dc184b7d3049a294.js", ... }

We want the absoluteUrl to be https://vagr9k.github.io/gatsby-advanced-starter/app-7ab9dc184b7d3049a294.js, but we got https://vagr9k.github.io/app-7ab9dc184b7d3049a294.js.

How to Fix the Problem

Let the generated precachedConfig array items to be prefixed(lol):

/app-7ab9dc184b7d3049a294.js => /gatsby-advanced-starter/app-7ab9dc184b7d3049a294.js

There exists a replacePrefix option in sw-precache. If pathPrefix is configured by user, we should replace the public prefix(rootDir, used as stripPrefix) with pathPrefix. That's how this pull request fix it.

Note 1:
We can also use the stripPrefixMulti option, but replacePrefix is enough here.

Note 2:
User can also set the replacePrefix or stripPrefixMulti option manually to fix the problem:

// in gatsby-config.js
{
  resolve: 'gatsby-plugin-offline',
  options: {
    replacePrefix: '/myprefix',
  },
}

But obviously this should be fixed by offline plugin.

How to Verify the Fixes

I have manually tested it, and it works. But you can quickly view the result of replacePrefix option by:

  1. install sw-precache cli:
$ npm install --global sw-precache
  1. generate service-worker.js by:
$ sw-precache --root=public --static-file-globs='public/**/*.js' --stripPrefix='public' --replacePrefix='/blog'
  1. view the content of generated service-worker.js.

Service worker failed to install page caches when
`pathPrefix` option was set.

There exists a
[`replacePrefix` option in `sw-precache`](https://github.com/GoogleChrome/sw-precache#replaceprefix-string).
If `pathPrefix` is configured by user, we should
replace the `public` prefix with `pathPrefix`.

We can also use the
[`stripPrefixMulti` option](https://github.com/GoogleChrome/sw-precache#stripprefixmulti-object),
but `replacePrefix` is enough here.
@gatsbybot
Copy link
Collaborator

gatsbybot commented Aug 25, 2017

Deploy preview ready!

Built with commit 13d49ff

https://deploy-preview-1912--gatsbygram.netlify.com

Copy link
Contributor

@KyleAMathews KyleAMathews left a comment

Choose a reason for hiding this comment

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

Thanks for the research and nice bug fix!

// the `public` prefix with `pathPrefix`.
// See more at:
// https://github.com/GoogleChrome/sw-precache#replaceprefix-string
replacePrefix: args.pathPrefix || '',
Copy link
Contributor

Choose a reason for hiding this comment

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

You also need to check if the user is actually building with paths prefixed. You can set a pathPrefix but build without them e.g. for local testing.

publicPath: program.prefixPaths
? `${store.getState().config.pathPrefix}/`

Copy link
Contributor Author

@vincentbel vincentbel Aug 25, 2017

Choose a reason for hiding this comment

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

@KyleAMathews I think pathPrefix will only be set if program.prefixPaths === true for plugin API(onPostBuild here), otherwise it will be ''.

if (store.getState().program.prefixPaths) {
pathPrefix = store.getState().config.pathPrefix
}

P.S.
It seems no full documents for plugin API like onPostBuild now, I got the pathPrefix argument by referring the source 😅. And can I get the program.prefixPaths variable in onPostBuild function?

Copy link
Contributor

Choose a reason for hiding this comment

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

Haha so it is :-)

Copy link
Contributor

Choose a reason for hiding this comment

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

Forgot I did that… :-)

@gatsbybot
Copy link
Collaborator

gatsbybot commented Aug 25, 2017

Deploy preview ready!

Built with commit 13d49ff

https://deploy-preview-1912--using-drupal.netlify.com

@KyleAMathews KyleAMathews dismissed their stale review August 25, 2017 17:05

Review was incorrect :-)

@KyleAMathews KyleAMathews merged commit 80d8b6a into gatsbyjs:master Aug 25, 2017
@KyleAMathews
Copy link
Contributor

Thanks!

@vincentbel
Copy link
Contributor Author

You rock, The Great Gatsby. 🤗

@jlengstorf
Copy link
Contributor

Hiya @vincentbel! 👋

This is definitely late, but on behalf of the entire Gatsby community, I wanted to say thank you for being here.

Gatsby is built by awesome people like you. Let us say “thanks” in two ways:

  1. We’d like to send you some Gatsby swag. As a token of our appreciation, you can go to the Gatsby Swag Store and log in with your GitHub account to get a coupon code good for one free piece of swag. (We’ve got t-shirts and hats, plus some socks that are really razzing our berries right now.)
  2. If you’re not already part of it, we just invited you to join the Gatsby organization on GitHub. This will add you to our team of maintainers. You’ll receive an email shortly asking you to confirm. By joining the team, you’ll be able to label issues, review pull requests, and merge approved pull requests.

If you have questions, please don’t hesitate to reach out to us: tweet at @gatsbyjs and we’ll come a-runnin’.

Thanks again! 💪💜

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants