fix(semantic): recognize .mts/.mjs/.cts/.cjs as source files - #75
Conversation
Files using the explicit ESM/CJS TypeScript & JavaScript extensions (.mts/.mjs/.cts/.cjs) were classified as assets rather than source, so they skipped Oxc symbol analysis entirely and changes to them never propagated to importers. This silently breaks true-affected detection for any monorepo whose package entrypoints/contracts use these extensions (e.g. modern NestJS services, native Node ESM packages). Completes frontops-dev#24, which added .js/.jsx ESM import resolution but not the .mjs/.cjs runtime-extension variants. - utils.rs: add mts/mjs/cts/cjs to SOURCE_EXTENSIONS (source/asset partition) - analyzer.rs: include them in the sourceRoot directory walk - semantic/mod.rs: remap .mjs->.mts and .cjs->.cts (TS ESM emits runtime exts), probe .mts/.mjs/.cts + /index.mts variants - resolve_options.rs: add the extensions + .mjs/.cjs extension_alias to the oxc resolver
|
Warning Review limit reached
More reviews will be available in 34 minutes and 15 seconds. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits. 🚦 How do rate limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughSource-file detection and module resolution now include ChangesExtended module extension support
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
src/semantic/reference_finder.rs (1)
535-582: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd a mirror test for
.cjs→.ctsremapping.The new remap branch for
.cjsis unverified in this test set; a small symmetric test would guard the added fallback path.Suggested test pattern
+#[test] +fn test_simple_resolve_cjs_to_cts_remapping() { + let temp_dir = TempDir::new().expect("Failed to create temp dir"); + let cwd = temp_dir.path(); + let src_dir = cwd.join("src"); + fs::create_dir_all(&src_dir).expect("Failed to create src dir"); + fs::write(src_dir.join("legacy.cts"), "export const schema = 1;") + .expect("Failed to write test file"); + + let profiler = Arc::new(Profiler::new(false)); + let analyzer = + WorkspaceAnalyzer::new(vec![], cwd, profiler.clone()).expect("Failed to create analyzer"); + let reference_finder = ReferenceFinder::new(&analyzer, cwd, profiler); + + let resolved = reference_finder.simple_resolve(src_dir.as_path(), "./legacy.cjs"); + assert_eq!(resolved, Some(PathBuf::from("src/legacy.cts"))); +}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/semantic/reference_finder.rs` around lines 535 - 582, Add a symmetric unit test in ReferenceFinder’s simple_resolve test suite to cover .cjs resolving to .cts, mirroring the existing .mjs→.mts case. Use ReferenceFinder::simple_resolve with a temporary workspace containing a .cts source file and assert the remapped PathBuf is returned, so the new fallback branch is verified alongside test_simple_resolve_mjs_to_mts_remapping and test_simple_resolve_index_mts.src/semantic/analyzer.rs (1)
247-252: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDeduplicate source-extension matching to a single shared definition.
This extension list now exists in multiple places; a future mismatch can silently break affected detection again. Reuse the shared source-file predicate/constant here instead of re-declaring literals.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/semantic/analyzer.rs` around lines 247 - 252, The source-extension matching logic is duplicated in the analyzer, which risks drifting from the shared definition. Update the extension check in analyzer.rs to reuse the existing shared source-file predicate or constant instead of hardcoding the literals again, using the relevant shared helper/definition that already powers source-file detection in the codebase.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@src/semantic/analyzer.rs`:
- Around line 247-252: The source-extension matching logic is duplicated in the
analyzer, which risks drifting from the shared definition. Update the extension
check in analyzer.rs to reuse the existing shared source-file predicate or
constant instead of hardcoding the literals again, using the relevant shared
helper/definition that already powers source-file detection in the codebase.
In `@src/semantic/reference_finder.rs`:
- Around line 535-582: Add a symmetric unit test in ReferenceFinder’s
simple_resolve test suite to cover .cjs resolving to .cts, mirroring the
existing .mjs→.mts case. Use ReferenceFinder::simple_resolve with a temporary
workspace containing a .cts source file and assert the remapped PathBuf is
returned, so the new fallback branch is verified alongside
test_simple_resolve_mjs_to_mts_remapping and test_simple_resolve_index_mts.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: ae560e9f-1e51-4b25-a578-f10cd574fcd5
📒 Files selected for processing (6)
src/core.rssrc/semantic/analyzer.rssrc/semantic/mod.rssrc/semantic/reference_finder.rssrc/semantic/resolve_options.rssrc/utils.rs
Address review feedback: - analyzer.rs: reuse the shared is_source_file predicate in the sourceRoot walk instead of re-hardcoding the extension literals, so it can't drift from SOURCE_EXTENSIONS (the drift that caused the original bug) - reference_finder.rs: add test_simple_resolve_cjs_to_cts_remapping mirroring the .mjs->.mts case
|
Thanks for the review — both nitpicks addressed in d25aa25:
Full suite green: |
|
@gbleu Thanks for the PR! |
📦 Preview Release AvailableA preview release has been published for commit d25aa25. Installationnpm install https://github.com/frontops-dev/domino/releases/download/pr-75-d25aa25/front-ops-domino-1.4.0.tgzRunning the previewnpx https://github.com/frontops-dev/domino/releases/download/pr-75-d25aa25/front-ops-domino-1.4.0.tgz affectedDetails |
|
Thank you @shaharkazaz! Validated the darwin-arm64 binary locally on a small repro, would also need #77 and #78 to drop the fork |
|
@gbleu approved both workflows there as well, a preview will be up soon. |
2 participants
Problem
Files using the explicit ESM/CJS TypeScript & JavaScript extensions —
.mts,.mjs,.cts,.cjs— are not recognized as source files. They are classified as assets, so they skip Oxc symbol analysis entirely. As a result, a change to any such file never propagates to the projects that import it:affectedreports only the package that directly owns the changed file, silently missing every downstream consumer.This is invisible in mixed repos —
.ts/.tsxanalysis keeps working — so the gap only shows up as under-reporting on the.mts-based parts of the graph. In a monorepo whose package entrypoints and cross-package contracts are.mts(e.g. modern NestJS services, native Node ESM packages), that's the entire cross-service dependency layer.Minimal repro
One package, one file, the same single-symbol change — only the extension differs:
affected --debugpartition.tsPartitioned files: 1 source, 0 assets✓.tsx1 source, 0 assets✓.mts0 source, **1 asset**✗.mjs0 source, **1 asset**✗.cts0 source, **1 asset**✗With the changed file treated as an asset, the debug log shows
Found 0 references to asset "...index.mts"and the consumers are never reached.Root cause
SOURCE_EXTENSIONSinsrc/utils.rs(which drivesis_source_file→ the source/asset partition) lists only["ts", "tsx", "js", "jsx"]. The samets | tsx | js | jsxset is hardcoded in the sourceRoot directory walk and the resolver config.This is the natural completion of #24, which added
.js/.jsx→.ts/.tsxESM import resolution but did not cover the.mjs/.cjsruntime-extension variants that TypeScript emits for.mts/.ctssources (export * from "./foo.mjs"where the source isfoo.mts).Fix
utils.rs— addmts/mjs/cts/cjstoSOURCE_EXTENSIONS(the source/asset partition)semantic/analyzer.rs— include them in the sourceRoot directory walksemantic/mod.rs— remap.mjs→.mtsand.cjs→.ctsin the relative resolver (TS ESM emits runtime extensions), and probe.mts/.mjs/.cts+/index.mtsvariantssemantic/resolve_options.rs— add the extensions and the.mjs/.cjsextension_aliasentries to the oxc resolverTests
is_source_fileassertions for.mts/.mjs/.cts/.cjs.mjs → .mtsremap and/index.mtsdirectory-entry probing209 passed; 0 failed.mtstemporal-contract now correctly marks the importing worker project as affected (previously missed), while projects importing unchanged symbols from the same package remain correctly pruned — i.e. precision is preserved, only the false negatives are fixed.Summary by CodeRabbit
New Features
.mts,.mjs,.cts, and.cjsfiles.Bug Fixes