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

Exclude the file passed to write() from being picked up #107

Merged
merged 2 commits into from
Apr 22, 2016

Conversation

jeffposnick
Copy link
Contributor

R: @gauntface @wibblymat @addyosmani
CC: @stuartlangridge

This ensures that a previously generated version of the service worker output file isn't picked up in the list of files to be precached.

Insert "We need to go deeper!" meme here.

Fixes #101

@addyosmani
Copy link
Contributor

Change LGTM. Would be good to get a quick check from Stuart as I think his tweet was in ref to this issue.

@stuartlangridge
Copy link

Works for me!

// Return null if we want to exclude this file, and it will be excluded in
// the subsequent filter().
return excludeFilePath === absolutePath(file) ?
null :

Choose a reason for hiding this comment

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

Is there a reason not to just do this exclude check in the filter?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You save an unneeded fs.statSync() by filtering it out early and avoiding the call to getFileAndSizeAndHashForFile(). That was the main reason.

Choose a reason for hiding this comment

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

Koolio - ta Jeff.

On Thu, 21 Apr 2016, 15:51 Jeffrey Posnick, notifications@github.com
wrote:

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

return glob.sync(globPattern.replace(path.sep, '/')).map(function(file) {

  • return getFileAndSizeAndHashForFile(file);
  • // Return null if we want to exclude this file, and it will be excluded in
  • // the subsequent filter().
  • return excludeFilePath === absolutePath(file) ?
  •  null :
    

You save an unneeded fs.statSync() by filtering it out early and avoiding
the call to getFileAndSizeAndHashForFile(). That was the main reason.


You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
https://github.com/GoogleChrome/sw-precache/pull/107/files/be0bd63f40fea5dcd5efa1cf799fb47d43afebe6#r60594149

@gauntface
Copy link

Lgtm apart from one comment

@addyosmani
Copy link
Contributor

Looks like we have two LGTMs and Matt's comment was responded to :)

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.

None yet

4 participants