General Feedback

[gauntface]

• Error cases just throw errors, there is no helpful error messages before the problem is reached.
  • Examples
    • dynamicUrlToDependencies, I first just gave it an array assuming it would take an array of urls, this causes an error when a sort method it called
    • The tool was struggling to find some file somewhere and I get: Error: ENOENT: no such file or directory, stat 'server/app-shell.handlebars'
• dynamicUrlToDependencies just feels a bit bluergh to me compared to vanilla SW JS where you say here's the url, cache it. This is much more involved and not sure of the benefit.
• I have no affirmation of what has been cached. I was partly hoping that the generated sw.js file would be fairly clean to parse and see what files are cached, but the following threw me off:
  • it's heavy on eslint comments (Not sure why)
  • the array of files isn't formatted particularly friendly for quick parsing (i.e. no new lines and the file name plus hash is a little weird).
  • functions are first in the doc rather than the sw event listeners which is a shame, kind of hides SW from first glance of the file
• Would be nice of the options had examples of the input. When I started using them I felt like I had to guess what was expected (which is both before and after reading the docs - I'm a bad person I know).
• Caching mechanisms are basically non-existent in this scenario, it seems like it's always cache first and network if it's not cached.
• Last bit of feedback which is a bit more open ended. I feel this is all a bit too divorced from SW for me to be able to "use" or "change" swprecache. In an ideal world I'd just have the gulp plugin that would find my SW file, inject the files I globbed for, and then inside the SW file I'd have a thirdparty script that would basically give me lots of helper methods to manage a fetch request. For example, navigateFallback didn't work at first and I thought it was set up correctly, but in terms of debugging, I had to guess what was wrong and try and fix it. Flip side of this is that when it does work its awesome.

[jeffposnick]

Very much appreciate the fresh pair of eyes you're providing here, @gauntface. I'm roughly breaking up my response to correspond to each of your high-level points.

• See Friendlier error messages for two types of bad parameters. #45 for a discussion around bad parameters/friendlier errors.
• There's room for improvement in the docs, especially around positioning what the library is and isn't trying to do and how it fits in with other libraries. More inline examples is another good request. @jpmedley has graciously volunteered to help with a doc overhaul, so I'm looping him in here.
• The default logging used to be more noisy, and include information about exactly what was being precached. I turned that off in 26d7328 at the request of the Web Starter Kit folks. You can turn it on via verbose.
• Re: the necessity of dynamicUrlToDependencies, here's the use case (and again, this could be explained better in the doc): you have a URL whose contents depend on several different partials or inlined resources. In order to properly detect when that URL's contents have changed in the SW, you need to create a unique hash that represents the union of the contents of all the dependencies. The fundamental point of this library is that you can't safely serve resources cache-first from a SW unless you know when to expire the previously cached entry due to underlying changes. Given that explanation, is it clearer why that option exists and how just passing a single URL in to cache.add() at runtime isn't sufficient?
• Re: the generated SW file, it's not optimized for readability. It's genereated via _.template and it does things like serialize arrays and functions in ways that don't follow the style conventions used elsewhere (hence the eslint ignores). The functions in the file are just as important to the SW as the event listeners, so I'm not sure that whether they're injected into the top or bottom of the file. And regarding the paths + hashnames that are written out, that's a critical part of the functionality of the library, so weirdness aside, they need to stay as-is. This file is effectively the output of an automated process that shouldn't be edited, so I don't think this amounts to a significant problem.
• The point of the library is to implement a cache-first strategy for local resources in the most efficient manner possible. The library can be used in conjunction with additional service worker logic to handle runtime caching via different scenarios. Positioning this distinction is tricky ("Why do we need sw-precache and sw-toolbox?) and I've tried to explain the situation in articles, but this is definitely an area that I would appreciate @jpmedley's help with in the docs revamp.
• sw-precache assumes that it's going to generate the SW script file for you, and that you will extend the functionality as needed via importScripts(). Generating the SW script means that sw-precache gets to control when the SW update cycle kicks in due to changes to the precached resources—each change will generate a script file with a different set of inlined hashes, giving the install and activate handlers a chances to manage the caches. Given the degree of experience you have working with SWs already, I can understand how providing your own SW file and expected sw-precache to just hook into that would appeal to you, but I don't think the majority of the users will either have an existing SW script, and this approach works well for them. The sample that I'm working on the app-shell-demo branch might help clarify how more experienced developers, or developers who have runtime caching needs, can extend the SW script that sw-precache generates. Ditto for improved docs.

[gauntface]

Regarding:

Re: the necessity of dynamicUrlToDependencies, here's the use case (and again, this could be
explained better in the doc): you have a URL whose contents depend on several different partials or
inlined resources. In order to properly detect when that URL's contents have changed in the SW, you
need to create a unique hash that represents the union of the contents of all the dependencies. The
fundamental point of this library is that you can't safely serve resources cache-first from a SW unless
you know when to expire the previously cached entry due to underlying changes. Given that
explanation, is it clearer why that option exists and how just passing a single URL in to cache.add() at
runtime isn't sufficient?

I understand what its doing and how it achieves it, but I now have a really tight dependency on what the server is doing and my build process which doesn't exist otherwise. I follow the importance but because the caching rules are removed from the user, you can't use cache and update in the background, so yes it becomes super risky.

I'd almost rather encourage a package number get bumped for each production build and that get used to update the SW file regardless of whether any of the individual files are updated.

---

Agree with the readability issues of final output. it's generated so little point of optimizing for readability. But does entrench the - it's all sw-precache or nothing vibe of the tool.

---

Cache first I'm totally for, but the library seems to cache only - if it gets a hit, it won't update will it?

I'd actually prefer sw-precache used sw-toolbox tbh and that's just so that I wouldn't feel so locked in.

---

I suppose the thing that has rung true in your final comment: "sw-precache gets to control when the SW update cycle kicks in due to changes to the precached resources" I'n my head, thinking of sw-precache as something just for install time only assets, it's perfect. I feel that for most sites, there is plenty of fun to be had in the fetch event.

Ultimately, I have concerns that over time, sw-precache will get in the way enough that I can see projects dropping back to custom sw files, where as sw-precache to inject static resources and sw-toolbox to manage extra scenarios / helper methods, sounds much more flexible and understandable (if not more error prone).

[jeffposnick]

I understand what its doing and how it achieves it, but I now have a really tight dependency on what the server is doing and my build process which doesn't exist otherwise.

Using gulp-rev or any of the other solutions to automate detecting file changes also places a tight dependency between the site's structure and the build process. sw-precache is very much akin to that. You pay a setup cost, and end up with an automated solution that handles the details for you moving forward. I don't want to downplay the existence of the setup cost, but in my eyes, the end efficiency outweighs it.

I'd almost rather encourage a package number get bumped for each production build and that get used to update the SW file regardless of whether any of the individual files are updated.

I'd contend that you'd end up with a build process that's more fragile and prone to omitting crucial resources, especially for large projects. And are you going to end up redownloading the entire set of resources each time anything gets deployed to your site? That's a throwback to the bad old days when the Play Store had to redownload the entire APK each time an Android app got updated, vs. the current setup where only changed resources need to be downloaded.

But does entrench the - it's all sw-precache or nothing vibe of the tool.

It's all sw-precache for all the site's local resources, or at least the ones that you specify in the build config. I can see that alienating a developer who knows SW and wants more control, but for new developers, having sw-precache handle everything should be seen as a plus. And while we need to do a better job of explaining it, there are ways of hooking into sw-precache to extend the functionality when needed.

Cache first I'm totally for, but the library seems to cache only - if it gets a hit, it won't update will it?

The only time things get updated is when a change to a local resource is detected as part of the build process, and the updated site + SW gets deployed. The update is handled via the SW lifecycle, and the controlled page can hook into the lifecycle events and display a "something is updated; please refresh" message. There's no need to fire off a network request to check to see if something changed. That's a good thing, no?

Ultimately, I have concerns that over time, sw-precache will get in the way enough that I can see projects dropping back to custom sw files

I guess my vision for sw-precache is that, over time, we'll add in enough functionality to make it compelling for developers who would otherwise feel the need to roll their own solutions. I'm happy to continue to try out sw-precache in different use cases and hear about others doing so, and let that guide development. For instance, I added in the navigateFallback functionality when I realized that's a common pattern for App Shells.

[gauntface]

I agree with all of the above.

The one thing I think we disagree on is what is easiest for the user.

I have experience with SW and I struggled with this and when it didn't work I was lost in terms of how to fix it. How would someone new to SW get on with it?

'Extending' it to do things fills me with dread because I don't know what it does in the first place, so how would I extend it?

It's ultimately just different approach of how to solve this problem is all. We'll see how things go in future projects.

[jeffposnick]

That's a great point to focus on—removing the rough edges and making this friendlier for developers new to SW via improved documentation, examples and simplified options is high priority right now.

Thanks for putting it through its paces, @gauntface!