feat(mapper): preserve node route mount prefixes#98
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5a69a00fa9
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const pathlessExpressMount = | ||
| method === "use" && | ||
| (isPathlessExpressMount(args) || isSingleExpressChildMount(args, parent, targets)); | ||
| const prefixes = literalPrefixes ?? (pathlessExpressMount ? ["/"] : null); |
There was a problem hiding this comment.
use(mw, router) mounts before suppression
When app.use() is called with middleware followed by a router (for example app.use(mw, apiRouter)), isPathlessExpressMount only treats the call as pathless if the first argument matches a narrow middleware-name heuristic. If the middleware identifier is generic (e.g. mw), this branch marks the child router as unknownMountedTargets, and routePathsForTarget drops all of that router’s routes. This is a regression in route discovery for common Express patterns that use short middleware variable names.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 5c3f77a: generic middleware identifiers are now treated as pathless only when they resolve to function-like declarations, so app.use(mw, router) preserves mounted child routes while dynamic mw prefixes remain suppressed. Added regressions for sync/async generic middleware and dynamic mw prefixes.
| function isMountPrefix(path: string): boolean { | ||
| return path.startsWith("/"); |
There was a problem hiding this comment.
isMountPrefix rejects literal "*" mount paths, so valid mounts like app.use('*', router) are treated as unknown and can cause all child routes to be suppressed. Since wildcard mounts are legitimate Express mount paths, this drops discoverable routes that previously appeared.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 5c3f77a: literal * mount prefixes are now accepted and normalized to slash-prefixed wildcard route output, with regression coverage for app.use("*", router).
|
ClawSweeper status: review started. I am starting a fresh review of this pull request: feat(mapper): preserve node route mount prefixes This is item 1/1 in the current shard. Shard 0/1. This placeholder means the worker is alive and reading the current context. I will edit this same comment with the actual review when the claws are done clicking. Crustacean status: shell secured, claws on keyboard, evidence pebbles being sorted. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 87fc9fc9cf
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (isLikelyDynamicMountPrefix(source, first)) { | ||
| return false; |
There was a problem hiding this comment.
In isPathlessExpressMount, the early isLikelyDynamicMountPrefix(first) check runs before function/import-based middleware detection, so middleware identifiers containing substrings like route, path, or mount (for example routeGuard) are misclassified as dynamic prefixes. For calls such as app.use(routeGuard, apiRouter), apiRouter is then added to unknownMountedTargets and routePathsForTarget suppresses all of its routes, even though this is a valid pathless mount. This causes route loss in common middleware+router patterns.
Useful? React with 👍 / 👎.
2 participants
Summary
This updates the Node mapper so routes declared inside mounted Express routers/apps and Hono sub-apps keep their literal mount prefix. A route declared inside a router mounted at /api is now reported under /api instead of as an unmounted child route.
Dynamic mounts stay conservative. If a router is only mounted through an unknown prefix, the mapper suppresses the raw child route instead of guessing a path that may be wrong.
Verification