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

[feature request] Cache-control: Immutable #97

Closed
tinchou opened this Issue Jan 29, 2017 · 6 comments

Comments

Projects
None yet
6 participants
@tinchou

tinchou commented Jan 29, 2017

This is a very recent feature, but one that could improve many sites performance. If there already exists a way to achieve this, it would be great to have it on the Docs.

Info: https://code.facebook.com/posts/557147474482256, https://hacks.mozilla.org/2017/01/using-immutable-caching-to-speed-up-the-web/

Thanks!

@guardrex

This comment has been minimized.

guardrex commented Jan 29, 2017

it would be great to have it on the Docs

It's not currently documented because OnPrepareResponse isn't in the static files doc. I opened an issue to discuss it over there. aspnet/Docs#2625

Regarding the feature request: Is your request to add something to the ResponseCache attribute for this? If so, wouldn't that be an MVC feature request? It doesn't seem like the middleware here would have anything to do with this directive.

@tinchou

This comment has been minimized.

tinchou commented Jan 29, 2017

I believe this middleware adds the Cache-Control headers, doesn't it? But now that I think about it, it doesn't handle the static files which are the main target for immutable.

Please feel free to close/move this if this is not the right place!

@guardrex

This comment has been minimized.

guardrex commented Jan 29, 2017

This middleware reads the header to determine if and how long to cache a resource. It doesn't set it.

Speaking of setting it, yes, either it's something for you to do with Static File Middleware (i.e., in OnPrepareResponse), or you could make an argument that something should be added to the ResponseCache attribute bits that would allow you to configure it easily for MVC-based responses. The attribute is an MVC thing, so the request would have to be made over in the MVC repo. @JunTaoLuo will confirm or correct me on these points.

@Tratcher

This comment has been minimized.

Member

Tratcher commented Jan 30, 2017

Interesting feature. This middleware is not the right place for it though, it doesn't set any headers, it only reads them. Using OnPrepareResponse sounds like your best place to implement this. Note we've previously declined requests to add other cache control options directly to the static files options, so I doubt we'd add this one as a first class option, especially before it becomes a formal spec.

Adding this option to the MVC attribute seems out of place, as MVC content is inherently dynamic. It should be pretty easy to append this value to the header in your MVC controller if you really want it. Give it a try and feel free to share the code for review.

@RehanSaeed

This comment has been minimized.

RehanSaeed commented Oct 19, 2017

I think the HttpContext could use the following extension method being added to it:

public static HttpContext ApplyCacheProfile(this HttpContext context, CacheProfile cacheProfile)

The CacheProfile class also needs an IsImmutable boolean flag on it. I've added it to my Boilerplate.AspNetCore NuGet package here if anyone wants to see the code. I think I remember @andrewlock had raised a GitHub issue to get this method added but the issue was closed.

@aspnet-hello

This comment has been minimized.

aspnet-hello commented Jan 2, 2018

This issue was moved to aspnet/AspNetCore#2611

@aspnet aspnet locked and limited conversation to collaborators Jan 2, 2018

@aspnet-hello aspnet-hello removed this from the Backlog milestone Jan 2, 2018

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.