Skip to content

Commit

Permalink
Fix redirect under suspense boundary with basePath (#62597)
Browse files Browse the repository at this point in the history
Fixes `redirect()` call under suspense boundary, redirect url should
include the `basePath` in the url.

`redirect()` under suspense boundaries will go through server html
insertion, which is being used every time by suspense boundary resolved,
new errors will triggered from SSR streaming. It was missing before.

Fixes NEXT-2615
Fixes #62407
  • Loading branch information
huozhi committed Mar 4, 2024
1 parent 714020f commit 400b57c
Show file tree
Hide file tree
Showing 5 changed files with 34 additions and 5 deletions.
1 change: 1 addition & 0 deletions packages/next/src/server/app-render/app-render.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -893,6 +893,7 @@ async function renderToHTMLOrFlightImpl(
polyfills,
renderServerInsertedHTML,
hasPostponed,
basePath: renderOpts.basePath,
})

const renderer = createStaticRenderer({
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,15 +8,18 @@ import {
import { renderToReadableStream } from 'react-dom/server.edge'
import { streamToString } from '../stream-utils/node-web-streams-helper'
import { RedirectStatusCode } from '../../client/components/redirect-status-code'
import { addPathPrefix } from '../../shared/lib/router/utils/add-path-prefix'

export function makeGetServerInsertedHTML({
polyfills,
renderServerInsertedHTML,
basePath,
hasPostponed,
}: {
polyfills: JSX.IntrinsicElements['script'][]
renderServerInsertedHTML: () => React.ReactNode
hasPostponed: boolean
basePath: string
}) {
let flushedErrorMetaTagsUntilIndex = 0
// If the render had postponed, then we have already flushed the polyfills.
Expand All @@ -38,7 +41,10 @@ export function makeGetServerInsertedHTML({
) : null
)
} else if (isRedirectError(error)) {
const redirectUrl = getURLFromRedirectError(error)
const redirectUrl = addPathPrefix(
getURLFromRedirectError(error),
basePath
)
const statusCode = getRedirectStatusCodeFromError(error)
const isPermanent =
statusCode === RedirectStatusCode.PermanentRedirect ? true : false
Expand Down
4 changes: 4 additions & 0 deletions test/e2e/app-dir/app-basepath/app/dynamic/[id]/loading.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
// Have loading.js to trigger suspense boundary for /dynamic/[id] page
export default function Loading() {
return <div>loading</div>
}
11 changes: 11 additions & 0 deletions test/e2e/app-dir/app-basepath/app/dynamic/[id]/page.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
import { redirect } from 'next/navigation'

export default async function Page({ params: { id } }) {
if (id === 'source') redirect('/dynamic/dest')

return (
<div>
<p>{`id:${id}`}</p>
</div>
)
}
15 changes: 11 additions & 4 deletions test/e2e/app-dir/app-basepath/index.test.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { createNextDescribe } from 'e2e-utils'
import { check } from 'next-test-utils'
import { retry } from 'next-test-utils'

createNextDescribe(
'app dir - basepath',
Expand Down Expand Up @@ -42,10 +42,9 @@ createNextDescribe(

it('should prefix redirect() with basePath', async () => {
const browser = await next.browser('/base/redirect')
await check(async () => {
await retry(async () => {
expect(await browser.url()).toBe(`${next.url}/base/another`)
return 'success'
}, 'success')
})
})

it('should render usePathname without the basePath', async () => {
Expand All @@ -56,5 +55,13 @@ createNextDescribe(
})
await Promise.all(validatorPromises)
})

it('should handle redirect in dynamic in suspense boundary routes with basePath', async () => {
const browser = await next.browser('/base/dynamic/source')
await retry(async () => {
expect(await browser.url()).toBe(`${next.url}/base/dynamic/dest`)
expect(await browser.elementByCss('p').text()).toBe(`id:dest`)
})
})
}
)

0 comments on commit 400b57c

Please sign in to comment.