feat(mapper): add Node server route mapping#47
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: d86b5fd15d
ℹ️ 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".
| function routeTargetNames(source: string, framework: ServerFramework): Set<string> { | ||
| if (framework === "express") { | ||
| return declaredTargetNames(source, [ | ||
| /\b(?:const|let|var)\s+([A-Za-z_$][A-Za-z0-9_$]*)\s*=\s*(?:express\s*\(\s*\)|express\s*\.\s*Router\s*\(\s*\)|Router\s*\(\s*\))/gu, |
There was a problem hiding this comment.
Update the Express target regex to allow non-empty Router(...) calls. Right now only express.Router()/Router() with an empty argument list match, so common declarations like const router = Router({ mergeParams: true }) are skipped and none of that file’s router.get/post/... routes are mapped. This silently drops legitimate API features in typical Express codebases.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 7ff0004: Express target detection now accepts non-empty express(...), express.Router(...), and Router(...) calls, with regression coverage for Router({ mergeParams: true }) mapping a real route.
| if (char === "/" && next === "/") { | ||
| cursor += 1; | ||
| state = "line-comment"; | ||
| } else if (char === "/" && next === "*") { | ||
| cursor += 1; | ||
| state = "block-comment"; | ||
| } else if (char === "'") { |
There was a problem hiding this comment.
Treat JavaScript regex literals as non-code when checking match context. isInsideCommentOrString only tracks comments and quoted strings, so a pattern like /app.get('\/health')/ in an Express file is parsed as a real route and emits a bogus feature with route metadata/tests. That creates false positives and can misdirect workflow focus for repositories that store route-shaped regexes.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 7ff0004: the match-context scanner now treats likely JavaScript regex literals as non-code, including escaped slashes and character classes. I also added regression coverage for route-like regex text, postfix-division, and JSX closing-tag edge cases.
steipete
added
enhancement
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cd53244584
ℹ️ 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".
| } | ||
| return named.split(",").some((part) => { | ||
| const binding = part.trim(); | ||
| return !binding.startsWith("type ") && /^Router(?:\s+as\s+Router)?$/u.test(binding); |
There was a problem hiding this comment.
importClauseHasBareValueRouter only treats Router (or Router as Router) as a valid value import, so import { Router as ExpressRouter } from 'express' is ignored. In that common aliasing pattern, routeTargetNames never enables Router(...) target discovery and routes like const r = ExpressRouter(); r.get('/x', ...) are silently skipped, reducing route mapping coverage for real Express APIs.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Good catch. This is fixed in follow-up PR #52: Router factory discovery now tracks the local factory names from Router aliases, CommonJS destructuring aliases, and direct express.Router assignments, while keeping type-only imports and non-Router properties excluded.
2 participants
Summary
Closes #46.
Adds deterministic route mapping for common Express, Fastify, and Hono server declarations so Node API projects get focused route-level feature slices instead of only broad source groups.
Mapped shapes include:
app.get("/path", handler)/router.post("/path", handler)fastify.post("/path", handler)app.delete("/path", handler)for Honorouter.route("/path").get(...).delete(...)Express chainsThe mapper stays conservative: it requires route receivers to be variables initialized from framework constructors, skips comments/strings, avoids client-call false positives, and does not infer cross-file mount prefixes.
Tests
./node_modules/.bin/oxfmt --check docs/feature-mapping.md src/mapper.test.ts src/mapper.ts src/mappers/node-routes.ts./node_modules/.bin/tsc -p tsconfig.json --noEmit./node_modules/.bin/oxlint . --config oxlint.json./node_modules/.bin/vitest run src/mapper.test.tsnode -e "require('fs').rmSync('dist',{recursive:true,force:true})" && ./node_modules/.bin/tsc -p tsconfig.build.json