[next]: fix issues with rewrite normalization of index routes (#11707)

The Next.js builder currently normalizes user rewrites to ensure the internal `.rsc` / `.action` rewrites are still handled even after following the rewrite. However we have special rewrites: `/index.rsc` and `/index.action`. When those routes pass through the normalization logic, a request routed to a catch-all route (e.g. `[[...param]]`) will go from `/` -> `/index.rsc` -> `<someRewriteString>?param=index`. This is inconsistent `next start` or other spots where the param should be an empty string. Similarly, if a user rewrites their entire app to a subdirectory (eg `/:path*` -> `/base/:path*`, a route will go from `/` -> `/index.rsc` -> `/base/index.rsc` which won't exist, causing a 404. The solution here is to return the `index.rsc` / `index.action` routes back to `/` so they can be handled by user rewrites, if necessary. This also disables the `hasActionOutputSupport` flag if `routesManifest.i18n` is detected as they are not compatible.
vercel · Jun 6, 2024 · fa9789a · fa9789a
1 parent 5c12ed6
commit fa9789a
Show file tree

Hide file tree

Showing 18 changed files with 273 additions and 2 deletions.
diff --git a/.changeset/rare-jeans-attend.md b/.changeset/rare-jeans-attend.md
@@ -0,0 +1,5 @@
+---
+'@vercel/next': patch
+---
+
+prevent /index from being incorrectly normalized in rewrites
diff --git a/packages/next/src/server-build.ts b/packages/next/src/server-build.ts
@@ -212,7 +212,8 @@ export async function serverBuild({
   );
   const hasActionOutputSupport =
     semver.gte(nextVersion, ACTION_OUTPUT_SUPPORT_VERSION) &&
-    Boolean(process.env.NEXT_EXPERIMENTAL_STREAMING_ACTIONS);
+    Boolean(process.env.NEXT_EXPERIMENTAL_STREAMING_ACTIONS) &&
+    !routesManifest.i18n;
   const projectDir = requiredServerFilesManifest.relativeAppDir
     ? path.join(baseDir, requiredServerFilesManifest.relativeAppDir)
     : requiredServerFilesManifest.appDir || entryPath;
@@ -2085,6 +2086,22 @@ export async function serverBuild({
           ]
         : []),
 
+      // before processing rewrites, remove any special `/index` routes that were added
+      // as these won't be properly normalized by `afterFilesRewrites` / `dynamicRoutes`
+      ...(appPathRoutesManifest
+        ? [
+            {
+              src: path.posix.join(
+                '/',
+                entryDirectory,
+                '/index(\\.action|\\.rsc)'
+              ),
+              dest: path.posix.join('/', entryDirectory),
+              continue: true,
+            },
+          ]
+        : []),
+
       // These need to come before handle: miss or else they are grouped
       // with that routing section
       ...afterFilesRewrites,

diff --git a/...ext/test/fixtures/00-app-dir-actions-experimental-streaming-index-handling/app/actions.js b/...ext/test/fixtures/00-app-dir-actions-experimental-streaming-index-handling/app/actions.js
@@ -0,0 +1,6 @@
+'use server';
+
+export async function increment(value) {
+  await new Promise(resolve => setTimeout(resolve, 500));
+  return value + 1;
+}
diff --git a/...next/test/fixtures/00-app-dir-actions-experimental-streaming-index-handling/app/layout.js b/...next/test/fixtures/00-app-dir-actions-experimental-streaming-index-handling/app/layout.js
@@ -0,0 +1,10 @@
+export default function Root({ children }) {
+    return (
+      <html>
+        <head>
+          <title>Hello World</title>
+        </head>
+        <body>{children}</body>
+      </html>
+    );
+  }
diff --git a/...test/fixtures/00-app-dir-actions-experimental-streaming-index-handling/app/static/page.js b/...test/fixtures/00-app-dir-actions-experimental-streaming-index-handling/app/static/page.js
@@ -0,0 +1,25 @@
+"use client";
+
+import { useState } from "react";
+import { increment } from "../actions";
+
+export default function Home() {
+  const [count, setCount] = useState(0);
+
+  return (
+    <div>
+      {count}
+      <button
+        onClick={async () => {
+          const actionResult = await increment(count);
+          // @ts-ignore
+          setCount(actionResult);
+          console.log(actionResult);
+        }}
+      >
+        Trigger
+      </button>
+      Static
+    </div>
+  );
+}
diff --git a/...next/test/fixtures/00-app-dir-actions-experimental-streaming-index-handling/index.test.js b/...next/test/fixtures/00-app-dir-actions-experimental-streaming-index-handling/index.test.js
@@ -0,0 +1,55 @@
+/* eslint-env jest */
+const path = require('path');
+const { deployAndTest } = require('../../utils');
+const fetch = require('../../../../../test/lib/deployment/fetch-retry');
+
+const ctx = {};
+
+function findActionId(page, runtime) {
+  page = `app${page}/page`; // add /app prefix and /page suffix
+
+  for (const [actionId, details] of Object.entries(
+    ctx.actionManifest[runtime]
+  )) {
+    if (details.workers[page]) {
+      return actionId;
+    }
+  }
+
+  throw new Error("Couldn't find action ID");
+}
+
+describe(`${__dirname.split(path.sep).pop()}`, () => {
+  beforeAll(async () => {
+    const info = await deployAndTest(__dirname);
+
+    const actionManifest = await fetch(
+      `${info.deploymentUrl}/server-reference-manifest.json`
+    ).then(res => res.json());
+
+    ctx.actionManifest = actionManifest;
+
+    Object.assign(ctx, info);
+  });
+
+  it('should work when there is a rewrite targeting the root page', async () => {
+    const actionId = findActionId('/static', 'node');
+
+    const res = await fetch(ctx.deploymentUrl, {
+      method: 'POST',
+      body: JSON.stringify([1337]),
+      headers: {
+        'Content-Type': 'text/plain;charset=UTF-8',
+        'Next-Action': actionId,
+      },
+    });
+
+    expect(res.status).toEqual(200);
+    expect(res.headers.get('x-matched-path')).toBe('/static/');
+    expect(res.headers.get('x-vercel-cache')).toBe('BYPASS');
+
+    const body = await res.text();
+    // The action incremented the provided count by 1
+    expect(body).toContain('1338');
+  });
+});
diff --git a/...ext/test/fixtures/00-app-dir-actions-experimental-streaming-index-handling/next.config.js b/...ext/test/fixtures/00-app-dir-actions-experimental-streaming-index-handling/next.config.js
@@ -0,0 +1,10 @@
+module.exports = {
+  rewrites() {
+    return [
+      {
+        source: '/:path*',
+        destination: '/static/:path*',
+      },
+    ];
+  },
+};
diff --git a/.../next/test/fixtures/00-app-dir-actions-experimental-streaming-index-handling/package.json b/.../next/test/fixtures/00-app-dir-actions-experimental-streaming-index-handling/package.json
@@ -0,0 +1,9 @@
+{
+  "dependencies": {
+    "next": "canary"
+  },
+  "scripts": {
+    "build": "next build && cp .next/server/server-reference-manifest.json public/"
+  },
+  "ignoreNextjsUpdates": true
+}
diff --git a/.../next/test/fixtures/00-app-dir-actions-experimental-streaming-index-handling/public/.keep b/.../next/test/fixtures/00-app-dir-actions-experimental-streaming-index-handling/public/.keep
diff --git a/...s/next/test/fixtures/00-app-dir-actions-experimental-streaming-index-handling/vercel.json b/...s/next/test/fixtures/00-app-dir-actions-experimental-streaming-index-handling/vercel.json
@@ -0,0 +1,14 @@
+{
+  "builds": [
+    {
+      "src": "package.json",
+      "use": "@vercel/next"
+    }
+  ],
+  "build": {
+    "env": {
+      "NEXT_EXPERIMENTAL_STREAMING_ACTIONS": "1"
+    }
+  },
+  "probes": []
+}
diff --git a/packages/next/test/fixtures/00-app-dir-actions-experimental-streaming/index.test.js b/packages/next/test/fixtures/00-app-dir-actions-experimental-streaming/index.test.js
@@ -15,7 +15,8 @@ function findActionId(page, runtime) {
       return actionId;
     }
   }
-  return null;
+
+  throw new Error("Couldn't find action ID");
 }
 
 function generateFormDataPayload(actionId) {
@@ -313,6 +314,21 @@ describe(`${__dirname.split(path.sep).pop()}`, () => {
         expect(res.headers.get('x-edge-runtime')).toBe('1');
       }
     });
+
+    it('should work on the index route', async () => {
+      const canonicalPath = '/';
+      const actionId = findActionId('', 'node');
+
+      const res = await fetch(
+        `${ctx.deploymentUrl}${canonicalPath}`,
+        generateFormDataPayload(actionId)
+      );
+
+      expect(res.status).toEqual(200);
+      expect(res.headers.get('x-matched-path')).toBe('/index.action');
+      expect(res.headers.get('content-type')).toBe('text/x-component');
+      expect(res.headers.get('x-vercel-cache')).toBe('MISS');
+    });
   });
 
   describe('rewrite to index', () => {
@@ -330,6 +346,28 @@ describe(`${__dirname.split(path.sep).pop()}`, () => {
       expect(res.headers.get('content-type')).toBe('text/x-component');
       expect(res.headers.get('x-vercel-cache')).toBe('MISS');
     });
+
+    it('should work when entire path is rewritten', async () => {
+      const actionId = findActionId('/static', 'node');
+
+      const res = await fetch(ctx.deploymentUrl, {
+        method: 'POST',
+        body: JSON.stringify([1337]),
+        headers: {
+          'Content-Type': 'text/plain;charset=UTF-8',
+          'Next-Action': actionId,
+          'x-rewrite-me': '1',
+        },
+      });
+
+      expect(res.status).toEqual(200);
+      expect(res.headers.get('x-matched-path')).toBe('/index.action');
+      expect(res.headers.get('x-vercel-cache')).toBe('MISS');
+
+      const body = await res.text();
+      // The action incremented the provided count by 1
+      expect(body).toContain('1338');
+    });
   });
 
   describe('pages', () => {

diff --git a/packages/next/test/fixtures/00-app-dir-actions-experimental-streaming/next.config.js b/packages/next/test/fixtures/00-app-dir-actions-experimental-streaming/next.config.js
@@ -21,6 +21,16 @@ module.exports = {
         source: '/rewritten-to-index',
         destination: '/?fromRewrite=1',
       },
+      {
+        source: '/:path*',
+        destination: '/static/:path*',
+        has: [
+          {
+            type: 'header',
+            key: 'x-rewrite-me',
+          },
+        ],
+      },
     ];
   },
 };
diff --git a/packages/next/test/fixtures/00-app-dir-catchall-index-handling/app/[[...slug]]/page.js b/packages/next/test/fixtures/00-app-dir-catchall-index-handling/app/[[...slug]]/page.js
@@ -0,0 +1,14 @@
+import Link from 'next/link';
+
+const Page = ({ params }) => {
+  return (
+    <div>
+      <div id="page-param">page-param-{params.slug?.[0] ?? ''}</div>
+      <Link href="/">Home</Link>
+      <Link href="/foo">Foo</Link>
+      <Link href="/bar">Bar</Link>
+    </div>
+  );
+};
+
+export default Page;
diff --git a/packages/next/test/fixtures/00-app-dir-catchall-index-handling/app/layout.js b/packages/next/test/fixtures/00-app-dir-catchall-index-handling/app/layout.js
@@ -0,0 +1,10 @@
+export default function Root({ children }) {
+    return (
+      <html>
+        <head>
+          <title>Hello World</title>
+        </head>
+        <body>{children}</body>
+      </html>
+    );
+  }
diff --git a/packages/next/test/fixtures/00-app-dir-catchall-index-handling/index.test.js b/packages/next/test/fixtures/00-app-dir-catchall-index-handling/index.test.js
@@ -0,0 +1,12 @@
+/* eslint-env jest */
+const path = require('path');
+const { deployAndTest } = require('../../utils');
+
+const ctx = {};
+
+describe(`${__dirname.split(path.sep).pop()}`, () => {
+  it('should deploy and pass probe checks', async () => {
+    const info = await deployAndTest(__dirname);
+    Object.assign(ctx, info);
+  });
+});
diff --git a/packages/next/test/fixtures/00-app-dir-catchall-index-handling/next.config.js b/packages/next/test/fixtures/00-app-dir-catchall-index-handling/next.config.js
@@ -0,0 +1 @@
+module.exports = {};
diff --git a/packages/next/test/fixtures/00-app-dir-catchall-index-handling/package.json b/packages/next/test/fixtures/00-app-dir-catchall-index-handling/package.json
@@ -0,0 +1,8 @@
+{
+  "dependencies": {
+    "next": "canary",
+    "react": "19.0.0-rc-f994737d14-20240522",
+    "react-dom": "19.0.0-rc-f994737d14-20240522"
+  },
+  "ignoreNextjsUpdates": true
+}
diff --git a/packages/next/test/fixtures/00-app-dir-catchall-index-handling/vercel.json b/packages/next/test/fixtures/00-app-dir-catchall-index-handling/vercel.json
@@ -0,0 +1,27 @@
+{
+  "builds": [
+    {
+      "src": "package.json",
+      "use": "@vercel/next"
+    }
+  ],
+  "probes": [
+    {
+      "path": "/",
+      "status": 200,
+      "mustContain": "\"page-param-\",\"\"",
+      "mustNotContain": "\"page-param-\",\"index\"",
+      "headers": {
+        "RSC": "1"
+      }
+    },
+    {
+      "path": "/foo",
+      "status": 200,
+      "mustContain": "\"page-param-\",\"foo\"",
+      "headers": {
+        "RSC": "1"
+      }
+    }
+  ]
+}