diff --git a/packages/low-router/src/LowRouter.ts b/packages/low-router/src/LowRouter.ts index ec86ea7..efa46a8 100644 --- a/packages/low-router/src/LowRouter.ts +++ b/packages/low-router/src/LowRouter.ts @@ -1,5 +1,5 @@ import debug from "@wbe/debug" -import { PathnameOrObject, Resolve, Route, RouteContext, RouteParams, RouterOptions } from "./types" +import { PathnameOrObject, QueryParams, Resolve, Route, RouteContext, RouteParams, RouterOptions } from "./types" import { createMatcher, Matcher } from "./utils/createMatcher" import { compilePath as defaultCompilePath, CompilePath } from "./utils/compilePath" import { normalizePath } from "./utils/normalizePath" @@ -35,10 +35,10 @@ export class LowRouter { */ public async resolve(pathnameOrObject: PathnameOrObject): Promise { const obj = this.#resolver(pathnameOrObject) - if (typeof obj.context.route?.action === "function") { + if (typeof obj.context?.route?.action === "function") { obj.response = await obj.context.route.action(obj.context) } - this.options.onResolve?.(obj) + if (obj.context) this.options.onResolve?.(obj) return Promise.resolve(obj) } @@ -51,7 +51,7 @@ export class LowRouter { if (typeof obj.context?.route?.action === "function") { obj.response = obj.context.route.action(obj.context) } - this.options.onResolve?.(obj) + if (obj.context) this.options.onResolve?.(obj) return obj } @@ -62,15 +62,15 @@ export class LowRouter { */ #resolver(pathnameOrObject: PathnameOrObject) { // match route - const routeContext = this.matchRoute( + const pathname = typeof pathnameOrObject === "string" ? pathnameOrObject - : this.createUrl({ name: pathnameOrObject?.name, params: pathnameOrObject?.params }) - ) + : this.createUrl(pathnameOrObject) + const routeContext = pathname ? this.matchRoute(pathname) : undefined // error if (!routeContext) { - this.#log(`No matching route found with pathname ${pathnameOrObject}`, this.routes) - this.options.onError?.() + this.#log(`No matching route found with pathname ${pathname}`, this.routes) + this.options.onError?.({ pathname, input: pathnameOrObject }) return { response: undefined, context: undefined } } // save current context @@ -81,7 +81,14 @@ export class LowRouter { } public dispose(): void { - this.currentContext = null + // walk the parent chain to break references and help GC + let ctx: RouteContext | undefined | null = this.currentContext + while (ctx) { + const parent = ctx.parent + ctx.parent = null + ctx = parent + } + this.currentContext = undefined this.options.onDispose?.() } @@ -102,7 +109,9 @@ export class LowRouter { let relativePathname: string | undefined try { relativePathname = this.compilePath(route.path)(params) - } catch {} + } catch (e) { + this.#log("compilePath error", route.path, e) + } const ctx = { pathname, @@ -132,9 +141,20 @@ export class LowRouter { * ex: * createUrl({ name: "home" }) => "/" * createUrl({ name: "user", params: { id: "1" } }) => "/user/1" + * createUrl({ name: "user", params: { id: "1" }, query: { tab: "x" }, hash: "top" }) => "/user/1?tab=x#top" */ - public createUrl({ name, params = {} }: { name: string; params?: RouteParams }): string { - const next = (name, params, routes, curBase): string => { + public createUrl({ + name, + params = {}, + query, + hash, + }: { + name: string + params?: RouteParams + query?: QueryParams + hash?: string + }): string | undefined { + const next = (name, params, routes, curBase): string | undefined => { for (let route of routes) { const compiledPath = normalizePath(this.compilePath(curBase + route.path)(params)) if (route.name === name) { @@ -145,7 +165,23 @@ export class LowRouter { } } } - return next(name, params, this.routes, this.options.base) + const pathname = next(name, params, this.routes, this.options.base) + if (!pathname) return undefined + + // append query string + let qs = "" + if (query) { + const sp = new URLSearchParams() + for (const k in query) { + const v = query[k] + if (v != null) sp.append(k, String(v)) + } + const s = sp.toString() + if (s) qs = `?${s}` + } + // append hash + const h = hash ? (hash[0] === "#" ? hash : `#${hash}`) : "" + return pathname + qs + h } #log(...rest: any[]): void { diff --git a/packages/low-router/src/types.ts b/packages/low-router/src/types.ts index 5402783..ed560ac 100644 --- a/packages/low-router/src/types.ts +++ b/packages/low-router/src/types.ts @@ -5,7 +5,9 @@ export type RouteParams = { [paramName: string]: string } export type QueryParams = { [paramName: string]: string } export type Hash = string export type RouteProps = Record -export type PathnameOrObject = string | { name: string; params: RouteParams } +export type PathnameOrObject = + | string + | { name: string; params?: RouteParams; query?: QueryParams; hash?: string } export interface RouteContext { pathname: string @@ -20,7 +22,7 @@ export interface RouteContext { export interface Resolve { response: any - context: RouteContext + context: RouteContext | undefined } export interface Route { @@ -36,7 +38,7 @@ export interface RouterOptions { onInit: () => void onResolve: ({ response, context }: Resolve) => void onDispose: () => void - onError: () => void + onError: (info: { pathname?: string; input: PathnameOrObject }) => void matcher: Matcher compilePath: CompilePath id?: number | string diff --git a/packages/low-router/tests/createUrl.test.ts b/packages/low-router/tests/createUrl.test.ts index 52e9930..2ad63ff 100644 --- a/packages/low-router/tests/createUrl.test.ts +++ b/packages/low-router/tests/createUrl.test.ts @@ -99,4 +99,32 @@ describe.concurrent("createUrl", () => { ) expect(router.createUrl({ name: "f", params: { lang: "en" } })).toBe("/en/test/f") }) + + it("should append query and hash", () => { + const routes = [ + { path: "/", name: "home" }, + { path: "/user/:id", name: "user" }, + ] + const router = new LowRouter(routes) + + expect(router.createUrl({ name: "user", params: { id: "1" }, query: { tab: "profile" } })).toBe( + "/user/1?tab=profile" + ) + expect(router.createUrl({ name: "user", params: { id: "1" }, hash: "top" })).toBe("/user/1#top") + expect( + router.createUrl({ + name: "user", + params: { id: "1" }, + query: { tab: "x", y: "z", }, + hash: "#anchor", + }) + ).toBe("/user/1?tab=x&y=z#anchor") + // undefined/null values are skipped + expect( + router.createUrl({ + name: "home", + query: { a: "1", b: undefined, c: null }, + }) + ).toBe("/?a=1") + }) }) diff --git a/packages/low-router/tests/resolve.test.ts b/packages/low-router/tests/resolve.test.ts index b0933ea..3b4cbb0 100644 --- a/packages/low-router/tests/resolve.test.ts +++ b/packages/low-router/tests/resolve.test.ts @@ -97,6 +97,51 @@ describe.concurrent("resolve", () => { }) }) + it("should not throw on unmatched pathname", async () => { + const router = new LowRouter([{ path: "/foo", action: () => "foo" }]) + const { response, context } = await router.resolve("/nope") + expect(response).toBeUndefined() + expect(context).toBeUndefined() + }) + + it("should call onError and skip onResolve on 404", async () => { + const onResolve = vi.fn() + const onError = vi.fn() + const router = new LowRouter([{ path: "/foo" }], { onResolve, onError }) + await router.resolve("/nope") + expect(onError).toHaveBeenCalledTimes(1) + expect(onError).toHaveBeenCalledWith({ pathname: "/nope", input: "/nope" }) + expect(onResolve).not.toHaveBeenCalled() + }) + + it("should pass input object to onError when name is unknown", async () => { + const onError = vi.fn() + const router = new LowRouter([{ path: "/foo", name: "foo" }], { onError }) + const input = { name: "missing", params: {} } + await router.resolve(input) + expect(onError).toHaveBeenCalledWith({ pathname: undefined, input }) + }) + + it("should resolve by { name, params }", async () => { + const routes = [{ path: "/u/:id", name: "user", action: (ctx: any) => ctx.params.id }] + const router = new LowRouter(routes) + const { response } = await router.resolve({ name: "user", params: { id: "42" } }) + expect(response).toBe("42") + }) + + it("should break parent chain and call onDispose", () => { + const onDispose = vi.fn() + const router = new LowRouter( + [{ path: "/", children: [{ path: "/z", children: [{ path: "/c" }] }] }], + { onDispose } + ) + router.resolveSync("/z/c") + expect(router.currentContext).toBeDefined() + router.dispose() + expect(router.currentContext).toBeUndefined() + expect(onDispose).toHaveBeenCalledTimes(1) + }) + it("should resolve child route", () => { return new Promise(async (resolve: any) => { const routes = [ diff --git a/packages/low-router/tests/resolveSync.test.ts b/packages/low-router/tests/resolveSync.test.ts index ce60a5e..332cfc8 100644 --- a/packages/low-router/tests/resolveSync.test.ts +++ b/packages/low-router/tests/resolveSync.test.ts @@ -85,6 +85,13 @@ describe.concurrent("resolveSync", () => { expect(response).toBe("action response!") }) + it("should not throw on unmatched pathname", () => { + const router = new LowRouter([{ path: "/foo", action: () => "foo" }]) + const { response, context } = router.resolveSync("/nope") + expect(response).toBeUndefined() + expect(context).toBeUndefined() + }) + it("should resolve child route", () => { const routes = [ {