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

feat(v2): Plugin for Offline/PWA support #2205

Merged
merged 52 commits into from
Jul 8, 2020
Merged

feat(v2): Plugin for Offline/PWA support #2205

merged 52 commits into from
Jul 8, 2020

Conversation

codemonkey800
Copy link
Contributor

@codemonkey800 codemonkey800 commented Jan 11, 2020

Changes

  • Implements @docusaurus/plugin-pwa
  • Add popup for notifying users of updated website content
  • Adds @docusaurus/plugin-pwa to v2 website, adding offline/PWA support

Motivation

Implements offline support as requested in #458. Having offline support is beneficial for a dev that needs documentation to work on a project where internet is slow or inaccessible (i.e. airplanes, trains, etc.)

Have you read the Contributing Guidelines on pull requests?

Yes 😄

Test Plan

I've manually tested offline support in the browser and as an installed app. I also ran a lighthouse audit.

Loading Offline

load-offline

Installing as a Chrome App

install-app

Open app in chrome://apps

load-app

Lighthouse Audit

image

Redirect HTTP to HTTPs is server dependent so it's checked off in the audit.

Update Popup

update-popup

Related PRs

@facebook-github-bot
Copy link
Contributor

Hi codemonkey800! Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file.In order for us to review and merge your code, please sign at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need to sign the corporate CLA.

If you have received this in error or have any questions, please contact us at cla@fb.com. Thanks!

@docusaurus-bot
Copy link
Contributor

docusaurus-bot commented Jan 11, 2020

Deploy preview for docusaurus-2 ready!

Built with commit 0e0b219

https://deploy-preview-2205--docusaurus-2.netlify.app

@facebook-github-bot facebook-github-bot added the CLA Signed Signed Facebook CLA label Jan 11, 2020
@facebook-github-bot
Copy link
Contributor

Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks!

@bravo-kernel
Copy link
Contributor

This is pretty amazing, I recently tested oflline and noticed the site went down so this is ❤️

@nebrelbug
Copy link
Contributor

@codemonkey800 wow, I love this! One possible idea is to implement something like VuePress's plugin (https://vuepress.vuejs.org/plugin/official/plugin-pwa.html) that creates a popup when content has been refreshed.

@yangshun
Copy link
Contributor

This is such amazing and great work from the community. We're shorthanded now and I'll only have time to review next weekend though. Thank you from the bottom of my heart ❤️

Maybe @lex111 can take a stab at reviewing it too.

@codemonkey800
Copy link
Contributor Author

@nebrelbug great suggestion :) I'd love to add something like this, I'll see if it's something I can get around to implementing. I mostly wanted to get a foundation for offline/PWA functionality first

@lex111
Copy link
Contributor

lex111 commented Jan 17, 2020

@codemonkey800 This is HUGE and the long-awaited feature! Thanks a lot!

I have no experience in PWA with Workbox, but how does the mobile application update when the documentation is updated? I also noticed that pictures from FB are not showed in offline mode, is this the way it should be?
Can we also fix Lighthouse audit errors?

@codemonkey800
Copy link
Contributor Author

codemonkey800 commented Jan 17, 2020

@lex111 thank you!

Updating

I need to verify this myself, but updating shouldn't be a problem when the documentation is updated. First off, the service worker URL should never change between application builds. You can read more about it here, but basically changing the URL will cause the outdated service worker to load if you cache your index.html.

Now since the service worker URL will always point to the up-to-date service worker, whenever we update the documentation site, a new precache manifest will be generated at the top of the service worker for each build using the InjectManifest plugin:

importScripts('/precache-manifest-server.[hash].js');
importScripts(
  '/precache-manifest-client.[hash].js',
  '/workbox-v[version]/workbox-sw.js',
);

The [hash] will be unique between each build, so we can guarantee that the website will be updated by the service worker using the updated precache manifest. What I do need to check though is when the precache manifest is updated, does the service worker:

  1. Show the old content and update the cache or
  2. Update the cache and then show the new content.

FB Images

The FB images (or more generally images from Markdown posts using authorImageUrl) are not precached like the Webpack assets because we're not processing them through Webpack. Precaching only works through Webpack because of the precache list.

A requirement for the precache list to work is that the resource URL has a way to identify different revisions (like a hash in the URL). Since the FB images aren't processed through Webpack and generated with a unique hash, we can't add them to the precache list.

So instead, I added FB images as Workbox routes so that when you load it the first time over the network, it'll be cached by the service worker. But if you install the app offline before loading the FB images, they won't show up.

I think one way we can fix this is by scanning all authorImageUrls, attach a revision (maybe overall build hash) to it, and add it to the precache manifest.

Lighthouse Errors

Yeah 😆

So these lighthouse errors are because some tags are missing for iOS and Android. Normally webpack-pwa-manifest would have generated all those tags for us, but because we don't use HtmlWebpackPlugin, they don't get injected in production.

I think what I can do here instead is add those tags manually instead of having webpack-pwa-manifest do it for me.

@codemonkey800
Copy link
Contributor Author

Just wanted to let y'all know I'll be a bit slow to responding to feedback and addressing it in my PR due to studying + work. I'm hoping we can get this merged eventually though 😄

@lex111 lex111 added the pr: new feature This PR adds a new API or behavior. label Jan 24, 2020
@lex111
Copy link
Contributor

lex111 commented Feb 24, 2020

@codemonkey800 how goes it? I am wondering if we can refuse to use external dependency (Workbox) for SW? For example, how is this done in VuePress?

@codemonkey800
Copy link
Contributor Author

@lex111 i've been swamped with work but I'll push my remaining changes either tonight or tomorrow. It includes the PWA popup to reload the page and processes the service worker in Webpack.

It is definitely possible to make this work without Workbox. The main things we use Workbox for is:

  • Generating service worker manifest
  • Using Workbox strategies to cache things on the fly
  • Handling when to show the popup

VuePress also uses workbox, specifically the generateSW function: https://git.io/JvEPw

@lex111
Copy link
Contributor

lex111 commented Feb 26, 2020

@codemonkey800 I will check it soon, will I get recommendations on how to correctly check PWA? What could be the negative consequences of this?

@codemonkey800
Copy link
Contributor Author

@lex111 You could run PWA Lighthouse audits in your dev tools on the preview website. You can also refer to the PWA checklist: https://web.dev/pwa-checklist/

To check the update popup, you'll need to:

  1. Run yarn build in website/ and run an http server. This is to register the initial service worker
  2. Make changes to website/, like update a markdown file somewhere
  3. Run yarn build again and run an http server. If you look in Chrome Dev Tools on the Application tab, you should see a second service worker being installed, eventually moving in the waiting stage, and after that the popup should show

Some possible negatives I can think of are:

  1. Normal browser reloads will not update the website anymore. Either a hard refresh or clicking Refresh on the popup will update the contents.
  2. That said, if a developer introduces a bug that prevents rendering of the popup dialog, the user may not be able to update the website without doing a hard refresh. For example, consider this scenario:
  • User visits docusuaurs site for the first time and the service worker gets registered
  • Developer introduces bug that causes renderPopup() to fail
  • User visits docusuaurs site again. Because their current cached renderPopup() works, it'll show the popup indicating that there's a new version of the website
  • Developer updates markdown or something
  • The update popup never shows because the renderPopup() function is buggy, so the user stays stuck with the cached version of the docusuaurs site until they hard refresh the page

There are definitely tradeoffs when making a PWA, but I think it should be up to the discretion of the user if they want to add it. However, whether we add it to the docusuaurs site itself is up for debate 😅

@lex111
Copy link
Contributor

lex111 commented Feb 26, 2020

@codemonkey800 thanks for detail input. Hmm, maybe it makes sense to activate PWA only in certain cases? For example, if the user explicitly install the site as an application? If the user visited the site with mobile?

I described these cases in this pseudocode:

let forceOffline = location.search.includes('offline');

function initSW() {
 navigator.serviceWorker.register('/sw.js')
}

function isSaveDate() {
  let saveData = navigator.connection && navigator.connection.saveData
  return window.innerWidth <= 940 || saveData
}

if ((forceOffline || !isSaveDate()) && navigator.serviceWorker) {
  // init SW if the user turned on save data mode or they just went to a page from a mobile OR `force` option exists (for testing purposes)
  // See https://developers.google.com/web/fundamentals/performance/optimizing-content-efficiency/save-data/ for details
} else if (localStorage.appinstalled) {
  // init SW if user installed site as an app and refresh a page
  } else {
  window.addEventListener('appinstalled', () => {
    // init SW if installed site as an app
    
    localStorage.appinstalled = true;
  });
}

What do you think?

@codemonkey800
Copy link
Contributor Author

@lex111 For sure, I think that sounds like a good idea. How about we make this the default behavior and add an option like hybrid that defaults to true. If hybrid is false, then make the app entirely PWA without the above cases for users who want a fully PWA doc site

I will need to research and see if it's possible to install an app and run the service worker after because usually it's the other way around

@lex111
Copy link
Contributor

lex111 commented Feb 26, 2020

@codemonkey800 I’m not sure exactly whether this option is needed, because in my view PWA + Offline is only needed for certain cases (mobile devices, or if the site is implicitly using it as an application). I updated example above to show the init SW, isn’t this the case in reality?

@codemonkey800
Copy link
Contributor Author

codemonkey800 commented Feb 26, 2020

@lex111 Oh okay never mind I think I understand now. One thing I'm wondering is how should we let the user force offline mode? Aside from adding /#offline /?offline directly to the URL?

@lex111
Copy link
Contributor

lex111 commented Feb 26, 2020

@codemonkey800 yep, something like "https://v2.docusaurus.io/?offline" (location.search.includes('offline')). This will be needed for testing, and besides this, in the future I want to add Lighthouse check to CI.

@nebrelbug
Copy link
Contributor

@codemonkey800 will there be a way to specify the Workbox caching strategy in the docusaurus.config.js? Ex. strategy: 'stale-while-revalidate' or strategy: 'network-only'

https://developers.google.com/web/tools/workbox/modules/workbox-strategies

@codemonkey800
Copy link
Contributor Author

@nebrelbug There's currently no way to specify the strategy for the precache manifest unfortunately. See GoogleChrome/workbox#1767

However, you can set up your own runtime strategies using the custom service worker but it won't be added to the cache at the very start like resources in the precache manifest

@codemonkey800
Copy link
Contributor Author

codemonkey800 commented Feb 27, 2020

@lex111 In order for the app install button to show, there needs to be a running service worker, so we can't do registration after 😢

image

An app will also not be installable unless it's available offline:

image

@lex111
Copy link
Contributor

lex111 commented Feb 28, 2020

@codemonkey800 too bad, are there any options to get around this? For example, check the conditions for the use of PWA in the service worker?

@codemonkey800
Copy link
Contributor Author

@lex111 We could possibly pass an empty array into precacheAndRoute() to make the browser think it has offline support. Since the array is empty, every resource will be downloaded over the network.

Then when the user installs the app, we can emit a CACHE_FILES message to the service worker so it can precache the actual manifest.

An issue I see here is if the user uninstalls the app, I don't think there's currently a way to detect that, so the service worker will continue to precache files. Is this okay?

@slorber
Copy link
Collaborator

slorber commented Jul 7, 2020

Hey @codemonkey800 , I think this PR is ready. If you have some time to review my changes, so that I can merge it asap, that would be great.

Otherwise I'll merge it tomorrow. As it's a plugin, and we are in alpha, we think it's better to get it merged, and see if we have some feedbacks and bug reports to improve it.

Major changes:

  • custom babel/webpack setup more adapted for SW code
  • debug mode (custom logs + workbox logs)
  • configurable offline mode / precaching activation strategies + sensible defaults
  • better QS params passing between registerSW and SW
  • use default browser ui for app installation
  • minor changes (refactors, renamings, docs...)

Overall your code worked great, thanks for your work.

Doc: https://deploy-preview-2205--docusaurus-2.netlify.app/docs/next/using-plugins/#docusaurusplugin-pwa

@codemonkey800
Copy link
Contributor Author

@slorber wow, this is amazing work 😄 I'll take a look at all the code today, but from what I've seen it's looking very good. thank you so much for the help

@codemonkey800
Copy link
Contributor Author

@slorber I finished reviewing the new code. I love all the new changes you added, it makes the PWA a lot more robust 😄

@slorber
Copy link
Collaborator

slorber commented Jul 8, 2020

awesome @codemonkey800 , let's merge this finally! 🎉

@slorber slorber merged commit 9b3da59 into facebook:master Jul 8, 2020
@yangshun
Copy link
Contributor

yangshun commented Jul 8, 2020

Finally! Thanks @codemonkey800 @slorber @lex111!

@codemonkey800
Copy link
Contributor Author

Thank you @slorber and @lex111 with all the help in reviewing this and finally getting it merged. I couldn't have done it without y'all 😃

@hobadams
Copy link
Contributor

hobadams commented Jul 9, 2020

Nice work guys! Can't wait for the release :-) @codemonkey800 @slorber @lex111

@slorber
Copy link
Collaborator

slorber commented Jul 24, 2020

Hey all, this is feature is now released!
If you like it, don't forget to retweet 😄
https://twitter.com/docusaurus/status/1286715187983048704

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed Signed Facebook CLA pr: new feature This PR adds a new API or behavior.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet