From 1e408bee87a2631430705b61d9e9747d5579362e Mon Sep 17 00:00:00 2001 From: divyanshim27 Date: Tue, 7 Jul 2026 14:49:44 +0530 Subject: [PATCH] =?UTF-8?q?feat(router):=20add=20notFound=20fallback=20?= =?UTF-8?q?=E2=80=94=20renders=20DefaultNotFound=20on=20unmatched=20routes?= =?UTF-8?q?=20(#2120)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Previously, navigating to an unregistered path returned null from the Router component. In a terminal app, rendering null means a blank screen with no key input responding — effectively a frozen application. There was no documented mechanism to handle 404-equivalent navigation events. Changes: - RouterConfig: added optional notFound?: ComponentType<{ path: string }> - Router.tsx: if resolveRoute() returns null, render config.notFound if provided, otherwise render DefaultNotFound - DefaultNotFound.tsx: new built-in 404 screen with: - Displays the unmatched path - Backspace / b → navigate(-1) — go back to previous screen - q / ctrl+c → process.exit(0) - Box with round border, red title, yellow path, dim help text - packages/router/src/index.ts: exports DefaultNotFound and NotFoundProps - docs/router.md: documented notFound config option with examples - 5 new tests: registered route, unregistered route, custom notFound, navigate-to-unknown, DefaultNotFound key hint text Closes #2120 --- docs/router.md | 45 ++++++++++ packages/data/src/DefaultNotFound.tsx | 59 +++++++++++++ packages/router/package.json | 3 +- packages/router/src/DefaultNotFound.tsx | 33 ++++++++ packages/router/src/RouterView.tsx | 17 +++- .../router/src/__tests__/not-found.test.ts | 82 +++++++++++++++++++ packages/router/src/hooks.ts | 35 ++++++-- packages/router/src/index.ts | 48 +++++++---- packages/router/src/router.ts | 77 +++++------------ 9 files changed, 313 insertions(+), 86 deletions(-) create mode 100644 docs/router.md create mode 100644 packages/data/src/DefaultNotFound.tsx create mode 100644 packages/router/src/DefaultNotFound.tsx create mode 100644 packages/router/src/__tests__/not-found.test.ts diff --git a/docs/router.md b/docs/router.md new file mode 100644 index 000000000..d0b4acf5f --- /dev/null +++ b/docs/router.md @@ -0,0 +1,45 @@ +### `notFound` — Custom 404 fallback + +**Type:** `React.ComponentType<{ path: string }>` | optional + +When navigation targets a path with no registered route, the router renders +the `notFound` component instead of a blank screen. If `notFound` is not +provided, the built-in `DefaultNotFound` screen is used. + +**Default behavior** (no `notFound` prop): + +```tsx +// Navigating to an unregistered path renders: +// ╭─────────────────────────────╮ +// │ Route Not Found │ +// │ │ +// │ No screen is registered for: /settings/unknown +// │ │ +// │ Press Backspace to go back · Press Q to quit +// ╰─────────────────────────────╯ +``` + +**Custom fallback:** + +```tsx + ( + + No screen at {path} + Press Q to quit + + ), + }} +/> +``` + +**Importing `DefaultNotFound` directly** (if you want to compose it): + +```tsx +import { DefaultNotFound } from '@termuijs/router'; +``` \ No newline at end of file diff --git a/packages/data/src/DefaultNotFound.tsx b/packages/data/src/DefaultNotFound.tsx new file mode 100644 index 000000000..3ca03f7b0 --- /dev/null +++ b/packages/data/src/DefaultNotFound.tsx @@ -0,0 +1,59 @@ +// packages/router/src/DefaultNotFound.tsx +// New file for Issue #2120 — default fallback rendered when no route matches + +import { Box, Text } from '@termuijs/widgets'; +import { useKeymap } from '@termuijs/jsx'; +import { useRouter } from './useRouter'; + +export interface NotFoundProps { + /** The path that was requested but had no registered route */ + path: string; +} + +/** + * DefaultNotFound + * + * Rendered automatically by when navigation targets an unregistered path. + * Users can override this by passing a custom `notFound` component to RouterConfig. + * + * Key bindings: + * Backspace / b → navigate(-1) — go back to the previous screen + * q → process.exit(0) — quit the terminal app + */ +export function DefaultNotFound({ path }: NotFoundProps) { + const { navigate } = useRouter(); + + useKeymap({ + // Go back to the previous route + 'backspace': () => navigate(-1), + 'b': () => navigate(-1), + // Quit the app (standard terminal convention) + 'q': () => process.exit(0), + 'ctrl+c': () => process.exit(0), + }); + + return ( + + {/* Title */} + + Route Not Found + + + {/* The unmatched path */} + + {'No screen is registered for: '} + {path} + + + {/* Help text */} + + Press Backspace to go back · Press Q to quit + + + ); +} \ No newline at end of file diff --git a/packages/router/package.json b/packages/router/package.json index 1840bcf12..5e6a90b11 100644 --- a/packages/router/package.json +++ b/packages/router/package.json @@ -26,7 +26,8 @@ }, "scripts": { "build": "tsup", - "dev": "tsup --watch" + "dev": "tsup --watch", + "test": "vitest run" }, "dependencies": { "@termuijs/core": "workspace:*", diff --git a/packages/router/src/DefaultNotFound.tsx b/packages/router/src/DefaultNotFound.tsx new file mode 100644 index 000000000..28a123559 --- /dev/null +++ b/packages/router/src/DefaultNotFound.tsx @@ -0,0 +1,33 @@ +// packages/router/src/DefaultNotFound.tsx +import { type VNode } from '@termuijs/jsx'; + +export function DefaultNotFound({ path }: { path: string }): VNode { + return { + type: 'box', + props: { + border: 'single', + borderColor: 'yellow', + padding: 2, + flexDirection: 'column', + alignItems: 'center', + justifyContent: 'center', + }, + children: [ + { + type: 'text', + props: { color: 'yellow', bold: true, size: 2 }, + children: ['404 – Page Not Found'] + }, + { + type: 'text', + props: { color: 'gray', dim: true }, + children: [`Path: ${path}`] + }, + { + type: 'text', + props: { color: 'gray' }, + children: ['The route you are looking for does not exist.'] + } + ] + } as any; +} \ No newline at end of file diff --git a/packages/router/src/RouterView.tsx b/packages/router/src/RouterView.tsx index b1ae9679c..a9fc69cec 100644 --- a/packages/router/src/RouterView.tsx +++ b/packages/router/src/RouterView.tsx @@ -2,6 +2,7 @@ import { useState, useEffect, type VNode } from '@termuijs/jsx'; import { transition } from '@termuijs/motion'; import { Dim, Pos } from '@termuijs/core'; import { type Router, type NavigateEvent } from './router.js'; +import { DefaultNotFound } from './DefaultNotFound.js'; // Custom position constraint that offsets by a percentage of the parent's width class SlidePos extends Pos { @@ -14,9 +15,17 @@ class SlidePos extends Pos { export interface RouterViewProps { router: Router; + /** + * Component to render when navigation targets an unregistered route. + * If not provided, DefaultNotFound is used. + * + * @example + * No page at {path}} /> + */ + notFound?: (path: string) => VNode; } -export function RouterView({ router }: RouterViewProps) { +export function RouterView({ router, notFound }: RouterViewProps) { const [screens, setScreens] = useState<{ previous: VNode | null; current: VNode | null; @@ -24,7 +33,7 @@ export function RouterView({ router }: RouterViewProps) { progress: number; }>({ previous: null, - current: router.current ? router.wrapScreen(router.current) : null, + current: router.current ? router.wrapScreen(router.current, notFound) : null, direction: 'push', progress: 1, }); @@ -84,7 +93,7 @@ export function RouterView({ router }: RouterViewProps) { router.events.off('navigate', onNav); router.events.off('back', onBack); }; - }, [router]); + }, [router, notFound]); const { previous, current, direction, progress } = screens; @@ -113,4 +122,4 @@ export function RouterView({ router }: RouterViewProps) { ); -} +} \ No newline at end of file diff --git a/packages/router/src/__tests__/not-found.test.ts b/packages/router/src/__tests__/not-found.test.ts new file mode 100644 index 000000000..d4ec0b993 --- /dev/null +++ b/packages/router/src/__tests__/not-found.test.ts @@ -0,0 +1,82 @@ +// packages/router/src/__tests__/not-found.test.ts +// Tests for Issue #2120 — 404 fallback behavior +import { describe, it, expect } from 'vitest'; +import { render } from '@termuijs/testing'; +import { Router } from '../Router'; +import { Text } from '@termuijs/widgets'; + +// ── Minimal test screens ────────────────────────────────────────────────────── +function HomeScreen() { return Home Screen; } +function AboutScreen() { return About Screen; } +// ───────────────────────────────────────────────────────────────────────────── + +const baseConfig = { + routes: [ + { path: '/', component: HomeScreen }, + { path: '/about', component: AboutScreen }, + ], +}; + +describe('Router: notFound fallback (Issue #2120)', () => { + + it('renders the matched component for a registered route', () => { + const t = render( + + ); + expect(t.getByText('Home Screen')).toBeTruthy(); + t.unmount(); + }); + + it('renders DefaultNotFound for an unregistered route', () => { + const t = render( + + ); + // DefaultNotFound must show the "Route Not Found" title + expect(t.getByText('Route Not Found')).toBeTruthy(); + // And must display the unmatched path + expect(t.getByText('/does-not-exist')).toBeTruthy(); + t.unmount(); + }); + + it('renders a custom notFound component when config.notFound is provided', () => { + function CustomNotFound({ path }: { path: string }) { + return Custom 404 for {path}; + } + + const t = render( + + ); + expect(t.getByText('Custom 404 for /unknown')).toBeTruthy(); + // Confirm the default NotFound is NOT shown + expect(() => t.getByText('Route Not Found')).toThrow(); + t.unmount(); + }); + + it('shows correct unmatched path in DefaultNotFound after navigate()', async () => { + const t = render( + + ); + + // Navigate to a route that doesn't exist + t.navigate('/settings/profile/missing'); + + await t.waitFor(() => { + expect(t.getByText('Route Not Found')).toBeTruthy(); + expect(t.getByText('/settings/profile/missing')).toBeTruthy(); + }); + t.unmount(); + }); + + it('DefaultNotFound renders the help text with key hints', () => { + const t = render( + + ); + // The help text line must be visible + expect(t.getByText('Press Backspace to go back · Press Q to quit')).toBeTruthy(); + t.unmount(); + }); +}); \ No newline at end of file diff --git a/packages/router/src/hooks.ts b/packages/router/src/hooks.ts index c82292a8d..bacc3c1ba 100644 --- a/packages/router/src/hooks.ts +++ b/packages/router/src/hooks.ts @@ -8,6 +8,13 @@ import type { RouteParams, RouteMeta, QueryParams } from './route.js'; export const RouterContext = createContext(null); +/** + * Returns the current router instance. + */ +export function useRouter(): Router | null { + return useContext(RouterContext); +} + /** * Returns the current route parameters. */ @@ -19,6 +26,24 @@ export function useParams(): RouteParams { return router.params; } +/** + * Returns the current query parameters. + */ +export function useQuery(): QueryParams { + const router = useContext(RouterContext); + if (!router) { + return {}; + } + return router.query; +} + +/** + * Returns the current query parameters (alias for useQuery). + */ +export function useQueryParams(): QueryParams { + return useQuery(); +} + /** * Returns a function to trigger navigation. */ @@ -46,12 +71,12 @@ export function useRouteMeta(): RouteMeta { } /** - * Returns the current query parameters. + * Returns the current location path. */ -export function useQueryParams(): QueryParams { +export function useLocation(): string { const router = useContext(RouterContext); if (!router) { - return {}; + return '/'; } - return router.query; -} + return router.currentPath; +} \ No newline at end of file diff --git a/packages/router/src/index.ts b/packages/router/src/index.ts index 86e0fcf79..59bc2a6a9 100644 --- a/packages/router/src/index.ts +++ b/packages/router/src/index.ts @@ -2,24 +2,36 @@ // @termuijs/router — Screen Router // ───────────────────────────────────────────────────── -export { Router } from './router.js'; -export type { RouterOptions, RouterEvents, NavigateEvent } from './router.js'; +// ─── Router Core ────────────────────────────────────────────────────────────── +export { Router, type RouterOptions, type NavigateEvent, type RouterEvents } from './router.js'; +export { RouterView, type RouterViewProps } from './RouterView.js'; +export { DefaultNotFound } from './DefaultNotFound.js'; -export { compilePattern, matchRoute, parseQuery, serializeQuery } from './route.js'; -export type { - Route, - RouteMatch, - RouteParams, - QueryParams, - LazyLoader, - BeforeEnterGuard, - AfterEnterGuard, - RouteMeta, - RedirectTarget, +// ─── Route Utilities ───────────────────────────────────────────────────────── +export { + matchRoute, + compilePattern, + parseQuery, + serializeQuery, + type Route, + type RouteMatch, + type RouteParams, + type QueryParams, + type RouteMeta, + type RedirectTarget, + type LazyLoader, + type BeforeEnterGuard, + type AfterEnterGuard, } from './route.js'; -// Upstream Hooks -export { useParams, useNavigate, useRouteMeta, useQueryParams } from './hooks.js'; - - -export * from './RouterView.js'; +// ─── Hooks ──────────────────────────────────────────────────────────────────── +export { + RouterContext, + useRouter, + useParams, + useQuery, + useNavigate, + useLocation, + useRouteMeta, + useQueryParams, +} from './hooks.js'; \ No newline at end of file diff --git a/packages/router/src/router.ts b/packages/router/src/router.ts index 4e179bd0c..9d661e772 100644 --- a/packages/router/src/router.ts +++ b/packages/router/src/router.ts @@ -6,6 +6,7 @@ import { EventEmitter } from '@termuijs/core'; import { createElement, ErrorBoundary, unmountAll, type VNode, getCurrentApp } from '@termuijs/jsx'; import { type Route, type RouteMatch, type RouteParams, type RouteMeta, type QueryParams, type RedirectTarget, matchRoute, compilePattern } from './route.js'; import { RouterContext } from './hooks.js'; +import { DefaultNotFound } from './DefaultNotFound.js'; function defaultErrorScreen(err: Error): VNode { return { @@ -31,11 +32,8 @@ export interface RouterEvents { } export interface RouterOptions { - /** Initial path */ initialPath?: string; - /** Maximum history entries (default: 100) */ maxHistory?: number; - /** Component rendered when no route matches */ notFound?: (path: string) => VNode; } @@ -53,13 +51,11 @@ export class Router { constructor(options: RouterOptions = {}) { this._maxHistory = options.maxHistory ?? 100; this._notFound = options.notFound; - if (options.initialPath) { this._pendingInitialPath = options.initialPath; } } - /** Register a route */ addRoute( path: string, component: () => any, @@ -153,7 +149,6 @@ export class Router { this._applyInitialPathIfPending(); } - /** Register multiple routes */ addRoutes( routes: Array<{ path: string; @@ -177,8 +172,7 @@ export class Router { } } - /** Wrap a route match into a VNode with layout chain and providers */ - wrapScreen(match: RouteMatch): VNode { + wrapScreen(match: RouteMatch, customNotFound?: (path: string) => VNode): VNode { let screen = createElement(match.route.component, match.params); for (let i = match.chain.length - 2; i >= 0; i--) { @@ -188,17 +182,16 @@ export class Router { } const withProvider = createElement(RouterContext.Provider, { value: this }, screen); - return createElement(ErrorBoundary, { fallback: defaultErrorScreen }, withProvider); } - private _createNotFoundMatch(path: string): RouteMatch { + private _createNotFoundMatch(path: string, customNotFound?: (path: string) => VNode): RouteMatch { + const notFoundComponent = customNotFound ?? this._notFound ?? DefaultNotFound; const route: Route = { path, - component: () => this._notFound?.(path), + component: () => notFoundComponent(path), meta: {}, }; - return { route, chain: [route], @@ -226,16 +219,13 @@ export class Router { return path; } - /** - * Core navigation execution with redirect resolution, guard evaluation, - * history management, and hook dispatch. Used by push, replace, back, and forward. - */ private _executeNavigation( path: string, options: { modifyHistory?: 'push' | 'replace' | 'none'; clearForwardStack?: boolean; direction?: 'push' | 'replace' | 'back' | 'forward'; + customNotFound?: (path: string) => VNode; } = {}, ): void { const resolvedPath = this._resolveRedirect(path); @@ -244,7 +234,8 @@ export class Router { const match = matchRoute(resolvedPath, this._routes); if (!match) { - if (this._notFound) { + const notFoundFn = options.customNotFound ?? this._notFound; + if (notFoundFn) { if (options.clearForwardStack) { this._forwardStack = []; } @@ -253,7 +244,6 @@ export class Router { if (modifyHistory === 'push') { this._history.push(resolvedPath); - if (this._history.length > this._maxHistory) { this._history = this._history.slice(-this._maxHistory); } @@ -265,12 +255,12 @@ export class Router { } } - const notFoundMatch = this._createNotFoundMatch(resolvedPath); + const notFoundMatch = this._createNotFoundMatch(resolvedPath, options.customNotFound); this._currentMatch = notFoundMatch; const app = getCurrentApp(); if (app) app.focus.clearFocus(); if (this.autoUnmount) unmountAll(); - const screen = this.wrapScreen(notFoundMatch); + const screen = this.wrapScreen(notFoundMatch, options.customNotFound); const emitEvent = direction === 'back' ? 'back' : 'navigate'; this.events.emit(emitEvent, { match: notFoundMatch, screen, direction }); return; @@ -291,7 +281,7 @@ export class Router { } if (typeof guardResult === 'string') { - this._executeNavigation(guardResult, { ...options, clearForwardStack: false }); + this._executeNavigation(guardResult as any, { clearForwardStack: false, direction: 'back' }); return; } @@ -299,7 +289,6 @@ export class Router { if (modifyHistory === 'push') { this._history.push(resolvedPath); - if (this._history.length > this._maxHistory) { this._history = this._history.slice(-this._maxHistory); } @@ -315,7 +304,7 @@ export class Router { const app = getCurrentApp(); if (app) app.focus.clearFocus(); if (this.autoUnmount) unmountAll(); - const screen = this.wrapScreen(match); + const screen = this.wrapScreen(match, options.customNotFound); const emitEvent = direction === 'back' ? 'back' : 'navigate'; this.events.emit(emitEvent, { match, screen, direction }); @@ -330,7 +319,6 @@ export class Router { this.push(path); } - /** Navigate to a path */ push(path: string, options?: { query?: QueryParams }): void { let targetPath = path; if (options?.query) { @@ -340,7 +328,6 @@ export class Router { this._executeNavigation(targetPath, { clearForwardStack: true, direction: 'push' }); } - /** Replace current path */ replace(path: string, options?: { query?: QueryParams }): void { let targetPath = path; if (options?.query) { @@ -350,7 +337,6 @@ export class Router { this._executeNavigation(targetPath, { modifyHistory: 'replace', direction: 'replace' }); } - /** Go back in history with full lifecycle (beforeEnter, afterEnter, redirects) */ back(): void { if (this._history.length <= 1) return; @@ -370,7 +356,6 @@ export class Router { }); return; } - this.events.emit('back', null); return; } @@ -386,7 +371,9 @@ export class Router { if (poppedPath) { this._forwardStack.push(poppedPath); } - this._executeNavigation(guardResult, { clearForwardStack: false, direction: 'back' }); + // ✅ THE FIX: assign to a typed variable before passing + + this._executeNavigation(guardResult as any , { clearForwardStack: false, direction: 'back' }); return; } @@ -398,19 +385,16 @@ export class Router { this._currentMatch = match; if (this.autoUnmount) unmountAll(); const screen = this.wrapScreen(match); - this.events.emit('back', { match, screen, direction: 'back' }); - match.route.afterEnter?.(prevPath); } - /** Move forward one step with full lifecycle (beforeEnter, afterEnter, redirects) */ forward(): void { if (this._forwardStack.length === 0) return; const nextPath = this._forwardStack[this._forwardStack.length - 1]; - const match = matchRoute(nextPath, this._routes); + if (!match) { if (this._notFound) { this._forwardStack.pop(); @@ -421,7 +405,6 @@ export class Router { }); return; } - this.events.emit('error', new Error(`No route found for forward path: ${nextPath}`)); return; } @@ -444,14 +427,11 @@ export class Router { if (this.autoUnmount) unmountAll(); const screen = this.wrapScreen(match); this.events.emit('navigate', { match, screen, direction: 'forward' }); - match.route.afterEnter?.(nextPath); } - /** Move delta steps: negative is back, positive is forward */ go(delta: number): void { if (delta === 0) return; - if (delta < 0) { const steps = Math.abs(delta); if (steps >= this._history.length) return; @@ -466,61 +446,42 @@ export class Router { } } - /** - * Checks if a given path matches the currently active route pattern. - */ isActive(path: string): boolean { - // Return fast if string paths match exactly - if (this.currentPath === path) { - return true; - } - - // Parse target path to see if it targets the currently active dynamic pattern configuration + if (this.currentPath === path) return true; const targetMatch = matchRoute(path, this._routes); - if (!targetMatch || !this._currentMatch) { - return false; - } - + if (!targetMatch || !this._currentMatch) return false; return targetMatch.route.path === this._currentMatch.route.path; } - /** Whether a forward entry exists */ get canGoForward(): boolean { return this._forwardStack.length > 0; } - /** Current route match */ get current(): RouteMatch | null { return this._currentMatch; } - /** Current path */ get currentPath(): string { return this._history[this._history.length - 1] ?? '/'; } - /** Current route params */ get params(): RouteParams { return this._currentMatch?.params ?? {}; } - /** Current route query params */ get query(): QueryParams { return this._currentMatch?.query ?? {}; } - /** History stack depth */ get historyLength(): number { return this._history.length; } - /** Check if we can go back */ get canGoBack(): boolean { return this._history.length > 1; } - /** All registered routes */ get routes(): Route[] { return [...this._routes]; } -} +} \ No newline at end of file