Skip to content
This repository has been archived by the owner on Jan 23, 2021. It is now read-only.

Always run sw-precache's fetch handler before sw-toolbox's #131

Merged
merged 2 commits into from
Jun 13, 2016

Conversation

jeffposnick
Copy link
Contributor

R: @addyosmani @gauntface @wibblymat

This reorders the sw-toolbox inlined code, to ensure that its fetch handler only runs if sw-precache's handler does not call event.respondWith().

This addresses an issue we were seeing in Polymer/old-docs-site#1641 and fixes #128 as well.

There's a chance that folks were relying on the previous behavior (though I don't think that's a great use of sw-precache if they were...), so I'll call this change out explicitly in the 4.0 release notes.

@@ -183,7 +183,7 @@ function generate(params, callback) {
if (params.runtimeCaching) {
var pathToSWToolbox = require.resolve('sw-toolbox/sw-toolbox.js');
swToolboxCode = fs.readFileSync(pathToSWToolbox, 'utf8')
.replace('//# sourceMappingURL=sw-toolbox.map.json', '');
.replace('//# sourceMappingURL=./build/sw-toolbox.map.json', '');

Choose a reason for hiding this comment

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

This is at the root on v3.2.1 and looking v3.2.0 the map wasn't added the final bundle

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The version of sw-toolbox.js that gets installed via npm currently has the line //# sourceMappingURL=./build/sw-toolbox.map.json in it.

If that's not intentional then it may need to be updated in a sw-toolbox 3.2.2.

Choose a reason for hiding this comment

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

Yeah that'll be the case. I'll raise a bug.

On Fri, 10 Jun 2016, 19:23 Jeffrey Posnick, notifications@github.com
wrote:

In lib/sw-precache.js
#131 (comment)
:

@@ -183,7 +183,7 @@ function generate(params, callback) {
if (params.runtimeCaching) {
var pathToSWToolbox = require.resolve('sw-toolbox/sw-toolbox.js');
swToolboxCode = fs.readFileSync(pathToSWToolbox, 'utf8')

  •    .replace('//# sourceMappingURL=sw-toolbox.map.json', '');
    
  •    .replace('//# sourceMappingURL=./build/sw-toolbox.map.json', '');
    

The version of sw-toolbox.js that gets installed via npm currently has
the line //# sourceMappingURL=./build/sw-toolbox.map.json in it.

If that's not intentional then it may need to be updated in a sw-toolbox
3.2.2.


You are receiving this because you were assigned.

Reply to this email directly, view it on GitHub
https://github.com/GoogleChrome/sw-precache/pull/131/files/320c709b19231052d64728cc045f956771d08581#r66659215,
or mute the thread
https://github.com/notifications/unsubscribe/AAIh8EYzZkDgkPLQaQUHxXgCm7rdrUa5ks5qKauggaJpZM4IzL2u
.

@gauntface
Copy link

This largely seems ok to me, although difficult to fully appreciate given it looks like template imports to me.

@jeffposnick jeffposnick mentioned this pull request Jun 13, 2016
@jeffposnick
Copy link
Contributor Author

Yes, it's just shuffling around the order of things in the template.

@jeffposnick jeffposnick merged commit 1ca4494 into master Jun 13, 2016
@jeffposnick jeffposnick deleted the delay-sw-toolbox-fetch-handler branch June 13, 2016 14:44
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Clarify routing
4 participants