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

Add not found boundary and move head cache to app router #47688

Merged
merged 9 commits into from
Mar 30, 2023
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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
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>
}