fix: handle ipx redirect that visitors might have browser cached from…

… v4 (#390)

* test: add setup replicating runtime v4 next/image handling to test handling of potentially browser-cached v4 image handling redirects

* fix: handle ipx redirect that visitors might have browser cached from v4

src/build/image-cdn.ts

@@ -18,15 +18,27 @@ export const setImageConfig = async (ctx: PluginContext): Promise<void> => {
     return
   }
 
-  ctx.netlifyConfig.redirects.push({
-    from: imageEndpointPath,
-    // w and q are too short to be used as params with id-length rule
-    // but we are forced to do so because of the next/image loader decides on their names
-    // eslint-disable-next-line id-length
-    query: { url: ':url', w: ':width', q: ':quality' },
-    to: '/.netlify/images?url=:url&w=:width&q=:quality',
-    status: 200,
-  })
+  ctx.netlifyConfig.redirects.push(
+    {
+      from: imageEndpointPath,
+      // w and q are too short to be used as params with id-length rule
+      // but we are forced to do so because of the next/image loader decides on their names
+      // eslint-disable-next-line id-length
+      query: { url: ':url', w: ':width', q: ':quality' },
+      to: '/.netlify/images?url=:url&w=:width&q=:quality',
+      status: 200,
+    },
+    // when migrating from @netlify/plugin-nextjs@4 image redirect to ipx might be cached in the browser
+    {
+      from: '/_ipx/*',
+      // w and q are too short to be used as params with id-length rule
+      // but we are forced to do so because of the next/image loader decides on their names
+      // eslint-disable-next-line id-length
+      query: { url: ':url', w: ':width', q: ':quality' },
+      to: '/.netlify/images?url=:url&w=:width&q=:quality',
+      status: 200,
+    },
+  )
 
   if (remotePatterns?.length !== 0 || domains?.length !== 0) {
     ctx.netlifyConfig.images ||= { remote_images: [] }

tests/e2e/simple-app.test.ts

@@ -129,6 +129,8 @@ test.describe('next/image is using Netlify Image CDN', () => {
 
     const nextImageResponse = await nextImageResponsePromise
     expect(nextImageResponse.request().url()).toContain('_next/image?url=%2Fsquirrel.jpg')
+
+    expect(nextImageResponse.status()).toBe(200)
     // ensure next/image is using Image CDN
     // source image is jpg, but when requesting it through Image CDN avif will be returned
     expect(await nextImageResponse.headerValue('content-type')).toEqual('image/avif')
@@ -152,8 +154,10 @@ test.describe('next/image is using Netlify Image CDN', () => {
       )}`,
     )
 
-    await expect(nextImageResponse?.status()).toBe(200)
-    await expect(await nextImageResponse.headerValue('content-type')).toEqual('image/avif')
+    expect(nextImageResponse.status()).toBe(200)
+    expect(await nextImageResponse.headerValue('content-type')).toEqual('image/avif')
+
+    await expectImageWasLoaded(page.locator('img'))
   })
 
   test('Remote images: remote patterns #2 (just hostname starting with wildcard)', async ({
@@ -172,8 +176,10 @@ test.describe('next/image is using Netlify Image CDN', () => {
       )}`,
     )
 
-    await expect(nextImageResponse?.status()).toBe(200)
-    await expect(await nextImageResponse.headerValue('content-type')).toEqual('image/avif')
+    expect(nextImageResponse.status()).toBe(200)
+    expect(await nextImageResponse.headerValue('content-type')).toEqual('image/avif')
+
+    await expectImageWasLoaded(page.locator('img'))
   })
 
   test('Remote images: domains', async ({ page, simpleNextApp }) => {
@@ -189,8 +195,30 @@ test.describe('next/image is using Netlify Image CDN', () => {
       )}`,
     )
 
-    await expect(nextImageResponse?.status()).toBe(200)
-    await expect(await nextImageResponse.headerValue('content-type')).toEqual('image/avif')
+    expect(nextImageResponse?.status()).toBe(200)
+    expect(await nextImageResponse.headerValue('content-type')).toEqual('image/avif')
+
+    await expectImageWasLoaded(page.locator('img'))
+  })
+
+  test('Handling of browser-cached Runtime v4 redirect', async ({ page, simpleNextApp }) => {
+    // Runtime v4 redirects for next/image are 301 and would be cached by browser
+    // So this test checks behavior when migrating from v4 to v5 for site visitors
+    // and ensure that images are still served through Image CDN
+    const nextImageResponsePromise = page.waitForResponse('**/_ipx/**')
+
+    await page.goto(`${simpleNextApp.url}/image/migration-from-v4-runtime`)
+
+    const nextImageResponse = await nextImageResponsePromise
+    // ensure fixture is replicating runtime v4 redirect
+    expect(nextImageResponse.request().url()).toContain(
+      '_ipx/w_384,q_75/%2Fsquirrel.jpg?url=%2Fsquirrel.jpg&w=384&q=75',
+    )
+
+    expect(nextImageResponse.status()).toEqual(200)
+    expect(await nextImageResponse.headerValue('content-type')).toEqual('image/avif')
+
+    await expectImageWasLoaded(page.locator('img'))
   })
 })
 

tests/fixtures/simple-next-app/app/image/migration-from-v4-runtime/next-image-runtime-v4.js

@@ -0,0 +1,17 @@
+// Moved to separate component marked with "use client" to avoid the following error when attempting to do similar on page itself
+// Error: Functions cannot be passed directly to Client Components unless you explicitly expose it by marking it with "use server".
+//   {0: ..., loader: function}
+
+'use client'
+
+import Image from 'next/image'
+
+function RuntimeV4SimulatorImageLoader({ src, width, quality }) {
+  // replicate default Next.js image loader, just using custom endpoint that will simulate the runtime V4 behavior
+  // https://github.com/vercel/next.js/blob/c9439b5654432df6488e178e5ade6f4ad2d1cf6a/packages/next/src/shared/lib/image-loader.ts#L60
+  return `/_nextRuntimeV4ImageHandler?url=${encodeURIComponent(src)}&w=${width}&q=${quality || 75}`
+}
+
+export function NextImageWithLoaderSimulatingRuntimeV4(props) {
+  return <Image {...props} loader={RuntimeV4SimulatorImageLoader} />
+}

tests/fixtures/simple-next-app/app/image/migration-from-v4-runtime/page.js

@@ -0,0 +1,15 @@
+import { NextImageWithLoaderSimulatingRuntimeV4 } from './next-image-runtime-v4'
+
+export default function NextImageUsingNetlifyImageCDN() {
+  return (
+    <main>
+      <h1>Next/Image + Netlify Image CDN</h1>
+      <NextImageWithLoaderSimulatingRuntimeV4
+        src="/squirrel.jpg"
+        alt="a cute squirrel (next/image)"
+        width={300}
+        height={278}
+      />
+    </main>
+  )
+}

tests/fixtures/simple-next-app/netlify.toml

@@ -1,3 +1,12 @@
+[[redirects]]
+from = "/_nextRuntimeV4ImageHandler"
+# replicate runtime V4 image handler
+# https://github.com/netlify/next-runtime/blob/637e08c3f3437e5e302ec230b8c849bb61495566/packages/runtime/src/helpers/functions.ts#L254-L259
+query = { url = ":url", w = ":width", q = ":quality" }
+to = "/_ipx/w_:width,q_:quality/:url"
+status = 301
+
+
 [functions]
 directory = "netlify/functions"
 included_files = [

tests/integration/simple-app.test.ts

@@ -58,6 +58,7 @@ test<FixtureTestContext>('Test that the simple next app is working', async (ctx)
   expect(blobEntries.map(({ key }) => decodeBlobKey(key)).sort()).toEqual([
     '/404',
     '/image/local',
+    '/image/migration-from-v4-runtime',
     '/image/remote-domain',
     '/image/remote-pattern-1',
     '/image/remote-pattern-2',