Skip to content

Commit

Permalink
log a dev warning when a missing parallel slot results in a 404 (#60186)
Browse files Browse the repository at this point in the history
### What & Why?
When visiting a route that attempts to render a slot with no page & no default, the fallback behavior is to trigger a 404. However this can lead to a confusing development experience for complex parallel routing cases as you might not realize a default is missing, or which slot is causing the error.

Previous issues where this caused confusion:
- #51805
- #49569


### How?
This is a dev-only modification to track parallel slots that are using the default fallback (aka missing). When the `NotFoundBoundary` is triggered in development mode, this will log a warning about why it 404ed, along with a list of slot(s) that were determined to be missing.

![CleanShot 2024-01-03 at 14 34 30@2x](https://github.com/vercel/next.js/assets/1939140/1a00ea49-24b6-4ba0-9bac-8773c7e10a75)

### Future
We should eventually lift this into some sort of dev-only UI to help catch it when not monitoring the browser console (similar to the error overlay). However, this will require some design thought and isn't necessary for the first iteration.

Closes NEXT-1798
  • Loading branch information
ztanner authored Jan 4, 2024
1 parent 1216422 commit aa48b65
Show file tree
Hide file tree
Showing 19 changed files with 195 additions and 8 deletions.
5 changes: 2 additions & 3 deletions packages/next/src/build/webpack/loaders/next-app-loader.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ import {
import { getFilesInDir } from '../../../lib/get-files-in-dir'
import { normalizeAppPath } from '../../../shared/lib/router/utils/app-paths'
import type { PageExtensions } from '../../page-extensions-type'
import { PARALLEL_ROUTE_DEFAULT_PATH } from '../../../client/components/parallel-route-default'

export type AppLoaderOptions = {
name: string
Expand Down Expand Up @@ -424,8 +425,6 @@ async function createTreeCodeFromPath(
if (!props[normalizeParallelKey(adjacentParallelSegment)]) {
const actualSegment =
adjacentParallelSegment === 'children' ? '' : adjacentParallelSegment
const fallbackDefault =
'next/dist/client/components/parallel-route-default'
let defaultPath = await resolver(
`${appDirPrefix}${segmentPath}/${actualSegment}/default`
)
Expand All @@ -440,7 +439,7 @@ async function createTreeCodeFromPath(
)

// if a default is found, use that. Otherwise use the fallback, which will trigger a `notFound()`
defaultPath = normalizedDefault ?? fallbackDefault
defaultPath = normalizedDefault ?? PARALLEL_ROUTE_DEFAULT_PATH
}

props[normalizeParallelKey(adjacentParallelSegment)] = `[
Expand Down
11 changes: 10 additions & 1 deletion packages/next/src/client/components/app-router.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import {
AppRouterContext,
LayoutRouterContext,
GlobalLayoutRouterContext,
MissingSlotContext,
} from '../../shared/lib/app-router-context.shared-runtime'
import type {
CacheNode,
Expand Down Expand Up @@ -104,6 +105,7 @@ type AppRouterProps = Omit<
buildId: string
initialHead: ReactNode
assetPrefix: string
missingSlots: Set<string>
}

function isExternalURL(url: URL) {
Expand Down Expand Up @@ -266,6 +268,7 @@ function Router({
initialCanonicalUrl,
initialSeedData,
assetPrefix,
missingSlots,
}: AppRouterProps) {
const initialState = useMemo(
() =>
Expand Down Expand Up @@ -598,7 +601,13 @@ function Router({
if (typeof window !== 'undefined') {
const DevRootNotFoundBoundary: typeof import('./dev-root-not-found-boundary').DevRootNotFoundBoundary =
require('./dev-root-not-found-boundary').DevRootNotFoundBoundary
content = <DevRootNotFoundBoundary>{content}</DevRootNotFoundBoundary>
content = (
<DevRootNotFoundBoundary>
<MissingSlotContext.Provider value={missingSlots}>
{content}
</MissingSlotContext.Provider>
</DevRootNotFoundBoundary>
)
}
const HotReloader: typeof import('./react-dev-overlay/hot-reloader-client').default =
require('./react-dev-overlay/hot-reloader-client').default
Expand Down
37 changes: 34 additions & 3 deletions packages/next/src/client/components/not-found-boundary.tsx
Original file line number Diff line number Diff line change
@@ -1,7 +1,10 @@
'use client'

import React from 'react'
import React, { useContext } from 'react'
import { usePathname } from './navigation'
import { isNotFoundError } from './not-found'
import { warnOnce } from '../../shared/lib/utils/warn-once'
import { MissingSlotContext } from '../../shared/lib/app-router-context.shared-runtime'

interface NotFoundBoundaryProps {
notFound?: React.ReactNode
Expand All @@ -12,6 +15,7 @@ interface NotFoundBoundaryProps {

interface NotFoundErrorBoundaryProps extends NotFoundBoundaryProps {
pathname: string
missingSlots: Set<string>
}

interface NotFoundErrorBoundaryState {
Expand All @@ -31,9 +35,34 @@ class NotFoundErrorBoundary extends React.Component<
}
}

componentDidCatch(): void {
if (
process.env.NODE_ENV === 'development' &&
// A missing children slot is the typical not-found case, so no need to warn
!this.props.missingSlots.has('children')
) {
let warningMessage =
'No default component was found for a parallel route rendered on this page. Falling back to nearest NotFound boundary.\n' +
'Learn more: https://nextjs.org/docs/app/building-your-application/routing/parallel-routes#defaultjs\n\n'

if (this.props.missingSlots.size > 0) {
const formattedSlots = Array.from(this.props.missingSlots)
.sort((a, b) => a.localeCompare(b))
.map((slot) => `@${slot}`)
.join(', ')

warningMessage += 'Missing slots: ' + formattedSlots
}

warnOnce(warningMessage)
}
}

static getDerivedStateFromError(error: any) {
if (error?.digest === 'NEXT_NOT_FOUND') {
return { notFoundTriggered: true }
if (isNotFoundError(error)) {
return {
notFoundTriggered: true,
}
}
// Re-throw if error is not for 404
throw error
Expand Down Expand Up @@ -86,12 +115,14 @@ export function NotFoundBoundary({
children,
}: NotFoundBoundaryProps) {
const pathname = usePathname()
const missingSlots = useContext(MissingSlotContext)
return notFound ? (
<NotFoundErrorBoundary
pathname={pathname}
notFound={notFound}
notFoundStyles={notFoundStyles}
asNotFound={asNotFound}
missingSlots={missingSlots}
>
{children}
</NotFoundErrorBoundary>
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
import { notFound } from './not-found'

export default function NoopParallelRouteDefault() {
export const PARALLEL_ROUTE_DEFAULT_PATH =
'next/dist/client/components/parallel-route-default'

export default function ParallelRouteDefault() {
notFound()
}
6 changes: 6 additions & 0 deletions packages/next/src/server/app-render/app-render.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -352,6 +352,7 @@ async function ReactServerApp({
const injectedCSS = new Set<string>()
const injectedJS = new Set<string>()
const injectedFontPreloadTags = new Set<string>()
const missingSlots = new Set<string>()
const {
getDynamicParamFromSegment,
query,
Expand Down Expand Up @@ -387,6 +388,7 @@ async function ReactServerApp({
rootLayoutIncluded: false,
asNotFound: asNotFound,
metadataOutlet: <MetadataOutlet />,
missingSlots,
})

return (
Expand All @@ -410,6 +412,9 @@ async function ReactServerApp({
</>
}
globalErrorComponent={GlobalError}
// This is used to provide debug information (when in development mode)
// about which slots were not filled by page components while creating the component tree.
missingSlots={missingSlots}
/>
</>
)
Expand Down Expand Up @@ -485,6 +490,7 @@ async function ReactServerError({
initialHead={head}
globalErrorComponent={GlobalError}
initialSeedData={initialSeedData}
missingSlots={new Set()}
/>
)
}
Expand Down
14 changes: 14 additions & 0 deletions packages/next/src/server/app-render/create-component-tree.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import { createComponentStylesAndScripts } from './create-component-styles-and-s
import { getLayerAssets } from './get-layer-assets'
import { hasLoadingComponentInTree } from './has-loading-component-in-tree'
import { validateRevalidate } from '../lib/patch-fetch'
import { PARALLEL_ROUTE_DEFAULT_PATH } from '../../client/components/parallel-route-default'

type ComponentTree = {
seedData: CacheNodeSeedData
Expand Down Expand Up @@ -43,6 +44,7 @@ export async function createComponentTree({
asNotFound,
metadataOutlet,
ctx,
missingSlots,
}: {
createSegmentPath: CreateSegmentPath
loaderTree: LoaderTree
Expand All @@ -55,6 +57,7 @@ export async function createComponentTree({
asNotFound?: boolean
metadataOutlet?: React.ReactNode
ctx: AppRenderContext
missingSlots?: Set<string>
}): Promise<ComponentTree> {
const {
renderOpts: { nextConfigOutput, experimental },
Expand Down Expand Up @@ -372,6 +375,16 @@ export async function createComponentTree({
// client router.
} else {
// Create the child component

if (process.env.NODE_ENV === 'development' && missingSlots) {
// When we detect the default fallback (which triggers a 404), we collect the missing slots
// to provide more helpful debug information during development mode.
const parsedTree = parseLoaderTree(parallelRoute)
if (parsedTree.layoutOrPagePath === PARALLEL_ROUTE_DEFAULT_PATH) {
missingSlots.add(parallelRouteKey)
}
}

const { seedData, styles: childComponentStyles } =
await createComponentTree({
createSegmentPath: (child) => {
Expand All @@ -386,6 +399,7 @@ export async function createComponentTree({
asNotFound,
metadataOutlet,
ctx,
missingSlots,
})

currentStyles = childComponentStyles
Expand Down
2 changes: 2 additions & 0 deletions packages/next/src/server/app-render/create-error-handler.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,8 @@ export function createErrorHandler({
return (err) => {
if (allCapturedErrors) allCapturedErrors.push(err)

// These errors are expected. We return the digest
// so that they can be properly handled.
if (isDynamicUsageError(err)) {
return err.digest
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -161,3 +161,5 @@ if (process.env.NODE_ENV !== 'production') {
GlobalLayoutRouterContext.displayName = 'GlobalLayoutRouterContext'
TemplateContext.displayName = 'TemplateContext'
}

export const MissingSlotContext = React.createContext<Set<string>>(new Set())
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
export default function Page() {
return <div>@bar slot</div>
}
3 changes: 3 additions & 0 deletions test/e2e/app-dir/parallel-route-not-found/app/@bar/page.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
export default function Page() {
return <div>@bar slot</div>
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
export default function Page() {
return <div>@foo slot</div>
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
export default function Page() {
return <div>@foo slot</div>
}
3 changes: 3 additions & 0 deletions test/e2e/app-dir/parallel-route-not-found/app/@foo/page.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
export default function Page() {
return <div>@foo slot</div>
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
export default function Page() {
return <div>Both Slots Missing</div>
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
export default function Page() {
return <div>Has Both Slots</div>
}
15 changes: 15 additions & 0 deletions test/e2e/app-dir/parallel-route-not-found/app/layout.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
export default function Layout(props: {
children: React.ReactNode
foo: React.ReactNode
bar: React.ReactNode
}) {
return (
<html>
<body>
<div id="children">{props.children}</div>
{props.foo}
{props.bar}
</body>
</html>
)
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
export default function Page() {
return <div>No @bar Slot</div>
}
9 changes: 9 additions & 0 deletions test/e2e/app-dir/parallel-route-not-found/app/page.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
import Link from 'next/link'

export default function Page() {
return (
<div>
<Link href="/nested">To /nested</Link>
</div>
)
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,73 @@
import { createNextDescribe } from 'e2e-utils'

createNextDescribe(
'parallel-route-not-found',
{
files: __dirname,
},
({ next, isNextDev }) => {
it('should handle a layout that attempts to render a missing parallel route', async () => {
const browser = await next.browser('/no-bar-slot')
const logs = await browser.log()
expect(await browser.elementByCss('body').text()).toContain(
'This page could not be found'
)
const warnings = logs.filter((log) => log.source === 'warning')
if (isNextDev) {
expect(warnings.length).toBe(1)
expect(warnings[0].message).toContain(
'No default component was found for a parallel route rendered on this page'
)
expect(warnings[0].message).toContain('Missing slots: @bar')
} else {
expect(warnings.length).toBe(0)
}
})

it('should handle multiple missing parallel routes', async () => {
const browser = await next.browser('/both-slots-missing')
const logs = await browser.log()

expect(await browser.elementByCss('body').text()).toContain(
'This page could not be found'
)

const warnings = logs.filter((log) => log.source === 'warning')
if (isNextDev) {
expect(warnings.length).toBe(1)
expect(warnings[0].message).toContain(
'No default component was found for a parallel route rendered on this page'
)
expect(warnings[0].message).toContain('Missing slots: @bar, @foo')
} else {
expect(warnings.length).toBe(0)
}
})

it('should render the page & slots if all parallel routes are found', async () => {
const browser = await next.browser('/has-both-slots')
const logs = await browser.log()

expect(await browser.elementByCss('body').text()).toContain(
'Has Both Slots'
)
expect(await browser.elementByCss('body').text()).toContain('@foo slot')
expect(await browser.elementByCss('body').text()).toContain('@bar slot')

const warnings = logs.filter((log) => log.source === 'warning')
expect(warnings.length).toBe(0)
})

if (isNextDev) {
it('should not log any warnings for a regular not found page', async () => {
const browser = await next.browser('/this-page-doesnt-exist')
const logs = await browser.log()
expect(await browser.elementByCss('body').text()).toContain(
'This page could not be found'
)
const warnings = logs.filter((log) => log.source === 'warning')
expect(warnings.length).toBe(0)
})
}
}
)

0 comments on commit aa48b65

Please sign in to comment.