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

Cache static assets #7611

Closed
wants to merge 1 commit into from
Closed

Cache static assets #7611

wants to merge 1 commit into from

Conversation

Bargs
Copy link
Contributor

@Bargs Bargs commented Jul 2, 2016

By default Hapi sends cache-control: no-cache headers. Because we version our static assets anyway, setting a nice long max-age on these responses is a huge and easy performance win. I've set max-age to an arbitrary, large period of one year.

You'll need to test this in production mode, dev mode still sets cache-control: no-cache. I'm guessing that's due to the base path proxy overriding headers, as it is wont to do. But I think that's actually desirable for dev mode anyway.

Fixes #6520

By default Hapi sends cache-control: no-cache headers. Because we
version our static assets anyway, setting a nice long max-age on these
responses is a huge and easy performance win. I've set max-age to an
arbitrary, large period of one year.
@Bargs Bargs added the review label Jul 2, 2016
@epixa
Copy link
Contributor

epixa commented Jul 5, 2016

I don't think we can do this until we version the static assets by shasum instead of build number. Plugin installs will result in the bundles changing, and it's imperative that they not be cached as a result.

@Bargs
Copy link
Contributor Author

Bargs commented Jul 5, 2016

Do you feel like users needing to CMD + SHIFT + R on those rare occasions when a plugin is installed is reason enough to degrade performance for the other 99.9% of the time?

@epixa
Copy link
Contributor

epixa commented Jul 5, 2016

No, but I don't think this is a dichotomy. Using hashes is the right solution, so let's get that done and then we can tweak cache parameters all we want.

I know the two are technically not the same issue, but users shouldn't have to care about technicalities. Caching is the issue here, so let's get the caching right.

@Bargs
Copy link
Contributor Author

Bargs commented Jul 5, 2016

Alright, I'll take a crack at it. Generating fingerprints with webpack appears to be pretty trivial, the trick is just making sure it jives with all the idiosyncrasies of our build process.

@epixa epixa removed the review label Jul 5, 2016
@Bargs Bargs removed their assignment Jul 26, 2016
@epixa epixa changed the title Fixes #6520 Cache static assets Cache static assets Oct 8, 2016
@epixa
Copy link
Contributor

epixa commented Oct 8, 2016

@Bargs Do you still plan to work on this?

@Bargs
Copy link
Contributor Author

Bargs commented Oct 10, 2016

someday, I hope so...

@Bargs
Copy link
Contributor Author

Bargs commented Nov 3, 2016

This is probably more appropriate for the platform team now

@Bargs Bargs closed this Nov 3, 2016
@ChrisCinelli
Copy link

Could you at least cache the index.js that is 5Mb?
It would also be nice to have gzip compression enabled.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants