Pre-cache key assets with Workboxjs. #23533

MikeyBeLike · 2017-08-17T22:44:59Z

First time attempting to contribute, so bare with me please if i've missed anything or done anything wrong.

But I noticed this was merged ( #23101 ) so in addition, thought it would make sense to pre-cache those main assets.

Also not too sure how it works but I think Jekyll is serving/parsing the markdown doc files dynamically. It would be a cool idea if we could maybe get them to static html files? That way we can have offline support with this too!

Screenshot to show speed difference:
https://snag.gy/KzD0Rv.jpg - throttled slow 3G
https://snag.gy/YHsyzB.jpg - throttled slow 3G w/ workboxjs

Also ran grunt test and everything passed fine.

XhmikosR · 2017-08-17T23:58:08Z

Jekyll builds the md files to html.

Please make sure we are not caching useless stuff.

XhmikosR · 2017-08-17T23:59:01Z

sw-src.js

@@ -0,0 +1,7 @@
+'use strict';


Why do we need this file? Can't you do what you want in sw.js?

We can, but this serves as a template to generate the main sw.js file

XhmikosR · 2017-08-17T23:59:33Z

sw.js

@@ -1 +1,156 @@
-// empty for now


If this file is generated, then it should be removed now.

And output the generated sw.js in _gh_pages folder.

XhmikosR · 2017-08-18T00:08:52Z

Then see my comment about the dist folder.

…

On Aug 18, 2017 03:06, "Michael" ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In sw-src.js <#23533 (comment)>: > @@ -0,0 +1,7 @@ +'use strict'; We can, but this serves as a template to generate the main sw.js file — You are receiving this because you were assigned. Reply to this email directly, view it on GitHub <#23533 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAVVtb-dJDwr7rj1VlHsNshVrC_yvTjhks5sZNWMgaJpZM4O65qg> .

MikeyBeLike · 2017-08-18T00:18:56Z

Should be all good now?

XhmikosR · 2017-08-18T15:11:33Z

build/workbox.config.json

+        "assets/**/*.{js,html,svg,png,css,jpg,ico}",
+        "dist/**/*.{js,html,svg,png,css,jpg,ico}"
+    ],
+    "swSrc": "sw-src.js",


How about just naming this sw.js? We output the generated file in the _gh_pages folder so we don't have any conflicts.

makes more sense, will update

XhmikosR · 2017-08-18T15:12:09Z

sw-src.js

@@ -0,0 +1,6 @@
+importScripts('https://unpkg.com/workbox-sw@1.3.0');


Why do you use unpkg and not a local devDependency?

that makes more sense actually, working on it.

XhmikosR · 2017-08-18T15:13:50Z

package.json

@@ -52,7 +52,8 @@
    "test": "npm-run-all dist js-test docs",
    "watch": "npm-run-all --parallel watch-css watch-js",
    "watch-css": "nodemon --ignore js/ --ignore dist/ -e scss -x \"npm run css && npm run css-docs\"",
-    "watch-js": "nodemon --ignore scss/ --ignore js/dist/ --ignore dist/ -e js -x \"npm run js-compile-plugins\""
+    "watch-js": "nodemon --ignore scss/ --ignore js/dist/ --ignore dist/ -e js -x \"npm run js-compile-plugins\"",
+    "workbox-precache": "workbox-cli inject:manifest --configFile=\"build/workbox.config.json\""


We need to hook this script in the other scripts, say docs. And also rename it to docs-workbox-precache.

It fails on travis due to incompatibility with node 4, so I had to remove it from the docs script to pass, but i've put in a pull request to fix it though, so waiting on that for now

XhmikosR · 2017-08-18T15:31:28Z

@Lyricalz: apart from the previous stuff I commented, you need to update package-lock.json too after you fetch and rebase from v4-dev.

XhmikosR · 2017-08-18T15:34:36Z

sw-src.js

@@ -0,0 +1,6 @@
+importScripts('https://unpkg.com/workbox-sw@1.3.0');
+
+const VERSION = 1;


Wouldn't it make sense to use the version from package.json + another variable?

yeah that's a good idea, since this is linked to the docs, are there any vars that are updated whenever the docs are rebuilt?

MikeyBeLike · 2017-08-19T12:43:24Z

Made some updates
-removed workbox cli, now using workbox build directly
-included generated html to be pre-cached too so the docs are now fully available offline
-removed version const, turns out that serviceworkers are prompted to update when there's a byte difference, the revision hashes take care of this automatically
-automatically copies local workbox-sw library in case of any version changes
-renamed npm script also added it to docs-compile script
-set page to reload automatically when there's a service worker update

Note: build still hasn't passed on node 4 but once the workbox-team have merged & released i'll update to that version and it should pass then

XhmikosR · 2017-08-19T22:07:00Z

build/workbox.js

+  }
+})
+
+config.manifestTransforms = [mdToHtml]


Why do we need workbox to do MD -> HTML?

The _gh_pages folder already has the html files.

see comment below

XhmikosR · 2017-08-19T22:07:36Z

build/workbox.config.json

+    "globPatterns": [
+        "assets/**/*.{js,svg,png,css}",
+        "dist/**/*.{js,css}",
+        "docs/**/*.{md,html,css,jpg,js}"


We shouldn't cache md files.

Also, this seems to generic. You should change the whole config to work against the _gh_pages folder which only has the needed assets and not the src stuff.

Okay so I added a transformer for this as if I specified the _gh_pages folder, workbox will attempt to pre-cache an url like.. http://getbootstrap.com/gh_pages/docs/4.0/, instead of http://getbootstrap.com/docs/4.0/. The transformer basically renames the .md files to their correct .html files but keeps the docs/ structure intact instead of _gh_pages/docs/

XhmikosR · 2017-08-19T22:09:28Z

build/workbox.config.json

+    "templatedUrls": {
+      "/": ["_gh_pages/docs/index.html"],
+      "docs": ["_gh_pages/docs/index.html"],
+      "docs/4.0/": ["_gh_pages/docs/4.0/index.html"] 


Why do we need this? If we cache the whole _gh_pages folder, doesn't workbox take care of this for us?

So 'templatedUrls' is just a way to redirect urls to their relative sources. Since the docs uses /docs/index instead.html of _gh_pages/docs/index.html and _gh_pages/* doesn't actually exist via 'http://getbootstrap.com/gh_pages/*', we're basically saying get the source contents of _gh_pages/docs/index.html and the cache the contents of it as if you're visiting the url '/'.

XhmikosR · 2017-08-19T22:10:13Z

build/workbox.js

@@ -0,0 +1,28 @@
+'use strict'
+
+var fse = require('fs-extra')


You don't list fs-extra in devDependencies? BTW can't we just use fs?

Oh, also, you could/should ES6-ify this file.

XhmikosR · 2017-08-19T22:10:47Z

@Lyricalz : thanks for your patience, we'll get there :)

MikeyBeLike · 2017-08-19T23:04:48Z

@XhmikosR actually I wasn't thinking right about the md-html thing, should just go straight to the _gh_pages as you suggested, just updated and moved back to just normal fs instead. I think we've got it now, i've also updated the transformer to just remove the '_gh_pages' in the url

MikeyBeLike · 2017-09-09T10:17:08Z

@XhmikosR I've just rebased and upgraded workbox-build to 2.0.1 which includes your backslash fix, we ready to merge? 😄

XhmikosR · 2017-09-13T14:50:46Z

@Lyricalz: I rebased the branch, please make sure you don't overwrite my changes.

Now, 3 final issues, I promise :)

We get some warnings from workbox:

C:\Users\xmr\Desktop\bootstrap>node -v && npm -v
v8.5.0
5.3.0

C:\Users\xmr\Desktop\bootstrap>npm run docs-compile

> bootstrap@4.0.0-beta docs-compile C:\Users\xmr\Desktop\bootstrap
> bundle exec jekyll build && npm run docs-workbox-precache

Configuration file: C:/Users/xmr/Desktop/bootstrap/_config.yml
            Source: .
       Destination: ./_gh_pages
 Incremental build: disabled. Enable with --incremental
      Generating...
                    done in 4.297 seconds.
 Auto-regeneration: disabled. Use --watch to enable.

> bootstrap@4.0.0-beta docs-workbox-precache C:\Users\xmr\Desktop\bootstrap
> node build/workbox.js

(node:3884) [DEP0013] DeprecationWarning: Calling an asynchronous function without callback is deprecated.
(node:3884) UnhandledPromiseRejectionWarning: Unhandled promise rejection (rejection id: 1): TypeError: Cannot read property 'replace' of undefined
(node:3884) [DEP0018] DeprecationWarning: Unhandled promise rejections are deprecated. In the future, promise rejections that are not handled will terminate the Node.js process with a non-zero exit code.

Not sure how to make our scripts generate the workbox stuff so that we can test things without us having to run 2 separate cmds simultaneously. Currently I have to do bundle exec jekyll s and npm run docs-workbox-precache in two separate cmd windows
I'm not sure how to debug this being that the service worker isn't being registered locally without https.

MikeyBeLike · 2017-09-13T23:43:21Z

@XhmikosR i've updated 1) to get rid of this error
not sure about 2) but doesn't the script in docs-compile take care of this?
3) as far as I know localhost/127.0.0.1 is considered secure origin, so your service worker should still register, unless i'm missing something here

XhmikosR · 2017-09-14T07:21:14Z

@Lyricalz:

The error I was getting must be because the service worker wasn't properly being registered without your last patch :)

Now I can navigate the docs offline.
re: docs-compile just compiles everything; it doesn't run the local server.

[2017-09-14 10:14:31] ERROR '/assets/js/vendor/workbox-sw.prod.v2.0.1.js.map' not found. Maybe we should disable map for workbox or output it
I added *.json files too in precache otherwise manifest.json wasn't cached. Not sure if this is the right thing to do...
And the favicon not being cached for some weird reason

I think we are almost done :)

@gauntface: do you have any idea why we get this for the favicons?

:9001/assets/img/favicons/favicon-32x32.png:1 GET http://localhost:9001/assets/img/favicons/favicon-32x32.png net::ERR_INTERNET_DISCONNECTED
:9001/assets/img/favicons/favicon-16x16.png:1 GET http://localhost:9001/assets/img/favicons/favicon-16x16.png net::ERR_INTERNET_DISCONNECTED

I see sw.js has them so something else must be going on.

MikeyBeLike · 2017-09-14T08:13:06Z

@XhmikosR

was thinking about this last night, i'll add the map file later this evening it should just be a matter of appending .map to the the workbox path and copying it too
yes, that's the right thing to do!
I think the favicon issue is independent of this as even viewing the file directly & outside of the docs produces this strange error: https://snag.gy/1a6DvW.jpg

MikeyBeLike · 2017-09-14T08:20:14Z

Also re: docs-serve i'm assuming this command re-builds & runs a local server with that one command? If so, I guess we won't be able to generate the manifest for pre-caching before the local server starts. Is that something that we need to address? i've been just running docs-workbox-precache after docs-serve, which isn't a big deal.. But yeah, would be nice if we could get the serve to build docs, build pre-cache manifest & then run server

addyosmani · 2017-09-14T17:56:59Z

build/workbox.config.json

@@ -1,7 +1,7 @@
 {
  "globDirectory": "./",
  "globPatterns": [
-    "_gh_pages/**/*.{html,css,js,png,jpg}"
+    "_gh_pages/**/*.{html,css,js,json,png,jpg}"


With respect to the favicon issue you're seeing, is it possible to view your current build in Chrome Canary and take a screenshot of what is in Application Panel -> Cache Storage? Favicon issues like this are often due to the asset either not existing in the final build, the globPattern not being sufficient (don't believe this is an issue here) or something else.

@addyosmani: I see the favicons present in the workbox cache:

And thanks for the reply :)

XhmikosR · 2017-09-15T10:38:33Z

Thanks to everyone for the help and ofc @Lyricalz for your patience!

I'll keep an eye for any regressions or fixes for the favicon issue.

MikeyBeLike · 2017-09-15T10:43:03Z

Awesome!

gauntface · 2017-09-15T15:05:39Z

Just a cheeky FYI, if you have any further issues, feature requests or feedback for Workbox please raise an issue (thanks for you help fixing the windows bug ;) ).

MikeyBeLike · 2017-09-15T15:40:34Z

@gauntface would you mind reviewing this file please? Would you say that is the recommended way to prompt an update with service workers?

XhmikosR · 2017-09-15T15:44:52Z

@Lyricalz: another thing I noticed is that if you visit the homepage, go offline, then the docs page isn't cached. I think is something with the page being redirected.

MikeyBeLike · 2017-09-15T16:31:57Z

@XhmikosR I noticed this too, but that's because the live link is hardcoded in the redirect, rather than the local url. I was thinking about making a PR, to make these links relative, rather than using absolute URLs.

When you click documentation it actually goes here: docs/4.0/ then redirects to docs/4.0/getting-started/introduction/ where the actual source is, looking at the source - view-source:https://getbootstrap.com/docs/4.0/ you can see the redirects are absolute URLs so when your offline, it's still attempting to redirect you to the online docs

XhmikosR · 2017-09-15T22:49:05Z

@Lyricalz: we use jekyll-redirect-from for the redirects so I'm not sure if it's even possible since I haven't looked at it. If you make a PR please CC me.

gauntface · 2017-09-25T16:20:03Z

@Lyricalz that looks fine to me.

Workbox-sw does provide an alternative way to listen for a broadcast message when assets are updated: https://github.com/GoogleChromeLabs/workbox-microsite/blob/master/src/themes/scripts/sw-controller.js#L18

XhmikosR self-assigned this Aug 17, 2017

MikeyBeLike force-pushed the service-worker branch from 196a5bd to 11c0af1 Compare August 17, 2017 23:57

XhmikosR reviewed Aug 17, 2017

View reviewed changes

MikeyBeLike force-pushed the service-worker branch from 11c0af1 to 7c2b2eb Compare August 18, 2017 00:00

MikeyBeLike force-pushed the service-worker branch from 7c2b2eb to 1590326 Compare August 18, 2017 00:13

Johann-S added docs js v4 labels Aug 18, 2017

XhmikosR reviewed Aug 18, 2017

View reviewed changes

MikeyBeLike force-pushed the service-worker branch 3 times, most recently from 4b99f70 to 286b5ef Compare August 19, 2017 12:34

MikeyBeLike force-pushed the service-worker branch from 286b5ef to 8c5f98f Compare August 19, 2017 13:33

XhmikosR reviewed Aug 19, 2017

View reviewed changes

MikeyBeLike force-pushed the service-worker branch from 8c5f98f to b54fbfc Compare August 19, 2017 23:04

MikeyBeLike force-pushed the service-worker branch from b54fbfc to 756ea5a Compare August 19, 2017 23:07

addyosmani mentioned this pull request Sep 1, 2017

Fix backslashes replacement. GoogleChrome/workbox#774

Merged

XhmikosR force-pushed the service-worker branch 2 times, most recently from 933d7f2 to 04bab42 Compare September 6, 2017 12:12

MikeyBeLike force-pushed the service-worker branch from aa3e86b to 07dccca Compare September 9, 2017 10:09

XhmikosR force-pushed the service-worker branch 2 times, most recently from 063d3f9 to e8c8b76 Compare September 13, 2017 14:40

MikeyBeLike force-pushed the service-worker branch 2 times, most recently from 86a6c04 to c1273bf Compare September 13, 2017 23:51

addyosmani reviewed Sep 14, 2017

View reviewed changes

XhmikosR force-pushed the service-worker branch from 6821a78 to 53e448c Compare September 15, 2017 10:32

Pre-cache key assets with Workboxjs.

fa03833

XhmikosR force-pushed the service-worker branch from 53e448c to fa03833 Compare September 15, 2017 10:34

XhmikosR approved these changes Sep 15, 2017

View reviewed changes

XhmikosR merged commit 5951508 into twbs:v4-dev Sep 15, 2017

mdo mentioned this pull request Sep 15, 2017

Beta 2 ship list #23278

Closed

mdo mentioned this pull request Jul 25, 2018

Remove service worker #26958

Merged

1 task

		@@ -0,0 +1,6 @@
		importScripts('https://unpkg.com/workbox-sw@1.3.0');

		@@ -0,0 +1,6 @@
		importScripts('https://unpkg.com/workbox-sw@1.3.0');

		const VERSION = 1;

Pre-cache key assets with Workboxjs. #23533

Pre-cache key assets with Workboxjs. #23533

Conversation

MikeyBeLike commented Aug 17, 2017 • edited Loading

XhmikosR commented Aug 17, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

XhmikosR Aug 17, 2017 • edited Loading

Choose a reason for hiding this comment

XhmikosR commented Aug 18, 2017 via email

MikeyBeLike commented Aug 18, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MikeyBeLike Aug 18, 2017 • edited Loading

Choose a reason for hiding this comment

XhmikosR commented Aug 18, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MikeyBeLike commented Aug 19, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

XhmikosR commented Aug 19, 2017

MikeyBeLike commented Aug 19, 2017 • edited Loading

MikeyBeLike commented Sep 9, 2017

XhmikosR commented Sep 13, 2017 • edited Loading

MikeyBeLike commented Sep 13, 2017

XhmikosR commented Sep 14, 2017 • edited Loading

MikeyBeLike commented Sep 14, 2017 • edited Loading

MikeyBeLike commented Sep 14, 2017

Choose a reason for hiding this comment

XhmikosR Sep 14, 2017 • edited Loading

Choose a reason for hiding this comment

XhmikosR commented Sep 15, 2017

MikeyBeLike commented Sep 15, 2017

gauntface commented Sep 15, 2017

MikeyBeLike commented Sep 15, 2017

XhmikosR commented Sep 15, 2017

MikeyBeLike commented Sep 15, 2017 • edited Loading

XhmikosR commented Sep 15, 2017

gauntface commented Sep 25, 2017

MikeyBeLike commented Aug 17, 2017 •

edited

Loading

XhmikosR Aug 17, 2017 •

edited

Loading

MikeyBeLike Aug 18, 2017 •

edited

Loading

MikeyBeLike commented Aug 19, 2017 •

edited

Loading

XhmikosR commented Sep 13, 2017 •

edited

Loading

XhmikosR commented Sep 14, 2017 •

edited

Loading

MikeyBeLike commented Sep 14, 2017 •

edited

Loading

XhmikosR Sep 14, 2017 •

edited

Loading

MikeyBeLike commented Sep 15, 2017 •

edited

Loading