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

nginx: add Cache-Control header #791

Merged
merged 6 commits into from
Mar 14, 2024
Merged

nginx: add Cache-Control header #791

merged 6 commits into from
Mar 14, 2024

Conversation

deer-wmde
Copy link
Contributor

@deer-wmde deer-wmde commented Mar 13, 2024

https://phabricator.wikimedia.org/T331423

Adds the Cache-Control header to the nginx response. Forces revalidation for index.html

Copy link

Deployment previews on netlify for branch refs/pull/791/merge will be at the following locations (when build is done):

@m90
Copy link
Contributor

m90 commented Mar 13, 2024

I would think we still need to set different values for assets that are not index.html, i.e. revisioned JS and CSS assets that will never change and thus can be cached:

JS and CSS bundles are revisioned by the build system. However, we do not send a Cache-Control header here either. As these files will never change after being created (as opposed to index.html), we can set "hard" cache control values to make sure users will never have to redownload the same bundle twice. Something like Cache-Control: max-age: 31536000, immutable should work well.

@deer-wmde
Copy link
Contributor Author

I wonder if we should lower the max-age for everything else though. I'm thinking of assets like images that may not change their filename (I think?)

@m90
Copy link
Contributor

m90 commented Mar 13, 2024

I'm thinking of assets like images that may not change their filename (I think?)

A cursory check tells me all assets that are handled by the Vue App are revisioned. We do load some stuff from GCS as well, but those aren't affected by this change anyways.

nginx/default.conf Outdated Show resolved Hide resolved
@deer-wmde
Copy link
Contributor Author

I'm thinking of assets like images that may not change their filename (I think?)

A cursory check tells me all assets that are handled by the Vue App are revisioned. We do load some stuff from GCS as well, but those aren't affected by this change anyways.

Indeed! I didn't know this, amazing. In that case we should be good, thanks for mentioning this

@m90
Copy link
Contributor

m90 commented Mar 13, 2024

Indeed! I didn't know this, amazing. In that case we should be good, thanks for mentioning this

Unfortunately I was wrong and we're serving an unrevisioned /config.js file which we'll need to handle just like index.html I would assume.

Copy link
Contributor

@m90 m90 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Works well for me when testing this locally.

On subsequent loads, index.html and config.js will be requested from the server, yielding a 304 response, all other assets will skip the request, reading from disk cache.

When adding arbitrary changes, I will the correct version after reload.

@deer-wmde deer-wmde merged commit 4b353d9 into main Mar 14, 2024
7 checks passed
@deer-wmde deer-wmde deleted the de/cache branch March 14, 2024 15:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants