sw-build support for templatedUrls #295
Conversation
# Conflicts: # packages/sw-build/src/index.js # packages/sw-build/src/lib/generate-sw.js
There was a problem hiding this comment.
One very minor comment. As far as I can tell there shouldn't be changes needed over in sw-cli to support this, but will allow the honorable @gauntface to confirm for sure.
The rest of this looks fairly straight-forward and implem LGTM as is.
| @@ -2,7 +2,7 @@ const errors = require('./errors'); | |||
| const filterFiles = require('./utils/filter-files'); | |||
| const getCompositeDetails = require('./utils/get-composite-details'); | |||
| const getFileDetails = require('./utils/get-file-details'); | |||
| const getStringDetails = require('./utils/get-string-hash'); | |||
| const getStringDetails = require('./utils/get-string-details'); | |||
There was a problem hiding this comment.
This was changed from get-string-hash -> get-string-details. I found the original name more specific description wise. Although you're generating hash -> updating -> returning digest. Small nit to consider.
There was a problem hiding this comment.
get-string-details and get-string-hash are two distinct modules, and the reference to get-string-hash in the previous commit was a copy/paste error 😦
|
LGTM from my side - only concern was accepting strings - not sure it makes sense vs just adding a custom {url: '', revision: ''} object in the service - for sw-precache this wasn't an option so the linked PR was required. Happy to accept with or without for now - just raise an issue to discuss if you land as is :) Otherwise code +1 |
|
Actually, since this is already reviewed, I'll merge as-is, and opened #300 to discuss whether to remove string support. |
R: @addyosmani @gauntface
Fixes #263
This implements what's basically
sw-precache'sdynamicUrlToDependencies, but with a name that @gauntface agreed on. It also has support for both strings and globs as the value that uniquely determines the URL's contents, so there should be parity with @HenrikJoreteg's changes made in GoogleChromeLabs/sw-precache#262I'm not 100% sure that I've made the changes in all the right places (do I need to touch something in
sw-clitoo?), as I'm not as familiar with this portion of the codebase. Feedback is appreciated.