Skip to content
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

HMR for server routes #3817

Open
rtritto opened this issue Jan 9, 2025 · 3 comments
Open

HMR for server routes #3817

rtritto opened this issue Jan 9, 2025 · 3 comments
Labels
enhancement New feature or request.

Comments

@rtritto
Copy link

rtritto commented Jan 9, 2025

What is the feature you are proposing?

During development environment, HMR (Hot Module Replacement) is needed to develop: after the server load all route handlers, any change to to handler must be updated to the server.

HMR improves the DX (Developer Experience) without manually restart the server every time a file is changed.

Example with vite

Having a file handler:

// /api/index.ts
export default (c) => {
  return c.text('Ok')
}

and the server file:

// /server.ts
import { serve } from '@hono/node-server'
import { Hono } from 'hono'

const filepath = './api/index.ts'

const registerRoute = async () => {
  // Register the file to intercept changes
  const { default: handler } = await viteDevServer.ssrLoadModule(filepath, { fixStacktrace: true })
  app.get('/', handler)
}

const app = new Hono()

// Register route first time
await registerRoute()

// Event for file changes
viteDevServer.watcher.on('change', async (file) => {
  // Re-register route with the new file handler
  await registerRoute()
})

serve({ fetch: app.fetch })

Actual

On re-register an handler, Hono routers throws an Error:

Error: Can not add a route since the matcher is already built.
    at SmartRouter.add (file:///C:/Users/<USER>/AppData/Local/Yarn/Berry/cache/hono-npm-4.6.16-e1b105c322-10c0.zip/node_modules/hono/dist/router/smart-router/router.js:12:13)
    at Hono.#addRoute (file:///C:/Users/<USER>/AppData/Local/Yarn/Berry/cache/hono-npm-4.6.16-e1b105c322-10c0.zip/node_modules/hono/dist/hono-base.js:157:17) 
    at file:///C:/Users/<USER>/AppData/Local/Yarn/Berry/cache/hono-npm-4.6.16-e1b105c322-10c0.zip/node_modules/hono/dist/hono-base.js:42:25
    at Array.forEach (<anonymous>)
    at Hono.<computed> [as get] (file:///C:/Users/<USER>/AppData/Local/Yarn/Berry/cache/hono-npm-4.6.16-e1b105c322-10c0.zip/node_modules/hono/dist/hono-base.js:41:14)
    at file:///C:/<PROJECT>/.yarn/unplugged/universal-autorouter-npm-0.2.3-5b1bcaec9b/node_modules/universal-autorouter/dist/index.mjs:62:20

Expected

On a re-register route, Hono routers should replace the old handler with the new one.

Implementation (draft)

Some points that can be achieved one or all to the check of Hono routers:

  • add process.env.NODE_ENV === 'production' to condition of the check. Eg:

    import { Hono } from 'hono'
    
    const app = new Hono()
    
    app.get('/', (c) => c.text('foo'))
    
    await app.request('/')
    
    app.get('/', (c) => c.text('foo')) // No Error in development / Error in production
  • the error is provided only for new routes (check current method and route with the map of routes). Eg:

    import { Hono } from 'hono'
    
    const app = new Hono()
    
    app.get('/', (c) => c.text('foo'))
    
    await app.request('/')
    
    app.get('/', (c) => c.text('foo')) // No Error
    
    app.get('/extra', (c) => c.text('foo')) // Error! (expected)

Initial discussion: #3805

@rtritto rtritto added the enhancement New feature or request. label Jan 9, 2025
@rtritto
Copy link
Author

rtritto commented Jan 9, 2025

Draft for implementation of /src/router/smart-router/router.ts:

export class SmartRouter<T> implements Router<T> {
  name: string = 'SmartRouter'
  #routers: Router<T>[] = []
-  #routes?: [string, string, T][] = []
+  #routes?: Record<string, T> = {} 

  constructor(init: { routers: Router<T>[] }) {
    this.#routers = init.routers
  }

  add(method: string, path: string, handler: T) {
    if (!this.#routes) {
      throw new Error(MESSAGE_MATCHER_IS_ALREADY_BUILT)
    }

-    this.#routes.push([method, path, handler])
+    this.#routes[`${path}${method}`] = handler
  }

  match(method: string, path: string): Result<T> {
    if (!this.#routes) {
      throw new Error('Fatal error')
    }

    const routers = this.#routers
    const routes = this.#routes

    const len = routers.length
    let i = 0
    let res
    for (; i < len; i++) {
      const router = routers[i]
      try {
-        for (let i = 0, len = routes.length; i < len; i++) {
-          router.add(...routes[i])
+        for (const [key, handler] of Object.entries(routes)) {
+          const [path, method] = key.split('')
+          router.add(method, path, handler) 
        }
        res = router.match(method, path)
      } catch (e) {
        if (e instanceof UnsupportedPathError) {
          continue
        }
        throw e
      }

      this.match = router.match.bind(router)
      this.#routers = [router]
-      this.#routes = undefined
      break
    }

@yusukebe
Copy link
Member

@rtritto

Have you ever tried @hono/vite-dev-server? It can HMR as a dev-server for Vite.

@rtritto
Copy link
Author

rtritto commented Jan 10, 2025

@yusukebe Yes, I did some try but got some trouble (vite config conflicts) with Vike ecosystem.
@hono/vite-dev-server doesn't use filesystem to load routes and use hot-reload (inject to fetch-client) instead of HMR (inject to server).
I also did some look to code and didn't find anything useful to this use case (only viteDevServer.ssrLoadModule is used).

How can an updated handler be injected in Hono context to replace the handler of an already regirtered route?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request.
Projects
None yet
Development

No branches or pull requests

2 participants