-
-
Notifications
You must be signed in to change notification settings - Fork 546
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
PoC: feat(reg-exp-router): Introduced PreparedRegExpRouter #1796
base: main
Are you sure you want to change the base?
Conversation
Hi, @yusukebe. |
Hi @usualoma Interesting approach. I'm glad you are interested in file-based routing!
Exactly right. We could make Hono specialize in file-based routing and improve the router. However, it might be too much optimization. I'd like to take some more time to think about it. This might not be related, but it's one idea. Can we improve performance like this with explicitly app.get('/', (c) => c.json(0))
app.get('/a', (c) => c.json(0))
app.get('/b', (c) => c.json(0))
export default app.build() |
@yusukebe Thank you for your comment. After creating the PR, I thought about it for a while. File-based routing can easily extract paths, but it is expensive to run before buildInitParams({
routes: [
{ method: 'ALL', path: '*' },
{ method: 'ALL', path: '/posts/:id/*' },
{ method: 'GET', path: '*' },
{ method: 'GET', path: '/' },
{ method: 'GET', path: '/static' },
{ method: 'GET', path: '/posts/:id/*' },
{ method: 'GET', path: '/posts/:id' },
{ method: 'GET', path: '/posts/:id/comments' },
{ method: 'POST', path: '/posts' },
{ method: 'PUT', path: '/posts/:id' },
],
}) after buildInitParams({
paths: ['*', '/static', '/posts/:id/*', '/posts/:id', '/posts/:id/comments', '/posts'],
}) |
I guess you mean "finish initializing the router before the first request comes in". If so, I think the following changes will do it. diff --git a/src/hono-base.ts b/src/hono-base.ts
index f118a741..b4539b74 100644
--- a/src/hono-base.ts
+++ b/src/hono-base.ts
@@ -236,10 +236,14 @@ class Hono<
}
get routerName() {
- this.matchRoute('GET', '/')
+ this.build()
return this.router.name
}
+ build() {
+ this.router.build?.()
+ return this
+ }
+
/**
* @deprecated
* `app.head()` is no longer used.
diff --git a/src/router.ts b/src/router.ts
index 78cd65e5..71e45005 100644
--- a/src/router.ts
+++ b/src/router.ts
@@ -8,6 +8,7 @@ export interface Router<T> {
name: string
add(method: string, path: string, handler: T): void
match(method: string, path: string): Result<T>
+ build?(): void
}
export type ParamIndexMap = Record<string, number>
diff --git a/src/router/reg-exp-router/router.ts b/src/router/reg-exp-router/router.ts
index 82a8d0ea..30affc24 100644
--- a/src/router/reg-exp-router/router.ts
+++ b/src/router/reg-exp-router/router.ts
@@ -208,6 +208,11 @@ export class RegExpRouter<T> implements Router<T> {
}
match(method: string, path: string): Result<T> {
+ this.build()
+ return this.match(method, path)
+ }
+
+ build() {
clearWildcardRegExpCache() // no longer used.
const matchers = this.buildAllMatchers()
@@ -228,8 +233,6 @@ export class RegExpRouter<T> implements Router<T> {
const index = match.indexOf('', 1)
return [matcher[1][index], match]
}
-
- return this.match(method, path)
}
private buildAllMatchers(): Record<string, Matcher<T>> { |
And maybe SmartRouter. I have not confirmed that it works. diff --git a/src/router/smart-router/router.ts b/src/router/smart-router/router.ts
index 7b0510d8..3a970ca8 100644
--- a/src/router/smart-router/router.ts
+++ b/src/router/smart-router/router.ts
@@ -20,6 +20,11 @@ export class SmartRouter<T> implements Router<T> {
}
match(method: string, path: string): Result<T> {
+ this.build()
+ return this.match(method, path)
+ }
+
+ build() {
if (!this.routes) {
throw new Error('Fatal error')
}
@@ -27,14 +32,13 @@ export class SmartRouter<T> implements Router<T> {
const { routers, routes } = this
const len = routers.length
let i = 0
- let res
for (; i < len; i++) {
const router = routers[i]
try {
routes.forEach((args) => {
router.add(...args)
})
- res = router.match(method, path)
+ router.build?.()
} catch (e) {
if (e instanceof UnsupportedPathError) {
continue
@@ -55,8 +59,6 @@ export class SmartRouter<T> implements Router<T> {
// e.g. "SmartRouter + RegExpRouter"
this.name = `SmartRouter + ${this.activeRouter.name}`
-
- return res as Result<T>
}
get activeRouter() { |
For example, in this project that provides file-based routing: https://github.com/yusukebe/file-base-routing-framework We can write diff --git a/src/framework.ts b/src/framework.ts
index d75e3b9..e781263 100644
--- a/src/framework.ts
+++ b/src/framework.ts
@@ -35,7 +35,19 @@ const ROUTES = import.meta.glob<RouteFile>('/app/routes/**/[a-z0-9[-][a-z0-9[_-]
})
export const createApp = () => {
- const app = new Hono()
+ const paths: string[] = []
+ Object.keys(ROUTES).map((key) => {
+ paths.push(filePathToPath(key.replace(/^\/app\/routes/, '')))
+ })
+
+ const preparedParams = buildInitParams({
+ paths
+ })
+
+ const app = new HonoBase({
+ // @ts-ignore
+ router: new PreparedRegExpRouter(preparedParams[0], preparedParams[1])
+ })
for (const [key, routes] of Object.entries(RENDERERS)) {
const dirPath = pathToDirPath(key) The bundle size difference:
The difference isn't as significant as I expected, but this approach allows us to implement it in the framework without creating Vite or Rollup plugins. |
Yes, I mean that. But, I think we can make it like the following: diff --git a/src/hono-base.ts b/src/hono-base.ts
index f118a74..40a3ec4 100644
--- a/src/hono-base.ts
+++ b/src/hono-base.ts
@@ -5,6 +5,7 @@ import { HTTPException } from './http-exception'
import { HonoRequest } from './request'
import type { Router } from './router'
import { METHOD_NAME_ALL, METHOD_NAME_ALL_LOWERCASE, METHODS } from './router'
+import { PreparedRegExpRouter, buildInitParams } from './router/reg-exp-router'
import type {
Env,
ErrorHandler,
@@ -251,10 +252,7 @@ class Hono<
}
private addRoute(method: string, path: string, handler: H) {
- method = method.toUpperCase()
- path = mergePath(this.#basePath, path)
const r: RouterRoute = { path: path, method: method, handler: handler }
- this.router.add(method, path, [handler, r])
this.routes.push(r)
}
@@ -373,6 +371,24 @@ class Hono<
event.respondWith(this.dispatch(event.request, event, undefined, event.request.method))
})
}
+
+ build = () => {
+ const paths = this.routes.map((route) => {
+ return route.path
+ })
+ console.log(paths)
+ const preparedParams = buildInitParams({
+ paths,
+ })
+ // eslint-disable-next-line @typescript-eslint/ban-ts-comment
+ // @ts-ignore
+ this.router = new PreparedRegExpRouter(preparedParams[0], preparedParams[1])
+ this.routes.map((route) => {
+ const r: RouterRoute = { path: route.path, method: route.method, handler: route.handler }
+ this.router.add(route.method.toUpperCase(), route.path, [route.handler, r])
+ })
+ return this
+ }
}
export { Hono as HonoBase } The bundle size difference: yusuke $ wrangler deploy --minify --dry-run src/default-router.ts
⛅️ wrangler 3.9.0 (update available 3.19.0)
------------------------------------------------------
Total Upload: 21.95 KiB / gzip: 8.04 KiB
--dry-run: exiting now.
yusuke $ wrangler deploy --minify --dry-run src/prepared-router.ts
⛅️ wrangler 3.9.0 (update available 3.19.0)
------------------------------------------------------
Total Upload: 18.27 KiB / gzip: 7.09 KiB
--dry-run: exiting now. Nevertheless, this code actually works, but it is not practical to write this process in |
@yusukebe Thank you so much!
The main point of PreparedRegExpRouter is to exclude the following source code from the bundle src/router/reg-exp-router/{trie,node}.ts Therefore, we need to somehow work with the bundling tool to remove the following code.
|
Ah, I didn't write that, but it imports them like this. So, it does not import import { HonoBase } from '/Users/yusuke/work/honojs/hono/src/hono-base'
import {
PreparedRegExpRouter,
buildInitParams
} from '/Users/yusuke/work/honojs/hono/src/router/reg-exp-router/prepared-router'
import type { H, MiddlewareHandler } from '/Users/yusuke/work/honojs/hono/src/types' The entire diff: diff --git a/src/framework.ts b/src/framework.ts
index d75e3b9..96d4316 100644
--- a/src/framework.ts
+++ b/src/framework.ts
@@ -1,5 +1,9 @@
-import { Hono } from 'hono'
-import type { H, MiddlewareHandler } from 'hono/types'
+import { HonoBase } from '/Users/yusuke/work/honojs/hono/src/hono-base'
+import {
+ PreparedRegExpRouter,
+ buildInitParams
+} from '/Users/yusuke/work/honojs/hono/src/router/reg-exp-router/prepared-router'
+import type { H, MiddlewareHandler } from '/Users/yusuke/work/honojs/hono/src/types'
const METHODS = ['GET', 'POST', 'PUT', 'DELETE'] as const
@@ -35,7 +39,19 @@ const ROUTES = import.meta.glob<RouteFile>('/app/routes/**/[a-z0-9[-][a-z0-9[_-]
})
export const createApp = () => {
- const app = new Hono()
+ const paths: string[] = []
+ Object.keys(ROUTES).map((key) => {
+ paths.push(filePathToPath(key.replace(/^\/app\/routes/, '')))
+ })
+
+ const preparedParams = buildInitParams({
+ paths
+ })
+
+ const app = new HonoBase({
+ // @ts-ignore
+ router: new PreparedRegExpRouter(preparedParams[0], preparedParams[1])
+ })
for (const [key, routes] of Object.entries(RENDERERS)) {
const dirPath = pathToDirPath(key) |
Yeah, I know, but src/router/reg-exp-router/{trie,node}.ts And |
Ah, but I don't deny the following opinion.
This code is very high-performing, but it might be too complicated. We might be happier to optimize PatternRouter than this PR approach. |
I see, you are right. But, I don't want to create this router and plugins for bundlers just for this optimization. Perhaps we could enhance performance further with static analytics before bundling, but that would be really too much optimization. Either way, this PR has inspired us. Let's keep it open still. |
In addition, It is a good idea to have a "RegExpRouter" preset for file-based routing. |
What is the PR to improve?
With this PR, we aim to improve the reduction of RegExpRouter bundle size and initial addition time.
As you can see in the code I added to the following unit test, we can prepare regular expressions, etc. in advance by passing the routing information to
buildInitParams()
. This can be used to simplify the initialization process at startup.src/router/reg-exp-router/router.test.ts
Benchmark
In Node.js, it is more than 10 times faster than RegExpRouter and close to LinearRouter; in Bun, it may be faster than LinearRouter.
Bundlesize
I compared the app created by
npm create sonik@latest
with the following changes.Add
RegExpRouter
presetAdd rollup plugin
With this setup, the
npx vite build
resulted in71.27 kB
->55.30 kB
.When can we use it?
It can be used for general applications, but it is a bit difficult to use.
File-based routing has the following characteristics that make it easy to implement.
Author should do the followings, if applicable
yarn denoify
to generate files for Deno