Use permanent cache for translation files on production #181377
Conversation
|
/ci |
|
/ci |
pgayvallet
added
Team:Core
|
/ci |
|
/ci |
|
/ci |
💛 Build succeeded, but was flaky
Failed CI StepsTest Failures
Metrics [docs]Public APIs missing comments
Unknown metric groupsAPI count
History
To update your PR or re-run it, just comment with: |
| 'content-type': 'application/json', | ||
| 'cache-control': 'must-revalidate', | ||
| etag: translationCache.hash, | ||
| ['/translations/{locale}.json', `/translations/${translationHash}/{locale}.json`].forEach( |
There was a problem hiding this comment.
I kept the old route for two reasons:
- functional tests against the endpoint (it's a pain to retrieve the translation hash from the API integration tests...)
- BWC: some integrations/customers might be reaching this endpoint, you never know
There was a problem hiding this comment.
This path is not a public API so i dont think we should worry about custom integrations especially since i dont see any use case for this.. Maybe worth creating an issue to remove it later on in a separate PR
| if (isDist) { | ||
| headers = { | ||
| 'content-type': 'application/json', | ||
| 'cache-control': `public, max-age=${365 * DAY}, immutable`, | ||
| }; | ||
| } else { | ||
| headers = { | ||
| 'content-type': 'application/json', | ||
| 'cache-control': 'must-revalidate', | ||
| etag: translationCache.hash, | ||
| }; |
There was a problem hiding this comment.
I kept the etag cache control for non-distribution build, even if I'm not sure how useful this is, given the translation hash will change when translations are added/removed/changed.
| const translationHash = i18n.getTranslationHash(); | ||
| const translationsUrl = `${serverBasePath}/translations/${translationHash}/${i18nLib.getLocale()}.json`; | ||
|
|
There was a problem hiding this comment.
Changing the translationsUrl that will be sent to the browser to load the translations
|
Pinging @elastic/kibana-core (Team:Core) |
| registerTranslationsRoute({ | ||
| router, | ||
| locale: 'en', | ||
| isDist: true, |
There was a problem hiding this comment.
I may have missed it, but is it worth having a jest integration style test for this "isDist" Boolean and how it affects response headers?
## Summary Fix elastic#83409 Use a permanent cache (`public, max-age=365d, immutable`) for translation files when in production (`dist`), similar to what we're doing for static assets. Translation files cache busting is a little tricky, because it doesn't only depend on the version (enabling or disabling a custom plugin can change the translations while not changing the build hash), so we're using a custom hash generated from the content of the current translation file (which was already used to generate the `etag` header previously). --------- Co-authored-by: kibanamachine <[email protected]>
Summary
Fix #83409
Use a permanent cache (
public, max-age=365d, immutable) for translation files when in production (dist), similar to what we're doing for static assets.Translation files cache busting is a little tricky, because it doesn't only depend on the version (enabling or disabling a custom plugin can change the translations while not changing the build hash), so we're using a custom hash generated from the content of the current translation file (which was already used to generate the
etagheader previously).