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

Set service-worker max-age to 0 #428

Merged

Conversation

nolanlawson
Copy link
Contributor

As of Chrome 68 and Firefox 57 (and Edge and Safari), service workers are always revalidated regardless of the cache-control directive. So it's recommended to use cache-control: max-age=0, since it aligns older browsers with the newer behavior and also makes it clear to developers what's going on.

For the service worker sourcemap file I don't think it really matters what we do, since it's a request that is only made when DevTools are open, but I'd say align it with service-worker.js just in case the two get out of sync somehow.

Sources:

@lukeed
Copy link
Member

lukeed commented Sep 7, 2018

They should be private, no-cache, no?

@nolanlawson
Copy link
Contributor Author

I mean, max-age=0 should be the same as no-cache, right? Not sure about private; my intuition is actually to set public for everything, but I don't know if that's a sensible default since I'm not very familiar with how proxies handle that stuff.

@lukeed
Copy link
Member

lukeed commented Sep 8, 2018

Well, service-workers should completely bypass any cache so that it doesn't ever risk being stale.

While it's true that max-age=0 is effectively the same thing, you're still adding an entry into the browser and CDN cache, only to have them immediately expire. It's like doing this:

let obj = {};

function onRequest()
  obj.foo = 'bar';
  if (obj.foo) {
    delete obj.foo;
  }
}

onRequest();
onRequest();
// etc

The combination of private and no-cache completely skips the cache assignment(s), avoiding useless work. It's as equally supported as the public directive.

@Rich-Harris
Copy link
Member

From that MDN page:

Preventing caching

To turn off caching, you can send the following response header. In addition, see also the Expires and Pragma headers.

Cache-Control: no-cache, no-store, must-revalidate

@Rich-Harris Rich-Harris merged commit 7bd684a into sveltejs:master Sep 8, 2018
@Rich-Harris
Copy link
Member

I went with the MDN recommendation, we can always change it later if we think it's wrong. Thanks

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.

3 participants