Fix TypeError when using params in RootLayout with parallel routes (#60401)

### What?
When accessing `params` on a `RootLayout`, while also using parallel
routes, two potential errors would occur:
- A `Warning: React.createElement: type is invalid` error when
attempting to render a `NotFound` component that doesn't exist
- A `TypeError: Cannot read properties of undefined` error when
attempting to access params in the root layout.

### Why?
`createComponentTree` will render a duplicate `RootLayout` (to ensure
the `notFound()` fallback in unmatched parallel slots have a
`NotFoundBoundary` to catch them) but it currently doesn't ensure a
`NotFound` component exists nor does it forward `params` to the layout.

### How?
This forwards the params to the `RootLayout` and doesn't render a
`NotFoundComponent` if one doesn't exist. This replaces a few `any`
types with more sound types that would have helped catch these mistakes.
There's still a lot more typing that needs to be done (left a comment
below with some additional details) but I opted to make the minimal
changes related to this issue.

Longer term we should remove this duplicate `RootLayout` (see #60220)
which will require special UI to show unmatched slots (similar to the
error overlay, but less harsh)

Closes NEXT-1909
Fixes #59711
This commit is contained in:
Zack Tanner 2024-01-09 07:06:24 -08:00 committed by GitHub
parent a3f4ad9887
commit 1481b2649f
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
13 changed files with 156 additions and 15 deletions

View file

@ -30,13 +30,14 @@ export type ErrorComponent = React.ComponentType<{
export interface ErrorBoundaryProps {
children?: React.ReactNode
errorComponent: ErrorComponent
errorComponent: ErrorComponent | undefined
errorStyles?: React.ReactNode | undefined
errorScripts?: React.ReactNode | undefined
}
interface ErrorBoundaryHandlerProps extends ErrorBoundaryProps {
pathname: string
errorComponent: ErrorComponent
}
interface ErrorBoundaryHandlerState {

View file

@ -509,7 +509,7 @@ export default function OuterLayoutRouter({
}: {
parallelRouterKey: string
segmentPath: FlightSegmentPath
error: ErrorComponent
error: ErrorComponent | undefined
errorStyles: React.ReactNode | undefined
errorScripts: React.ReactNode | undefined
templateStyles: React.ReactNode | undefined

View file

@ -16,7 +16,7 @@ export async function createComponentStylesAndScripts({
injectedCSS: Set<string>
injectedJS: Set<string>
ctx: AppRenderContext
}): Promise<[any, React.ReactNode, React.ReactNode]> {
}): Promise<[React.ComponentType<any>, React.ReactNode, React.ReactNode]> {
const { styles: cssHrefs, scripts: jsHrefs } = getLinkAndScriptTags(
ctx.clientReferenceManifest,
filePath,

View file

@ -17,6 +17,10 @@ type ComponentTree = {
styles: ReactNode
}
type Params = {
[key: string]: string | string[]
}
/**
* This component will call `React.postpone` that throws the postponed error.
*/
@ -48,7 +52,7 @@ export async function createComponentTree({
}: {
createSegmentPath: CreateSegmentPath
loaderTree: LoaderTree
parentParams: { [key: string]: any }
parentParams: Params
rootLayoutIncluded: boolean
firstItem?: boolean
injectedCSS: Set<string>
@ -231,7 +235,7 @@ export async function createComponentTree({
throw staticGenerationStore.dynamicUsageErr
}
const LayoutOrPage = layoutOrPageMod
const LayoutOrPage: React.ComponentType<any> | undefined = layoutOrPageMod
? interopDefault(layoutOrPageMod)
: undefined
@ -242,20 +246,31 @@ export async function createComponentTree({
const parallelKeys = Object.keys(parallelRoutes)
const hasSlotKey = parallelKeys.length > 1
if (hasSlotKey && rootLayoutAtThisLevel) {
Component = (componentProps: any) => {
// TODO-APP: This is a hack to support unmatched parallel routes, which will throw `notFound()`.
// This ensures that a `NotFoundBoundary` is available for when that happens,
// but it's not ideal, as it needlessly invokes the `NotFound` component and renders the `RootLayout` twice.
// We should instead look into handling the fallback behavior differently in development mode so that it doesn't
// rely on the `NotFound` behavior.
if (hasSlotKey && rootLayoutAtThisLevel && LayoutOrPage) {
Component = (componentProps: { params: Params }) => {
const NotFoundComponent = NotFound
const RootLayoutComponent = LayoutOrPage
return (
<NotFoundBoundary
notFound={
<>
{layerAssets}
<RootLayoutComponent>
{notFoundStyles}
<NotFoundComponent />
</RootLayoutComponent>
</>
NotFoundComponent ? (
<>
{layerAssets}
{/*
* We are intentionally only forwarding params to the root layout, as passing any of the parallel route props
* might trigger `notFound()`, which is not currently supported in the root layout.
*/}
<RootLayoutComponent params={componentProps.params}>
{notFoundStyles}
<NotFoundComponent />
</RootLayoutComponent>
</>
) : undefined
}
>
<RootLayoutComponent {...componentProps} />

View file

@ -0,0 +1,3 @@
export default function Page() {
return <div>Interception Modal</div>
}

View file

@ -0,0 +1,3 @@
export default function Page() {
return null
}

View file

@ -0,0 +1,15 @@
export default function Layout(props: {
children: React.ReactNode
params: { locale: string }
modal: React.ReactNode
}) {
return (
<html>
<body>
<div id="children">{props.children}</div>
<div>Locale: {props.params.locale}</div>
{props.modal}
</body>
</html>
)
}

View file

@ -0,0 +1,3 @@
export default function NotFound() {
return <div>Custom Not Found</div>
}

View file

@ -0,0 +1,9 @@
import Link from 'next/link'
export default function Page() {
return (
<div>
<Link href="/en/show">To /en/show</Link>
</div>
)
}

View file

@ -0,0 +1,12 @@
'use client'
import { notFound } from 'next/navigation'
export default function Page({ params }) {
console.log(params)
if (params.locale !== 'en') {
notFound()
}
return <div>Regular Modal Page</div>
}

View file

@ -0,0 +1,6 @@
/**
* @type {import('next').NextConfig}
*/
const nextConfig = {}
module.exports = nextConfig

View file

@ -0,0 +1,73 @@
import { createNextDescribe } from 'e2e-utils'
import { check } from 'next-test-utils'
createNextDescribe(
'parallel-route-not-found',
{
files: __dirname,
},
({ next }) => {
it('should behave correctly without any errors', async () => {
const browser = await next.browser('/en')
await check(() => {
if (
next.cliOutput.includes('TypeError') ||
next.cliOutput.includes('Warning')
) {
return 'has-errors'
}
return 'success'
}, 'success')
expect(await browser.elementByCss('body').text()).not.toContain(
'Interception Modal'
)
expect(await browser.elementByCss('body').text()).toContain('Locale: en')
await browser.elementByCss("[href='/en/show']").click()
await check(() => {
if (
next.cliOutput.includes('TypeError') ||
next.cliOutput.includes('Warning')
) {
return 'has-errors'
}
return 'success'
}, 'success')
await check(
() => browser.elementByCss('body').text(),
/Interception Modal/
)
await check(() => browser.elementByCss('body').text(), /Locale: en/)
await browser.refresh()
await check(
() => browser.elementByCss('body').text(),
/Regular Modal Page/
)
await check(() => browser.elementByCss('body').text(), /Locale: en/)
})
it('should handle the not found case correctly without any errors', async () => {
const browser = await next.browser('/de/show')
await check(() => {
if (
next.cliOutput.includes('TypeError') ||
next.cliOutput.includes('Warning')
) {
return 'has-errors'
}
return 'success'
}, 'success')
expect(await browser.elementByCss('body').text()).toContain(
'Custom Not Found'
)
})
}
)

View file

@ -82,7 +82,8 @@
"test/e2e/app-dir/ppr-*/**/*",
"test/e2e/app-dir/app-prefetch*/**/*",
"test/e2e/app-dir/interception-middleware-rewrite/interception-middleware-rewrite.test.ts",
"test/e2e/app-dir/searchparams-static-bailout/searchparams-static-bailout.test.ts"
"test/e2e/app-dir/searchparams-static-bailout/searchparams-static-bailout.test.ts",
"test/e2e/app-dir/parallel-route-not-found-params/parallel-route-not-found-params.test.ts"
]
}
}