GoogleChrome / workbox

📦 Workbox: JavaScript libraries for Progressive Web Apps
https://developers.google.com/web/tools/workbox/
MIT License
12.38k stars 820 forks source link

More precise documentation around StaleWhileRevalidate's behavior #2807

Closed rubas closed 3 years ago

rubas commented 3 years ago

Library Affected: workbox-strategies

Browser & Platform: all browsers

Feature Request Description:

The stale-while-revalidate Cache-Control Extension When present in an HTTP response, the stale-while-revalidate Cache- Control extension indicates that caches MAY serve the response in which it appears after it becomes stale, up to the indicated number of seconds. https://tools.ietf.org/html/rfc5861#section-3

The current implementation of the stale-while-revalidate is very inflexible and just assumes the current cache content is always stale.

Normally I would use this strategy alongside an indication, when the cache content should be consider stale. For example Firefox and Chrome support stale-while-revalidate next to max-age. We use a similar solution with our varnish layer.

Our your goal is to leverage the local cache and reduce the load (requests) on the origin.

This is why I propose an option for the stale-while-revalidate strategy that allows you to set a number of second during which the cache is considered fresh.

This option in combination with the ExpirationPlugin (how long stale content is allowed be serve...) gives us a complete implementation of stale-while-revalidate.

jeffposnick commented 3 years ago

Hello @rubas!

Apologies if I'm misunderstanding your request, but are you asking for the ability to use the StaleWhileRevalidate strategy and completely skip the revalidation (i.e. network) step if some criteria are met?

I'd normally say that one way to handle that would be to ensure that you're using the HTTP cache and associated Cache-Control: headers effectively, since if you are, the revalidation request could be fulfilled from the HTTP cache instead of the network. This means you won't have to change anything in Workbox.

If you can't accomplish this with Cache-Control: headers, then you have some options for extending Workbox to accomplish what you describe. (If you do end up implementing something, feel free to release it on npm so that others can use it!)

Custom strategy

Writing a Strategy subclass that implemented the specific logic you're looking for is probably the easiest approach. Workbox's StaleWhileRevalidate class is only a handful of lines of code (when you omit the comments and logging), and could be adapted to accomplish what you describe. The nice part about writing a custom Strategy subclass is that all the existing plugins will "just work", as long as you interact with the network and cache via the wrapper methods exposed by the handler parameter.

Here's a rough sketch:

import {Strategy} from 'workbox-strategies';

class CustomSWR extends Strategy {
  async _responseIsFresh(response: Response): boolean {
    // Your logic for determining whether response is fresh
    // goes here.
  }

  // Your overridden _handle() method will be called by the base class's handle().
  async _handle(request: Request, handler: StrategyHandler): Promise<Response> {
    // cachedResponseWillBeUsed plugins will automatically be run here, so you
    // could theoretically avoid needing to implement your own _responseIsFresh()
    // if you just require that folks always use a cachedResponseWillBeUsed plugin.
    // But if you want it to be self-contained, you can run logic embedded in the class.
    let response = await handler.cacheMatch(request);
    if (await this._responseIsFresh(response)) {
      return response;
    }

    // fetchAndCachePut() will handle both the network request
    // and updating the cache.
    return await handler.fetchAndCachePut(request);
  }
}

Custom plugin (not feasible)

Another alternative for extending Workbox is to write a custom plugin.

I could imagine a plugin that implemented cachedResponseWillBeUsed and requestWillFetch, making use of the state object that's passed to both to signal from cachedResponseWillBeUsed whether requestWillFetch should return a valid Request or just null. The problem with this approach, though, is that the StaleWhileRevalidate strategy doesn't guarantee that cachedResponseWillBeUsed will execute before requestWillFetch (they're effectively run simultaneously), so you couldn't reliably set state in cachedResponseWillBeUsed before requestWillFetch runs. Additionally, if you did return null in requestWillFetch, then you'd end up with a fetch() failure that you'd have to account for.

I'm going to close this issue for now, but if for some reason that doesn't address your use case, let us know and we can revisit.

rubas commented 3 years ago

Thank you, Jeff. I don't know anybody who answer all issues always this fast and extensive!!

Apologies if I'm misunderstanding your request, but are you asking for the ability to use the StaleWhileRevalidate strategy and completely skip the revalidation (i.e. network) step if some criteria are met?

Correct. As the name implies the idea is that you are allowed to use/send "stale" content while fetching an updated version in the background.

Normally (in the context of http cache, varnish strategies ...) you always have two parameters with this strategy:

I do understand that I could get my desired behaviour with a custom strategy (or just with a http cache afterwards if supported by the browser) but suggest to rethink the name of the strategy (or at least add a note in the docs about the the difference / limitation) - or better add the missing functionality to make it whole 🌻

At the end of the day it is a wording issue. Treating the cache always as stale, is not in the spirit of the "original" strategy and is imo confusing. This term is always used in a context, where you define first the freshness (lifetime) of the cache. The corresponding RFC has the title HTTP Cache-Control **Extensions** for Stale Content.

Why do I care?

I treat SW as every other cache layer I work with. If you are only looking for speed and offline functionality, the current implementation works fine. But I also want to reduce unnecessary requests (💰💰💰) to the origin/next cache layer ...


jeffposnick commented 3 years ago

Yeah, that's a valid point w.r.t. Workbox's naming—I did think about it when I was writing https://web.dev/stale-while-revalidate/

I don't know that we're going to change anything at this point, as the (technically inaccurate) StaleWhileRevalidate strategy name is used very pervasively in the codebase and supporting build tools. I think it's more likely that we'll add a caveat in the documentation, linking to this issue for folks who want more context and potential workarounds.

I'll reopen this issue to track that documentation change.