Fix .svg image optimization with a loader prop (#34452)

* Fix .svg image optimization with a `loader` prop * Add test * Revert test back to 266x266
vercel · Feb 16, 2022 · 2264d35 · 2264d35
1 parent 59714db
commit 2264d35
Show file tree

Hide file tree

Showing 3 changed files with 54 additions and 6 deletions.
diff --git a/packages/next/client/image.tsx b/packages/next/client/image.tsx
@@ -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(
@@ -466,7 +462,7 @@ export default function Image({
       )
     }
 
-    if (!unoptimized) {
+    if (!unoptimized && loader !== defaultImageLoader) {
       const urlStr = loader({
         config,
         src,
@@ -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
   }`

diff --git a/test/integration/image-component/default/pages/loader-svg.js b/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
diff --git a/test/integration/image-component/default/test/index.test.js b/test/integration/image-component/default/test/index.test.js
@@ -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`),
@@ -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 () => {