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

Add support for Cache-Control: immutable #140

Closed
wants to merge 2 commits into from

Conversation

jcready
Copy link
Contributor

@jcready jcready commented Apr 29, 2017

If the maxAge option is set to Infinity then the resulting header will be set:

Cache-Control: public, max-age=31536000, immutable

Resolves expressjs/express#3197

@jcready jcready changed the title Cache control immutable Add support for Cache-Control: immutable Apr 29, 2017
@dougwilson dougwilson self-assigned this Sep 27, 2017
@dougwilson
Copy link
Contributor

dougwilson commented Sep 27, 2017

So after some (light) reading, I realize (and I don't think I did before) that "immutable" is actually useful for any cache duration, so saying max age 4 hours + immutable is a valid configuration. I am going to land this still, still with full credit to @jcready , but with the following API change: instead of { maxAge: Infinity } = add immutable flag, a new immutable option will be added, with the default as false, such that { immutable: true } = immutable (and you can do { maxAge: '4h', immutable: true }).

@dougwilson
Copy link
Contributor

Another option, just to throw it out there (though it's high visibility), is to actually deprecate the maxAge option and have cacheControl accept an object so you can { cacheControl: { maxAge: '4h' } } or { cacheControl: { maxAge: '6h', immutable: true } } or some other combination.

I'm not 100% sure how I feel about deprecating the maxAge option since it's been here for so long. Perhaps deliver the separate { immutable: true } option now and in a later release deprecate both maxAge and immutable to move them into cacheControl 🤔

@jcready
Copy link
Contributor Author

jcready commented Sep 27, 2017

Now I’m curious, how is “immutable” useful for any cache duration? Could you point me to what you were reading?

@dougwilson
Copy link
Contributor

dougwilson commented Sep 27, 2017

I was specifically reading through the Firefox bug for the implementation https://bugzilla.mozilla.org/show_bug.cgi?id=1267474 and MDN https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Cache-Control

Basically, it is a directive that tells the web browser that, if the browser has a valid cached version (which means it needs to have some directive to tell it to cache the response), it will just not send the revalidation requests until the resource expires out of the browser's cache.

@jcready
Copy link
Contributor Author

jcready commented Sep 27, 2017

Interesting. I had (I guess incorrectly) assumed that “immutable” meant the resource could never change and that the maxAge part of it was only a fallback for clients that didn’t support the immutable part of the directive.

@dougwilson
Copy link
Contributor

Yea, I had the same assumption as well.

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

Successfully merging this pull request may close these issues.

2 participants