Skip to content

Commit a3d6f47

Browse files
misrasaurabh1claude
andcommitted
fix: stop adding .js extensions when converting CommonJS to ESM
The `add_js_extension()` function in `module_system.py` was adding .js extensions when converting CommonJS requires to ESM imports. This caused "Cannot find module" errors because TypeScript/Jest module resolution doesn't require explicit extensions. This change modifies `add_js_extension()` to NOT add extensions, as TypeScript and modern bundlers handle extension resolution automatically. Note: The aiservice now handles stripping any extensions that LLMs might add (see codeflash-internal PR #2341), so CLI-side stripping is removed. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
2 parents 92a18ed + 7e34379 commit a3d6f47

5 files changed

Lines changed: 61 additions & 154 deletions

File tree

codeflash/discovery/functions_to_optimize.py

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -216,6 +216,32 @@ def get_files_for_language(
216216
return files
217217

218218

219+
def _is_js_ts_function_exported(file_path: Path, function_name: str) -> tuple[bool, str | None]:
220+
"""Check if a JavaScript/TypeScript function is exported from its module.
221+
222+
For JS/TS, functions that are not exported cannot be imported by tests,
223+
making them impossible to optimize.
224+
225+
Args:
226+
file_path: Path to the source file.
227+
function_name: Name of the function to check.
228+
229+
Returns:
230+
Tuple of (is_exported, export_name). export_name may be 'default' for default exports.
231+
232+
"""
233+
from codeflash.languages.treesitter_utils import get_analyzer_for_file
234+
235+
try:
236+
source = file_path.read_text(encoding="utf-8")
237+
analyzer = get_analyzer_for_file(file_path)
238+
return analyzer.is_function_exported(source, function_name)
239+
except Exception as e:
240+
logger.debug(f"Failed to check export status for {function_name}: {e}")
241+
# Return True to avoid blocking in case of errors
242+
return True, None
243+
244+
219245
def _find_all_functions_in_python_file(file_path: Path) -> dict[Path, list[FunctionToOptimize]]:
220246
"""Find all optimizable functions in a Python file using AST parsing.
221247
@@ -338,6 +364,36 @@ def get_functions_to_optimize(
338364
exit_with_message(
339365
f"Function {only_get_this_function} not found in file {file}\nor the function does not have a 'return' statement or is a property"
340366
)
367+
368+
# For JavaScript/TypeScript, verify that the function (or its parent class) is exported
369+
# Non-exported functions cannot be imported by tests
370+
if found_function.language in ("javascript", "typescript"):
371+
# For class methods, check if the parent class is exported
372+
# For standalone functions, check if the function itself is exported
373+
if found_function.parents:
374+
# It's a class method - check if the class is exported
375+
name_to_check = found_function.top_level_parent_name
376+
else:
377+
# It's a standalone function - check if the function is exported
378+
name_to_check = found_function.function_name
379+
380+
is_exported, export_name = _is_js_ts_function_exported(file, name_to_check)
381+
if not is_exported:
382+
if found_function.parents:
383+
logger.debug(
384+
f"Class '{name_to_check}' containing method '{found_function.function_name}' "
385+
f"is not exported from {file}. "
386+
f"In JavaScript/TypeScript, only exported classes/functions can be optimized "
387+
f"because tests need to import them."
388+
)
389+
else:
390+
logger.debug(
391+
f"Function '{found_function.function_name}' is not exported from {file}. "
392+
f"In JavaScript/TypeScript, only exported functions can be optimized because "
393+
f"tests need to import them."
394+
)
395+
return {}, 0, None
396+
341397
functions[file] = [found_function]
342398
else:
343399
logger.info("Finding all functions modified in the current git diff ...")

codeflash/languages/javascript/instrument.py

Lines changed: 0 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -18,43 +18,6 @@
1818
from codeflash.discovery.functions_to_optimize import FunctionToOptimize
1919

2020

21-
# Patterns to strip file extensions from import paths
22-
# LLMs sometimes add .js extensions to TypeScript imports, which breaks module resolution
23-
_JS_EXTENSION_PATTERN = re.compile(
24-
r"""(from\s+['"])(\.{0,2}/[^'"]+?)(\.(?:js|ts|tsx|jsx|mjs|mts))(['"])"""
25-
)
26-
_REQUIRE_EXTENSION_PATTERN = re.compile(
27-
r"""(require\s*\(\s*['"])(\.{0,2}/[^'"]+?)(\.(?:js|ts|tsx|jsx|mjs|mts))(['"]\s*\))"""
28-
)
29-
_JEST_MOCK_EXTENSION_PATTERN = re.compile(
30-
r"""(jest\.(?:mock|doMock|unmock|requireActual|requireMock)\s*\(\s*['"])(\.{0,2}/[^'"]+?)(\.(?:js|ts|tsx|jsx|mjs|mts))(['"])"""
31-
)
32-
33-
34-
def strip_js_extensions(source: str) -> str:
35-
"""Strip .js/.ts/.tsx/.jsx extensions from relative import paths.
36-
37-
TypeScript and Jest's module resolution automatically resolve file extensions,
38-
so adding them explicitly can cause "Cannot find module" errors when the LLM
39-
adds incorrect extensions (e.g., .js to a .ts file).
40-
41-
This function removes extensions from:
42-
- ES module imports: import { x } from '../path/file.js'
43-
- CommonJS requires: require('../path/file.js')
44-
- Jest mocks: jest.mock('../path/file.js')
45-
46-
Args:
47-
source: The test source code.
48-
49-
Returns:
50-
Source code with extensions stripped from relative import paths.
51-
52-
"""
53-
source = _JS_EXTENSION_PATTERN.sub(r"\1\2\4", source)
54-
source = _REQUIRE_EXTENSION_PATTERN.sub(r"\1\2\4", source)
55-
return _JEST_MOCK_EXTENSION_PATTERN.sub(r"\1\2\4", source)
56-
57-
5821
class TestingMode:
5922
"""Testing mode constants."""
6023

codeflash/verification/verifier.py

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,6 @@ def generate_tests(
6767
from codeflash.languages.javascript.instrument import (
6868
TestingMode,
6969
instrument_generated_js_test,
70-
strip_js_extensions,
7170
validate_and_fix_import_style,
7271
)
7372
from codeflash.languages.javascript.module_system import ensure_module_system_compatibility
@@ -76,9 +75,6 @@ def generate_tests(
7675
func_name = function_to_optimize.function_name
7776
qualified_name = function_to_optimize.qualified_name
7877

79-
# Strip incorrect file extensions from import paths (LLMs sometimes add .js to .ts imports)
80-
generated_test_source = strip_js_extensions(generated_test_source)
81-
8278
# Validate and fix import styles (default vs named exports)
8379
generated_test_source = validate_and_fix_import_style(generated_test_source, source_file, func_name)
8480

tests/test_javascript_function_discovery.py

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -346,11 +346,11 @@ def test_get_specific_function(self, tmp_path):
346346
"""Test getting a specific function by name."""
347347
js_file = tmp_path / "math_utils.js"
348348
js_file.write_text("""
349-
function add(a, b) {
349+
export function add(a, b) {
350350
return a + b;
351351
}
352352
353-
function subtract(a, b) {
353+
export function subtract(a, b) {
354354
return a - b;
355355
}
356356
""")
@@ -378,7 +378,7 @@ def test_get_class_method(self, tmp_path):
378378
"""Test getting a specific class method."""
379379
js_file = tmp_path / "calculator.js"
380380
js_file.write_text("""
381-
class Calculator {
381+
export class Calculator {
382382
add(a, b) {
383383
return a + b;
384384
}
@@ -388,7 +388,7 @@ class Calculator {
388388
}
389389
}
390390
391-
function standaloneFunc() {
391+
export function standaloneFunc() {
392392
return 42;
393393
}
394394
""")

tests/test_languages/test_javascript_instrumentation.py

Lines changed: 1 addition & 109 deletions
Original file line numberDiff line numberDiff line change
@@ -651,112 +651,4 @@ def test_this_method_call_exact_output(self):
651651

652652
expected = " return codeflash.capture('Class.fibonacci', '1', this.fibonacci.bind(this), n - 1);"
653653
assert transformed == expected, f"Expected:\n{expected}\nGot:\n{transformed}"
654-
assert counter == 1
655-
656-
657-
class TestStripJsExtensions:
658-
"""Tests for stripping file extensions from import paths."""
659-
660-
def test_strip_js_extension_from_esm_import(self):
661-
"""Test stripping .js from ES module imports."""
662-
from codeflash.languages.javascript.instrument import strip_js_extensions
663-
664-
code = "import { getDifferences } from '../src/utils/DynamicBindingUtils.js';"
665-
expected = "import { getDifferences } from '../src/utils/DynamicBindingUtils';"
666-
667-
result = strip_js_extensions(code)
668-
assert result == expected
669-
670-
def test_strip_ts_extension_from_esm_import(self):
671-
"""Test stripping .ts from ES module imports."""
672-
from codeflash.languages.javascript.instrument import strip_js_extensions
673-
674-
code = "import { func } from './module.ts';"
675-
expected = "import { func } from './module';"
676-
677-
result = strip_js_extensions(code)
678-
assert result == expected
679-
680-
def test_strip_extension_from_require(self):
681-
"""Test stripping extensions from require() calls."""
682-
from codeflash.languages.javascript.instrument import strip_js_extensions
683-
684-
code = "const { func } = require('../utils/helper.js');"
685-
expected = "const { func } = require('../utils/helper');"
686-
687-
result = strip_js_extensions(code)
688-
assert result == expected
689-
690-
def test_strip_extension_from_jest_mock(self):
691-
"""Test stripping extensions from jest.mock() calls."""
692-
from codeflash.languages.javascript.instrument import strip_js_extensions
693-
694-
code = "jest.mock('../src/utils/DynamicBindingUtils.js');"
695-
expected = "jest.mock('../src/utils/DynamicBindingUtils');"
696-
697-
result = strip_js_extensions(code)
698-
assert result == expected
699-
700-
def test_strip_extension_from_jest_doMock(self):
701-
"""Test stripping extensions from jest.doMock() calls."""
702-
from codeflash.languages.javascript.instrument import strip_js_extensions
703-
704-
code = "jest.doMock('./helper.ts');"
705-
expected = "jest.doMock('./helper');"
706-
707-
result = strip_js_extensions(code)
708-
assert result == expected
709-
710-
def test_preserve_external_package_imports(self):
711-
"""Test that external package imports are not modified."""
712-
from codeflash.languages.javascript.instrument import strip_js_extensions
713-
714-
code = "import lodash from 'lodash';"
715-
716-
result = strip_js_extensions(code)
717-
assert result == code # Should be unchanged
718-
719-
def test_preserve_absolute_imports(self):
720-
"""Test that absolute/alias imports are not modified."""
721-
from codeflash.languages.javascript.instrument import strip_js_extensions
722-
723-
code = "import { Component } from '@/components/Button';"
724-
725-
result = strip_js_extensions(code)
726-
assert result == code # Should be unchanged
727-
728-
def test_strip_multiple_extensions_in_file(self):
729-
"""Test stripping multiple extensions in a single file."""
730-
from codeflash.languages.javascript.instrument import strip_js_extensions
731-
732-
code = """
733-
import { func1 } from '../utils/helper.js';
734-
import { func2 } from './local.ts';
735-
const { func3 } = require('../lib/util.tsx');
736-
jest.mock('../mocks/mock.jsx');
737-
"""
738-
expected = """
739-
import { func1 } from '../utils/helper';
740-
import { func2 } from './local';
741-
const { func3 } = require('../lib/util');
742-
jest.mock('../mocks/mock');
743-
"""
744-
745-
result = strip_js_extensions(code)
746-
assert result == expected
747-
748-
def test_strip_mjs_mts_extensions(self):
749-
"""Test stripping .mjs and .mts extensions."""
750-
from codeflash.languages.javascript.instrument import strip_js_extensions
751-
752-
code = """
753-
import { a } from './module.mjs';
754-
import { b } from '../util.mts';
755-
"""
756-
expected = """
757-
import { a } from './module';
758-
import { b } from '../util';
759-
"""
760-
761-
result = strip_js_extensions(code)
762-
assert result == expected
654+
assert counter == 1

0 commit comments

Comments
 (0)