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

Adding expiry headers to Nginx Configuration #24

Closed
wants to merge 2 commits into from
Closed

Adding expiry headers to Nginx Configuration #24

wants to merge 2 commits into from

Conversation

MikeRogers0
Copy link

Summary

This should add expiry headers to https://guides.rubyonrails.org/getting_started.html which will solve:

image

I saw the above when I ran Lighthouse against the current guides site.

I based the nginx config changes on this digital ocean article.

Other Information

I've not touched nginx configurations for a while (I normally just use Netlify & Vercel right now) & I wasn't able to confirm locally that the configuration changes are valid, plus I'm totally open to debate about if 24h is the right level of caching for html files.

@rafaelfranca
Copy link
Member

Edge builds happen after every commit to rails/rails, so we can't cache the pages and the assets unless we have a way to invalidate the cache based in the content of the files.

For the stable build. I still think we need a way to invalidate the cache. It is not always that it happens but sometimes we need to regenerate stable documentation and sometimes the content changes.

I think we need to investigate adding source digest in the assets path and remove the page cache for edge guides and doc.

@MikeRogers0
Copy link
Author

@rafaelfranca I've updated the rules to only affect the images, I'm assuming those are fairly unlikely to change :) This should allow us to confirm this nginx config change is valid :)

I think we need to investigate adding source digest in the assets path and remove the page cache for edge guides and doc.

This would be something I'd be keen to look into :D Currently it links to quite a few seperate JS/CSS files, I'd be very keen to copy the asset pipeline approach of combining them & adding as content hash to them.

@fxn
Copy link
Member

fxn commented Feb 24, 2021

I'm assuming those are fairly unlikely to change

I prefer to have flexibility, we do not have a problem serving these files. The web server automates conditional GETs, it's a good compromise.

@MikeRogers0
Copy link
Author

MikeRogers0 commented Feb 24, 2021

Should I close this off & come back to it when we have content hashes at the end of asset files?

@MikeRogers0
Copy link
Author

MikeRogers0 commented Feb 26, 2021

Closing off - I'll look into adding a digest / other improvements, then come back to this.

Edit: I made a ticket ( #25 ) to track this :)

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.

3 participants