mirror of
https://github.com/LukeHagar/vercel.git
synced 2025-12-06 04:22:01 +00:00
[next]: ensure unmatched action rewrites are routed to correct handler (#11686)
User defined rewrites are "normalized" so that our internal rewrites are still properly handled. Before normalizing these rewrites, the Next.js builder will attempt to match server action requests to a`.action` variant. Then the user-defined rewrites flow through the afterFiles normalization ([this part](https://github.com/vercel/vercel/blob/fix/unmatched-action-rewrites/packages/next/src/server-build.ts#L254-L279)) so that when we add `.action` in the builder, we don't drop the suffix. But this normalization can lead to a malformed `dest`. e.g., if I had rewrite like this: ```js { source: '/greedy-rewrite/static/:path*', destination: '/static/:path*', } ``` The builder would go through this flow on an action request to `/greedy-rewrite/static`: 1. It'll attempt to match it to a `.action` output, so `/greedy-rewrite/static` -> `/greedy-rewrite/static.action` 2. The afterFiles normalization will take place, so the original `dest` of `/static/$1` will become `/static/$1$rscsuff` 3. $1 will be an empty string, because it doesn't match the existing capture group. So now `/greedy-rewrite/static.action` -> `/greedy-rewrite/static/.action` 4. `static/.action` is not a valid output, so it'll 404 and the action will break. Existing handling exists for `.rsc` outputs for a similar reason, but only on the index route. I added a similar fix for this in #11688.
This commit is contained in:
5
.changeset/mean-peas-visit.md
Normal file
5
.changeset/mean-peas-visit.md
Normal file
@@ -0,0 +1,5 @@
|
||||
---
|
||||
'@vercel/next': patch
|
||||
---
|
||||
|
||||
ensure unmatched action rewrites are routed to correct handler
|
||||
@@ -2089,6 +2089,25 @@ export async function serverBuild({
|
||||
// with that routing section
|
||||
...afterFilesRewrites,
|
||||
|
||||
// Ensure that after we normalize `afterFilesRewrites`, unmatched actions are routed to the correct handler
|
||||
// e.g. /foo/.action -> /foo.action. This should only ever match in cases where we're routing to an action handler
|
||||
// and the rewrite normalization led to something like /foo/$1$rscsuff, and $1 had no match.
|
||||
// This is meant to have parity with the .rsc handling below.
|
||||
...(hasActionOutputSupport
|
||||
? [
|
||||
{
|
||||
src: `${path.posix.join('/', entryDirectory, '/\\.action$')}`,
|
||||
dest: `${path.posix.join('/', entryDirectory, '/index.action')}`,
|
||||
check: true,
|
||||
},
|
||||
{
|
||||
src: `${path.posix.join('/', entryDirectory, '(.+)/\\.action$')}`,
|
||||
dest: `${path.posix.join('/', entryDirectory, '$1.action')}`,
|
||||
check: true,
|
||||
},
|
||||
]
|
||||
: []),
|
||||
|
||||
// ensure non-normalized /.rsc from rewrites is handled
|
||||
...(appPathRoutesManifest
|
||||
? [
|
||||
|
||||
25
packages/next/test/fixtures/00-app-dir-actions-experimental-streaming/app/edge/static/page.js
vendored
Normal file
25
packages/next/test/fixtures/00-app-dir-actions-experimental-streaming/app/edge/static/page.js
vendored
Normal file
@@ -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>
|
||||
);
|
||||
}
|
||||
25
packages/next/test/fixtures/00-app-dir-actions-experimental-streaming/app/page.js
vendored
Normal file
25
packages/next/test/fixtures/00-app-dir-actions-experimental-streaming/app/page.js
vendored
Normal file
@@ -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>
|
||||
);
|
||||
}
|
||||
25
packages/next/test/fixtures/00-app-dir-actions-experimental-streaming/app/static/page.js
vendored
Normal file
25
packages/next/test/fixtures/00-app-dir-actions-experimental-streaming/app/static/page.js
vendored
Normal file
@@ -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>
|
||||
);
|
||||
}
|
||||
@@ -293,6 +293,43 @@ describe(`${__dirname.split(path.sep).pop()}`, () => {
|
||||
expect(res.headers.get('x-edge-runtime')).toBe('1');
|
||||
}
|
||||
});
|
||||
|
||||
it('should work when a rewrite greedy matches an action rewrite', async () => {
|
||||
const targetPath = `${basePath}/static`;
|
||||
const canonicalPath = `/greedy-rewrite/${basePath}/static`;
|
||||
const actionId = findActionId(targetPath, runtime);
|
||||
|
||||
const res = await fetch(
|
||||
`${ctx.deploymentUrl}${canonicalPath}`,
|
||||
generateFormDataPayload(actionId)
|
||||
);
|
||||
|
||||
expect(res.status).toEqual(200);
|
||||
expect(res.headers.get('x-matched-path')).toBe(targetPath + '.action');
|
||||
expect(res.headers.get('content-type')).toBe('text/x-component');
|
||||
if (runtime === 'node') {
|
||||
expect(res.headers.get('x-vercel-cache')).toBe('MISS');
|
||||
} else {
|
||||
expect(res.headers.get('x-edge-runtime')).toBe('1');
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
describe('rewrite to index', () => {
|
||||
it('should work when user has a rewrite to the index route', async () => {
|
||||
const canonicalPath = '/rewritten-to-index';
|
||||
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('pages', () => {
|
||||
|
||||
@@ -9,6 +9,18 @@ module.exports = {
|
||||
source: '/rewrite/edge/rsc/static',
|
||||
destination: '/edge/rsc/static',
|
||||
},
|
||||
{
|
||||
source: '/greedy-rewrite/static/:path*',
|
||||
destination: '/static/:path*',
|
||||
},
|
||||
{
|
||||
source: '/greedy-rewrite/edge/static/:path*',
|
||||
destination: '/edge/static/:path*',
|
||||
},
|
||||
{
|
||||
source: '/rewritten-to-index',
|
||||
destination: '/?fromRewrite=1',
|
||||
},
|
||||
];
|
||||
},
|
||||
};
|
||||
|
||||
Reference in New Issue
Block a user