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.

Sign up for GitHub

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

Enable custom Cache-Control header for remote images #35596

Closed
wants to merge 2 commits into from
Closed

Enable custom Cache-Control header for remote images #35596

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
2e7c9f6
feat(image): add remote cache control option to image optimizer (#35595)
pedrosodre Mar 25, 2022
eb4b7c7
docs(image): add remote cache control documentation
pedrosodre Mar 25, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions docs/api-reference/next/image.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -497,6 +497,18 @@ The default [loader](#loader) will automatically bypass Image Optimization for a

Auto-detection for animated files is best-effort and supports GIF, APNG, and WebP. If you want to explicitly bypass Image Optimization for a given animated image, use the [unoptimized](#unoptimized) prop.

### Remote Cache Control

By default, when an image came from a remote server the `Cache-Control` header is set as `public, max-age=0, must-revalidate`. To disable cache revalidation and change this behavior you can rewrite the header value by configuring `remoteCacheControl` option.

```js
module.exports = {
images: {
remoteCacheControl: 'public, max-age=315360000',
Copy link
Contributor

@belgattitude belgattitude Mar 30, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wondering something... Could it be a function too ?

remoteCacheControl: string | (originalUrl: string, urlReturnedByTheLoader: string) => string

To allow customization based on the given url (or the transformed one).

i.e:

remoteCacheControl: (originalUrl: string, urlReturnedByTheLoader: string) => {
  if (isLocalImage(url)) {
    return 'public, max-age=0'
  }
  if(isS3Url(url)) {
    return 'public, max-age=315360000';
  } 
  // ...other cases
}

Just a concept, not a proper API... Might need more thoughts (are other headers useful ?...).

PS: Personally I would have preferred the loader function to return something like:

{ 
   url: 'transformed', 
   responseHeaders: Record<string, string> // So we can add any headers
}

But I guess it would be a bc.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@belgattitude I've checked the code and I'm rewriting my anwser:

I think we can let the remoteCacheControl variable be a string or a function, but not related with the loader, since the problem this PR tries to solve only happens when we use next/image to optimize/transform remote images. If we use a loader to point image to an external CDN without optimize it using next, the problem does not happens (since next/image points directly to external URL).

In any case, if we understand that remoteCacheControl is the way to go ahead with this pull request, I think that make sense let user decide the Cache-Control header based on remote image URL, that way we would be able to set different caching controls for each origin, like that:

remoteCacheControl: (url: string) => {
  if (isLocalImage(url)) {
    return 'public, max-age=0'
  }
  if(isS3Url(url)) {
    return 'public, max-age=315360000';
  } 
  // ...other cases
}

What do you guys think?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think a function is useful here because its not controlling Next.js built-in cache.

Furthermore, I don't think we should allow customizing the cache header for local images since those are hashed by content and already use the immutable header.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@styfle Actually the example was not good, but we can have multiple remote locations, like a CDN from AWS, an image from API and any others, and the cache-control can be different for them, but the function works only for remote images.

I'll modify my code to allow another review.

},
}
```

## Related

For an overview of the Image component features and usage guidelines, see:
Expand Down
13 changes: 7 additions & 6 deletions packages/next/server/image-optimizer.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -565,14 +565,13 @@ function setResponseHeaders(
contentType: string | null,
isStatic: boolean,
xCache: XCacheHeader,
contentSecurityPolicy: string
contentSecurityPolicy: string,
remoteCacheControl: string
) {
res.setHeader('Vary', 'Accept')
res.setHeader(
'Cache-Control',
isStatic
? 'public, max-age=315360000, immutable'
: `public, max-age=0, must-revalidate`
isStatic ? 'public, max-age=315360000, immutable' : remoteCacheControl
)
if (sendEtagResponse(req, res, etag)) {
// already called res.end() so we're finished
Expand Down Expand Up @@ -606,7 +605,8 @@ export function sendResponse(
buffer: Buffer,
isStatic: boolean,
xCache: XCacheHeader,
contentSecurityPolicy: string
contentSecurityPolicy: string,
remoteCacheControl: string
) {
const contentType = getContentType(extension)
const etag = getHash([buffer])
Expand All @@ -618,7 +618,8 @@ export function sendResponse(
contentType,
isStatic,
xCache,
contentSecurityPolicy
contentSecurityPolicy,
remoteCacheControl
)
if (!result.finished) {
res.end(buffer)
Expand Down
3 changes: 2 additions & 1 deletion packages/next/server/next-server.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -263,7 +263,8 @@ export default class NextNodeServer extends BaseServer {
cacheEntry.value.buffer,
paramsResult.isStatic,
cacheEntry.isMiss ? 'MISS' : cacheEntry.isStale ? 'STALE' : 'HIT',
imagesConfig.contentSecurityPolicy
imagesConfig.contentSecurityPolicy,
imagesConfig.remoteCacheControl
)
} catch (err) {
if (err instanceof ImageError) {
Expand Down
4 changes: 4 additions & 0 deletions packages/next/shared/lib/image-config.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,9 @@ export type ImageConfigComplete = {

/** @see [Dangerously Allow SVG](https://nextjs.org/docs/api-reference/next/image#dangerously-allow-svg) */
contentSecurityPolicy: string

/** @see [Remote Cache Control](https://nextjs.org/docs/api-reference/next/image#remote-cache-control) */
remoteCacheControl: string
}

export type ImageConfig = Partial<ImageConfigComplete>
Expand All @@ -60,4 +63,5 @@ export const imageConfigDefault: ImageConfigComplete = {
formats: ['image/webp'],
dangerouslyAllowSVG: false,
contentSecurityPolicy: `script-src 'none'; frame-src 'none'; sandbox;`,
remoteCacheControl: `public, max-age=0, must-revalidate`,
}