Fix TypeScript module system detection in CommonJS packages

[mohammedahmed18]

Issue #10: TypeScript Module System Detection Bug

Problem

TypeScript files (.ts, .tsx) were incorrectly detected as ESM regardless of package.json "type" field. This caused "Cannot use import statement outside a module" errors when the AI service generated ESM imports for TypeScript files in CommonJS packages.

Root Cause

File: /opt/codeflash/codeflash/languages/javascript/module_system.py (lines 73-77)

if suffix in (".ts", ".tsx", ".mts"):
    # TypeScript always uses ESM syntax (import/export)  ← WRONG COMMENT
    # even if package.json doesn't have "type": "module" ← WRONG LOGIC
    logger.debug("Detected ES Module from TypeScript file extension")
    return ModuleSystem.ES_MODULE  ← RETURNS TOO EARLY

The code confused:

• TypeScript SOURCE syntax (always uses import/export)
• RUNTIME module system (depends on package.json and tsconfig)

Impact

• Systematic bug (reproducible for ALL TypeScript files in CommonJS packages)
• Severity: HIGH (blocks test generation)
• Affected: 2 logs in current batch (both from backend-core/inMemoryQueue.ts)
• Trace IDs: 3fccd364-bc04-4196-9f50-f45f29a632d3, 6d220d09-3292-41de-a52c-d0e961e615e9

Fix

1. For .ts/.tsx files: Defer to package.json "type" field (don't return early)
2. If no "type" field: Default to CommonJS (Node.js default behavior)
3. Explicit extensions: .mts (always ESM) and .cts (always CommonJS) still override package.json

Testing

✅ 5 new regression tests covering all scenarios:

• TypeScript in CommonJS package (no "type" field)
• TypeScript in explicit CommonJS package ("type": "commonjs")
• TypeScript in ESM package ("type": "module")
• .mts files (always ESM)
• .cts files (always CommonJS)

✅ 318 existing JavaScript tests pass (no regressions)
✅ No linting/type errors (uv run prek)

Before/After

Before (broken):

// backend-core/package.json has no "type" field → CommonJS
// src/queue/inMemoryQueue.ts
export class InMemoryQueue { ... }

// Generated test imports ESM style:
import { jest, describe, it } from '@jest/globals';  ← FAILS!

// Error: Cannot use import statement outside a module

After (fixed):

// backend-core/package.json has no "type" field → CommonJS
// src/queue/inMemoryQueue.ts
export class InMemoryQueue { ... }

// Generated test respects CommonJS:
// (Jest globals available automatically in CommonJS mode)
describe('InMemoryQueue.on', () => { ... });  ← WORKS!

Files Changed

• codeflash/languages/javascript/module_system.py - Fix detection logic
• tests/test_languages/test_javascript_module_system.py - Update tests to match correct behavior
• tests/test_languages/test_typescript_commonjs_module_detection.py - New regression tests

Category

Systematic bug (reproducible on every TypeScript file in CommonJS packages)

[claude]

Claude finished @mohammedahmed18's task in 3m 6s —— View job

---

PR Review Summary

Prek Checks

• Auto-fixed a minor ruff formatting issue: a multi-line logger.debug(...) call in module_system.py was reformatted to a single line (within 120-char limit). Committed as style: auto-fix ruff formatting in module_system.py.
• All checks pass after the fix.
• 6 pre-existing mypy errors in module_system.py (unrelated to this PR, in add_js_extensions_to_relative_imports).

Code Review

The fix is correct and well-reasoned. The core logic change in detect_module_system (module_system.py):

1. .mts and .cts explicit extension handling — correctly added. .cts was missing before entirely.
2. .ts/.tsx defer to package.json — the key fix. pkg_type_from_json is initialized to None before the package_json.exists() block, so the fallback to CommonJS works correctly whether package.json is absent or present-but-missing-"type".
3. Fallthrough edge case — if a .ts file's package.json contains an unrecognised "type" value (not "module" or "commonjs"), execution falls through to Strategy 3 (file content analysis). Since TypeScript source always uses ESM import/export syntax, that would incorrectly detect ESM. This is an extremely narrow edge case that Node.js itself doesn't support, so it's fine to ignore.

One minor observation: the test_typescript_commonjs_module_detection.py test file doesn't follow the project's class-based test structure used in the existing test_javascript_module_system.py (which uses class TestDetectModuleSystem). This is cosmetic and not a blocker.

Duplicate Detection

No duplicates detected. detect_module_system is defined only in module_system.py and consumed via import in support.py.

Test Coverage

SMALL PR — coverage analysis skipped per policy. The PR adds 5 focused regression tests that directly cover the fixed code paths, including all combinations of .ts/.tsx with and without "type" field, plus .mts/.cts explicit overrides.

---

Verdict: LGTM. The fix correctly addresses the root cause (confusing TypeScript's source syntax with its runtime module system) and is consistent with Node.js behavior.

Last updated: 2026-04-04