Skip to content

Commit

Permalink
fix(#39807): ignore width/height from webpack with "fill" (#39849)
Browse files Browse the repository at this point in the history
Fixes #39807.

When statically importing an image, the `width` and `height` will always be provided alongside the `src` by the Webpack. `next/image` will ignore `width` and `height` come from Webpack when `layout === 'fill'`, while `next/future/image` will not, hence the issue. The PR fixes that. The corresponding integration test cases are also added.

cc @styfle 

## Bug

- [x] Related issues linked using `fixes #number`
- [x] Integration tests added
- [ ] Errors have helpful link attached, see `contributing.md`
  • Loading branch information
SukkaW committed Aug 23, 2022
1 parent cb430cc commit 8dfab19
Show file tree
Hide file tree
Showing 3 changed files with 82 additions and 41 deletions.
7 changes: 5 additions & 2 deletions packages/next/client/future/image.tsx
Expand Up @@ -591,8 +591,11 @@ export default function Image({
blurDataURL = blurDataURL || staticImageData.blurDataURL
staticSrc = staticImageData.src

height = height || staticImageData.height
width = width || staticImageData.width
// Ignore width and height (come from the bundler) when "fill" is used
if (!fill) {
height = height || staticImageData.height
width = width || staticImageData.width
}
if (!staticImageData.height || !staticImageData.width) {
throw new Error(
`An object should only be passed to the image component src parameter if it comes from a static image import. It must include height and width. Received ${JSON.stringify(
Expand Down
4 changes: 4 additions & 0 deletions test/integration/image-future/default/pages/static-img.js
Expand Up @@ -36,6 +36,10 @@ const Page = () => {
<Image id="static-gif" src={testGIF} />
<Image id="static-bmp" src={testBMP} />
<Image id="static-ico" src={testICO} />
<Image id="static-svg-fill" src={testSVG} fill />
<Image id="static-gif-fill" src={testGIF} fill />
<Image id="static-bmp-fill" src={testBMP} fill />
<Image id="static-ico-fill" src={testICO} fill />
<br />
<Image id="static-unoptimized" src={testJPG} unoptimized />
</div>
Expand Down
112 changes: 73 additions & 39 deletions test/integration/image-future/default/test/static.test.ts
Expand Up @@ -6,6 +6,7 @@ import {
renderViaHTTP,
File,
waitFor,
launchApp,
} from 'next-test-utils'
import webdriver from 'next-webdriver'
import { join } from 'path'
Expand All @@ -18,7 +19,7 @@ let html

const indexPage = new File(join(appDir, 'pages/static-img.js'))

const runTests = () => {
const runTests = (isDev = false) => {
it('Should allow an image with a static src to omit height and width', async () => {
expect(await browser.elementById('basic-static')).toBeTruthy()
expect(await browser.elementById('blur-png')).toBeTruthy()
Expand All @@ -29,44 +30,62 @@ const runTests = () => {
expect(await browser.elementById('static-gif')).toBeTruthy()
expect(await browser.elementById('static-bmp')).toBeTruthy()
expect(await browser.elementById('static-ico')).toBeTruthy()
expect(await browser.elementById('static-svg-fill')).toBeTruthy()
expect(await browser.elementById('static-gif-fill')).toBeTruthy()
expect(await browser.elementById('static-bmp-fill')).toBeTruthy()
expect(await browser.elementById('static-ico-fill')).toBeTruthy()
expect(await browser.elementById('static-unoptimized')).toBeTruthy()
})
it('Should use immutable cache-control header for static import', async () => {
await browser.eval(
`document.getElementById("basic-static").scrollIntoView()`
)
await waitFor(1000)
const url = await browser.eval(
`document.getElementById("basic-static").src`
)
const res = await fetch(url)
expect(res.headers.get('cache-control')).toBe(
'public, max-age=315360000, immutable'
)
})
it('Should use immutable cache-control header even when unoptimized', async () => {
await browser.eval(
`document.getElementById("static-unoptimized").scrollIntoView()`
)
await waitFor(1000)
const url = await browser.eval(
`document.getElementById("static-unoptimized").src`
)
const res = await fetch(url)
expect(res.headers.get('cache-control')).toBe(
'public, max-age=31536000, immutable'
)
})

if (!isDev) {
// cache-control is set to "0, no-store" in dev mode
it('Should use immutable cache-control header for static import', async () => {
await browser.eval(
`document.getElementById("basic-static").scrollIntoView()`
)
await waitFor(1000)
const url = await browser.eval(
`document.getElementById("basic-static").src`
)
const res = await fetch(url)
expect(res.headers.get('cache-control')).toBe(
'public, max-age=315360000, immutable'
)
})

it('Should use immutable cache-control header even when unoptimized', async () => {
await browser.eval(
`document.getElementById("static-unoptimized").scrollIntoView()`
)
await waitFor(1000)
const url = await browser.eval(
`document.getElementById("static-unoptimized").src`
)
const res = await fetch(url)
expect(res.headers.get('cache-control')).toBe(
'public, max-age=31536000, immutable'
)
})
}
it('Should automatically provide an image height and width', async () => {
expect(html).toContain('width="400" height="300"')
})
it('Should allow provided width and height to override intrinsic', async () => {
expect(html).toContain('width="150" height="150"')
})

it('Should add a blur to a statically imported image in "raw" mode', async () => {
expect(html).toContain(
`style="background-size:cover;background-position:0% 0%;background-image:url(&quot;data:image/svg+xml;charset=utf-8,%3Csvg xmlns=&#x27;http%3A//www.w3.org/2000/svg&#x27; viewBox=&#x27;0 0 400 300&#x27;%3E%3Cfilter id=&#x27;b&#x27; color-interpolation-filters=&#x27;sRGB&#x27;%3E%3CfeGaussianBlur stdDeviation=&#x27;50&#x27;/%3E%3C/filter%3E%3Cimage filter=&#x27;url(%23b)&#x27; x=&#x27;0&#x27; y=&#x27;0&#x27; height=&#x27;100%25&#x27; width=&#x27;100%25&#x27; href=&#x27;data:image/jpeg;base64,/9j/4AAQSkZJRgABAQAAAQABAAD/2wCEAAoKCgoKCgsMDAsPEA4QDxYUExMUFiIYGhgaGCIzICUgICUgMy03LCksNy1RQDg4QFFeT0pPXnFlZXGPiI+7u/sBCgoKCgoKCwwMCw8QDhAPFhQTExQWIhgaGBoYIjMgJSAgJSAzLTcsKSw3LVFAODhAUV5PSk9ecWVlcY+Ij7u7+//CABEIAAYACAMBIgACEQEDEQH/xAAnAAEBAAAAAAAAAAAAAAAAAAAABwEBAAAAAAAAAAAAAAAAAAAAAP/aAAwDAQACEAMQAAAAmgP/xAAcEAACAQUBAAAAAAAAAAAAAAASFBMAAQMFERX/2gAIAQEAAT8AZ1HjrKZX55JysIc4Ff/EABQRAQAAAAAAAAAAAAAAAAAAAAD/2gAIAQIBAT8Af//EABQRAQAAAAAAAAAAAAAAAAAAAAD/2gAIAQMBAT8Af//Z&#x27;/%3E%3C/svg%3E&quot;)`
)
// next-image-loader uses different blur behavior for dev/prod mode
// See next/build/webpack/loaders/next-image-loader
if (isDev) {
expect(html).toContain(
`style="background-size:cover;background-position:0% 0%;filter:blur(20px);background-image:url(&quot;/_next/image?url=%2F_next%2Fstatic%2Fmedia%2Ftest-rect.f323a148.jpg&amp;w=8&amp;q=70&quot;)"`
)
} else {
expect(html).toContain(
`style="background-size:cover;background-position:0% 0%;background-image:url(&quot;data:image/svg+xml;charset=utf-8,%3Csvg xmlns=&#x27;http%3A//www.w3.org/2000/svg&#x27; viewBox=&#x27;0 0 400 300&#x27;%3E%3Cfilter id=&#x27;b&#x27; color-interpolation-filters=&#x27;sRGB&#x27;%3E%3CfeGaussianBlur stdDeviation=&#x27;50&#x27;/%3E%3C/filter%3E%3Cimage filter=&#x27;url(%23b)&#x27; x=&#x27;0&#x27; y=&#x27;0&#x27; height=&#x27;100%25&#x27; width=&#x27;100%25&#x27; href=&#x27;data:image/jpeg;base64,/9j/4AAQSkZJRgABAQAAAQABAAD/2wCEAAoKCgoKCgsMDAsPEA4QDxYUExMUFiIYGhgaGCIzICUgICUgMy03LCksNy1RQDg4QFFeT0pPXnFlZXGPiI+7u/sBCgoKCgoKCwwMCw8QDhAPFhQTExQWIhgaGBoYIjMgJSAgJSAzLTcsKSw3LVFAODhAUV5PSk9ecWVlcY+Ij7u7+//CABEIAAYACAMBIgACEQEDEQH/xAAnAAEBAAAAAAAAAAAAAAAAAAAABwEBAAAAAAAAAAAAAAAAAAAAAP/aAAwDAQACEAMQAAAAmgP/xAAcEAACAQUBAAAAAAAAAAAAAAASFBMAAQMFERX/2gAIAQEAAT8AZ1HjrKZX55JysIc4Ff/EABQRAQAAAAAAAAAAAAAAAAAAAAD/2gAIAQIBAT8Af//EABQRAQAAAAAAAAAAAAAAAAAAAAD/2gAIAQMBAT8Af//Z&#x27;/%3E%3C/svg%3E&quot;)`
)
}
})
}

Expand All @@ -90,15 +109,30 @@ describe('Build Error Tests', () => {
})
})
describe('Future Static Image Component Tests', () => {
beforeAll(async () => {
await nextBuild(appDir)
appPort = await findPort()
app = await nextStart(appDir, appPort)
html = await renderViaHTTP(appPort, '/static-img')
browser = await webdriver(appPort, '/static-img')
describe('production mode', () => {
beforeAll(async () => {
await nextBuild(appDir)
appPort = await findPort()
app = await nextStart(appDir, appPort)
html = await renderViaHTTP(appPort, '/static-img')
browser = await webdriver(appPort, '/static-img')
})
afterAll(() => {
killApp(app)
})
runTests(false)
})
afterAll(() => {
killApp(app)

describe('dev mode', () => {
beforeAll(async () => {
appPort = await findPort()
app = await launchApp(appDir, appPort)
html = await renderViaHTTP(appPort, '/static-img')
browser = await webdriver(appPort, '/static-img')
})
afterAll(() => {
killApp(app)
})
runTests(true)
})
runTests()
})

0 comments on commit 8dfab19

Please sign in to comment.