chore(routing): routing code refactoring#193
Merged
Merged
Conversation
Member
Author
|
@EmanFateen FYI |
Contributor
There was a problem hiding this comment.
Pull request overview
Refactors the routing subsystem by splitting “route declaration” contracts from normalization/registration logic, aiming to reduce coupling to the underlying router while keeping legacy decorator routing working through the shared registrar.
Changes:
- Introduces new routing declaration contracts (
RouteProps,NormalizedRouteProps,RouteSource,RouteGroupDefinition,RouterMethod,HttpMethod) and a normalization pipeline (normalizeRouteProps,normalizeRouteSources,mergeRouteOptions). - Replaces route expansion/validation flow with
createRouteRegistrations+validateRouteRegistrations, and updates registration to consume normalized routes. - Adds path-template resolution utilities (
resolvePathTemplate) with tests; updates test setup (tsconfig + imports) and adds watch scripts.
Reviewed changes
Copilot reviewed 50 out of 57 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| tests/tsconfig.json | Adds a dedicated TS config for the tests workspace. |
| tests/function-first-routing.e2e.test.ts | Updates imports to use path aliases and new routing/Testing entrypoints. |
| src/Testing/TestAgentFactory.ts | Makes createTestAgent generic over request type to align with generic KoalaConfig. |
| src/routing/source/route-source.ts | Removes old RouteSource definition (moved to declaration layer). |
| src/routing/route.ts | Removes old Route declaration API (moved to declaration layer). |
| src/routing/route.test.ts | Removes old tests tied to removed src/routing/route.ts. |
| src/routing/route-options.ts | Removes old RouteOptions type (moved to declaration layer). |
| src/routing/registration/validate-route-registrations.ts | Updates validation to operate on normalized routes + generic registrations. |
| src/routing/registration/validate-route-registrations.test.ts | Renames/updates tests to target validateRouteRegistrations. |
| src/routing/registration/route-registration.ts | Updates registration types to be generic and based on normalized route props. |
| src/routing/registration/register-routes.ts | Updates route registration pipeline to use new normalization/registration/validation modules. |
| src/routing/registration/register-routes.test.ts | Updates tests to use new declaration module import paths. |
| src/routing/registration/expand-route-definitions.ts | Removes old expansion implementation (replaced by createRouteRegistrations). |
| src/routing/registration/create-route-registrations.ts | Adds new route-to-registrations expansion function (incl. koa-body insertion). |
| src/routing/registration/create-route-registrations.test.ts | Updates tests for the new createRouteRegistrations. |
| src/routing/path/resolve-path-template.ts | Adds reusable path-template resolver for named-route path generation. |
| src/routing/path/resolve-path-template.test.ts | Adds tests for path-template resolution and missing-param errors. |
| src/routing/path/create-path-for.ts | Updates imports to use new declaration/normalization modules. |
| src/routing/path/create-path-for.test.ts | Updates test imports to new declaration paths. |
| src/routing/path/create-path-catalog.ts | Updates imports to new declaration/normalization modules. |
| src/routing/path/create-path-catalog.test.ts | Updates test imports to new declaration paths. |
| src/routing/normalization/resolve-route-options.ts | Adds new merge helper for route options during normalization. |
| src/routing/normalization/normalize-route-sources.ts | Refactors normalization to work on the new declaration contracts and generics. |
| src/routing/normalization/normalize-route-sources.test.ts | Updates tests to new declaration import paths. |
| src/routing/normalization/normalize-route-props.ts | Adds normalization for RouteProps into canonical NormalizedRouteProps. |
| src/routing/index.ts | Updates routing subpath exports to the new module structure. |
| src/routing/http-method.type.ts | Adds a new HttpMethod type (case-insensitive variants + any/all). |
| src/routing/helpers/route-group.ts | Removes old route-group helper (moved to declaration layer). |
| src/routing/deprecated-decorator/verify-routing-mode.ts | Updates verifyRoutingMode to be generic over request type. |
| src/routing/deprecated-decorator/verify-routing-mode.test.ts | Adds tests for routing-mode exclusivity behavior. |
| src/routing/deprecated-decorator/route.ts | Updates legacy decorator route type imports to new declaration/type locations. |
| src/routing/deprecated-decorator/route-metadata.ts | Updates legacy metadata imports to new declaration/type locations. |
| src/routing/deprecated-decorator/legacy-router.ts | Routes legacy decorators through the new canonical route contract and registrar. |
| src/routing/deprecated-decorator/legacy-router.test.ts | Adds tests for legacy decorator conversion + registration through shared registrar. |
| src/routing/definition/route-definition.ts | Removes old route-definition contract (replaced by normalized props contract). |
| src/routing/definition/resolve-route-options.ts | Removes old route-definition option resolver (replaced by normalization helpers). |
| src/routing/definition/resolve-route-options.test.ts | Removes tests for deleted option resolver. |
| src/routing/definition/create-route-definition.ts | Removes old route-definition factory (replaced by normalizeRouteProps + Route). |
| src/routing/declaration/router-method.type.ts | Adds canonical RouterMethod type for the registrar/router adapter. |
| src/routing/declaration/route.ts | Adds new Route factory that returns canonical normalized route props. |
| src/routing/declaration/route.test.ts | Updates tests to target new Route factory. |
| src/routing/declaration/route-source.type.ts | Adds new RouteSource union type in the declaration layer. |
| src/routing/declaration/route-props.type.ts | Adds new RouteProps contract (replacing old RouteDeclaration). |
| src/routing/declaration/route-options.type.ts | Adds new RouteOptions contract in the declaration layer. |
| src/routing/declaration/route-group.ts | Adds new RouteGroup + associated contracts in the declaration layer. |
| src/routing/declaration/route-group.test.ts | Updates tests to target declaration-layer RouteGroup. |
| src/routing/declaration/route-body-options.type.ts | Adds body options contract to support parse-body behavior without legacy definition types. |
| src/routing/declaration/normalized-route-props.type.ts | Adds canonical normalized route contract shared across routing internals. |
| src/routing/declaration/http-verb-helpers.ts | Updates verb helpers to return canonical normalized route props and use new request base type. |
| src/routing/declaration/http-verb-helpers.test.ts | Adds tests for verb helper behavior against canonical route contract. |
| src/index.ts | Updates legacy routing imports/exports to renamed deprecated-decorator paths and new routing types. |
| src/Http/Scope/types.ts | Introduces HttpRequestBase usage to decouple scope/middleware from raw Koa Request import. |
| src/Http/Request/types.ts | Adds HttpRequestBase alias and updates HttpRequest to extend it. |
| src/config/koala-config.ts | Makes KoalaConfig generic over HttpRequestBase to align request typing across routing. |
| src/application/create-application.ts | Updates legacy routing imports and makes create generic over request type. |
| src/application/create-application.test.ts | Updates import for exclusiveRoutingModeError to the new deprecated-decorator location. |
| package.json | Adds watch:test and watch:build scripts. |
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.
This existing routing module code is challenging to work with and heavily coupled to the underlying Koa-Router. Moreover, the concepts are concealed within the implementation details.
In general, this module requires refactoring and cleanup. This is a technical debt that we must address.
While working on this refactoring, I discovered an ownership leak. For instance, the Config module utilizes the
RouteSource, which is implemented within the routing component. Unfortunately, these leaks are not limited to the routing and configuration modules; they occur across multiple components at the moment.In my opinion, the primary reason for this issue is the absence of a shared kernel owned by the
applicationor even a newcoremodule that should also own the sharedcontracts. This centralcoremodule should own the concepts, delegate to underlying components, and orchestrate them.