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

next/image Cache-Control header does not match origin server #23562

Closed
aguilarm opened this issue Mar 30, 2021 · 6 comments
Closed

next/image Cache-Control header does not match origin server #23562

aguilarm opened this issue Mar 30, 2021 · 6 comments
Labels
bug Issue was opened via the bug report template.

Comments

@aguilarm
Copy link

What version of Next.js are you using?

10.1.0

What version of Node.js are you using?

14

What browser are you using?

Chrome

What operating system are you using?

macOS

How are you deploying your application?

next start

Describe the Bug

The Cache-Control header for images generated by the next/image component are set to public, max-age=0, must-revalidate and do not match the origin server as described in documentation.

Expected Behavior

Cache Control max-age should match the origin server.

To Reproduce

Use the Image component to generate a derivative image from a server that has a max-age value set.

The Cache-Control header for images generated by the next/image component are set to public, max-age=0, must-revalidate and do not match the origin server as described in documentation.

I traced this behavior from this issue about custom headers to this issue concerning etags and finally this PR where Etags were added.

The Etag PR is really close but has opted to ditch forwarding the Cache-Control max-age value, and set it to 0 + must-revalidate. In small deployments, this isn't a big deal because with Etag working the server is not transferring the whole file repeatedly unless the file actually changes.

However, this means clients both rely on the next image server to be up to display images and ping it excessively. Ideally, both Etag and a high max-age let clients cache images as long as is reasonable and do not receive a new version of the file.

I have a PR incoming that saves the original maxAge value and forwards it out to clients. I need to add testing and would like a general gut-check. It seems like there is some potential to overhaul the image optimizer a little more to mitigate things like #23436 or the issue with custom headers from config being superseded.

@aguilarm aguilarm added the bug Issue was opened via the bug report template. label Mar 30, 2021
@aguilarm
Copy link
Author

Will hold on opening a PR until I've fleshed this out more, here is my WIP potential fix canary...aguilarm:image-maxage

@egeriis
Copy link

egeriis commented Apr 7, 2021

One big issue that max-age=0 leads to, is that CDNs like Fastly don't automatically cache those assets.

@igordanchenko
Copy link

Are there any updates on this issue?

It makes no sense that NextJS forces browsers to revalidate immutable images over and over again. It really hurts visual performance of the app.

Upstream origin cache-control header:

Cache-Control: public, max-age=31536000, immutable

NextJS image cache-control header:

Cache-Control: public, max-age=0, must-revalidate

Here is the code sandbox demonstrating this issue - https://e6s3y.sse.codesandbox.io/

If you refresh the page multiple times, you can see that the top image (served from the upstream) is displayed instantaneously, while the bottom one (served via next/image) blinks before rendering.

@timneutkens
Copy link
Member

timneutkens commented Jun 25, 2021

Duplicate of #19914

I'm combining all these issues as they're practically the same report

@timneutkens timneutkens marked this as a duplicate of #19914 Jun 25, 2021
@igordanchenko
Copy link

These issues are not exactly the same... #19914 is requesting NextJS to respect custom /_next/image(.*) cache-control headers defined in next.config.js, while this issue is about the upstream origin cache-control headers. Also, #19914 is overloaded with use-cases and some confusing info. But if both can be addressed at the same time relatively soon then 👍

@balazsorban44
Copy link
Member

This issue has been automatically locked due to no recent activity. If you are running into a similar issue, please create a new issue with the steps to reproduce. Thank you.

@vercel vercel locked as resolved and limited conversation to collaborators Jan 28, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Issue was opened via the bug report template.
Projects
None yet
Development

No branches or pull requests

5 participants