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.

Sign up for GitHub

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

Jump to bottom

Replace the App Cache with a Service Worker #1022

Merged
merged 25 commits into from
Jul 19, 2019
Merged

Replace the App Cache with a Service Worker #1022

merged 25 commits into from
Jul 19, 2019

Conversation

jmerle
Copy link
Contributor

@jmerle jmerle commented Jul 11, 2019 •
edited
Loading

This PR contains the following changes:

  • Replaced the App Cache with a Service Worker. Everything still functions the same, we just don't rely on the deprecated App Cache anymore. This change bumps the browser version requirements for Edge from 16+ to 17+, for Safari from 9.1+ to 11.1+ and for iOS from 10+ to 11.3+. All of these were released in the beginning of 2018, so this should be fine under our "we support recent versions of modern browsers" policy. This partially implements Switch to a Service Worker #774.
  • Added a 192x192 icon and a background color to the PWA manifest so that DevDocs ticks all the requirements for an installable PWA. Not all browsers support this yet, but most major ones do. There is no explicit notification asking the user to install the PWA because there is no cross-browser way to show one, so users will have to install it manually. On Chrome Mobile and Firefox Mobile the button to install it is pretty obvious and on Chrome Desktop the user has to click on the three dots in the top-right and select "Install DevDocs". Google is working on improving this. This closes Can we make devdocs a chrome extension instead of a chrome app? #557.
  • Removed all server-side styling configuration (dark/light theme, sidebar size and more) and moved it all to the client so that the amount of requests to the server is minimized. The current version of DevDocs needs to re-fetch the index page every time a styling preference is updated because some of the styling is done server-side in the views/*.erb files. This extends on the work done in Fix layout preferences on Firefox #1011, which has already been merged into this PR.

The service worker is enabled by default in production and is opt-in in development. To enable the service worker in development, set the ENABLE_SERVICE_WORKER environment variable to true when running the server. I have updated our Dockerfiles to set this environment variable so that future versions of our Docker images will have the service worker enabled aswell.

These changes have been tested on all our supported browsers:

  • Chrome 75 on Ubuntu 18.04
  • Firefox 67 on Ubuntu 18.04
  • Opera 62 on Ubuntu 18.04
  • Chrome 75 on Android 9
  • Firefox 67 on Android 9
  • Safari on iOS 12.3.1
  • Safari 12.1 on macOS 10.14.4
  • EdgeHTML 18.18362 on Windows 10 version 1903

A live demo is running on sw.devdocs.jmerle.dev.

jmerle added 20 commits May 26, 2019 19:48
@jmerle
Load layout preferences client-side from cookies
35f6f71
@jmerle
Simplify toggleLayout
0c42db5
@jmerle
Simplify layouts array
d63487d
@jmerle
Merge branch 'fix-firefox-preferences' into service-worker
5edbb16
@jmerle
Replace AppCache with a service worker
8ed1f4a
@jmerle
Service worker refactoring
ec8616e
@jmerle
Add preference to bypass service worker cache
0e9aa88
@jmerle
Cache debug.js when not running in production
7af10a0
@jmerle
Refactoring
35dd78f
@jmerle
Remove redundant server-side styling
aaf9904
@jmerle
Move all styling configuration to the client
dd23a6c
@jmerle
Remove redundant server-side rendering
4a088ca
@jmerle
Re-add background color to manifest
0cd1527
@jmerle
Make service worker opt-in in development
576f32d
@jmerle
Update unsupported browser versions
b8a5c6b
@jmerle
Update offline page
27167a4
@jmerle
Update quotes
2bbbc66
@jmerle
Better cache name creation
5dc1437
@jmerle
Formatting
52e982b
@jmerle
Update page load error message
fb2d737
@jmerle jmerle mentioned this pull request Jul 11, 2019
Closed
@jmerle jmerle mentioned this pull request Jul 11, 2019
Closed
j-f1
j-f1 reviewed Jul 11, 2019
View reviewed changes
Copy link
Contributor

@j-f1 j-f1 left a comment

Choose a reason for hiding this comment

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

Thank you for doing this! I’ve got a few suggestions/questions/comments/whatever — nothing PR-breaking. Feel free to ask me to kick some of them into a separate issue since they’re not directly about changes made in this PR.

assets/javascripts/app/serviceworker.coffee Outdated Show resolved Hide resolved
assets/javascripts/app/settings.coffee Outdated Show resolved Hide resolved
assets/javascripts/app/update_checker.coffee Outdated Show resolved Hide resolved
assets/javascripts/templates/error_tmpl.coffee Outdated Show resolved Hide resolved
assets/javascripts/templates/pages/offline_tmpl.coffee Outdated Show resolved Hide resolved
assets/javascripts/app/settings.coffee Outdated Show resolved Hide resolved
assets/javascripts/app/settings.coffee Outdated Show resolved Hide resolved
docs/maintainers.md Outdated Show resolved Hide resolved
lib/app.rb Show resolved Hide resolved
views/service-worker.js.erb Outdated Show resolved Hide resolved
@jmerle
Copy link
Contributor Author

jmerle commented Jul 12, 2019

Thanks for the fast yet thorough review! I've implemented all of your suggestions, lets get this deployed asap.

@jmerle
Copy link
Contributor Author

jmerle commented Jul 12, 2019

The latest commit sets the theme-color based on whether DevDocs is set to the dark theme or not for a better user experience on mobile devices:

I couldn't really do this in a separate PR because it relies on a method added in #1011 (which has already been merged into this PR).

@cglong
Copy link

cglong commented Jul 12, 2019

According to Microsoft's PWABuilder, the demo environment received a 35/40 in the Service Worker category for failing to meet the following criterion:

Service Worker has a pushManager registration

@j-f1
Copy link
Contributor

j-f1 commented Jul 12, 2019

The Push API seems to be mainly a way to send users push notifications. That’s not something we need at the moment, but it might be useful to have an option in the future that prompts users to update their docs when updates become available.

@jmerle jmerle merged commit 6138f05 into freeCodeCamp:master Jul 19, 2019
@sbrl
Copy link

sbrl commented Jul 20, 2019

It works! Thank so much ❤️

@jmerle jmerle deleted the service-worker branch September 4, 2019 08:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@j-f1 j-f1 j-f1 approved these changes

@sbrl sbrl sbrl approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Can we make devdocs a chrome extension instead of a chrome app?
4 participants
@jmerle @cglong @j-f1 @sbrl