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

Fix .svg image optimization with a loader prop #34452

Merged
merged 4 commits into from Feb 16, 2022
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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
12 changes: 7 additions & 5 deletions packages/next/client/image.tsx
Expand Up @@ -385,10 +385,6 @@ export default function Image({
isLazy = false
}

if (src.endsWith('.svg') && !config.dangerouslyAllowSVG) {
unoptimized = true
}

if (process.env.NODE_ENV !== 'production') {
if (!src) {
throw new Error(
Expand Down Expand Up @@ -466,7 +462,7 @@ export default function Image({
)
}

if (!unoptimized) {
if (!unoptimized && loader !== defaultImageLoader) {
const urlStr = loader({
config,
src,
Expand Down Expand Up @@ -866,6 +862,12 @@ function defaultLoader({
}
}

if (src.endsWith('.svg') && !config.dangerouslyAllowSVG) {
// Special case to make svg serve as-is to avoid proxying
// through the built-in Image Optimization API.
return src
}

return `${config.path}?url=${encodeURIComponent(src)}&w=${width}&q=${
quality || 75
}`
Expand Down
22 changes: 22 additions & 0 deletions test/integration/image-component/default/pages/loader-svg.js
@@ -0,0 +1,22 @@
import React from 'react'
import Image from 'next/image'

const Page = () => {
return (
<div>
<h1>Should work with SVG</h1>
<Image
id="with-loader"
src="/test.svg"
width={100}
height={100}
loader={({ src, width }) => `${src}?size=${width}`}
/>
<br />
<Image id="without-loader" src="/test.svg" width={100} height={100} />
<footer>footer</footer>
</div>
)
}

export default Page
26 changes: 25 additions & 1 deletion test/integration/image-component/default/test/index.test.js
Expand Up @@ -224,7 +224,7 @@ function runTests(mode) {
)
await check(
() => browser.eval(`document.getElementById("msg3").textContent`),
'loaded 1 img3 with dimensions 400x400'
'loaded 1 img3 with dimensions 266x266'
)
await check(
() => browser.eval(`document.getElementById("msg4").textContent`),
Expand Down Expand Up @@ -785,6 +785,30 @@ function runTests(mode) {
/Image with src (.*)gif(.*) has "sizes" property but it will be ignored/gm
)
})

it('should not warn when svg, even if with loader prop or without', async () => {
const browser = await webdriver(appPort, '/loader-svg')
await browser.eval(`document.querySelector("footer").scrollIntoView()`)
const warnings = (await browser.log('browser'))
.map((log) => log.message)
.join('\n')
expect(await hasRedbox(browser)).toBe(false)
expect(warnings).not.toMatch(
/Image with src (.*) has a "loader" property that does not implement width/gm
)
expect(await browser.elementById('with-loader').getAttribute('src')).toBe(
'/test.svg?size=256'
)
expect(
await browser.elementById('with-loader').getAttribute('srcset')
).toBe('/test.svg?size=128 1x, /test.svg?size=256 2x')
expect(
await browser.elementById('without-loader').getAttribute('src')
).toBe('/test.svg')
expect(
await browser.elementById('without-loader').getAttribute('srcset')
).toBe('/test.svg 1x, /test.svg 2x')
})
} else {
//server-only tests
it('should not create an image folder in server/chunks', async () => {
Expand Down