Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix server action edge redirect with middleware rewrite #67148

Merged
merged 2 commits into from
Jun 24, 2024
Merged

Fix server action edge redirect with middleware rewrite #67148

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
6061936
Fix server action edge redirect with middleware rewrite
huozhi Jun 24, 2024
05b3f7d
revert
huozhi Jun 24, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion packages/next/src/server/web/adapter.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -300,7 +300,7 @@ export async function adapter(
* a data URL if the request was a data request.
*/
const rewrite = response?.headers.get('x-middleware-rewrite')
if (response && rewrite) {
if (response && rewrite && !isEdgeRendering) {
Copy link
Member Author

@huozhi huozhi Jun 24, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

aligning with redirect logic here using !isEdgeRendering.

I think we might able to delete these re-construction here later as web/adapater should always for edge runtime? but this time keep the fix simple cc @ijjk @ztanner

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess this is only necessary because we use this adapter in

next.js/packages/next/src/build/templates/middleware.ts

Line 24 in f9d12d1

return adapter({
where we don't know yet what runtime we have.

So I guess we just abstracted too early and templates/middleware.ts should use a different adapter?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just checked the __BUILD_MANIFEST is not avialble in middleware, I guess that's what it was trying to do there. Probably can give a different flag later to identify which is middleware and which are other normal edge functions

const rewriteUrl = new NextURL(rewrite, {
forceLocale: true,
headers: params.request.headers,
Expand Down
11 changes: 11 additions & 0 deletions test/e2e/app-dir/server-actions-redirect-middleware-rewrite/app/actions.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
'use server'

import { redirect } from 'next/navigation'

export async function relativeRedirect() {
return redirect('./subpage')
}

export async function absoluteRedirect() {
return redirect('/subpage')
}
11 changes: 11 additions & 0 deletions test/e2e/app-dir/server-actions-redirect-middleware-rewrite/app/layout.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
export default function RootLayout({
children,
}: Readonly<{
children: React.ReactNode
}>) {
return (
<html>
<body>{children}</body>
</html>
)
}
3 changes: 3 additions & 0 deletions test/e2e/app-dir/server-actions-redirect-middleware-rewrite/app/redirect/page.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
export default function Page() {
return <div id="redirected">Redirected</div>
}
7 changes: 7 additions & 0 deletions test/e2e/app-dir/server-actions-redirect-middleware-rewrite/app/server-action/_action.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
'use server'

import { redirect } from 'next/navigation'

export const redirectAction = async () => {
redirect('/redirect')
}
13 changes: 13 additions & 0 deletions test/e2e/app-dir/server-actions-redirect-middleware-rewrite/app/server-action/edge/page.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
'use client'

import { redirectAction } from '../_action'

export default function Page() {
return (
<form action={redirectAction}>
<button type="submit">Submit</button>
</form>
)
}

export const runtime = 'edge'
13 changes: 13 additions & 0 deletions test/e2e/app-dir/server-actions-redirect-middleware-rewrite/app/server-action/node/page.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
'use client'

import { redirectAction } from '../_action'

export default function Page() {
return (
<form action={redirectAction}>
<button type="submit">Submit</button>
</form>
)
}

export const runtime = 'nodejs'
5 changes: 5 additions & 0 deletions test/e2e/app-dir/server-actions-redirect-middleware-rewrite/middleware.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
import { NextRequest, NextResponse } from 'next/server'

export default function middleware(request: NextRequest) {
return NextResponse.rewrite(request.url)
}
33 changes: 33 additions & 0 deletions ...er-actions-redirect-middleware-rewrite/server-actions-redirect-middleware-rewrite.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
import { nextTestSetup } from 'e2e-utils'
import { retry } from 'next-test-utils'

describe('app-dir - server-actions-redirect-middleware-rewrite.test', () => {
const { next } = nextTestSetup({
files: __dirname,
})

it('should redirect correctly in nodejs runtime with middleware rewrite', async () => {
const browser = await next.browser('/server-action/node')
await browser.waitForElementByCss('button').click()

await retry(async () => {
expect(await browser.waitForElementByCss('#redirected').text()).toBe(
'Redirected'
)
})
expect(await browser.url()).toBe(`${next.url}/redirect`)
})

it('should redirect correctly in edge runtime with middleware rewrite', async () => {
const browser = await next.browser('/server-action/edge')
await browser.waitForElementByCss('button').click()

await retry(async () => {
expect(await browser.waitForElementByCss('#redirected').text()).toBe(
'Redirected'
)

expect(await browser.url()).toBe(`${next.url}/redirect`)
})
})
})
Loading