Skip to content

Commit

Permalink
Add not found boundary and move head cache to app router (#47688)
Browse files Browse the repository at this point in the history
### What?

- Add not found boundary to app router
- Move `head` cache back to app router

### Why?

We want the head to be rendered separately from body, previously to be
able to use `redirect()` and `notFound()` in `generateMetadata` we move
the head cache into layout-router to be wrapped by not found and
redirect boundaries. Since redirect boudary is already moved to
app-router, so we only need to add not found boundary and move head
cache to app router.

Notice: there's a limitation that we can't find the corresponding not
found of page if you throw notFound in generateMetadata, the root layout
+ root/default not found will be used to generate the 404 page

### How?

Closes NEXT-864
Fixes #46738

fix NEXT-888 ([link](https://linear.app/vercel/issue/NEXT-888))

---------
  • Loading branch information
huozhi authored Mar 30, 2023
1 parent b9d7732 commit 0616f1b
Show file tree
Hide file tree
Showing 15 changed files with 157 additions and 47 deletions.
23 changes: 20 additions & 3 deletions packages/next/src/client/components/app-router.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,8 @@ import { isBot } from '../../shared/lib/router/utils/is-bot'
import { addBasePath } from '../add-base-path'
import { AppRouterAnnouncer } from './app-router-announcer'
import { RedirectBoundary } from './redirect-boundary'
import { NotFoundBoundary } from './not-found-boundary'
import { findHeadInCache } from './router-reducer/reducers/find-head-in-cache'

const isServer = typeof window === 'undefined'

Expand Down Expand Up @@ -70,6 +72,10 @@ type AppRouterProps = Omit<
> & {
initialHead: ReactNode
assetPrefix: string
// Top level boundaries props
notFound: React.ReactNode | undefined
notFoundStyles: React.ReactNode | undefined
asNotFound?: boolean
}

function isExternalURL(url: URL) {
Expand All @@ -85,6 +91,9 @@ function Router({
initialCanonicalUrl,
children,
assetPrefix,
notFound,
notFoundStyles,
asNotFound,
}: AppRouterProps) {
const initialState = useMemo(
() =>
Expand Down Expand Up @@ -311,13 +320,22 @@ function Router({
}
}, [onPopState])

const head = useMemo(() => {
return findHeadInCache(cache, tree[1])
}, [cache, tree])

const content = (
<>
<NotFoundBoundary
notFound={notFound}
notFoundStyles={notFoundStyles}
asNotFound={asNotFound}
>
<RedirectBoundary>
{head}
{cache.subTreeData}
<AppRouterAnnouncer tree={tree} />
</RedirectBoundary>
</>
</NotFoundBoundary>
)

return (
Expand All @@ -338,7 +356,6 @@ function Router({
// Root node always has `url`
// Provided in AppTreeContext to ensure it can be overwritten in layout-router
url: canonicalUrl,
headRenderedAboveThisLevel: false,
}}
>
{HotReloader ? (
Expand Down
17 changes: 2 additions & 15 deletions packages/next/src/client/components/layout-router.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import type {
import type { ErrorComponent } from './error-boundary'
import { FocusAndScrollRef } from './router-reducer/router-reducer-types'

import React, { useContext, useMemo, use } from 'react'
import React, { useContext, use } from 'react'
import ReactDOM from 'react-dom'
import {
CacheStates,
Expand All @@ -22,7 +22,6 @@ import { createInfinitePromise } from './infinite-promise'
import { ErrorBoundary } from './error-boundary'
import { matchSegment } from './match-segments'
import { handleSmoothScroll } from '../../shared/lib/router/utils/handle-smooth-scroll'
import { findHeadInCache } from './router-reducer/reducers/find-head-in-cache'
import { RedirectBoundary } from './redirect-boundary'
import { NotFoundBoundary } from './not-found-boundary'

Expand Down Expand Up @@ -232,7 +231,6 @@ function InnerLayoutRouter({
// TODO-APP: implement `<Offscreen>` when available.
// isActive,
path,
headRenderedAboveThisLevel,
}: {
parallelRouterKey: string
url: string
Expand All @@ -242,7 +240,6 @@ function InnerLayoutRouter({
tree: FlightRouterState
isActive: boolean
path: string
headRenderedAboveThisLevel: boolean
}) {
const context = useContext(GlobalLayoutRouterContext)
if (!context) {
Expand All @@ -251,13 +248,6 @@ function InnerLayoutRouter({

const { changeByServerResponse, tree: fullTree, focusAndScrollRef } = context

const head = useMemo(() => {
if (headRenderedAboveThisLevel) {
return null
}
return findHeadInCache(childNodes, tree[1])
}, [childNodes, tree, headRenderedAboveThisLevel])

// Read segment path from the parallel router cache node.
let childNode = childNodes.get(path)

Expand Down Expand Up @@ -370,10 +360,8 @@ function InnerLayoutRouter({
childNodes: childNode.parallelRoutes,
// TODO-APP: overriding of url for parallel routes
url: url,
headRenderedAboveThisLevel: true,
}}
>
{head}
{childNode.subTreeData}
</LayoutRouterContext.Provider>
)
Expand Down Expand Up @@ -457,7 +445,7 @@ export default function OuterLayoutRouter({
throw new Error('invariant expected layout router to be mounted')
}

const { childNodes, tree, url, headRenderedAboveThisLevel } = context
const { childNodes, tree, url } = context

// Get the current parallelRouter cache node
let childNodesForParallelRouter = childNodes.get(parallelRouterKey)
Expand Down Expand Up @@ -528,7 +516,6 @@ export default function OuterLayoutRouter({
segmentPath={segmentPath}
path={preservedSegment}
isActive={currentChildSegment === preservedSegment}
headRenderedAboveThisLevel={headRenderedAboveThisLevel}
/>
</RedirectBoundary>
</NotFoundBoundary>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -95,10 +95,7 @@ describe('findHeadInCache', () => {
]),
}

const result = findHeadInCache(
cache.parallelRoutes.get('children')!,
routerTree[1]
)
const result = findHeadInCache(cache, routerTree[1])

expect(result).toMatchObject(
<>
Expand Down
Original file line number Diff line number Diff line change
@@ -1,16 +1,20 @@
import type { FlightRouterState } from '../../../../server/app-render/types'
import type { ChildSegmentMap } from '../../../../shared/lib/app-router-context'
import type { CacheNode } from '../../../../shared/lib/app-router-context'

export function findHeadInCache(
childSegmentMap: ChildSegmentMap,
cache: CacheNode,
parallelRoutes: FlightRouterState[1]
): React.ReactNode {
if (!childSegmentMap) {
return undefined
const isLastItem = Object.keys(parallelRoutes).length === 0
if (isLastItem) {
return cache.head
}
for (const key in parallelRoutes) {
const [segment, childParallelRoutes] = parallelRoutes[key]
const isLastItem = Object.keys(childParallelRoutes).length === 0
const childSegmentMap = cache.parallelRoutes.get(key)
if (!childSegmentMap) {
continue
}

const cacheKey = Array.isArray(segment) ? segment[1] : segment

Expand All @@ -19,14 +23,9 @@ export function findHeadInCache(
continue
}

if (isLastItem && cacheNode.head) return cacheNode.head

const segmentMap = cacheNode.parallelRoutes.get(key)
if (segmentMap) {
const item = findHeadInCache(segmentMap, childParallelRoutes)
if (item) {
return item
}
const item = findHeadInCache(cacheNode, childParallelRoutes)
if (item) {
return item
}
}

Expand Down
31 changes: 29 additions & 2 deletions packages/next/src/server/app-render/app-render.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -1124,17 +1124,35 @@ export async function renderToHTMLOrFlight(
}>(
async (props) => {
// Create full component tree from root to leaf.
const injectedCSS = new Set<string>()
const { Component: ComponentTree } = await createComponentTree({
createSegmentPath: (child) => child,
loaderTree: loaderTree,
loaderTree,
parentParams: {},
firstItem: true,
injectedCSS: new Set(),
injectedCSS,
injectedFontPreloadTags: new Set(),
rootLayoutIncluded: false,
asNotFound: props.asNotFound,
})

const { 'not-found': notFound, layout } = loaderTree[2]
const isLayout = typeof layout !== 'undefined'
const rootLayoutModule = layout?.[0]
const RootLayout = rootLayoutModule
? interopDefault(await rootLayoutModule())
: null
const rootLayoutAtThisLevel = isLayout
const [NotFound, notFoundStyles] = notFound
? await createComponentAndStyles({
filePath: notFound[1],
getComponent: notFound[0],
injectedCSS,
})
: rootLayoutAtThisLevel
? [DefaultNotFound]
: []

const initialTree = createFlightRouterStateFromLoaderTree(
loaderTree,
getDynamicParamFromSegment,
Expand All @@ -1155,6 +1173,15 @@ export async function renderToHTMLOrFlight(
</>
}
globalErrorComponent={GlobalError}
notFound={
NotFound && RootLayout ? (
<RootLayout params={{}}>
<NotFound />
</RootLayout>
) : undefined
}
notFoundStyles={notFoundStyles}
asNotFound={props.asNotFound}
>
<ComponentTree />
</AppRouter>
Expand Down
1 change: 0 additions & 1 deletion packages/next/src/shared/lib/app-router-context.ts
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,6 @@ export const LayoutRouterContext = React.createContext<{
childNodes: CacheNode['parallelRoutes']
tree: FlightRouterState
url: string
headRenderedAboveThisLevel: boolean
}>(null as any)
export const GlobalLayoutRouterContext = React.createContext<{
tree: FlightRouterState
Expand Down
12 changes: 12 additions & 0 deletions test/e2e/app-dir/metadata-suspense/app/layout.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
import { Suspense } from 'react'

export default function Layout({ children }) {
return (
<html>
<head></head>
<body>
<Suspense fallback={<div>loading...</div>}>{children}</Suspense>
</body>
</html>
)
}
3 changes: 3 additions & 0 deletions test/e2e/app-dir/metadata-suspense/app/loading.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
export default function Loading() {
return <h1>Loading...</h1>
}
9 changes: 9 additions & 0 deletions test/e2e/app-dir/metadata-suspense/app/page.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
export default function Page() {
return <div>Page</div>
}

export const metadata = {
title: 'My title',
description: 'My description',
themeColor: '#eee',
}
18 changes: 18 additions & 0 deletions test/e2e/app-dir/metadata-suspense/index.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
import { createNextDescribe } from 'e2e-utils'

createNextDescribe(
'app dir - metadata dynamic routes',
{
files: __dirname,
skipDeployment: true,
},
({ next }) => {
it('should render metadata in head even root layout is wrapped with Suspense', async () => {
const $ = await next.render$('/')
expect($('head title').text()).toBe('My title')
expect($('head meta[name="theme-color"]').attr('content')).toBe('#eee')

expect($('body meta').length).toBe(0)
})
}
)
5 changes: 5 additions & 0 deletions test/e2e/app-dir/metadata-suspense/next.config.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
module.exports = {
experimental: {
appDir: true,
},
}
24 changes: 24 additions & 0 deletions test/e2e/app-dir/metadata-suspense/tsconfig.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
{
"compilerOptions": {
"lib": ["dom", "dom.iterable", "esnext"],
"allowJs": true,
"skipLibCheck": true,
"strict": false,
"forceConsistentCasingInFileNames": true,
"noEmit": true,
"incremental": true,
"esModuleInterop": true,
"module": "esnext",
"moduleResolution": "node",
"resolveJsonModule": true,
"isolatedModules": true,
"jsx": "preserve",
"plugins": [
{
"name": "next"
}
]
},
"include": ["next-env.d.ts", ".next/types/**/*.ts", "**/*.ts", "**/*.tsx"],
"exclude": ["node_modules"]
}
3 changes: 3 additions & 0 deletions test/e2e/app-dir/metadata/app/not-found.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
export default function notFound() {
return <h2>root not found page</h2>
}
25 changes: 16 additions & 9 deletions test/e2e/app-dir/metadata/metadata.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -364,15 +364,22 @@ createNextDescribe(
)
})

it('should support notFound and redirect in generateMetadata', async () => {
const resNotFound = await next.fetch('/async/not-found')
expect(resNotFound.status).toBe(404)
const notFoundHtml = await resNotFound.text()
expect(notFoundHtml).not.toBe('not-found-text')
expect(notFoundHtml).toContain('This page could not be found.')

const resRedirect = await next.fetch('/async/redirect')
expect(resRedirect.status).toBe(307)
it('should support notFound in generateMetadata', async () => {
// TODO-APP: support custom not-found for generateMetadata
const res = await next.fetch('/async/not-found')
expect(res.status).toBe(404)
const html = await res.text()
expect(html).toContain('root not found page')

const browser = await next.browser('/async/not-found')
expect(await browser.elementByCss('h2').text()).toBe(
'root not found page'
)
})

it('should support redirect in generateMetadata', async () => {
const res = await next.fetch('/async/redirect')
expect(res.status).toBe(307)
})

it('should handle metadataBase for urls resolved as only URL type', async () => {
Expand Down
3 changes: 3 additions & 0 deletions test/e2e/app-dir/not-found/app/not-found/not-found.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
export default function notFound() {
return <h1>custom not found</h1>
}

0 comments on commit 0616f1b

Please sign in to comment.