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

Refactor image optimizer static immutable header #25909

Merged
merged 3 commits into from
Jun 8, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
16 changes: 4 additions & 12 deletions packages/next/client/image.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ export type ImageLoaderProps = {
src: string
width: number
quality?: number
isStatic?: boolean
}

type DefaultImageLoaderProps = ImageLoaderProps & { root: string }
Expand Down Expand Up @@ -190,7 +189,6 @@ type GenImgAttrsData = {
unoptimized: boolean
layout: LayoutValue
loader: ImageLoader
isStatic?: boolean
width?: number
quality?: number
sizes?: string
Expand All @@ -210,7 +208,6 @@ function generateImgAttrs({
quality,
sizes,
loader,
isStatic,
}: GenImgAttrsData): GenImgAttrsResult {
if (unoptimized) {
return { src, srcSet: undefined, sizes: undefined }
Expand All @@ -224,7 +221,7 @@ function generateImgAttrs({
srcSet: widths
.map(
(w, i) =>
`${loader({ src, quality, isStatic, width: w })} ${
`${loader({ src, quality, width: w })} ${
kind === 'w' ? w : i + 1
}${kind}`
)
Expand All @@ -236,7 +233,7 @@ function generateImgAttrs({
// updated by React. That causes multiple unnecessary requests if `srcSet`
// and `sizes` are defined.
// This bug cannot be reproduced in Chrome or Firefox.
src: loader({ src, quality, isStatic, width: widths[last] }),
src: loader({ src, quality, width: widths[last] }),
}
}

Expand Down Expand Up @@ -311,7 +308,6 @@ export default function Image({
delete rest['layout']
}

const isStatic = typeof src === 'object'
let staticSrc = ''
if (isStaticImport(src)) {
const staticImageData = isStaticRequire(src) ? src.default : src
Expand Down Expand Up @@ -339,7 +335,7 @@ export default function Image({
}
}
}
src = (isStatic ? staticSrc : src) as string
src = typeof src === 'string' ? src : staticSrc

if (process.env.NODE_ENV !== 'production') {
if (!src) {
Expand Down Expand Up @@ -516,7 +512,6 @@ export default function Image({
quality: qualityInt,
sizes,
loader,
isStatic,
})
}

Expand Down Expand Up @@ -644,7 +639,6 @@ function cloudinaryLoader({

function defaultLoader({
root,
isStatic,
src,
width,
quality,
Expand Down Expand Up @@ -692,7 +686,5 @@ function defaultLoader({
}
}

return `${root}?url=${encodeURIComponent(src)}&w=${width}&q=${quality || 75}${
isStatic ? '&s=1' : ''
}`
return `${root}?url=${encodeURIComponent(src)}&w=${width}&q=${quality || 75}`
}
13 changes: 4 additions & 9 deletions packages/next/next-server/server/image-optimizer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ export async function imageOptimizer(
}

const { headers } = req
const { url, w, q, s } = parsedUrl.query
const { url, w, q } = parsedUrl.query
const mimeType = getSupportedMimeType(MODERN_TYPES, headers.accept)
let href: string

Expand Down Expand Up @@ -111,13 +111,8 @@ export async function imageOptimizer(
return { finished: true }
}

if (s && s !== '1') {
res.statusCode = 400
res.end('"s" parameter must be "1" or omitted')
return { finished: true }
}

const isStatic = !!s
// Should match output from next-image-loader
const isStatic = url.startsWith('/_next/static/image')

const width = parseInt(w, 10)

Expand Down Expand Up @@ -381,7 +376,7 @@ function sendResponse(
res.setHeader(
'Cache-Control',
isStatic
? 'public, immutable, max-age=315360000'
? 'public, max-age=315360000, immutable'
: 'public, max-age=0, must-revalidate'
)
if (sendEtagResponse(req, res, etag)) {
Expand Down
10 changes: 0 additions & 10 deletions test/integration/image-component/default/test/static.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -37,16 +37,6 @@ const runTests = () => {
expect(html).toContain('width:200px;height:200px')
expect(html).not.toContain('width:400px;height:400px')
})
it('Should append "&s=1" to URLs of static images', async () => {
expect(
await browser.elementById('basic-static').getAttribute('src')
).toContain('&s=1')
})
it('Should not append "&s=1" to URLs of non-static images', async () => {
expect(
await browser.elementById('basic-non-static').getAttribute('src')
).not.toContain('&s=1')
})
it('Should add a blurry placeholder to statically imported jpg', async () => {
expect(html).toContain(
`style="position:absolute;top:0;left:0;bottom:0;right:0;box-sizing:border-box;padding:0;border:none;margin:auto;display:block;width:0;height:0;min-width:100%;max-width:100%;min-height:100%;max-height:100%;background-size:cover;background-image:url("data:image/jpeg;base64,/9j/4AAQSkZJRgABAQAAAQABAAD/2wCEAAoKCgoKCgsMDAsPEA4QDxYUExMUFiIYGhgaGCIzICUgICUgMy03LCksNy1RQDg4QFFeT0pPXnFlZXGPiI+7u/sBCgoKCgoKCwwMCw8QDhAPFhQTExQWIhgaGBoYIjMgJSAgJSAzLTcsKSw3LVFAODhAUV5PSk9ecWVlcY+Ij7u7+//CABEIAAgACAMBIgACEQEDEQH/xAAUAAEAAAAAAAAAAAAAAAAAAAAH/9oACAEBAAAAADX/xAAUAQEAAAAAAAAAAAAAAAAAAAAA/9oACAECEAAAAH//xAAUAQEAAAAAAAAAAAAAAAAAAAAA/9oACAEDEAAAAH//xAAdEAABAgcAAAAAAAAAAAAAAAATEhUAAwUUIzLS/9oACAEBAAE/AB0ZlUac43GqMYuo/8QAFBEBAAAAAAAAAAAAAAAAAAAAAP/aAAgBAgEBPwB//8QAFBEBAAAAAAAAAAAAAAAAAAAAAP/aAAgBAwEBPwB//9k=")"`
Expand Down
10 changes: 9 additions & 1 deletion test/integration/image-optimizer/pages/index.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,13 @@
import Image from 'next/image'
import Logo from '../public/test.jpg'

function Home() {
return <h1>Image Optimizer Home</h1>
return (
<>
<h1>Image Optimizer Home</h1>
<Image src={Logo} />
</>
)
}

export default Home
28 changes: 14 additions & 14 deletions test/integration/image-optimizer/test/index.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -217,13 +217,6 @@ function runTests({ w, isDev, domains }) {
)
})

it('should fail when s is present and not "1"', async () => {
const query = { url: '/test.png', w, q: 100, s: 'foo' }
const res = await fetchViaHTTP(appPort, '/_next/image', query, {})
expect(res.status).toBe(400)
expect(await res.text()).toBe(`"s" parameter must be "1" or omitted`)
})

it('should fail when domain is not defined in next.config.js', async () => {
const url = `http://vercel.com/button`
const query = { url, w, q: 100 }
Expand Down Expand Up @@ -511,13 +504,20 @@ function runTests({ w, isDev, domains }) {
})

it('should set cache-control to immutable for static images', async () => {
const query = { url: '/test.jpg', w, q: 100, s: '1' }
const opts = { headers: { accept: 'image/webp' } }
const res = await fetchViaHTTP(appPort, '/_next/image', query, opts)
expect(res.status).toBe(200)
expect(res.headers.get('cache-control')).toBe(
'public, immutable, max-age=315360000'
)
if (!isDev) {
const query = {
url:
'/_next/static/image/public/test.480a01e5ea850d0231aec0fa94bd23a0.jpg',
w,
q: 100,
}
const opts = { headers: { accept: 'image/webp' } }
const res = await fetchViaHTTP(appPort, '/_next/image', query, opts)
expect(res.status).toBe(200)
expect(res.headers.get('cache-control')).toBe(
'public, max-age=315360000, immutable'
)
}
})

it("should error if the resource isn't a valid image", async () => {
Expand Down