Skip to content

Commit

Permalink
Exclude srcset from svg image (#44308)
Browse files Browse the repository at this point in the history
The default behavior for svg is `dangerouslyAllowSVG: false` which means we won't try to optimize the image because its vector (see #34431 for more).

However, svg was incorrectly getting the `srcset` attribute assigned which would contain duplicate information like:

```
/test.svg 1x, /test.svg 2x
```

So this PR makes sure we treat svg the same as `unoptimized: true`, meaning there is no `srcset` generated. Note that this PR won't change the behavior if `loader` is defined or if `dangerouslyAllowSVG: true`.
  • Loading branch information
styfle authored Dec 23, 2022
1 parent 8a9133d commit fd0d0f5
Show file tree
Hide file tree
Showing 3 changed files with 14 additions and 10 deletions.
16 changes: 13 additions & 3 deletions packages/next/client/image.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -528,10 +528,11 @@ const Image = forwardRef<HTMLImageElement | null, ImageProps>(
let loader: ImageLoaderWithConfig = rest.loader || defaultLoader
// Remove property so it's not spread on <img> element
delete rest.loader
// This special value indicates that the user
// didn't define a "loader" prop or "loader" config.
const isDefaultLoader = '__next_img_default' in loader

if ('__next_img_default' in loader) {
// This special value indicates that the user
// didn't define a "loader" prop or config.
if (isDefaultLoader) {
if (config.loader === 'custom') {
throw new Error(
`Image with src "${src}" is missing "loader" prop.` +
Expand Down Expand Up @@ -625,6 +626,15 @@ const Image = forwardRef<HTMLImageElement | null, ImageProps>(
if (config.unoptimized) {
unoptimized = true
}
if (
isDefaultLoader &&
src.endsWith('.svg') &&
!config.dangerouslyAllowSVG
) {
// Special case to make svg serve as-is to avoid proxying
// through the built-in Image Optimization API.
unoptimized = true
}

const [blurComplete, setBlurComplete] = useState(false)
const [showAltText, setShowAltText] = useState(false)
Expand Down
6 changes: 0 additions & 6 deletions packages/next/shared/lib/image-loader.ts
Original file line number Diff line number Diff line change
Expand Up @@ -53,12 +53,6 @@ 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
2 changes: 1 addition & 1 deletion test/integration/next-image-new/default/test/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -951,7 +951,7 @@ function runTests(mode) {
).toBe('/test.svg')
expect(
await browser.elementById('without-loader').getAttribute('srcset')
).toBe('/test.svg 1x, /test.svg 2x')
).toBeFalsy()
})

it('should warn at most once even after state change', async () => {
Expand Down

0 comments on commit fd0d0f5

Please sign in to comment.