-
-
Notifications
You must be signed in to change notification settings - Fork 460
fix: route specificity for wildcard vs mounted routes (#1682, #1515) #1685
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
base: main
Are you sure you want to change the base?
Conversation
When comparing method-specific wildcard routes (e.g., GET /*) with ALL routes (e.g., mounted routes at ALL /api/*), prefer the more specific route based on wildcard capture length. Previously, GET /* would incorrectly override ALL /api/* because the lookup used nullish coalescing (`??`) which only fell back to ALL when the method- specific route didn't exist at all. Now when both routes match: - Compare wildcard capture lengths (shorter = more specific) - Method-specific exact matches (no wildcard) still take precedence - Method-specific routes win when specificity is equal This ensures that staticPlugin with prefix: "/" doesn't break mounted routes, while still allowing wildcards to catch unmatched paths. Fixes elysiajs#1682
WalkthroughReworked route resolution to compare method-specific and ALL-route candidates by inspecting wildcard captures and lengths, selecting the more specific route (preferring method-specific on ties). Added adapter logic to collect mount prefixes and skip conflicting static wildcards. Extended tests for mount/wildcard and SPA-fallback behaviors. Changes
Sequence Diagram(s)mermaid (Note: colored rectangles not required for this simple flow.) Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
commit: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/compose.ts`:
- Around line 2343-2366: The specificity check conflates missing wildcard vs an
empty capture by using params['*']||''; update the logic that builds
findDynamicRoute (the mr/ar/mw/aw handling from router.find) to detect presence
of the '*' param explicitly (e.g., check whether params hasOwnProperty '*' or
use ('*' in params) via Object.prototype.hasOwnProperty.call) so you can
distinguish "no wildcard" (undefined) from "empty capture" (''), then choose
route: if mr has no '*' prefer mr, else if ar has no '*' prefer ar, otherwise
compare mw.length vs aw.length to pick the shorter capture (more specific),
keeping existing tie-breaker preferring method-specific (mr).
…city
Use Object.prototype.hasOwnProperty.call to explicitly check for '*' param
presence, properly distinguishing:
- no wildcard (exact match) - highest priority
- empty capture ('') - wildcard matched nothing
- non-empty capture - compare lengths for specificity
This fixes edge cases where the previous ||'' fallback conflated undefined
(no wildcard) with empty string capture.
cb7982b to
57dcc20
Compare
…elysiajs#1515) When using static Response/HTMLBundle on root wildcard routes (e.g., GET /*), Bun's nativeStaticResponse optimization would add them to Bun's static route table, which has highest priority and bypasses Elysia's dynamic router. This caused SPA fallback patterns with mounted APIs to fail: app.mount('/api', backend) .get('/*', staticHtmlResponse) The /api/* routes would return HTML instead of API responses because the /* wildcard in Bun's static table matched everything. Solution: - During listen(), collect mount prefixes by scanning router.history for routes with hooks.config.mount flag - In createStaticRoute(), skip root wildcard paths (/* or /*/ or / or empty) when mounts exist - This forces root wildcards through the dynamic router where the route specificity fix correctly handles priority Fixes elysiajs#1515
Summary
This PR fixes two related routing issues:
Issue #1682 - staticPlugin without prefix destroys elysia.mount path
When using
staticPluginwithout a prefix alongside.mount(), the wildcard route from staticPlugin would take precedence over more specific mount routes.Issue #1515 - Cannot implement SPA fallback routing with mounted APIs
When using static
Response/HTMLBundleon root wildcard routes (e.g.,GET /*), Bun'snativeStaticResponseoptimization would bypass Elysia's dynamic router, causing mounted API routes to return HTML instead of JSON.Root Cause Analysis
#1682: The route matching logic used
??fallback which did not properly compare route specificity when both routes had wildcard captures.#1515: Static responses are added directly to Bun's static route table, which has highest priority and bypasses Elysia's dynamic router entirely. A root wildcard in the static table matches everything including mount routes.
Solution
For #1682 (src/compose.ts)
??fallback to comparing wildcard capture lengthsObject.prototype.hasOwnProperty.call(params, '*')to distinguish:For #1515 (src/adapter/bun/index.ts)
listen(), collect mount prefixes by scanningrouter.historyfor routes withhooks.config.mountflagcreateStaticRoute(), skip root wildcard paths (/*,/*/,/, ``) when mounts existWhy This Approach
.listen(), not per-requestTest Coverage
Added comprehensive tests in
test/core/spa-fallback.test.tscovering:/public/*) still workAll 1461 tests pass with no regressions.
Verification
Fixes #1682
Fixes #1515