Repository: vercel/next.js
Pull request: next-image: fix deployment id handling for unoptimized

Previously

```jsx
import img from "./x.png";

// x is "/_next/static/media/test.abc123.png?dpl=foo"

<Image id="static-unoptimized" src={x} unoptimized />
```

would lead to `/_next/static/media/test.abc123.png?dpl=foo&dpl=foo`

#90420 adds some related tests for the legacy loader as well

Changed files:
filename: packages/next/src/client/legacy/image.tsx
patch: @@ -420,6 +420,18 @@ function generateImgAttrs({
   loader,
 }: GenImgAttrsData): GenImgAttrsResult {
   if (unoptimized) {
+    if (src.startsWith('/') && !src.startsWith('//')) {
+      let deploymentId = getDeploymentId()
+      if (deploymentId) {
+        const srcUrl = new URL(src, 'http://n')
+        const srcDpl = srcUrl.searchParams.get('dpl')
+        if (!srcDpl) {
+          // src is missing the dpl parameter, but we have a deploymentId, so add it to the src URL
+          srcUrl.searchParams.set('dpl', deploymentId)
+          src = srcUrl.href.slice('http://n'.length)
+        }
+      }
+    }
     return { src, srcSet: undefined, sizes: undefined }
   }
 

filename: packages/next/src/shared/lib/get-img-props.ts
patch: @@ -231,10 +231,17 @@ function generateImgAttrs({
   loader,
 }: GenImgAttrsData): GenImgAttrsResult {
   if (unoptimized) {
-    const deploymentId = getDeploymentId()
-    if (src.startsWith('/') && !src.startsWith('//') && deploymentId) {
-      const sep = src.includes('?') ? '&' : '?'
-      src = `${src}${sep}dpl=${deploymentId}`
+    if (src.startsWith('/') && !src.startsWith('//')) {
+      let deploymentId = getDeploymentId()
+      if (deploymentId) {
+        const srcUrl = new URL(src, 'http://n')
+        const srcDpl = srcUrl.searchParams.get('dpl')
+        if (!srcDpl) {
+          // src is missing the dpl parameter, but we have a deploymentId, so add it to the src URL
+          srcUrl.searchParams.set('dpl', deploymentId)
+          src = srcUrl.href.slice('http://n'.length)
+        }
+      }
     }
     return { src, srcSet: undefined, sizes: undefined }
   }

filename: test/integration/next-image-new/app-dir/test/static.test.ts
patch: @@ -50,6 +50,8 @@ const runTests = (isDev) => {
         `document.getElementById("basic-static").src`
       )
       const res = await fetch(url)
+      expect(res.status).toBe(200)
+      expect(res.headers.get('content-type')).toStartWith('image/')
       expect(res.headers.get('cache-control')).toBe(
         'public, max-age=315360000, immutable'
       )
@@ -64,6 +66,8 @@ const runTests = (isDev) => {
         `document.getElementById("static-unoptimized").src`
       )
       const res = await fetch(url)
+      expect(res.status).toBe(200)
+      expect(res.headers.get('content-type')).toStartWith('image/')
       expect(res.headers.get('cache-control')).toBe(
         'public, max-age=31536000, immutable'
       )

filename: test/unit/next-image-get-img-props.test.ts
patch: @@ -1,8 +1,7 @@
 /* eslint-env jest */
 import { getImageProps } from 'next/image'
 
-let deploymentId
-
+let deploymentId: string | undefined
 jest.mock('next/dist/shared/lib/deployment-id.js', () => {
   return {
     __esModule: true,
@@ -622,7 +621,7 @@ describe('getImageProps()', () => {
       ['src', 'https://example.com/test.svg?v=1'],
     ])
   })
-  it('should add query string for imported local image when NEXT_DEPLOYMENT_ID defined', async () => {
+  it('should add query string for imported local image when deployment id is defined', async () => {
     try {
       deploymentId = 'dpl_123'
       const { props } = getImageProps({
@@ -652,7 +651,37 @@ describe('getImageProps()', () => {
       deploymentId = undefined
     }
   })
-  it('should add query string for imported local image from microfrontend when NEXT_DEPLOYMENT_ID defined', async () => {
+  it('should respect query string for imported local image when deployment id is defined', async () => {
+    try {
+      deploymentId = 'dpl_123'
+      const { props } = getImageProps({
+        alt: 'a nice desc',
+        src: '/_next/static/media/test.abc123.png?dpl=dpl_existing',
+        width: 100,
+        height: 200,
+      })
+      expect(warningMessages).toStrictEqual([])
+      expect(Object.entries(props)).toStrictEqual([
+        ['alt', 'a nice desc'],
+        ['loading', 'lazy'],
+        ['width', 100],
+        ['height', 200],
+        ['decoding', 'async'],
+        ['style', { color: 'transparent' }],
+        [
+          'srcSet',
+          '/_next/image?url=%2F_next%2Fstatic%2Fmedia%2Ftest.abc123.png&w=128&q=75&dpl=dpl_existing 1x, /_next/image?url=%2F_next%2Fstatic%2Fmedia%2Ftest.abc123.png&w=256&q=75&dpl=dpl_existing 2x',
+        ],
+        [
+          'src',
+          '/_next/image?url=%2F_next%2Fstatic%2Fmedia%2Ftest.abc123.png&w=256&q=75&dpl=dpl_existing',
+        ],
+      ])
+    } finally {
+      deploymentId = undefined
+    }
+  })
+  it('should add query string for imported local image from microfrontend when deployment id is defined', async () => {
     try {
       deploymentId = 'dpl_123'
       const { props } = getImageProps({
@@ -682,7 +711,37 @@ describe('getImageProps()', () => {
       deploymentId = undefined
     }
   })
-  it('should add query string for relative local image when NEXT_DEPLOYMENT_ID defined', async () => {
+  it('should respect existing query string for imported local image from microfrontend when deployment id is defined', async () => {
+    try {
+      deploymentId = 'dpl_123'
+      const { props } = getImageProps({
+        alt: 'a nice desc',
+        src: '/microfrontend/_next/static/media/test.abc123.png?dpl=dpl_existing', // simulating microfrontend path
+        width: 100,
+        height: 200,
+      })
+      expect(warningMessages).toStrictEqual([])
+      expect(Object.entries(props)).toStrictEqual([
+        ['alt', 'a nice desc'],
+        ['loading', 'lazy'],
+        ['width', 100],
+        ['height', 200],
+        ['decoding', 'async'],
+        ['style', { color: 'transparent' }],
+        [
+          'srcSet',
+          '/_next/image?url=%2Fmicrofrontend%2F_next%2Fstatic%2Fmedia%2Ftest.abc123.png&w=128&q=75&dpl=dpl_existing 1x, /_next/image?url=%2Fmicrofrontend%2F_next%2Fstatic%2Fmedia%2Ftest.abc123.png&w=256&q=75&dpl=dpl_existing 2x',
+        ],
+        [
+          'src',
+          '/_next/image?url=%2Fmicrofrontend%2F_next%2Fstatic%2Fmedia%2Ftest.abc123.png&w=256&q=75&dpl=dpl_existing',
+        ],
+      ])
+    } finally {
+      deploymentId = undefined
+    }
+  })
+  it('should add query string for relative local image when deployment id defined', async () => {
     try {
       deploymentId = 'dpl_123'
       const { props } = getImageProps({
@@ -709,7 +768,7 @@ describe('getImageProps()', () => {
       deploymentId = undefined
     }
   })
-  it('should not add query string for absolute remote image when NEXT_DEPLOYMENT_ID defined', async () => {
+  it('should not add query string for absolute remote image when deployment id is defined', async () => {
     try {
       deploymentId = 'dpl_123'
       const { props } = getImageProps({
@@ -739,7 +798,7 @@ describe('getImageProps()', () => {
       deploymentId = undefined
     }
   })
-  it('should add query string with question mark for unoptimized relative svg when NEXT_DEPLOYMENT_ID defined', async () => {
+  it('should add query string with question mark for unoptimized relative svg when deployment id is defined', async () => {
     try {
       deploymentId = 'dpl_123'
       const { props } = getImageProps({
@@ -762,7 +821,7 @@ describe('getImageProps()', () => {
       deploymentId = undefined
     }
   })
-  it('should add query string with ampersand for unoptimized relative svg when NEXT_DEPLOYMENT_ID defined', async () => {
+  it('should add query string with ampersand for unoptimized relative svg when deployment id is defined', async () => {
     try {
       deploymentId = 'dpl_123'
       const { props } = getImageProps({
@@ -785,7 +844,7 @@ describe('getImageProps()', () => {
       deploymentId = undefined
     }
   })
-  it('should not add query string for unoptimized absolute remote svg when NEXT_DEPLOYMENT_ID defined', async () => {
+  it('should not add query string for unoptimized absolute remote svg when deployment id is defined', async () => {
     try {
       deploymentId = 'dpl_123'
       const { props } = getImageProps({
@@ -808,7 +867,7 @@ describe('getImageProps()', () => {
       deploymentId = undefined
     }
   })
-  it('should not add query string for unoptimized with no protocol when NEXT_DEPLOYMENT_ID defined', async () => {
+  it('should not add query string for unoptimized with no protocol when deployment id is defined', async () => {
     try {
       deploymentId = 'dpl_123'
       const { props } = getImageProps({
@@ -832,4 +891,29 @@ describe('getImageProps()', () => {
       deploymentId = undefined
     }
   })
+
+  it('should respect existing query string for unoptimized relative image when deployment id is defined', async () => {
+    try {
+      deploymentId = 'dpl_123'
+      const { props } = getImageProps({
+        alt: 'a nice desc',
+        src: '/_next/static/media/test.abc123.png?dpl=dpl_existing',
+        unoptimized: true,
+        width: 100,
+        height: 200,
+      })
+      expect(warningMessages).toStrictEqual([])
+      expect(Object.entries(props)).toStrictEqual([
+        ['alt', 'a nice desc'],
+        ['loading', 'lazy'],
+        ['width', 100],
+        ['height', 200],
+        ['decoding', 'async'],
+        ['style', { color: 'transparent' }],
+        ['src', '/_next/static/media/test.abc123.png?dpl=dpl_existing'],
+      ])
+    } finally {
+      deploymentId = undefined
+    }
+  })
 })

filename: test/unit/next-image-legacy.test.ts
patch: @@ -4,6 +4,16 @@ import ReactDOM from 'react-dom/server'
 import Image from 'next/legacy/image'
 import cheerio from 'cheerio'
 
+let deploymentId: string | undefined
+jest.mock('next/dist/shared/lib/deployment-id.js', () => {
+  return {
+    __esModule: true,
+    getDeploymentId() {
+      return deploymentId
+    },
+  }
+})
+
 describe('Image Legacy Rendering', () => {
   it('should render Image on its own', async () => {
     const element = React.createElement(Image, {
@@ -94,4 +104,63 @@ describe('Image Legacy Rendering', () => {
       '/_next/image?url=%2Ftest.png&w=384&q=75 384w'
     )
   })
+
+  it.each([
+    {
+      name: 'without deployment add',
+      src: '/_next/static/media/test.3f1a293b.png',
+      expected: `/_next/image?url=${encodeURIComponent('/_next/static/media/test.3f1a293b.png')}&w=3840&q=75`,
+    },
+    {
+      name: 'adding a deployment id',
+      src: '/_next/static/media/test.3f1a293b.png',
+      dpl: 'dpl_123',
+      expected: `/_next/image?url=${encodeURIComponent('/_next/static/media/test.3f1a293b.png')}&w=3840&q=75&dpl=dpl_123`,
+    },
+    {
+      name: 'respect existing deployment id',
+      src: '/_next/static/media/test.3f1a293b.png?dpl=dpl_existing',
+      dpl: 'dpl_123',
+      expected: `/_next/image?url=${encodeURIComponent('/_next/static/media/test.3f1a293b.png')}&w=3840&q=75&dpl=dpl_existing`,
+    },
+    {
+      name: 'unoptimized without deployment add',
+      unoptimized: true,
+      src: '/_next/static/media/test.3f1a293b.png',
+      expected: '/_next/static/media/test.3f1a293b.png',
+    },
+    {
+      name: 'unoptimized adding a deployment id',
+      unoptimized: true,
+      src: '/_next/static/media/test.3f1a293b.png',
+      dpl: 'dpl_123',
+      expected: '/_next/static/media/test.3f1a293b.png?dpl=dpl_123',
+    },
+    {
+      name: 'unoptimized respect existing deployment id',
+      unoptimized: true,
+      src: '/_next/static/media/test.3f1a293b.png?dpl=dpl_existing',
+      dpl: 'dpl_123',
+      expected: '/_next/static/media/test.3f1a293b.png?dpl=dpl_existing',
+    },
+  ])(
+    'should correctly deployment ids: $name',
+    async ({ src, unoptimized, dpl, expected }) => {
+      deploymentId = dpl
+      try {
+        const element = React.createElement(Image, {
+          src,
+          unoptimized,
+          width: 100,
+          height: 100,
+          sizes: '50vw',
+        })
+        const $ = cheerio.load(ReactDOM.renderToString(element))
+        const noscriptImg = $('noscript img')
+        expect(noscriptImg.attr('src')).toBe(expected)
+      } finally {
+        deploymentId = undefined
+      }
+    }
+  )
 })