-
Notifications
You must be signed in to change notification settings - Fork 12
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
feat: set cache headers for static assets #999
Conversation
# as they have unique hashes thanks to the asset | ||
# fingerprinter | ||
if request.url.startswith(asset_fingerprinter._asset_root): | ||
response.headers.add('Cache-Control', 'public, max-age=31536000, immutable') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
just so you are aware: immutable is only support on Firefox, Safari
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Followed recommendations from MDN yeah https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Cache-Control#caching_static_assets + https://web.dev/http-cache/#versioned-urls
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thank you, looks good
Fixes cds-snc/notification-api#1152
This PR adds appropriate
Cache-Control
headers for static assets (JS, CSS, images) served by the admin. This will improve the user experience for users, reduce the number of requests we have to handle and our costs.Cache invalidation is done thanks to an asset fingerprinter. I've made sure that templates don't use hardcoded paths anymore by looking for thinks like
static/
,asset_path
andadmin_base_url
in the codebase.