Repository navigation
Conversation
mountRoutes() concatenates prefix + sub-path verbatim, so a "/" prefix registered root-mounted sub-router routes as "//health"-style paths and every root-mounted route 404'd. route() now normalizes a "/" prefix to "" before mounting; mountRoutes' existing empty-path guard still maps "" + "" to "/" for root handlers. "/api" and "" prefix behavior unchanged. Adds tests covering a root-mounted sub-route, a sub-handler at "/", param preservation through the root mount, and a "/api" prefix regression check.
Deploying chttpjs with
|
| Latest commit: |
2ec6765
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://f7eca125.chttpjs.pages.dev |
| Branch Preview URL: | https://fix-router-root-mount.chttpjs.pages.dev |
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
app.route("/", sub)registered the sub-router's paths with a doubled slash (//health), so every root-mounted route fell through to the catch-all and 404'd. This PR normalizes a"/"prefix to""inroute()before mounting, matching Hono'sapp.route("/", sub)behavior.Closes #97
Root cause
mountRoutesconcatenates prefix and sub-path verbatim (packages/router/src/index.ts):The
path === ""guard only covers the empty-prefix case. Inroute(), the trailing-slash strip is gated onp.length > 1, so a lone"/"prefix survived verbatim and fell through to//healthfor every root-mounted route.Repro
Pre-fix the repro prints 404; post-fix 200.
app.route("", sub)worked before and is unchanged.The fix
In
route(), collapse"/"to""ahead of the existing trailing-slash strip:mountRoutes' empty-path guard then turns"" + ""into"/"for root handlers, androute("/api", sub)/route("", sub)behavior is untouched. The change stays inside the scriptc compilable subset (string equality +slice, same primitives as the existing strip);scripts/scriptc-gate.shstill reports 100% static coverage on all gates.Tests
Added to
packages/router/test/router.test.js(tests written first; the three root-mount cases fail pre-fix):route("/", sub)resolves a sub-route (/health→ handler runs, 200).route("/", sub)resolves a handler registered at"/"in the sub-app (/→ 200)./item/:id).route("/api", sub)still mounts under the prefix (no regression).pnpm --filter @chttp/router build && pnpm --filter @chttp/router test: 105/105 pass. Full workspacepnpm -r test: all packages and examples pass, 0 failures.Context
Found while porting Brioso's web API (briosoco/brioso#859) from Hono to chttp:
miscAppmounts at the root and its/health,/ready,/robots.txt,/sitemap.xmlroutes all 404'd because of this bug.Changeset
.changeset/router-root-mount.md—@chttp/routerpatch intent;pnpm change checkpasses.