-
Notifications
You must be signed in to change notification settings - Fork 10.3k
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
[v2] [gatsby-plugin-offline] Service Workers throw errors when offline #6212
Comments
hey @fabe thanks for submitting this! I'm actually working on this problem right now! |
Same issue with: https://jason.stallin.gs |
The issue is that the static json data paths aren't getting stored in the service worker. Still not sure why, but that json path contains the pageContext. |
@kkemple Wanna have a look at this? |
@octalmage these were fixed in #7092 |
@KyleAMathews I'm also still having this issue with JSON data, with gatsby-plugin-offline version 2.0.0-beta.9 which contains the fixes in #7092, so I don't think it's actually fixed... |
Just checked and this is indeed still an issue - I forked @fabe's original repo and updated the packages. Link: https://github.com/davidbailey00/test-gatsby-v2-offline / deployed at: https://davidbailey00-test-gatsby-v2-offline.netlify.com/ |
I realised that I only updated package-lock.json and not yarn.lock, so Netlify was still deploying with the outdated packages. However - this is still an issue on multi-page sites, as they may load additional json data when switching pages, which is not cached by default. For example, the Gatsby 2 tutorial site does not work reliably offline - go to https://wizardly-babbage-0faa32.netlify.com/ and browse a few pages, then simulate offline network and reload. Upon visiting some pages, the same error "Cannot read property 'pageContext' of undefined" still sometimes occurs. I think a fix for this would simply be to add json to the array of default runtime cached extensions. @KyleAMathews please let me know if that sounds ok and I can easily submit a pull request, since it's only a one-line change. |
Update: I tried locally modifying gatsby-node.js inside my node_modules, including css and json in the runtime-cached extension list. However, this still didn't solve the problem as I still couldn't reliably load other pages on the site, even after visiting them when online - error 0 was shown in the console when trying to load these files offline. (I checked the generated sw.js to make sure it had updated the changes.) Either I'm misunderstanding how runtime caching works, or it doesn't seem to be working properly, since it isn't caching the json files even when json is added to the urlPattern regex. Edit: even on the official Gatsby v2 website this is broken. Open up the dev tools and simulate offline mode, then click around some pages. On some pages the whole screen will turn white and a bunch of For me the problem is triggered when going to the tutorial and clicking part two, even though I've been to this page before. On the v1 website, there are no such problems. I hope this issue can be reopened since it's clear that there are still some major problems. Additionally, for both the v1 and v2 websites, when offline, only the homepage can be opened directly without showing a "No Internet" error, but other cached pages can be navigated to. It seems odd not to cache all pages so that they can be opened directly, especially when their contents are all cached anyway. Shall I open another bug report for this? |
I've started work on a proper fix for this |
I'm almost ready with the fix but I've found another problem which might have been caused by my changes, so I'm just checking on that before sending a PR. |
I've determined the other problem is not caused by my changes, since it affects pages using the current version. I'll submit a PR shortly along with a new bug report. |
On second thoughts I've decided to hold off on submitting the PR, since my changes don't actually fix the root problem. I had refactored However, the root problem was only masked - given that for a large website it's not feasible to statically-cache all contents, the bug would still occur, assuming the options were changed to skip caching certain files, as soon as anyone visited a non-cached page. This is proving quite difficult to debug, since gatsby-plugin-offline isn't designed to work when running Now that I've familiarised myself somewhat with the codebase I want to have another crack at this tomorrow, although I feel like it might require some more complex changes, possibly to different plugins which gatsby-plugin-offline depends on, and/or the core Gatsby code. N.b. my original changes, which I was going to submit as a PR, can be seen at https://github.com/davidbailey00/gatsby/tree/fix-issue-6212 |
I'm making progress on a better fix, although it's taking me a long time due to various interdependent bugs which I'm also trying to fix. I'll post an update later when I've made more progress |
Thanks, @davidbailey00! Much appreciated! |
Most of the fixes are now done - I'm just trying to find a way to hard refresh when a page isn't found. Otherwise, either we get a blank page from the service worker, rather than a 404 / network error, or we get an infinite loop of blank pages if we do a normal refresh. Unfortunately, sw-precache seems to ignore search queries when matching against |
I've found a way to patch sw.js which allows me to do what I'd described in my last comment. Still some issues which I need to fix before PR'ing but lots of progress has been made. |
Everything fixed except I've uncovered what appears to be bug in React itself, in which className isn't applying the class correctly to the outermost DOM element - but only when loaded from the service worker (i.e. the offline shell) and only in Chrome (not in Firefox, haven't tested Edge etc). Trying to find a workaround for this. |
@davidbailey00 what are you working on exactly? The original issue described here was fixed last week with #7066 The 404 issue was fixed in #7268 It's often fastest to put up small PRs that solve slices of the overall problem. Would love to look at what you have! Going to close this issue now as it's fixed but please open up a new issue describing what you're trying to solve! |
Also, yeah, service workers are only for the built site so you need to debug it (unfortunately) by running gatsby build then gatsby serve to view the results. |
@KyleAMathews this was happening periodically for us on a client site we launched recently, we just updated gatsby yesterday to 2.0.15 and we are hoping that fixed it, still watching our log... but I went to Gatsbyjs.com and it happened there first time I ever went to site, maybe you need to update that site too to use latest v2.. or maybe it isn't fixed. |
@rcaracaus would you be able to open up the inspector and grab the Gatsby version for me?
We made a bunch of improvements to gatsby-plugin-offline (thanks @davidbailey00!), so it's possible you're on a cached version of the site that doesn't contain those improvements, which is unfortunate. Service workers are challenging 😱 |
"Gatsby 2.0.5" -- when I just did that on Gatsbyjs.com -- but I did hard refresh the site when I got the error and it went away... so that might mean if I had visited the site in the past I might have had a different version. Also probably a dumb question but (I am still learning serviceworker and PWA's).. on our site we don't specify gatsby-plugin-offline in our config file, should we be (assuming it is disabled)? Could not using it have caused the the pageContext issue for us? With the recent updates, if we have gatsby-plugin-offline disabled will we even benefit from the changes? Do Gatsby v2 sensible defaults recommend to have that plugin enabled? |
@DSchau ^ |
@rcaracaus not having the |
Ok, so if I set up a demo site on my local. Add the manifest & offline: Then edit gatsby-config.js Uncomment Then run: I should be able to evaluate http://localhost:8000/ with Google's Lighthouse and have 100% for my Progressive Web App settings, right? I'm seeing some failures here that I'm thinking could be fixed out of the box. Maybe I'm wrong about this though. I haven't played with this at all much. |
@mgifford To test Lighthouse scores/service workers, run |
Brilliant.. with that the only thing that doesn't work is "Does not redirect HTTP traffic to HTTPS" which really is an easy enough thing to do in Apache anyways. |
It seems that service workers are broken on v2. They are successfully installed, but return errors when the user is offline:
This also leads to this Lighthouse warning:
Could potentially be related to the
pageContext
rename?Steps to reproduce
Example repo: https://github.com/fabe/test-gatsby-v2-offline
Production build: https://peaceful-wozniak-ecfc89.netlify.com
Reload the site while offline (after installing the service worker).
Environment
The text was updated successfully, but these errors were encountered: