-
-
Notifications
You must be signed in to change notification settings - Fork 791
feat: pluggable router architecture with 49% performance improvement #3604
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: v6
Are you sure you want to change the base?
Conversation
- Add Object.create(null) for children and pathCache objects - Replace startsWith(':') with direct array access current[0] === ':' - Implement manual loops instead of array methods for placeholders - Add path caching with fast paths for common cases - Use in-place object mutation instead of object spreading - Add comprehensive performance benchmark showing 48-49% improvement - Preserve old implementation as router2.ts for comparison - Update tests to expect optimized Object.create(null) params
Adds support for catch-all routes using ::param syntax that capture remaining path segments as arrays. Key features: - ::param syntax captures all remaining segments as string[] - Proper priority: exact → named params → catch-all - Validates catch-all params only at end of routes - Maintains 48% performance improvement over old router - Full TypeScript support with string | string[] param types Examples: - /docs/::path matches /docs/api/guide → params.path = ['api', 'guide'] - /api/:version/docs/::path combines named and catch-all params - /docs exact route takes priority over /docs/::path catch-all This completes router feature parity for full Express/Koa independence.
- Add RouterInterface abstraction allowing custom router implementations - Update Feathers application to support router replacement via app.routes assignment - Add comprehensive tests for router swapping with mock implementations - Remove obsolete router2.ts and performance benchmarks - Clean up router tests with proper TypeScript typing - Maintain RouterInterface compatibility for external router implementations - Support router.caseSensitive property for case-insensitive routing
- Create new @feathersjs/routing package for Express/Koa migration support - Implement ExpressRouter using path-to-regexp for 100% Express compatibility - Implement KoaRouter using path-to-regexp for 100% Koa compatibility - Support named parameters (:id), wildcards (*), optional parameters, and regex constraints - Add comprehensive tests for both routers including Feathers integration - Include TypeScript definitions for path-to-regexp - Provide clean API: app.routes = new ExpressRouter() or app.routes = new KoaRouter() - Runtime agnostic - works across Node.js, Deno, Bun, and Cloudflare Workers
Deploying feathers-dove with
|
Latest commit: |
f5f4d55
|
Status: | ✅ Deploy successful! |
Preview URL: | https://3fe91a60.feathers.pages.dev |
Branch Preview URL: | https://v6-router-optimization.feathers.pages.dev |
- Extract shared logic into BaseRouter base class - Maintain framework-specific defaults (Express: case insensitive, Koa: case sensitive) - Fix path-to-regexp v8 compatibility with named wildcard syntax (/*path) - Remove test expectations for __id parameter (router-specific behavior) - Reduce code duplication by 68% while preserving all functionality - All 34 routing tests pass with full Feathers service integration Breaking changes: - Wildcard routes now use named syntax: /docs/*path instead of /docs/* - Parameter extraction returns 'path' key instead of '*' for wildcards
- Add BaseRouter tests to cover constructor options and edge cases - Change router tests to import from index.ts (realistic usage pattern) - Remove redundant index.test.ts file - Achieve 100% statement coverage and 92.59% branch coverage - Remaining uncovered branches are defensive fallbacks for unreachable edge cases Coverage improvements: - index.ts: 0% → 100% coverage through realistic imports - base-router.ts: better branch coverage for constructor options - All tests now use the same import pattern as end users
placeholders: RouteNode[] = [] | ||
catchAll?: RouteNode<T> |
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.
I'm not sure we need a catchAll?
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.
This one ironically came up twice in the last couple days. While I was making an npm server with Feathers over the weekend, I found a route that legitimately needed to capture a dynamic number of params. I was able to work around it, but the dynamic params would have made it cleaner. Then today Dallin had another use case that made sense to me. A root-level route that handles whatever other paths you throw at it:
/actions
/actions/test
/actions/test/test1/tes2
And with the catchAll feature, you can put a catch all custom param at the end of the route, so you could do this:
app.use('actions/::args', ..., { routeParams: ['args'] })
Then you get an array of args to handle as you please.
It's the most likely edge case to handle, so we cover 95% of routing use cases without any sacrifice of speed.
Then I realized that we had an opportunity to improve the upgrade path because there are likely users who are stuck with their old Express and Koa routes who might like to upgrade. You already kept the router decoupled, so I made an express router and koa router in a routing package. That might help people upgrade, or motivate them to do so if they feel like the upgrade path is easier.
// Fast path for root | ||
if (!path || path === '/') { | ||
const result = [''] | ||
this.pathCache[path] = result |
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.
pathCache
is never cleaned up so anybody can crash the server by making a lot of unique URL requests.
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.
Good catch. I added a simple LRU mechanism that defaults to 1000 cached paths before booting old ones.
Router Architecture Overhaul & Performance Optimization
🚀 Performance Improvements
optimized algorithms and caching
performance
🏗️ New Modular Architecture
RouterInterface
- swap routerimplementations at runtime
implementations
✨ Enhanced Routing Features
::param
syntax for handling wildcards (e.g.,/docs/::path
)🔧 Code Quality
implementations
optimizations
This enables both better performance for existing apps and flexible routing
architectures for complex use cases.