refactor: split src/map/extract.ts into cohesive modules (no behaviour change) - #127
Merged
Merged
Conversation
`src/map/extract.ts` had grown to ~1.6k lines covering the whole extraction pipeline. Split it, moving code verbatim, into modules that follow the pipeline's own layering: ast dependency-free AST helpers bindings per-file module bindings (identifier -> npm package) routes file-path/route-shape derivation + the route-register list sources project walk, pre-filter, framework detection coordinates input -> runtime rule parameter inputs validator schemas + request reads sinks sink recognizers, argument roles, candidate families module-graph one-hop cross-file helper tracing flows input -> sink linking, limitations entries entry-point recognizers extract the orchestrator only, plus re-exports No behaviour change: every moved line is byte-identical, and `extract.ts` re-exports what consumers and tests import from it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Refactored large extract.ts into focused, cohesive modules improving clarity. 🎯 Quality: 76% Good · 📦 Size: Oversized — strongly consider breaking this down 🛡️ Standards: no pre-flight fit check ran for this change — wire 📈 This month: Your 58th PR — above team average · Averaging Good |
Contributor
Author
|
/review |
daniloradovic
approved these changes
Aug 13, 2026
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.
Pure, behaviour-preserving split of
src/map/extract.ts(1,626 → 123 lines) into ten focused modules. No logic, comment semantics, control flow or public API changed.Why
The file had grown to 1,626 lines and become the single hardest thing in the repo to review — and the site of several subtle bugs (a global replace once made a helper call itself, causing infinite recursion; another silently no-op'd on a mismatched anchor; leaf-vs-path matching hid inside a 90-line function). The seams were already implicit: helpers had to be exported purely so tests could reach them, and
tests/map/*is already one file per concern.Layout
ast.tsbindings.tsroutes.tssources.tscoordinates.tsinputs.tssinks.tsmodule-graph.tsflows.tsentries.tsextract.tsDependency graph verified acyclic:
types → ast → bindings/routes → sources/coordinates → inputs/sinks → module-graph/flows → entries → extract.Proof it changed nothing
main.git diff --stat -- testsis empty: not one test was modified.extract.tsre-exportsruntimeCoordinate,functionNameFromPath,routeFromFilePath(plus theWalkStats/ModuleGraphtypes), so every existing import still resolves.mapCLI against the real reference app onmainand on this branch produces byte-identical JSON (9,693 bytes both).importlines, addedexportprefixes, and two comment lines. Every security-reasoning comment travelled with its code intact.Three deliberate deviations from the obvious layout
ROUTE_REGISTER_NAMESlives inroutes.ts, notentries.ts— its comment requires one list to drive both the AST recognizer and the textual pre-filter; putting it inentries.tswould have forced an upwardsources → entriesedge.ModuleGraphinterface lives insinks.ts—sinksFromneeds the type whilecreateModuleGraphneedscollectLocalSinks; moving the interface down breaks the cycle one-way.methodFromObjectArgwent toast.ts— dependency-free, and it shares a section heading withunwindChain.Latent issues found, deliberately NOT fixed here
Reported so they aren't lost; each needs its own PR because each is a behaviour change:
({ query }) => query.idcorrectly yieldsget.id, but({ query: q }) => q.idfalls through tobody→post.id. Worse forparams: an aliased route-param read would receive a runtime coordinate where the resolver exposes none. The alias is recorded but compared against the literal'query'/'params', so the binding key is discarded.escapeReis dead code.pathFromTaintedstrips a leading namespace segment only whensegs.length > 1, so a barereq.bodyread keepsbodyin its path — harmless today, inconsistent with the rule above it.