Caching images does not seem to work in V3. modifyResponseHeaders throws error

mhjmaas commented 2 months ago

Link to reproduction

Repo

Payload Version

3.0.0-beta.84

Node Version

v20.10.0

Next.js Version

15.0.0-canary.119

Describe the Bug

When using @payloadcms/storage-vercel-blob both in the admin UI and on the website itself the cache control headers are always set to: public, max-age=0, must-revalidate. (the images in the vercel blob store do show their default caching which is about a year I believe). I imagine this is because the payload api sets the cache to 0 and does not honor cache settings from the blob store. When wrapping images in a NextJS image this can be circumvented because it will then use nextjs image caching. However for background images and video this seems impossible.

I've tried using the "modifyResponseHeaders" property on the upload as it was added in this pull request: https://github.com/payloadcms/payload/pull/6926

upload: {
  modifyResponseHeaders: ({ headers }) => {
    headers.set('Cache-Control', 'public, max-age=86400')
    return headers
  }
},

However this results in an error while running locally without storage-vercel-blob in both admin UI, even though it works in the client website:

Error: Functions cannot be passed directly to Client Components unless you explicitly expose it by marking it with "use server". Or maybe you meant to call this function rather than return it.

Separating the code and making it server component results in an error in both the admin UI and client website:

ERROR: TypeError: headers.set is not a function
    at headersWithCors (webpack-internal:///(rsc)/./node_modules/.pnpm/@payloadcms+next@3.0.0-beta.84_@types+react@18.3.3_graphql@16.8.1_monaco-editor@0.48.0_next@1_sopbbpp676u7dtvtee63wvhbva/node_modules/@payloadcms/next/dist/utilities/headersWithCors.js:18:17)

If I deploy this code to vercel I get no errors however the cache control headers still seem to be set to 0.

The result is that every time there is a view on the webpage all images that are not cached by next/image are going to be downloaded again and this both detracts from the user experience as well as the usage limits by Vercel are going to be hit faster.

Reproduction Steps

Setup your project with a collection that has an upload: block
Upload an image in the collection
use the collection as a background image or just regular image (not using next/image)
Add the "modifyResponseHeaders" to the upload block as referenced in the pull request.
Refresh either the Admin UI or the collection and the error shows up in the log
Add @payloadcms/storage-vercel-blob to your payload.config. The error does not show up in the vercel logs but the images are still not cached like they should.

Adapters and Plugins

storage-vercel-blob

mhjmaas commented 2 months ago

To further help debugging the issue I've included a repository which demonstrates the issue. The readme includes instructions on how to set it up as well as a couple of observations I've made while setting it up which I will duplicate here as well:

It seems everything works as designed when using the images in a client website. However an error is thrown in the admin ui when browsing to the Media collection:

Error: Functions cannot be passed directly to Client Components unless you explicitly expose it by marking it with "use server". Or maybe you meant to call this function rather than return it.

To workaround this you can separate the modifyResponseHeaders into a file and use "use server" to mark it as a server component. however it then fails with the previously reported error. It seems to me it should just work inline without being a server component.

It seems the storage-vercel-blob plugin has nothing to do with the issue seeing as I can reproduce using the default local storage solution.

https://github.com/mhjmaas/payload-cache-issue

wellitongervickas commented 2 months ago

is this working within vercel blob storage? is it automatically? Because my images still not getting from cache, always requesting again. I updated to latest version beta 3.0.0-beta.98

mhjmaas commented 2 months ago

I'm not sure what the default behaviour should be, but seeing as there is a default cache setting in vercel blob storage I imagine it should simply use that if not set. With the current beta, when developing locally the 'modifyResponseHeaders' function seems to work, however when deployed on vercel and using blob storage it still requests the file again since the cache is set to 0.

wellitongervickas commented 2 months ago

https://github.com/payloadcms/payload/pull/8065 @mhjmaas