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.

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

Error on navigation API usage in pages router and middleware #73100

Merged
merged 6 commits into from
Nov 27, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
9 changes: 9 additions & 0 deletions packages/next/src/build/templates/middleware.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import { adapter } from '../../server/web/adapter'
// Import the userland code.
import * as _mod from 'VAR_USERLAND'
import { edgeInstrumentationOnRequestError } from '../../server/web/globals'
import { isNextRouterError } from '../../client/components/is-next-router-error'

const mod = { ..._mod }
const handler = mod.middleware || mod.default
Expand All @@ -26,6 +27,14 @@ function errorHandledHandler(fn: AdapterOptions['handler']) {
try {
return await fn(...args)
} catch (err) {
// In development, error the navigation API usage in runtime,
// since it's not allowed to be used in middleware as it's outside of react component tree.
if (process.env.NODE_ENV !== 'production') {
if (isNextRouterError(err)) {
err.message = `Next.js navigation API is not allowed to be used in Middleware.`
throw err
}
}
const req = args[0]
const url = new URL(req.url)
const resource = url.pathname + url.search
Expand Down
12 changes: 11 additions & 1 deletion packages/next/src/client/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ import {
import { onRecoverableError } from './react-client-callbacks/shared'
import tracer from './tracing/tracer'
import reportToSocket from './tracing/report-to-socket'
import { isNextRouterError } from './components/is-next-router-error'

/// <reference types="react-dom/experimental" />

Expand Down Expand Up @@ -927,7 +928,16 @@ export async function hydrate(opts?: { beforeRender?: () => Promise<void> }) {

error.name = initialErr!.name
error.stack = initialErr!.stack
throw getServerError(error, initialErr!.source!)
const errSource = initialErr.source!

// In development, error the navigation API usage in runtime,
// since it's not allowed to be used in pages router as it doesn't contain error boundary like app router.
if (isNextRouterError(initialErr)) {
error.message =
'Next.js navigation API is not allowed to be used in Pages Router.'
}

throw getServerError(error, errSource)
})
}
// We replaced the server-side error with a client-side error, and should
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
import { ReactNode } from 'react'
export default function Root({ children }: { children: ReactNode }) {
return (
<html>
<body>{children}</body>
</html>
)
}
3 changes: 3 additions & 0 deletions test/development/app-dir/server-navigation-error/app/page.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
export default function Page() {
return <p>hello world</p>
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
import { notFound } from 'next/navigation'

export async function GET(req) {
notFound()
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
import { redirect } from 'next/navigation'

export async function GET(req) {
redirect('/')
}
10 changes: 10 additions & 0 deletions test/development/app-dir/server-navigation-error/middleware.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
import { notFound, redirect } from 'next/navigation'
import { NextRequest } from 'next/server'

export default function middleware(req: NextRequest) {
if (req.nextUrl.pathname === '/middleware/not-found') {
notFound()
} else if (req.nextUrl.pathname === '/middleware/redirect') {
redirect('/')
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
/**
* @type {import('next').NextConfig}
*/
const nextConfig = {}

module.exports = nextConfig
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
import { notFound } from 'next/navigation'

export default function Page() {
notFound()
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
import { redirect } from 'next/navigation'

export default function Page() {
redirect('/')
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,151 @@
import { nextTestSetup } from 'e2e-utils'
import {
assertHasRedbox,
getRedboxDescription,
getRedboxSource,
} from 'next-test-utils'

describe('server-navigation-error', () => {
const { next } = nextTestSetup({
files: __dirname,
})

describe('pages router', () => {
it('should error on navigation API redirect', async () => {
const browser = await next.browser('/pages/redirect')
await assertHasRedbox(browser)
expect(await getRedboxDescription(browser)).toMatch(
`Next.js navigation API is not allowed to be used in Pages Router.`
)
const source = await getRedboxSource(browser)
if (process.env.TURBOPACK) {
expect(source).toMatchInlineSnapshot(`
"pages/pages/redirect.tsx (4:10) @ Page

2 |
3 | export default function Page() {
> 4 | redirect('/')
| ^
5 | }
6 |"
`)
} else {
expect(source).toMatchInlineSnapshot(`
"pages/pages/redirect.tsx (4:12) @ Page

2 |
3 | export default function Page() {
> 4 | redirect('/')
| ^
5 | }
6 |"
`)
}
})

it('should error on navigation API notFound', async () => {
const browser = await next.browser('/pages/not-found')
await assertHasRedbox(browser)
expect(await getRedboxDescription(browser)).toMatch(
`Next.js navigation API is not allowed to be used in Pages Router.`
)
const source = await getRedboxSource(browser)
if (process.env.TURBOPACK) {
expect(source).toMatchInlineSnapshot(`
"pages/pages/not-found.tsx (4:10) @ Page

2 |
3 | export default function Page() {
> 4 | notFound()
| ^
5 | }
6 |"
`)
} else {
expect(source).toMatchInlineSnapshot(`
"pages/pages/not-found.tsx (4:11) @ notFound

2 |
3 | export default function Page() {
> 4 | notFound()
| ^
5 | }
6 |"
`)
}
})
})

describe('middleware', () => {
it('should error on navigation API redirect ', async () => {
const browser = await next.browser('/middleware/redirect')
// FIXME: the first request to middleware error load didn't show the redbox, need one more reload
await browser.refresh()
await assertHasRedbox(browser)
expect(await getRedboxDescription(browser)).toMatch(
`Next.js navigation API is not allowed to be used in Middleware.`
)
const source = await getRedboxSource(browser)
if (process.env.TURBOPACK) {
expect(source).toMatchInlineSnapshot(`
"middleware.ts (8:12) @ middleware

6 | notFound()
7 | } else if (req.nextUrl.pathname === '/middleware/redirect') {
> 8 | redirect('/')
| ^
9 | }
10 | }
11 |"
`)
} else {
expect(source).toMatchInlineSnapshot(`
"middleware.ts (8:14) @ middleware

6 | notFound()
7 | } else if (req.nextUrl.pathname === '/middleware/redirect') {
> 8 | redirect('/')
| ^
9 | }
10 | }
11 |"
`)
}
})

it('should error on navigation API not-found', async () => {
const browser = await next.browser('/middleware/not-found')
await assertHasRedbox(browser)
expect(await getRedboxDescription(browser)).toMatch(
`Next.js navigation API is not allowed to be used in Middleware.`
)
const source = await getRedboxSource(browser)

if (process.env.TURBOPACK) {
expect(source).toMatchInlineSnapshot(`
"middleware.ts (6:12) @ middleware

4 | export default function middleware(req: NextRequest) {
5 | if (req.nextUrl.pathname === '/middleware/not-found') {
> 6 | notFound()
| ^
7 | } else if (req.nextUrl.pathname === '/middleware/redirect') {
8 | redirect('/')
9 | }"
`)
} else {
expect(source).toMatchInlineSnapshot(`
"middleware.ts (6:13) @ notFound

4 | export default function middleware(req: NextRequest) {
5 | if (req.nextUrl.pathname === '/middleware/not-found') {
> 6 | notFound()
| ^
7 | } else if (req.nextUrl.pathname === '/middleware/redirect') {
8 | redirect('/')
9 | }"
`)
}
})
})
})
Loading