Repository navigation
fix(js/ts): count CommonJS require() as an import edge - #750
Open
sandeep-patel-alepo-fifth wants to merge 2 commits into
Open
sandeep-patel-alepo-fifth wants to merge 2 commits into
sandeep-patel-alepo-fifth wants to merge 2 commits into
Conversation
The JavaScript and TypeScript tree-sitter specs matched only
(import_statement ...), so a CommonJS codebase produced an essentially empty
dependency graph. Every file then reported importer_count == 0, which the
orphaned, coupling, single_use and test_coverage detectors all read.
On a 1372-file Strapi backend (1128 files using require(), 48 using ESM
import) this reported 1090 orphaned files -- including the logger imported by
175 modules and the core chat orchestrator.
Add require('...') and dynamic import('...') patterns to both specs. The
require pattern uses an (#eq? @_require_fn "require") predicate so
notrequire() and obj.require() do not match. Same 1372-file backend now
resolves logger.js to 175 importers and drops the zero-importer set to 36.7%,
the residue being framework convention-loaded files.
ts_build_dep_graph normalized every resolved import with join(path.resolve(), resolved) whenever it was not absolute. But file_finder returns paths relative to the current directory, and resolve_js_import derives its candidate from the source file, so it also returns a cwd-relative path. Joining that onto the absolute scan root produced a doubled path that never matched file_set, and every edge was dropped. Key the lookup on the absolute form of each file_list entry and try the resolved path as-is before falling back to scan-root-relative, so edges match whether the resolver returns absolute, cwd-relative or scan-relative paths. Edges are now stored under the file_list key rather than the normalized string, keeping imports and importers consistent with the graph keys. With both fixes, the 1372-file Strapi backend goes from 1090 reported orphans to 268, and the coupling detector finds 320 single-use candidates where it previously found 0.
github-actions Bot
added a commit
to citizenadam/desloppify
that referenced
this pull request
Sep 15, 2026
This branch has not been deployed
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
1 participant
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.
Problem
JS_SPECandTYPESCRIPT_SPECindesloppify/languages/_framework/treesitter/specs/scripting.pydefineimport_queryas ESM-only:ts_build_dep_graphtherefore records no edge forrequire('./x'). On a CommonJS codebase the dependency graph comes out essentially empty and every file reportsimporter_count == 0.That count is read by the
orphaned,coupling,single_useandtest_coveragedetectors, so the damage is not limited to one detector.Repro
Scanning a Strapi 4 backend (1372 JS files: 1128 use
require(), 48 use ESMimport):Spot-checking those 1090 "orphaned" files:
src/utils/logger.jssrc/utils/chatModel.jssrc/utils/tokens.service.jssrc/chat/ChatService.js(core orchestrator)1090 of 1283 production files — 85% — were flagged.
Fix
Add
require('...')and dynamicimport('...')patterns to both specs. Therequirepattern uses an(#eq? @_require_fn "require")predicate, sonotrequire('./x')andobj.require('./x')do not match. The anchor.inarguments: (arguments . (string ...))restricts the capture to the first argument.Same 1372-file backend after the fix:
The 36.7% residue is framework convention-loaded files (Strapi autoloads
src/api/*/{controllers,services,routes}), which is the expected shape for this codebase rather than a graph failure.Tests
python -m pytest desloppify/tests/ -q→ 5810 passed, 3 skipped.Two failures in
test_review_commands.py::TestCmdReviewPrepare::test_do_run_batches_dry_run_generates_packet_and_promptsreproduce on a clean checkout ofmainat3a7735d5and are unrelated to this change.Added
test_js_dep_graph_records_commonjs_require_edgesindesloppify/tests/lang/common/test_treesitter_imports_direct.py, covering CommonJSrequire, ESMimport, dynamicimport(), and the negative cases.