Add static asset caching #615
Conversation
There was a problem hiding this comment.
This looks good to me Alex.
We need to document this change in the CHANGELOG: https://github.com/alphagov/govuk-prototype-kit/blob/master/CHANGELOG.md
I can do this for you if you want.
This kind of work makes me think that we could have a 'development mode' and a 'production mode' (something to indicate this is being put in front of a user), which would set these kinds of defaults for you.
|
Thanks, I can add it in no worries, I need to squash commits anyway and make a change @colinrotherham suggested 👍 |
| useBrowserSync: 'true', | ||
|
|
||
| // Set a value (in milliseconds) for the max age of all static assets in cache | ||
| assetsMaxAge: 0 |
There was a problem hiding this comment.
This all looks great, thanks, but I wonder if we can make the content and variable name here clearer, will have a think.
There was a problem hiding this comment.
Yeh definitely, the name here and names in server are bothering me a bit, maxAge is too vague but I'm not sure we are there yet.
There was a problem hiding this comment.
As discussed on Slack, it might be a more consistent approach to have useCache, with that only happening in production. Or not have a config at all and just cache on production.
|
@joelanman Yeh i'll take a look at that as well, it may be worth it there's definitely ways around it such as manually appending a query string to the relevant assets before deploying, but it's far from ideal. |
ad594f9 to
2a1af5c
Compare
2a1af5c to
bb0a6e5
Compare
|
closing in favour of #627 which adds cache busting |
Recently we have demoed our prototype on heroku and locally through presentations and video, the prototype doesn't cache any static assets so during these presentations theres often a flash of unstyled text and layout shifting due to the lack of CSS and webfonts.
This PR will allow a cache maxAge option to be set through config or environment variables, which will default to 0 if neither is set (should make it less of a breaking change as it won't be required in config)