kwhitley / itty-router

A little router.
MIT License
1.7k stars 77 forks source link

Unable to use nested routers #171

Closed josefaidt closed 1 year ago

josefaidt commented 1 year ago

Describe the Issue

A clear and concise description of what the issue is.

Hey :wave: I'm using itty-router and Bun to build a tiny API and am encountering an issue when using nested routers. The routes off the main router work fine, though.

Example Router Code

Please provide the itty-router code related to the issue. If possible, create a minimal, reproducible example.

// api.ts
import {
  error, // creates error responses
  json, // creates JSON responses
  Router, // the ~440 byte router itself
  withParams, // middleware: puts params directly on the Request
} from 'itty-router'

const router = Router()
const api = Router({ base: '/api' })
const v1 = Router({ base: '/v1' })

v1.get('/something', () => json({ status: 'ok' }))

api.all('/v1/*', v1.handle)

router
  .get('/healthcheck', () => json({ status: 'ok' }))
  .all('*', withParams)
  .all('/api/*', api.handle)
  .all('*', () => error(404))

Bun.serve({
  port: 3000,
  fetch(request) {
    return router
      .handle(request)
      .then(json) // send as JSON
      .catch(error) // catch errors
  },
})
// api-script.ts
const base = 'http://localhost:3000/'

// 200
fetch(new URL('/healthcheck', base))
  .then((r) => r.json())
  .then(console.log)
  .catch(console.error)

// 404
fetch(new URL('/api/v1/something', base))
  .then((r) => r.json())
  .then(console.log)
  .catch(console.error)

Request Details

Steps to Reproduce

Steps to reproduce the behavior:

  1. run the sample api, bun api.ts
  2. run the sample api script, bun api-script.ts
  3. observe the following response
    {
      status: "ok"
    }
    {
      status: 404,
      error: "Not Found"
    }

Expected Behavior

A clear and concise description of what you expected to happen.

Actual Behavior

This behavior seems to be specific to nesting multiple routers. If I add an arbitrary GET route to the API handler the call succeeds:

api
  .all('/v1/*', v1.handle)
  .get('/something', () => json({ status: 'ok' })

Environment (please complete the following information):

Additional Context

Add any other context about the problem here.

kwhitley commented 1 year ago

Hey there!

Looks like the v1 base should be /api/v1, rather than the /v1 base you currently have. Each subrouter has to be aware of its entire path, rather than just relative off the parent router (to preserve the tiny size of itty, we don't keep track of the parent tree in any way).

// api.ts
import {
  error, // creates error responses
  json, // creates JSON responses
  Router, // the ~440 byte router itself
  withParams, // middleware: puts params directly on the Request
} from 'itty-router'

const router = Router()
const api = Router({ base: '/api' })
const v1 = Router({ base: '/api/v1' }) // <-- this should contain the full path

v1.get('/something', () => json({ status: 'ok' }))

api.all('/v1/*', v1.handle)

router
  .get('/healthcheck', () => json({ status: 'ok' }))
  .all('*', withParams)
  .all('/api/*', api.handle)
  .all('*', () => error(404))

Bun.serve({
  port: 3000,
  fetch(request) {
    return router
      .handle(request)
      .then(json) // send as JSON
      .catch(error) // catch errors
  },
})
kwhitley commented 1 year ago

I'll add a more obvious note to this effect on https://itty.dev/itty-router/nesting. It's the one gotcha of using itty (specifically with nesting)!

josefaidt commented 1 year ago

Hey! Goodness, that makes total sense! I've read the bullet points at the bottom of that page and it didn't click.

This means each child router must include a base option to prefix every route within that router

This line makes a bit more sense now but I'm onboard with making it more obvious. Thanks for clearing this up!

kwhitley commented 1 year ago

Glad it was an easy fix! :)

I've added more explicit instruction to the docs (rolling out now): image