Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

New caching strategy suggestion - cacheFirstWithStaleWhileRevalidate #1816

Closed
adityapunjani opened this issue Jan 4, 2019 · 8 comments
Closed

Comments

@adityapunjani
Copy link

Issue or Feature Request Description:
In an ideal world to benefit from a warm cache, we want to pin the users to a certain application version (HTML + JS) for a good amount of time. This can be implemented by using cacheFirst for X number of days before it expires. However taking learnings from native apps, where the app download is async and decoupled from the app use itself, when you release a new app, you expect people to update but not while they are using it. The same could be extended to web apps with staleWhileRevalidate where the critical path is unblocked but the app updates in the background.

The best possible user experience from a caching point of view would be a combination cacheFirst for X numbers days followed by staleWhileRevalidate post cache expiry. I am not sure how something like this can be implemented with the current tools workbox provides. (Correct me if I'm wrong here)

I wanted to hear thoughts on adding a new strategy for this use case cacheFirstWithStaleWhileRevalidate ? cc @jeffposnick @philipwalton @prateekbh

@mrsharpoblunto
Copy link

An alternative suggestion to the same effect - a staleRevalidate flag option to the cacheFirst strategy

@prateekbh
Copy link
Collaborator

prateekbh commented Jan 11, 2019

I and @jeffposnick had some conversation about this. We think that a custom plugin would solve this instead of a new caching strategy.

All this solution requires is to use a plugin which extends workbox.expiration.Plugin only to call a new method of refreshEntries instead of expireEntries here:

cacheExpiration.expireEntries();

I'll write up a gist and put it here.

@prateekbh
Copy link
Collaborator

@adityapunjani here is a gist that achieves what you expect: https://gist.github.com/prateekbh/7f047938b5d1aab1b8a8b0f92d093ef2

@jeffposnick plz check if there's no blunder

@jeffposnick
Copy link
Contributor

Thanks, @prateekbh—I added a small comment to that gist, perhaps it satisfies @adityapunjani's use case.

But, re-reading Aditya's original post, I wonder if the behavior you're looking for can't just be accomplished by precaching a larger set of URLs? The behaviors you describe are things you already get "for free" if you use precaching rather than runtime caching.

I could imagine cases where you don't want to precache too many URLs, especially those that only a small subset of users will ever end up needing to visit. We've tried to think about that scenario in #155, but there's no concrete plan to move forward with that yet.

I'm always worried about using runtime caching for resources that have a strict "set of URLs that all work together" aspect, since you don't have any hard guarantees that all of the updates to URLs in that set will always happen. You do get those guarantees with precaching, though, because the install event will fail unless each URL that needs to be updated gets the update.

@arendtio
Copy link

@prateekbh I think I have a similar use-case so I tried your gist today, but I wonder if I made a mistake as the result did not match my expectations.

Use-Case: I want to cache request for 60 minutes. After that time, it should try to fetch newer versions, but if the fetch fails (e.g. offline), it should still use the cache versions.

Your gist does seem to cache the requests, but when the time is up, it deletes the cache so that if the browser is offline, all requests fail ultimately. Any idea if I did interpret the purpose of the gist wrong, use it wrong or is there a bug in the code?

@prateekbh
Copy link
Collaborator

@arendtio My bad I missed a line. Updated my gist to fix it

@arendtio
Copy link

Thanks, now it looks better 👍 The funny thing is: I tried to move that exact line around earlier today, but didn't realize I could just delete it :-D

@jeffposnick
Copy link
Contributor

I don't think that we're going to publish this as an "official" strategy, but we'd love to see folks from the community release custom Strategy subclasses to npm targeted at Workbox v6+.

@prateekbh's gist could be adapted to do that.

We'll have some official guidance in the v6 documentation, but in the meantime, #2593 (comment) is a good example of what the code for a custom Strategy subclass would look like.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants