[gatsby-plugin-offline] [gatsby/cache-dir] Fix various offline and caching issues #7355

vtenfys · 2018-08-15T17:13:29Z

Changes made:

<div> is replaced with <span> in gatsby-plugin-offline's app-shell.js to prevent an edge case in Chrome, where the the outer element's class attribute is unset despite its className being set in the VDOM, if the page was loaded from the offline shell.
A call to getResourcesForPathname was fixed - previously this prevented later code from calling since Promise.all(pathnameResources) would never resolve.
Styles are now searched as well as scripts when caching resources, and these are all cached regardless of whether they are included in pageResources.
Styles are also now searched in get-resources-from-html.js.
The offline shell and custom 404 page are now also scanned for resources when considering critical file paths.
- get-resources-from-html.js is modified so that if a custom 404 page doesn't exist, an empty array will be returned.
A glob for Webpack chunks is added to the precache options, as these are sometimes loaded from other chunks and therefore not detected by getResourcesFromHTML, causing errors to occur when trying to navigate to another page when offline (if they haven't all been cached).
The navigateFallbackWhitelist regex is modified to prevent loading the offline shell if the URL ends with ?no-cache=1 or &no-cache=1.
sw.js is patched to include search queries when matching URLs against navigateFallbackWhitelist. Unfortunately I couldn't find any options to enable this, so patching appears to be the only way until a change is made upstream.
- An error is thrown asking the user to report an issue if the patch fails - this will cause the build to fail, but will avoid confusion in the long run if the patch stops working.
- I checked the ignoreUrlParametersMatching options, but it's for ignoring differences, e.g. between /page/ and /page/?utm_source=github, not for disabling caching or fallback.
JSON and CSS resources are cached at runtime.
page-renderer.js now checks for this.state.pageResources.json, since errors were previously occurring even when this.state.pageResources existed due to the missing JSON data.
When a page isn't found, an attempt is made to load it directly, in order to display a proper error message, i.e. 404 or offline error, or simply load it directly if it's available online (e.g. /admin/ for CMS, which is not directly part of Gatsby).
A clause has been added to an if statement in production-app.js to prevent the 404 showing until an attempt has been made to load the page directly
- Load resources for 404 page if the page doesn't exist #7268 had actually broken support for static pages (e.g. Netlify CMS at /admin/) since the service worker would be unable to find the page from Gatsby and show a 404
onServiceWorkerInstalled is now delayed by 100ms, since it was being called before the fetch event listeners were added, thus preventing resources from being cached. Unfortunately I couldn't find a programmatic way to do this (other than more patching of sw.js, which is less than ideal).
- It might be worth increasing this to 500ms, since at 100ms it could still fail for more complex sites (despite working at as low as 20ms in local testing). Thoughts?
checkIfDoingInitialRenderForSW is removed from loader.js as it was causing the SW to be removed when navigating to a nonexistent page, even if the website had not been updated, causing it to stop working offline entirely - regardless, it is no longer needed with the other changes made.
README for gatsby-plugin-offline is updated to match the latest default options.

Closes #6212, Closes #7053, Closes #6688, possibly others

…offline-issues

KyleAMathews · 2018-08-15T17:17:31Z

Deploy preview for using-postcss-sass failed.

Built with commit 4eb02cd

https://app.netlify.com/sites/using-postcss-sass/deploys/5b75433b3813f010edac0916

KyleAMathews

This looks great! Very through! I'll give it a more through review, hopefully later, but thanks!

KyleAMathews · 2018-08-15T17:19:04Z

packages/gatsby-plugin-offline/package.json

@@ -1,7 +1,7 @@
 {
  "name": "gatsby-plugin-offline",
  "description": "Gatsby plugin which sets up a site to be able to run offline",
-  "version": "2.0.0-beta.9",
+  "version": "2.0.0-beta.10",


the version gets bumped automatically as part of deploys so please remove this

gatsbybot · 2018-08-15T17:19:56Z

Deploy preview for using-drupal ready!

Built with commit 4eb02cd

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

gatsbybot · 2018-08-15T17:20:52Z

Deploy preview for gatsbygram ready!

Built with commit 4eb02cd

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

KyleAMathews · 2018-08-15T17:27:11Z

Deploy preview for image-processing failed.

Built with commit 03138384e2592f0abca8626f22822d4fd213215e

https://app.netlify.com/sites/image-processing/deploys/5b7764dac6aed6786a016d16

vtenfys · 2018-08-15T17:30:49Z

@KyleAMathews thanks! I'd normally agree with small PRs, but the bugs were all so interdependent that I felt they would become fragmented with separate PRs, which might result in some of them reappearing after others having been fixed or some being forgotten. Looking forward to your full review though!

KyleAMathews · 2018-08-15T18:06:39Z

Deploy preview for gatsbyjs failed.

Built with commit 15ee192

https://app.netlify.com/sites/gatsbyjs/deploys/5b745f394ed62f238b914384

KyleAMathews · 2018-08-15T18:10:39Z

Deploy preview for gatsbyjs failed.

Built with commit 4eb02cd

https://app.netlify.com/sites/gatsbyjs/deploys/5b75433a3813f010edac090a

vtenfys · 2018-08-15T21:07:00Z

By the way, I just checked #7268 after rereading your comments on #6212, and it's a different issue to the one I was fixing -- #7268 serves the 404 page when running gatsby serve, whereas this PR is about handling what happens when the offline shell can't load the specified page, since the shell would previously try to handle all pages matching the regex /^.*([^.]{5}|.html)$/ regardless of what's happening on the server. Apologies, I see it's not just changing the behaviour of gatsby serve - I'll try to check tomorrow to see how custom 404 pages are handled with these changes. I believe they should work fine, but I'll need to make sure there aren't any edge cases or odd behaviours.

Update: ~~I've just pushed another commit which fixes some issues with regards to the changes in #7268.~~ There are some more issues I've just noticed - pushing another commit shortly!

Update 2: I'll need a bit more time to get this fixed - should be ready later today

wKovacs64 · 2018-08-15T22:44:24Z

@davidbailey00: Nice! ❤️

@jlengstorf: A lot going on here, but some of it may address some or even all of the things pertaining to gatsby-plugin-offline that we went over in our pairing session last week. (Did you end up creating that issue somewhere? We may want to link here if so.)

jlengstorf · 2018-08-15T22:48:27Z

@wKovacs64 I... definitely did not create that issue. I still have it written in my notebook to capture. 😬

Thanks for the heads up!

wKovacs64 · 2018-08-15T23:14:33Z

@jlengstorf Oh no sweat, no rush. Thanks!

vtenfys · 2018-08-16T09:35:24Z

I've fixed the remaining issues with regards to 404 pages, including a bug which #7268 introduced, and I've updated the changelog to reflect these fixes. Code should now be ready for a review! 😄

vtenfys · 2018-08-16T16:20:54Z

Just pushed a commit which throws an error if the sw.js patch fails - otherwise this might stop working in the future without users knowing why a problem is occurring.

KyleAMathews · 2018-08-16T16:44:01Z

Not necessarily for this PR but it'd be great to add this in the future #7382

dfmarulanda · 2018-08-29T21:15:51Z

It wasn't a problem of azure, because it is a server as a static website. The problem only happened when in the production build served by Azure. It never happened on the local build. @davidbailey00 I'm uploading the public folder because I use it to make a continuous deployment using my GitHub repo. It's fixed, but it was a weird bug generated in the last update.

vtenfys · 2018-08-29T21:18:32Z

@KyleAMathews @ssmith-wombatweb @dfmarulanda @jlengstorf perhaps we could have a runtime console warning after A page wasn't found for ... if it detects a page with the same name but different casing? We won't fix the Netlify+uppercase edge case but then it should be clear why it's not working.

On the note of this, I just noticed that URL search queries aren't always kept when redirecting to ?no-cache=1 - given the Netlify problem, https://www.simeonsmith.me/coding-with-kids/2018/08/10/crab-trap-alpha?no-cache=1 should keep adding &no-cache=1 when reloading. A better demonstration of the problem:

Go to https://wizardly-babbage-0faa32.netlify.com/ and wait for the SW to install
Then go to https://wizardly-babbage-0faa32.netlify.com/admin/?somequery=1 (should be handled by the service worker)
Expected behaviour: redirects to https://wizardly-babbage-0faa32.netlify.com/admin/?somequery=1&no-cache=1

Actual behaviour: redirects to https://wizardly-babbage-0faa32.netlify.com/admin/?no-cache=1

KyleAMathews · 2018-08-29T21:20:13Z

Yeah, that's the sort of warning I was thinking of.

vtenfys · 2018-08-29T21:29:15Z

@KyleAMathews Alright, I'm still doing work experience until the end of this week but I can try fixing that tomorrow evening, along with the other bug.

A warning should be easy to implement, although the bug I just discovered might take me a bit longer - it only happens from external links or direct URL entry, not from <Link>s; I think I vaguely understand why but I'll need to play around a bit. I can submit these as separate PRs since they're not directly related, unlike the issues from #7355.

(Also, unrelated to these issues but the company I'm doing work exp at seems impressed by Gatsby and is considering rewriting their current sites on legacy stacks using it 😄)

mjoynes-wombat-web · 2018-08-30T05:08:01Z

Just coming back to confirm that changing that to lowercase solves my issues on Netlify.
https://www.simeonsmith.me/coding-with-kids/2018/08/10/crab-trap-alpha

vtenfys · 2018-08-30T10:48:39Z

I've thought of a better solution than showing a warning - we can use Gatsby's global window.page.path to get the canonical path rather than using the page URL, which means that even if the page is loaded from a different location (potentially completely different), it can still find the resources. I'lll work on this tonight

KyleAMathews · 2018-08-30T18:36:44Z

(Also, unrelated to these issues but the company I'm doing work exp at seems impressed by Gatsby and is considering rewriting their current sites on legacy stacks using it 😄)

Nice! :-D

vtenfys · 2018-08-30T20:20:33Z

I've thought of a better solution than showing a warning - we can use Gatsby's global window.page.path to get the canonical path rather than using the page URL, which means that even if the page is loaded from a different location (potentially completely different), it can still find the resources. I'lll work on this tonight.

Just to update everyone, I've made some progress on this by replacing window.location.pathname with window.page.path in production-app.js but currently trying to understand how the <PageRenderer>'s properties are set, since modifying just production-app.js doesn't fix the issue.

Edit: I meant RouteHandler rather than PageRenderer

vtenfys · 2018-08-30T22:14:52Z

cc @KyleAMathews @jlengstorf @m-allanson I'm really struggling to understand where RouteHandler gets its props from, can someone help me out? I'm testing by making a copy of the about folder to about-test, to see if I can get it to load correctly using the canonical path (window.page.path) rather than the URL's actual path.

If I log the props...

...it's clear that the location prop is set from window.location:

But after running a trace I still have no idea how it gets these props. The element itself is created at line 126 (in my local code with added console.logs) when the renderer function is called:

...since it renders NewRoute, which wraps the RouteHandler element. It's clear that the only prop passed is the path prop:

All function calls between line 126 and the RouteHandler constructor are React calls, which can be seen via the console trace:

All the other calls are higher-level and have no mention of location or anything like that. My guess is that Reach Router must be adding the props, but I don't understand it well enough to know how this works... Could someone help me out please?

KyleAMathews · 2018-08-30T22:16:16Z

@davidbailey00 could you copy your previous comments over to a new issue? This one is getting rather lengthy :-)

vtenfys · 2018-08-30T22:21:26Z

@KyleAMathews sure, in hindsight I probably should've done that earlier :)

dfmarulanda · 2018-08-31T20:29:03Z

I updated gatsby today and now im getting

These dependencies were not found:

* @babel/runtime/core-js/get-iterator in ./node_modules/gatsby-plugin-offline/gatsby-browser.js
* @babel/runtime/core-js/promise in ./node_modules/gatsby-plugin-offline/gatsby-browser.js

vtenfys · 2018-08-31T20:34:03Z

@dfmarulanda Thanks for the heads up, I'll check this out.

vtenfys · 2018-08-31T20:39:51Z

@dfmarulanda hm, I'm unable to see what the issue is here since everything works locally for me... Can you try upgrading all packages again, and running yarn (or npm install) just to make sure any new dependencies are installed? If that still fails, can you output your package.json?

dfmarulanda · 2018-08-31T20:41:18Z

I already deleted node_modules and then yarn install. Also tried to add @babel/runtime by itself.

{
  "private": true,
  "scripts": {
    "build": "gatsby build",
    "develop": "gatsby develop",
    "lint:es": "eslint --ignore-path .gitignore \"**/*.{js,jsx}\"",
    "lint:style": "stylelint \"src/**/*.scss\"",
    "lint": "yarn lint:es && yarn lint:style",
    "format": "prettier-eslint --write \"**/*.{js,jsx,json,css,scss,md}\""
  },
  "dependencies": {
    "@babel/runtime": "^7.0.0",
    "@reach/router": "^1.1.1",
    "babel-eslint": "^8.2.6",
    "babel-plugin-styled-components": "^1.5.1",
    "fbjs": "^0.8.17",
    "gatsby": "^2.0.0-rc.7",
    "gatsby-plugin-canonical-urls": "^2.0.0-rc.1",
    "gatsby-plugin-drift": "^1.0.0",
    "gatsby-plugin-favicon": "^3.1.2",
    "gatsby-plugin-offline": "2.0.0-rc.0",
    "gatsby-plugin-react-helmet": "^3.0.0-rc.1",
    "gatsby-plugin-styled-components": "^3.0.0-rc.1",
    "gatsby-plugin-typography": "^2.2.0-rc.2",
    "grid-styled": "^5.0.2",
    "object-assign": "^4.1.1",
    "prop-types": "^15.6.2",
    "react": "^16.4.2",
    "react-dom": "^16.4.2",
    "react-emotion": "^9.2.4",
    "react-helmet": "^5.2.0",
    "react-router-dom": "^4.3.1",
    "react-typography": "^0.16.13",
    "styled-components": "^3.4.5",
    "styled-media-query": "^2.0.2",
    "typography": "^0.16.17"
  },
  "devDependencies": {
    "emotion": "^9.2.6",
    "eslint": "^4.19.1",
    "eslint-config-airbnb": "^17.0.0",
    "eslint-plugin-import": "^2.13.0",
    "eslint-plugin-jsx-a11y": "^6.1.1",
    "eslint-plugin-react": "^7.10.0",
    "prettier-eslint-cli": "^4.7.0",
    "stylelint": "^9.4.0",
    "stylelint-config-idiomatic-order": "^5.0.0",
    "stylelint-config-recommended-scss": "^3.0.0",
    "stylelint-scss": "^3.2.0",
    "webpack": "^4.16.4",
    "webpack-cli": "^3.1.0"
  }
}

vtenfys · 2018-08-31T20:46:01Z

You've got an old version of gatsby-plugin-offline which isn't updating because for some reason it doesn't have the caret (I can see that all other packages do). rc2 is the latest version; can you add the caret and then upgrade again?

dfmarulanda · 2018-08-31T20:49:58Z

Yeah. It seems it was the problem. Thanks!

dfmarulanda · 2018-09-06T16:51:16Z

I made an update yesterday and it seems it's broken again. I cannot figure out what's happening with it. You can check out my website at https://crowdswap.com

pieh · 2018-09-06T17:28:43Z

@dfmarulanda can you create new issue with more details of your setup - including output from gatsby info?

vtenfys · 2018-09-06T17:31:02Z

@pieh actually I'm just checking this out and the problem seems to be because the public folder isn't gitignored, so might not need a new issue

vtenfys · 2018-09-06T17:42:27Z

@dfmarulanda I've just checked and confirmed my above comment - you're not ignoring the public folder so the wrong files are being generated, causing additional errors. Please add public/ to your gitignore and then delete the folder, and commit all changes. Working demo: https://clever-villani-54e8ff.netlify.com/

Also, as @pieh said, in the future please create a new issue since GitHub's TTFB on this page is getting ridiculous 😀

dfmarulanda · 2018-09-06T20:12:26Z

But i don't understand why i have to gitignore the public folder. I use it as continous deployment to my website using azure web apps. I always delete the public folder before publishing. Why it would be the problem?

PD. Azure is connected with my public folder in my github repo.

vtenfys · 2018-09-06T20:25:11Z

Your HTML file is referencing JSON resources which don't exist on the server:

This is causing Gatsby to think that it shouldn't handle the page, since it can't find the resources, and so it tries loading the page directly. Since Gatsby does actually handle the page, it does the same thing again and again, forever.

I always delete the public folder before publishing.

In that case I think you're doing something wrong - after I ran rm -rf public .cache node_modules/.cache && gatsby build && gatsby serve, I had no problems. Even if you get this to work, it's considered bad practice to upload built files to git - you should use your Azure CI to build the Gatsby site, rather than doing it locally which should be done for testing only.

dfmarulanda · 2018-09-06T21:15:44Z

OK. I'll try to change my deploy process. It's super easy to do it using netlify, aws, heroku, but there is no an azure plugin. There is a docker plugin working in the latest version of Gatsby?

David Bailey added 3 commits August 15, 2018 17:32

Fix various bugs with offline support and caching

7391f2a

Merge branch 'master' of https://github.com/gatsbyjs/gatsby into fix-…

a4f477f

…offline-issues

Bump gatsby-plugin-offline version and update docs

15ee192

KyleAMathews previously approved these changes Aug 15, 2018

View reviewed changes

Restore previous gatsby-plugin-offline version

51c2e10

vtenfys dismissed KyleAMathews’s stale review via 51c2e10 August 15, 2018 17:21

Handle offline 404 pages properly

c0578ca

David Bailey added 4 commits August 16, 2018 09:50

Avoid incorrectly showing 404 pages

5ae987e

Tidy up get-resources-from-html.js

55448c1

Fix remaining issues with 404 pages

ef09930

404 page is no longer critical

4eb02cd

vtenfys mentioned this pull request Aug 16, 2018

Netlify CMS address printed in console redirects to gatsby development 404 page #6688

Closed

vtenfys changed the title ~~Fix various offline issues~~ [gatsby-plugin-offline] [gatsby/cache-dir] Fix various offline and caching issues Aug 16, 2018

KyleAMathews mentioned this pull request Aug 16, 2018

404 pages are blank in production #7371

Closed

vtenfys mentioned this pull request Aug 16, 2018

CSS does not load on direct visit of a page #7376

Closed

Throw an error if the sw.js patch fails

9c5f24a

KyleAMathews mentioned this pull request Aug 30, 2018

[v2] Refreshing a page then hitting back (browser back button) leads to 404 #7261

Closed

vtenfys mentioned this pull request Aug 30, 2018

[v2] Gatsby attempts to reload a page infinitely if the canonical path doesn't match window.location.pathname #7760

Closed

KyleAMathews mentioned this pull request Sep 3, 2018

Gatsby V2 website error #7821

Closed

vtenfys mentioned this pull request Sep 6, 2018

[v2] [unconfirmed] Gatsby doesn't properly delete files on build #7942

Closed

This was referenced Sep 8, 2018

Links not working after period of inactivity [tutorial] #4779

Closed

[v2][gatsby-plugin-offline] Fix offline handling with path-prefixed websites #7969

Merged

[gatsby-plugin-offline] [gatsby/cache-dir] Fix various offline and caching issues #7355

[gatsby-plugin-offline] [gatsby/cache-dir] Fix various offline and caching issues #7355

Conversation

vtenfys commented Aug 15, 2018 • edited Loading

KyleAMathews commented Aug 15, 2018 • edited Loading

KyleAMathews left a comment

Choose a reason for hiding this comment

KyleAMathews Aug 15, 2018

Choose a reason for hiding this comment

gatsbybot commented Aug 15, 2018 • edited Loading

gatsbybot commented Aug 15, 2018 • edited Loading

KyleAMathews commented Aug 15, 2018 • edited Loading

vtenfys commented Aug 15, 2018

KyleAMathews commented Aug 15, 2018

KyleAMathews commented Aug 15, 2018 • edited Loading

vtenfys commented Aug 15, 2018 • edited Loading

wKovacs64 commented Aug 15, 2018

jlengstorf commented Aug 15, 2018

wKovacs64 commented Aug 15, 2018

vtenfys commented Aug 16, 2018

vtenfys commented Aug 16, 2018

KyleAMathews commented Aug 16, 2018

dfmarulanda commented Aug 29, 2018

vtenfys commented Aug 29, 2018

KyleAMathews commented Aug 29, 2018

vtenfys commented Aug 29, 2018

mjoynes-wombat-web commented Aug 30, 2018

vtenfys commented Aug 30, 2018

KyleAMathews commented Aug 30, 2018

vtenfys commented Aug 30, 2018 • edited Loading

vtenfys commented Aug 30, 2018 • edited Loading

KyleAMathews commented Aug 30, 2018

vtenfys commented Aug 30, 2018

dfmarulanda commented Aug 31, 2018

vtenfys commented Aug 31, 2018

vtenfys commented Aug 31, 2018

dfmarulanda commented Aug 31, 2018

vtenfys commented Aug 31, 2018

dfmarulanda commented Aug 31, 2018

dfmarulanda commented Sep 6, 2018

pieh commented Sep 6, 2018

vtenfys commented Sep 6, 2018

vtenfys commented Sep 6, 2018 • edited Loading

dfmarulanda commented Sep 6, 2018 • edited Loading

vtenfys commented Sep 6, 2018

dfmarulanda commented Sep 6, 2018

vtenfys commented Aug 15, 2018 •

edited

Loading

KyleAMathews commented Aug 15, 2018 •

edited

Loading

gatsbybot commented Aug 15, 2018 •

edited

Loading

gatsbybot commented Aug 15, 2018 •

edited

Loading

KyleAMathews commented Aug 15, 2018 •

edited

Loading

KyleAMathews commented Aug 15, 2018 •

edited

Loading

vtenfys commented Aug 15, 2018 •

edited

Loading

vtenfys commented Aug 30, 2018 •

edited

Loading

vtenfys commented Aug 30, 2018 •

edited

Loading

vtenfys commented Sep 6, 2018 •

edited

Loading

dfmarulanda commented Sep 6, 2018 •

edited

Loading