Skip to content
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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions .changeset/friendly-outlets-warn.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
'@tanstack/react-router': patch
'@tanstack/solid-router': patch
'@tanstack/vue-router': patch
---

Warn when an Outlet is rendered inside a pending, error, or not-found component.
18 changes: 14 additions & 4 deletions packages/react-router/src/CatchBoundary.tsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
'use client'

import * as React from 'react'
import { wrapInNonRouteComponentContext } from './nonRouteComponentContext'
import type { ErrorRouteComponent } from './route'
import type { ErrorInfo } from 'react'

Expand Down Expand Up @@ -44,12 +45,21 @@ class CatchBoundaryImpl extends React.Component<{
}
render() {
const error = this.state.error
return error
? React.createElement(this.props.errorComponent ?? ErrorComponent, {
if (error) {
const element = React.createElement(
this.props.errorComponent ?? ErrorComponent,
{
error,
reset: this.reset,
})
: this.props.children
},
)

return process.env.NODE_ENV !== 'production'
? wrapInNonRouteComponentContext(element, 'errorComponent')
: element
}

return this.props.children
}
}

Expand Down
36 changes: 33 additions & 3 deletions packages/react-router/src/Match.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,10 @@ import { SafeFragment } from './SafeFragment'
import { renderRouteNotFound } from './renderRouteNotFound'
import { ScrollRestoration } from './scroll-restoration'
import { ClientOnly } from './ClientOnly'
import {
nonRouteComponentContext,
wrapInNonRouteComponentContext,
} from './nonRouteComponentContext'
import type {
AnyRoute,
AnyRouteMatch,
Expand All @@ -24,7 +28,14 @@ export function renderPending(
) {
const PendingComponent =
route?.options.pendingComponent ?? router.options.defaultPendingComponent
return PendingComponent ? <PendingComponent /> : null
if (!PendingComponent) {
return null
}

const pendingElement = <PendingComponent />
return process.env.NODE_ENV !== 'production'
? wrapInNonRouteComponentContext(pendingElement, 'pendingComponent')
: pendingElement
}

type OutletMatchSelection = [
Expand Down Expand Up @@ -123,10 +134,16 @@ function MatchView({
throw error
}

return React.createElement(
const notFoundElement = React.createElement(
routeNotFoundComponent!,
error as any,
)
return process.env.NODE_ENV !== 'production'
? wrapInNonRouteComponentContext(
notFoundElement,
'notFoundComponent',
)
: notFoundElement
}}
>
{resolvedNoSsr ? (
Expand Down Expand Up @@ -197,7 +214,7 @@ export const MatchInner = React.memo(function MatchInnerImpl({
(route.options.errorComponent ??
router.options.defaultErrorComponent) ||
ErrorComponent
return (
const errorElement = (
<RouteErrorComponent
error={match.error as any}
reset={undefined as any}
Expand All @@ -206,6 +223,9 @@ export const MatchInner = React.memo(function MatchInnerImpl({
}}
/>
)
return process.env.NODE_ENV !== 'production'
? wrapInNonRouteComponentContext(errorElement, 'errorComponent')
: errorElement
}
throw match.error
}
Expand All @@ -220,6 +240,16 @@ export const MatchInner = React.memo(function MatchInnerImpl({
* @link https://tanstack.com/router/latest/docs/framework/react/api/router/outletComponent
*/
export const Outlet = React.memo(function OutletImpl() {
if (process.env.NODE_ENV !== 'production') {
// eslint-disable-next-line react-hooks/rules-of-hooks
const nonRouteComponent = React.useContext(nonRouteComponentContext!)
if (nonRouteComponent) {
console.warn(
`Warning: An <Outlet /> was rendered inside a ${nonRouteComponent}. <Outlet /> should only be rendered inside a route component.`,
)
}
}

const router = useRouter()
const routeId = React.useContext(matchContext)!

Expand Down
21 changes: 21 additions & 0 deletions packages/react-router/src/nonRouteComponentContext.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
'use client'

import * as React from 'react'

export type NonRouteComponent =
| 'pendingComponent'
| 'errorComponent'
| 'notFoundComponent'

export const nonRouteComponentContext =
process.env.NODE_ENV !== 'production'
? React.createContext<NonRouteComponent | undefined>(undefined)
: undefined

export function wrapInNonRouteComponentContext(
element: React.ReactElement,
component: NonRouteComponent,
): React.ReactElement {
const Context = nonRouteComponentContext!
return <Context.Provider value={component}>{element}</Context.Provider>
}
13 changes: 11 additions & 2 deletions packages/react-router/src/renderRouteNotFound.tsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import * as React from 'react'
import { DefaultGlobalNotFound } from './not-found'
import { wrapInNonRouteComponentContext } from './nonRouteComponentContext'
import type { AnyRoute, AnyRouter } from '@tanstack/router-core'

/**
Expand All @@ -17,7 +18,12 @@ export function renderRouteNotFound(
) {
if (!route.options.notFoundComponent) {
if (router.options.defaultNotFoundComponent) {
return <router.options.defaultNotFoundComponent {...data} />
const notFoundElement = (
<router.options.defaultNotFoundComponent {...data} />
)
return process.env.NODE_ENV !== 'production'
? wrapInNonRouteComponentContext(notFoundElement, 'notFoundComponent')
: notFoundElement
}

if (process.env.NODE_ENV !== 'production') {
Expand All @@ -31,5 +37,8 @@ export function renderRouteNotFound(
return <DefaultGlobalNotFound />
}

return <route.options.notFoundComponent {...data} />
const notFoundElement = <route.options.notFoundComponent {...data} />
return process.env.NODE_ENV !== 'production'
? wrapInNonRouteComponentContext(notFoundElement, 'notFoundComponent')
: notFoundElement
}
146 changes: 146 additions & 0 deletions packages/react-router/tests/Outlet.test.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,146 @@
import { afterEach, expect, test, vi } from 'vitest'
import { cleanup, render, screen } from '@testing-library/react'
import { createMemoryHistory } from '@tanstack/history'
import { createControlledPromise, notFound } from '@tanstack/router-core'
import {
Outlet,
RouterProvider,
createRootRoute,
createRoute,
createRouter,
} from '../src'

const outletWarning = (
component: 'pendingComponent' | 'errorComponent' | 'notFoundComponent',
) =>
`Warning: An <Outlet /> was rendered inside a ${component}. <Outlet /> should only be rendered inside a route component.`

afterEach(() => {
cleanup()
vi.restoreAllMocks()
})

test('does not warn when Outlet is rendered inside a route component', async () => {
const warn = vi.spyOn(console, 'warn').mockImplementation(() => {})
const rootRoute = createRootRoute({
component: () => (
<>
<span>Root route</span>
<Outlet />
</>
),
})
const indexRoute = createRoute({
getParentRoute: () => rootRoute,
path: '/',
component: () => <span>Index route</span>,
})
const router = createRouter({
routeTree: rootRoute.addChildren([indexRoute]),
history: createMemoryHistory({ initialEntries: ['/'] }),
})

render(<RouterProvider router={router} />)

expect(await screen.findByText('Index route')).toBeInTheDocument()
expect(warn).not.toHaveBeenCalled()
})

test('warns when Outlet is rendered inside a pendingComponent', async () => {
const warn = vi.spyOn(console, 'warn').mockImplementation(() => {})
const pending = createControlledPromise<void>()
const rootRoute = createRootRoute({ component: Outlet })
const indexRoute = createRoute({
getParentRoute: () => rootRoute,
path: '/',
component: () => <span>Index route</span>,
})
const pendingRoute = createRoute({
getParentRoute: () => rootRoute,
path: '/pending',
loader: () => pending,
pendingMs: 0,
pendingComponent: () => (
<>
<span>Pending route</span>
<Outlet />
</>
),
component: () => <span>Resolved route</span>,
})
const router = createRouter({
routeTree: rootRoute.addChildren([indexRoute, pendingRoute]),
history: createMemoryHistory({ initialEntries: ['/'] }),
})

render(<RouterProvider router={router} />)
await screen.findByText('Index route')

const navigation = router.navigate({ to: '/pending' })
expect(await screen.findByText('Pending route')).toBeInTheDocument()
pending.resolve()
await navigation

expect(warn).toHaveBeenCalledWith(outletWarning('pendingComponent'))
})

test('warns when Outlet is rendered inside an errorComponent', async () => {
const warn = vi.spyOn(console, 'warn').mockImplementation(() => {})
const rootRoute = createRootRoute({ component: Outlet })
const indexRoute = createRoute({
getParentRoute: () => rootRoute,
path: '/',
loader: () => {
throw new Error('Loader failed')
},
errorComponent: () => (
<>
<span>Error route</span>
<Outlet />
</>
),
})
const router = createRouter({
routeTree: rootRoute.addChildren([indexRoute]),
history: createMemoryHistory({ initialEntries: ['/'] }),
})

render(<RouterProvider router={router} />)

expect(await screen.findByText('Error route')).toBeInTheDocument()
expect(warn).toHaveBeenCalledWith(outletWarning('errorComponent'))
})

test('warns when Outlet is rendered inside a notFoundComponent', async () => {
const warn = vi.spyOn(console, 'warn').mockImplementation(() => {})
const rootRoute = createRootRoute({ component: Outlet })
const indexRoute = createRoute({
getParentRoute: () => rootRoute,
path: '/',
component: () => <span>Index route</span>,
})
const notFoundRoute = createRoute({
getParentRoute: () => rootRoute,
path: '/not-found',
component: () => {
throw notFound()
},
notFoundComponent: () => (
<>
<span>Not found route</span>
<Outlet />
</>
),
})
const router = createRouter({
routeTree: rootRoute.addChildren([indexRoute, notFoundRoute]),
history: createMemoryHistory({ initialEntries: ['/'] }),
})

render(<RouterProvider router={router} />)
await screen.findByText('Index route')
await router.navigate({ to: '/not-found' })

expect(await screen.findByText('Not found route')).toBeInTheDocument()
expect(warn).toHaveBeenCalledWith(outletWarning('notFoundComponent'))
})
Comment on lines +49 to +146

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win

Add router-level fallback warning tests.

These tests cover only route-level pendingComponent, errorComponent, and notFoundComponent options. Add cases that configure defaultPendingComponent, defaultErrorComponent, and defaultNotFoundComponent on createRouter while the affected route omits its equivalent option.

This is required by the PR objective for router-level fallback coverage.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/react-router/tests/Outlet.test.tsx` around lines 49 - 146, Extend
the Outlet warning coverage with router-level fallback cases: configure
defaultPendingComponent, defaultErrorComponent, and defaultNotFoundComponent in
createRouter while omitting the corresponding route-level options. Mirror the
existing pending, error, and not-found scenarios and assert outletWarning uses
each fallback component name.

14 changes: 13 additions & 1 deletion packages/solid-router/src/CatchBoundary.tsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import * as Solid from 'solid-js'
import { Dynamic } from 'solid-js/web'
import { renderInNonRouteComponentContext } from './nonRouteComponentContext'
import type { ErrorRouteComponent } from './route'

export function CatchBoundary(
Expand All @@ -19,7 +20,18 @@ export function CatchBoundary(
Solid.on(props.getResetKey, () => reset(), { defer: true }),
)

return (
return process.env.NODE_ENV !== 'production' ? (
renderInNonRouteComponentContext(
() => (
<Dynamic
component={props.errorComponent ?? ErrorComponent}
error={error}
reset={reset}
/>
),
'errorComponent',
)
) : (
<Dynamic
component={props.errorComponent ?? ErrorComponent}
error={error}
Expand Down
Loading
Loading