From 1344ffe3d92fcce77e7bd7554d25e4f99a3b8afe Mon Sep 17 00:00:00 2001 From: Pooya Parsa Date: Mon, 8 Jul 2024 14:01:10 +0200 Subject: [PATCH 1/3] refactor!: improve router --- package.json | 2 +- pnpm-lock.yaml | 10 +- src/app/_utils.ts | 10 +- src/router.ts | 203 ++++++++++++++++++++------------------ src/types/context.ts | 4 +- src/types/handler.ts | 7 +- src/types/index.ts | 9 +- src/types/router.ts | 37 ++++--- test/bench/bundle.test.ts | 1 - test/resolve.test.ts | 22 ++--- test/router.test.ts | 15 +-- 11 files changed, 171 insertions(+), 149 deletions(-) diff --git a/package.json b/package.json index d14dd6a7..c28ec6f3 100644 --- a/package.json +++ b/package.json @@ -42,7 +42,7 @@ "cookie-es": "^1.1.0", "iron-webcrypto": "^1.2.1", "ohash": "^1.1.3", - "rou3": "^0.1.0", + "rou3": "^0.2.0", "ufo": "^1.5.3", "uncrypto": "^0.1.3" }, diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 87e843ed..99bd9bcd 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -21,8 +21,8 @@ importers: specifier: ^1.1.3 version: 1.1.3 rou3: - specifier: ^0.1.0 - version: 0.1.0 + specifier: ^0.2.0 + version: 0.2.0 ufo: specifier: ^1.5.3 version: 1.5.3 @@ -2752,8 +2752,8 @@ packages: engines: {node: '>=18.0.0', npm: '>=8.0.0'} hasBin: true - rou3@0.1.0: - resolution: {integrity: sha512-/XsAcVWWDQilIc8G15da40sPN77TZ91DdWBx+B8khqg6yjFhuFObtx90BNdRQBUfFPQPa4zf6myZu3gk5Rs+Yg==} + rou3@0.2.0: + resolution: {integrity: sha512-pM+Zd++BIMNcfZXOsErMc8ycuYFB6Y5dmanMfyIOXwiDhYx3EDDoLPKEn5oMSl/P9s4Yi0Y2+cqzrTCw83Q7iQ==} run-applescript@5.0.0: resolution: {integrity: sha512-XcT5rBksx1QdIhlFOCtgZkB99ZEouFZ1E2Kc2LHqNW13U3/74YGdkQRmThTwxy4QIyookibDKYZOPqX//6BlAg==} @@ -5921,7 +5921,7 @@ snapshots: '@rollup/rollup-win32-x64-msvc': 4.18.0 fsevents: 2.3.3 - rou3@0.1.0: {} + rou3@0.2.0: {} run-applescript@5.0.0: dependencies: diff --git a/src/app/_utils.ts b/src/app/_utils.ts index 0930f9d4..7e9565fc 100644 --- a/src/app/_utils.ts +++ b/src/app/_utils.ts @@ -8,6 +8,7 @@ import type { WebSocketOptions, App, EventHandler, + HTTPMethod, } from "../types"; import { _kRaw } from "../event"; import { defineLazyEventHandler } from "../handler"; @@ -44,7 +45,7 @@ export function use( } export function createResolver(stack: Stack): EventHandlerResolver { - return async (path: string) => { + return async (method, path) => { let _layerPath: string; for (const layer of stack) { if (layer.route === "/" && !layer.handler.__resolve__) { @@ -59,7 +60,7 @@ export function createResolver(stack: Stack): EventHandlerResolver { } let res = { route: layer.route, handler: layer.handler }; if (res.handler.__resolve__) { - const _res = await res.handler.__resolve__(_layerPath); + const _res = await res.handler.__resolve__(method, _layerPath); if (!_res) { continue; } @@ -94,14 +95,15 @@ export function normalizeLayer(input: InputLayer) { } export function resolveWebsocketOptions( - evResolver: EventHandlerResolver, + resolver: EventHandlerResolver, appOptions: AppOptions, ): WebSocketOptions { return { ...appOptions.websocket, async resolve(info) { const pathname = getPathname(info.url || "/"); - const resolved = await evResolver(pathname); + const method = ((info as any).method || "GET") as HTTPMethod; + const resolved = await resolver(method, pathname); return resolved?.handler?.__websocket__ || {}; }, }; diff --git a/src/router.ts b/src/router.ts index 726895f1..cf8694f1 100644 --- a/src/router.ts +++ b/src/router.ts @@ -1,102 +1,108 @@ -import { - createRouter as _createRouter, - findRoute as _findRoute, - addRoute as _addRoute, -} from "rou3"; import type { - CreateRouterOptions, + RouterOptions, EventHandler, - RouteNode, + HTTPMethod, + RouterEntry, Router, - RouterMethod, + H3Event, } from "./types"; +import { + type RouterContext, + createRouter as _createRouter, + findRoute, + addRoute, +} from "rou3"; import { createError } from "./error"; -import { withLeadingSlash } from "./utils/internal/path"; - -const RouterMethods: RouterMethod[] = [ - "connect", - "delete", - "get", - "head", - "options", - "post", - "put", - "trace", - "patch", -]; /** * Create a new h3 router instance. */ -export function createRouter(opts: CreateRouterOptions = {}): Router { - const _router = _createRouter(); +export function createRouter(opts: RouterOptions = {}): Router { + return new H3Router(opts); +} - const router: Router = {} as Router; +class H3Router implements Router { + _router: RouterContext; + _options: RouterOptions; + constructor(opts: RouterOptions = {}) { + this._router = _createRouter(); + this._options = opts; + this.handler = this.handler.bind(this); + } - // Utility to add a new route - function addRoute( - path: string, - handler: EventHandler, - method: RouterMethod | RouterMethod[] | "" | undefined, - ) { - if (Array.isArray(method)) { - for (const _method of method) { - addRoute(path, handler, _method); - } - } else { - const _method = (method || "").toLowerCase(); - _addRoute(_router, path, _method, { - handler, - path, - method: _method, - }); - } - return router; + all(path: string, handler: EventHandler) { + return this.add("", path, handler); } - // Shortcuts - router.use = router.add = (path, handler, method) => - addRoute(path, handler as EventHandler, method); - for (const method of RouterMethods) { - router[method] = (path, handle) => router.add(path, handle, method); + use(path: string, handler: EventHandler) { + return this.all(path, handler); } - // Handler matcher - function matchRoute( - path = "/", - method: RouterMethod = "get", - ): { error: Error } | { data: RouteNode; params?: Record } { - // Remove query parameters for matching - const qIndex = path.indexOf("?"); - if (qIndex !== -1) { - path = path.slice(0, Math.max(0, qIndex)); - } - // Match route - const match = _findRoute(_router, path, method); - if (!match) { - return { - error: createError({ - statusCode: 404, - name: "Not Found", - statusMessage: `Cannot find any route matching [${method}] ${path || "/"}.`, - }), - }; - } - return match as { data: RouteNode; params?: Record }; + get(path: string, handler: EventHandler) { + return this.add("GET", path, handler); + } + + post(path: string, handler: EventHandler) { + return this.add("POST", path, handler); + } + + put(path: string, handler: EventHandler) { + return this.add("PUT", path, handler); + } + + delete(path: string, handler: EventHandler) { + return this.add("DELETE", path, handler); + } + + patch(path: string, handler: EventHandler) { + return this.add("PATCH", path, handler); } - // Main handle - router.handler = (event) => { + head(path: string, handler: EventHandler) { + return this.add("HEAD", path, handler); + } + + options(path: string, handler: EventHandler) { + return this.add("OPTIONS", path, handler); + } + + connect(path: string, handler: EventHandler) { + return this.add("CONNECT", path, handler); + } + + trace(path: string, handler: EventHandler) { + return this.add("TRACE", path, handler); + } + + add( + method: HTTPMethod | Lowercase | "", + path: string, + handler: EventHandler, + ): this { + const _method = (method || "").toUpperCase(); + addRoute(this._router, _method, path, { + method: _method, + route: path, + handler, + }); + return this; + } + + handler(event: H3Event) { // Match handler - const match = matchRoute( + const match = this._findRoute( + event.method.toUpperCase() as HTTPMethod, event.path, - event.method.toLowerCase() as RouterMethod, ); // No match (method or route) - if ("error" in match) { - if (opts.preemptive) { - throw match.error; + if (!match) { + if (this._options.preemptive) { + throw createError({ + statusCode: 404, + name: "Not Found", + statusMessage: `Cannot find any route matching [${event.method}] ${event.path || "/"}`, + }); } else { return; // Let app match other handlers } @@ -108,33 +114,38 @@ export function createRouter(opts: CreateRouterOptions = {}): Router { // Call handler return Promise.resolve(match.data.handler(event)).then((res) => { - if (res === undefined && opts.preemptive) { + if (res === undefined && this._options.preemptive) { return null; // Send empty content } return res; }); - }; + } + + _findRoute(method: HTTPMethod = "GET", path = "/") { + // Remove query parameters for matching + const qIndex = path.indexOf("?"); + if (qIndex !== -1) { + path = path.slice(0, Math.max(0, qIndex)); + } + return findRoute(this._router, method, path) as + | { data: RouterEntry; params?: Record } + | undefined; + } - // Resolver - router.handler.__resolve__ = async (path) => { - path = withLeadingSlash(path); - const match = matchRoute(path); - if ("error" in match) { + async _resolveRoute(method: HTTPMethod = "GET", path: string) { + const match = this._findRoute(method, path); + if (!match) { return; } - let res = { - route: match.data.path, + const resolved = { + route: match.data.route, handler: match.data.handler, + params: match.params, }; - if (match.data.handler.__resolve__) { - const _res = await match.data.handler.__resolve__(path); - if (!_res) { - return; - } - res = { ...res, ..._res }; + if (resolved.handler.__resolve__) { + const _resolved = await resolved.handler.__resolve__(method, path); + return { ...resolved, ..._resolved }; } - return res; - }; - - return router; + return resolved; + } } diff --git a/src/types/context.ts b/src/types/context.ts index 757e4b1b..3c9d5d1d 100644 --- a/src/types/context.ts +++ b/src/types/context.ts @@ -1,5 +1,5 @@ import type { Session } from "./utils/session"; -import type { RouteNode } from "./router"; +import type { RouterEntry } from "./router"; export interface H3EventContext extends Record { /* Matched router parameters */ @@ -9,7 +9,7 @@ export interface H3EventContext extends Record { * * @experimental The object structure may change in non-major version. */ - matchedRoute?: RouteNode; + matchedRoute?: RouterEntry; /* Cached session data */ sessions?: Record; /* Trusted IP Address of client */ diff --git a/src/types/handler.ts b/src/types/handler.ts index 9bd11a26..665e8aa6 100644 --- a/src/types/handler.ts +++ b/src/types/handler.ts @@ -2,6 +2,8 @@ import type { Readable as NodeReadableStream } from "node:stream"; import type { QueryObject } from "ufo"; import type { H3Event } from "./event"; import type { Hooks as WSHooks } from "crossws"; +import type { HTTPMethod } from "./http"; +import type { RouterEntry } from "./router"; export type ResponseBody = | undefined // middleware pass @@ -26,8 +28,11 @@ export type InferEventInput< type MaybePromise = T | Promise; export type EventHandlerResolver = ( + method: HTTPMethod, path: string, -) => MaybePromise; +) => MaybePromise< + undefined | (Partial & { params?: Record }) +>; export interface EventHandler< Request extends EventHandlerRequest = EventHandlerRequest, diff --git a/src/types/index.ts b/src/types/index.ts index b19cab50..f6fbc42b 100644 --- a/src/types/index.ts +++ b/src/types/index.ts @@ -39,14 +39,7 @@ export type { } from "./web"; // Router -export type { - RouteNode, - Router, - RouterMethod, - RouterUse, - AddRouteShortcuts, - CreateRouterOptions, -} from "./router"; +export type { Router, RouterOptions, RouterEntry } from "./router"; // Context export type { H3EventContext } from "./context"; diff --git a/src/types/router.ts b/src/types/router.ts index ace66b30..f61dbad1 100644 --- a/src/types/router.ts +++ b/src/types/router.ts @@ -1,27 +1,36 @@ import type { EventHandler } from "./handler"; import type { HTTPMethod } from "./http"; -export type RouterMethod = Lowercase; +type AddRoute = (path: string, handler: EventHandler) => Router; -export type RouterUse = ( - path: string, - handler: EventHandler, - method?: RouterMethod | RouterMethod[], -) => Router; -export type AddRouteShortcuts = Record; +export interface Router { + all: AddRoute; + /** @deprecated please use router.all */ + use: Router["all"]; + get: AddRoute; + post: AddRoute; + put: AddRoute; + delete: AddRoute; + patch: AddRoute; + head: AddRoute; + options: AddRoute; + connect: AddRoute; + trace: AddRoute; + add: ( + method: "" | HTTPMethod | Lowercase, + path: string, + handler: EventHandler, + ) => Router; -export interface Router extends AddRouteShortcuts { - add: RouterUse; - use: RouterUse; handler: EventHandler; } -export interface RouteNode { +export interface RouterEntry { + method: HTTPMethod; + route: string; handler: EventHandler; - path: string; - method: string; } -export interface CreateRouterOptions { +export interface RouterOptions { preemptive?: boolean; } diff --git a/test/bench/bundle.test.ts b/test/bench/bundle.test.ts index 6a3f1745..bd955a2b 100644 --- a/test/bench/bundle.test.ts +++ b/test/bench/bundle.test.ts @@ -12,7 +12,6 @@ describe("benchmark", () => { export default toWebHandler(app); `; const { bytes, gzipSize } = await getBundleSize(code); - // console.log("bundle size", { bytes, gzipSize }); expect(bytes).toBeLessThanOrEqual(12_000); // <12kb expect(gzipSize).toBeLessThanOrEqual(5000); // <5kb }); diff --git a/test/resolve.test.ts b/test/resolve.test.ts index 7c35b268..eff1fcd8 100644 --- a/test/resolve.test.ts +++ b/test/resolve.test.ts @@ -33,25 +33,25 @@ describe("Event handler resolver", () => { describe("middleware", () => { it("does not resolves /", async () => { - expect(await app.resolve("/")).toBeUndefined(); + expect(await app.resolve("GET", "/")).toBeUndefined(); }); }); describe("path prefix", () => { it("resolves /test", async () => { - expect(await app.resolve("/test")).toMatchObject({ + expect(await app.resolve("GET", "/test")).toMatchObject({ route: "/test", handler: testHandlers[2], }); }); it("resolves /test/foo", async () => { - expect((await app.resolve("/test/foo"))?.route).toEqual("/test"); + expect((await app.resolve("GET", "/test/foo"))?.route).toEqual("/test"); }); }); it("resolves /lazy", async () => { - expect(await app.resolve("/lazy")).toMatchObject({ + expect(await app.resolve("GET", "/lazy")).toMatchObject({ route: "/lazy", handler: testHandlers[3], }); @@ -59,14 +59,14 @@ describe("Event handler resolver", () => { describe("nested app", () => { it("resolves /nested/path/foo", async () => { - expect(await app.resolve("/nested/path/foo")).toMatchObject({ + expect(await app.resolve("GET", "/nested/path/foo")).toMatchObject({ route: "/nested/path", handler: testHandlers[4], }); }); it.skip("resolves /nested/lazy", async () => { - expect(await app.resolve("/nested/lazy")).toMatchObject({ + expect(await app.resolve("GET", "/nested/lazy")).toMatchObject({ route: "/nested/lazy", handler: testHandlers[5], }); @@ -75,24 +75,24 @@ describe("Event handler resolver", () => { describe("router", () => { it("resolves /router", async () => { - expect(await app.resolve("/router")).toMatchObject({ + expect(await app.resolve("GET", "/router")).toMatchObject({ route: "/router", handler: testHandlers[6], }); - expect(await app.resolve("/router/")).toMatchObject( - (await app.resolve("/router")) as any, + expect(await app.resolve("GET", "/router/")).toMatchObject( + (await app.resolve("GET", "/router")) as any, ); }); it("resolves /router/:id", async () => { - expect(await app.resolve("/router/foo")).toMatchObject({ + expect(await app.resolve("GET", "/router/foo")).toMatchObject({ route: "/router/:id", handler: testHandlers[7], }); }); it("resolves /router/lazy", async () => { - expect(await app.resolve("/router/lazy")).toMatchObject({ + expect(await app.resolve("GET", "/router/lazy")).toMatchObject({ route: "/router/lazy", handler: testHandlers[8], }); diff --git a/test/router.test.ts b/test/router.test.ts index 15833752..e32925e0 100644 --- a/test/router.test.ts +++ b/test/router.test.ts @@ -10,9 +10,10 @@ describe("router", () => { beforeEach(() => { router = createRouter() - .add("/", () => "Hello") - .add("/test/?/a", () => "/test/?/a") - .add("/many/routes", () => "many routes", ["get", "post"]) + .get("/", () => "Hello") + .get("/test/?/a", () => "/test/?/a") + .get("/many/routes", () => "many routes") + .post("/many/routes", () => "many routes") .get("/test", () => "Test (GET)") .post("/test", () => "Test (POST)"); @@ -25,7 +26,7 @@ describe("router", () => { }); it("Multiple Routers", async () => { - const secondRouter = createRouter().add("/router2", () => "router2"); + const secondRouter = createRouter().get("/router2", () => "router2"); ctx.app.use(secondRouter); @@ -108,7 +109,7 @@ describe("router (preemptive)", () => { const res = await ctx.request.get("/404"); expect(JSON.parse(res.text)).toMatchObject({ statusCode: 404, - statusMessage: "Cannot find any route matching [get] /404.", + statusMessage: "Cannot find any route matching [GET] /404", }); }); @@ -214,7 +215,9 @@ describe("event.context.matchedRoute", () => { it("can return the matched path", async () => { const router = createRouter().get("/test/:template", (event) => { expect(event.context.matchedRoute).toMatchObject({ - path: "/test/:template", + method: "GET", + route: "/test/:template", + handler: expect.any(Function), }); return "200"; }); From 7e08889162808775dee1c07101ac09edff0fcbc1 Mon Sep 17 00:00:00 2001 From: Pooya Parsa Date: Mon, 8 Jul 2024 14:08:34 +0200 Subject: [PATCH 2/3] add back test comment --- test/bench/bundle.test.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/test/bench/bundle.test.ts b/test/bench/bundle.test.ts index bd955a2b..6a3f1745 100644 --- a/test/bench/bundle.test.ts +++ b/test/bench/bundle.test.ts @@ -12,6 +12,7 @@ describe("benchmark", () => { export default toWebHandler(app); `; const { bytes, gzipSize } = await getBundleSize(code); + // console.log("bundle size", { bytes, gzipSize }); expect(bytes).toBeLessThanOrEqual(12_000); // <12kb expect(gzipSize).toBeLessThanOrEqual(5000); // <5kb }); From 18cff5dd18d040fe05f5200122c0a8ebc5f3f46e Mon Sep 17 00:00:00 2001 From: Pooya Parsa Date: Mon, 8 Jul 2024 14:11:41 +0200 Subject: [PATCH 3/3] fix resolver --- src/router.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/src/router.ts b/src/router.ts index cf8694f1..9d62e398 100644 --- a/src/router.ts +++ b/src/router.ts @@ -28,6 +28,7 @@ class H3Router implements Router { this._router = _createRouter(); this._options = opts; this.handler = this.handler.bind(this); + (this.handler as EventHandler).__resolve__ = this._resolveRoute.bind(this); } all(path: string, handler: EventHandler) {