fix: prefer src/ over build output dirs in JS module detection
JS projects often have package.json main/module/exports pointing to build output directories (build/, dist/, out/) which contain compiled code. Now the detector prioritizes common source directories (src/, lib/, source/) and skips build output paths when determining module root.
This commit is contained in:
parent
aea1786899
commit
c7272ecdb9
3 changed files with 298 additions and 18 deletions
|
|
@ -310,14 +310,21 @@ def _detect_js_module_root(project_root: Path) -> tuple[Path, str]:
|
|||
"""Detect JavaScript/TypeScript module root.
|
||||
|
||||
Priority:
|
||||
1. package.json "exports" field
|
||||
2. package.json "module" field (ESM)
|
||||
3. package.json "main" field (CJS)
|
||||
4. src/ directory
|
||||
5. lib/ directory
|
||||
6. Project root
|
||||
1. src/, lib/, source/ directories (common source directories)
|
||||
2. package.json "exports" field (if not in build output directory)
|
||||
3. package.json "module" field (ESM, if not in build output directory)
|
||||
4. package.json "main" field (CJS, if not in build output directory)
|
||||
5. Project root
|
||||
|
||||
Build output directories (build/, dist/, out/) are skipped since they contain
|
||||
compiled code, not source files.
|
||||
|
||||
"""
|
||||
# Check for common source directories first - these are always preferred
|
||||
for src_dir in ["src", "lib", "source"]:
|
||||
if (project_root / src_dir).is_dir():
|
||||
return project_root / src_dir, f"{src_dir}/ directory"
|
||||
|
||||
package_json_path = project_root / "package.json"
|
||||
package_data: dict[str, Any] = {}
|
||||
|
||||
|
|
@ -334,32 +341,54 @@ def _detect_js_module_root(project_root: Path) -> tuple[Path, str]:
|
|||
entry_path = _extract_entry_path(exports)
|
||||
if entry_path:
|
||||
parent = Path(entry_path).parent
|
||||
if parent != Path() and parent.as_posix() != "." and (project_root / parent).is_dir():
|
||||
if (
|
||||
parent != Path()
|
||||
and parent.as_posix() != "."
|
||||
and (project_root / parent).is_dir()
|
||||
and not is_build_output_dir(parent)
|
||||
):
|
||||
return project_root / parent, f'{parent.as_posix()}/ (from package.json "exports")'
|
||||
|
||||
# Check module field (ESM)
|
||||
module_field = package_data.get("module")
|
||||
if module_field and isinstance(module_field, str):
|
||||
parent = Path(module_field).parent
|
||||
if parent != Path() and parent.as_posix() != "." and (project_root / parent).is_dir():
|
||||
if (
|
||||
parent != Path()
|
||||
and parent.as_posix() != "."
|
||||
and (project_root / parent).is_dir()
|
||||
and not is_build_output_dir(parent)
|
||||
):
|
||||
return project_root / parent, f'{parent.as_posix()}/ (from package.json "module")'
|
||||
|
||||
# Check main field (CJS)
|
||||
main_field = package_data.get("main")
|
||||
if main_field and isinstance(main_field, str):
|
||||
parent = Path(main_field).parent
|
||||
if parent != Path() and parent.as_posix() != "." and (project_root / parent).is_dir():
|
||||
if (
|
||||
parent != Path()
|
||||
and parent.as_posix() != "."
|
||||
and (project_root / parent).is_dir()
|
||||
and not is_build_output_dir(parent)
|
||||
):
|
||||
return project_root / parent, f'{parent.as_posix()}/ (from package.json "main")'
|
||||
|
||||
# Check for common source directories
|
||||
for src_dir in ["src", "lib", "source"]:
|
||||
if (project_root / src_dir).is_dir():
|
||||
return project_root / src_dir, f"{src_dir}/ directory"
|
||||
|
||||
# Default to project root
|
||||
return project_root, "project root"
|
||||
|
||||
|
||||
def is_build_output_dir(path: Path) -> bool:
|
||||
"""Check if a path is within a common build output directory.
|
||||
|
||||
Build output directories contain compiled code and should be skipped
|
||||
in favor of source directories.
|
||||
|
||||
"""
|
||||
build_dirs = {"build", "dist", "out", ".next", ".nuxt"}
|
||||
parts = path.as_posix().split("/")
|
||||
return any(part in build_dirs for part in parts)
|
||||
|
||||
|
||||
def _extract_entry_path(exports: Any) -> str | None:
|
||||
"""Extract entry path from package.json exports field."""
|
||||
if isinstance(exports, str):
|
||||
|
|
|
|||
|
|
@ -14,6 +14,7 @@ from codeflash.setup.detector import (
|
|||
_find_project_root,
|
||||
detect_project,
|
||||
has_existing_config,
|
||||
is_build_output_dir,
|
||||
)
|
||||
|
||||
|
||||
|
|
@ -139,15 +140,15 @@ class TestDetectModuleRoot:
|
|||
assert "pyproject.toml" in detail
|
||||
|
||||
def test_js_detects_from_exports(self, tmp_path):
|
||||
"""Should detect module root from package.json exports."""
|
||||
"""Should detect module root from package.json exports when no common src dir exists."""
|
||||
(tmp_path / "package.json").write_text(json.dumps({
|
||||
"name": "test",
|
||||
"exports": {".": "./src/index.js"}
|
||||
"exports": {".": "./packages/core/index.js"}
|
||||
}))
|
||||
(tmp_path / "src").mkdir()
|
||||
(tmp_path / "packages" / "core").mkdir(parents=True)
|
||||
|
||||
module_root, detail = _detect_js_module_root(tmp_path)
|
||||
assert module_root == tmp_path / "src"
|
||||
assert module_root == tmp_path / "packages" / "core"
|
||||
assert "exports" in detail
|
||||
|
||||
def test_js_detects_src_convention(self, tmp_path):
|
||||
|
|
@ -158,6 +159,214 @@ class TestDetectModuleRoot:
|
|||
module_root, detail = _detect_js_module_root(tmp_path)
|
||||
assert module_root == tmp_path / "src"
|
||||
|
||||
def test_js_prefers_src_over_build_src(self, tmp_path):
|
||||
"""Should prefer src/ over build/src/ even when package.json points to build/."""
|
||||
(tmp_path / "package.json").write_text(json.dumps({
|
||||
"name": "test",
|
||||
"main": "build/src/index.js",
|
||||
"module": "build/src/index.js"
|
||||
}))
|
||||
(tmp_path / "src").mkdir()
|
||||
(tmp_path / "build" / "src").mkdir(parents=True)
|
||||
|
||||
module_root, detail = _detect_js_module_root(tmp_path)
|
||||
assert module_root == tmp_path / "src"
|
||||
assert "src/ directory" in detail
|
||||
|
||||
def test_js_skips_build_dir_from_main(self, tmp_path):
|
||||
"""Should skip build output directories from package.json main field."""
|
||||
(tmp_path / "package.json").write_text(json.dumps({
|
||||
"name": "test",
|
||||
"main": "build/index.js"
|
||||
}))
|
||||
(tmp_path / "build").mkdir()
|
||||
|
||||
module_root, detail = _detect_js_module_root(tmp_path)
|
||||
assert module_root == tmp_path
|
||||
assert "project root" in detail
|
||||
|
||||
def test_js_skips_dist_dir_from_exports(self, tmp_path):
|
||||
"""Should skip dist output directories from package.json exports field."""
|
||||
(tmp_path / "package.json").write_text(json.dumps({
|
||||
"name": "test",
|
||||
"exports": {".": "./dist/index.js"}
|
||||
}))
|
||||
(tmp_path / "dist").mkdir()
|
||||
|
||||
module_root, detail = _detect_js_module_root(tmp_path)
|
||||
assert module_root == tmp_path
|
||||
assert "project root" in detail
|
||||
|
||||
def test_js_skips_out_dir_from_module(self, tmp_path):
|
||||
"""Should skip out output directories from package.json module field."""
|
||||
(tmp_path / "package.json").write_text(json.dumps({
|
||||
"name": "test",
|
||||
"module": "out/esm/index.js"
|
||||
}))
|
||||
(tmp_path / "out" / "esm").mkdir(parents=True)
|
||||
|
||||
module_root, detail = _detect_js_module_root(tmp_path)
|
||||
assert module_root == tmp_path
|
||||
assert "project root" in detail
|
||||
|
||||
def test_js_prefers_lib_over_build_dir(self, tmp_path):
|
||||
"""Should prefer lib/ over build output directories."""
|
||||
(tmp_path / "package.json").write_text(json.dumps({
|
||||
"name": "test",
|
||||
"main": "dist/index.js"
|
||||
}))
|
||||
(tmp_path / "lib").mkdir()
|
||||
(tmp_path / "dist").mkdir()
|
||||
|
||||
module_root, detail = _detect_js_module_root(tmp_path)
|
||||
assert module_root == tmp_path / "lib"
|
||||
assert "lib/ directory" in detail
|
||||
|
||||
def test_js_prefers_source_over_build_dir(self, tmp_path):
|
||||
"""Should prefer source/ over build output directories."""
|
||||
(tmp_path / "package.json").write_text(json.dumps({
|
||||
"name": "test",
|
||||
"main": "build/index.js"
|
||||
}))
|
||||
(tmp_path / "source").mkdir()
|
||||
(tmp_path / "build").mkdir()
|
||||
|
||||
module_root, detail = _detect_js_module_root(tmp_path)
|
||||
assert module_root == tmp_path / "source"
|
||||
assert "source/ directory" in detail
|
||||
|
||||
def test_js_falls_back_to_valid_exports_path(self, tmp_path):
|
||||
"""Should use exports path when no common source dirs exist and path is not build output."""
|
||||
(tmp_path / "package.json").write_text(json.dumps({
|
||||
"name": "test",
|
||||
"exports": {".": "./packages/core/index.js"}
|
||||
}))
|
||||
(tmp_path / "packages" / "core").mkdir(parents=True)
|
||||
|
||||
module_root, detail = _detect_js_module_root(tmp_path)
|
||||
assert module_root == tmp_path / "packages" / "core"
|
||||
assert "exports" in detail
|
||||
|
||||
def test_js_falls_back_to_valid_main_path(self, tmp_path):
|
||||
"""Should use main path when no common source dirs exist and path is not build output."""
|
||||
(tmp_path / "package.json").write_text(json.dumps({
|
||||
"name": "test",
|
||||
"main": "packages/main/index.js"
|
||||
}))
|
||||
(tmp_path / "packages" / "main").mkdir(parents=True)
|
||||
|
||||
module_root, detail = _detect_js_module_root(tmp_path)
|
||||
assert module_root == tmp_path / "packages" / "main"
|
||||
assert "main" in detail
|
||||
|
||||
def test_js_falls_back_to_valid_module_path(self, tmp_path):
|
||||
"""Should use module path when no common source dirs exist and path is not build output."""
|
||||
(tmp_path / "package.json").write_text(json.dumps({
|
||||
"name": "test",
|
||||
"module": "esm/index.js"
|
||||
}))
|
||||
(tmp_path / "esm").mkdir()
|
||||
|
||||
module_root, detail = _detect_js_module_root(tmp_path)
|
||||
assert module_root == tmp_path / "esm"
|
||||
assert "module" in detail
|
||||
|
||||
def test_js_returns_project_root_when_all_paths_are_build_output(self, tmp_path):
|
||||
"""Should return project root when all package.json paths point to build outputs."""
|
||||
(tmp_path / "package.json").write_text(json.dumps({
|
||||
"name": "test",
|
||||
"main": "dist/cjs/index.js",
|
||||
"module": "dist/esm/index.js",
|
||||
"exports": {".": "./build/index.js"}
|
||||
}))
|
||||
(tmp_path / "dist" / "cjs").mkdir(parents=True)
|
||||
(tmp_path / "dist" / "esm").mkdir(parents=True)
|
||||
(tmp_path / "build").mkdir()
|
||||
|
||||
module_root, detail = _detect_js_module_root(tmp_path)
|
||||
assert module_root == tmp_path
|
||||
assert "project root" in detail
|
||||
|
||||
def test_js_handles_malformed_package_json(self, tmp_path):
|
||||
"""Should handle malformed package.json gracefully."""
|
||||
(tmp_path / "package.json").write_text("{ invalid json }")
|
||||
|
||||
module_root, detail = _detect_js_module_root(tmp_path)
|
||||
assert module_root == tmp_path
|
||||
assert "project root" in detail
|
||||
|
||||
|
||||
class TestIsBuildOutputDir:
|
||||
"""Tests for is_build_output_dir function."""
|
||||
|
||||
def test_detects_build_dir(self):
|
||||
"""Should detect build/ as build output."""
|
||||
from pathlib import Path
|
||||
assert is_build_output_dir(Path("build"))
|
||||
assert is_build_output_dir(Path("build/src"))
|
||||
assert is_build_output_dir(Path("build/src/index.js"))
|
||||
|
||||
def test_detects_dist_dir(self):
|
||||
"""Should detect dist/ as build output."""
|
||||
from pathlib import Path
|
||||
assert is_build_output_dir(Path("dist"))
|
||||
assert is_build_output_dir(Path("dist/esm"))
|
||||
assert is_build_output_dir(Path("dist/cjs/index.js"))
|
||||
|
||||
def test_detects_out_dir(self):
|
||||
"""Should detect out/ as build output."""
|
||||
from pathlib import Path
|
||||
assert is_build_output_dir(Path("out"))
|
||||
assert is_build_output_dir(Path("out/src"))
|
||||
|
||||
def test_detects_next_dir(self):
|
||||
"""Should detect .next/ as build output."""
|
||||
from pathlib import Path
|
||||
assert is_build_output_dir(Path(".next"))
|
||||
assert is_build_output_dir(Path(".next/static"))
|
||||
|
||||
def test_detects_nuxt_dir(self):
|
||||
"""Should detect .nuxt/ as build output."""
|
||||
from pathlib import Path
|
||||
assert is_build_output_dir(Path(".nuxt"))
|
||||
assert is_build_output_dir(Path(".nuxt/dist"))
|
||||
|
||||
def test_detects_nested_build_dir(self):
|
||||
"""Should detect build dir nested in path."""
|
||||
from pathlib import Path
|
||||
assert is_build_output_dir(Path("packages/build/index.js"))
|
||||
assert is_build_output_dir(Path("foo/dist/bar"))
|
||||
|
||||
def test_does_not_detect_src(self):
|
||||
"""Should not detect src/ as build output."""
|
||||
from pathlib import Path
|
||||
assert not is_build_output_dir(Path("src"))
|
||||
assert not is_build_output_dir(Path("src/index.js"))
|
||||
|
||||
def test_does_not_detect_lib(self):
|
||||
"""Should not detect lib/ as build output."""
|
||||
from pathlib import Path
|
||||
assert not is_build_output_dir(Path("lib"))
|
||||
assert not is_build_output_dir(Path("lib/utils"))
|
||||
|
||||
def test_does_not_detect_source(self):
|
||||
"""Should not detect source/ as build output."""
|
||||
from pathlib import Path
|
||||
assert not is_build_output_dir(Path("source"))
|
||||
|
||||
def test_does_not_detect_packages(self):
|
||||
"""Should not detect packages/ as build output."""
|
||||
from pathlib import Path
|
||||
assert not is_build_output_dir(Path("packages"))
|
||||
assert not is_build_output_dir(Path("packages/core"))
|
||||
|
||||
def test_does_not_detect_similar_names(self):
|
||||
"""Should not detect directories with similar but different names."""
|
||||
from pathlib import Path
|
||||
assert not is_build_output_dir(Path("builder"))
|
||||
assert not is_build_output_dir(Path("distribution"))
|
||||
assert not is_build_output_dir(Path("output"))
|
||||
|
||||
|
||||
class TestDetectTestsRoot:
|
||||
"""Tests for tests root detection."""
|
||||
|
|
|
|||
|
|
@ -857,6 +857,48 @@ class TestE2ECLIFlags:
|
|||
# Should complete without error
|
||||
_handle_show_config()
|
||||
|
||||
def test_show_config_displays_config_path_when_saved(self, project_with_existing_config, monkeypatch):
|
||||
"""Should display config file path when saved config exists."""
|
||||
monkeypatch.chdir(project_with_existing_config)
|
||||
|
||||
# Track what gets printed
|
||||
printed_messages = []
|
||||
|
||||
def mock_print(msg="", *args, **kwargs):
|
||||
printed_messages.append(str(msg))
|
||||
|
||||
from codeflash.cli_cmds import console
|
||||
monkeypatch.setattr(console.console, "print", mock_print)
|
||||
|
||||
from codeflash.cli_cmds.cli import _handle_show_config
|
||||
_handle_show_config()
|
||||
|
||||
# Verify config path is displayed
|
||||
all_output = "\n".join(printed_messages)
|
||||
assert "pyproject.toml" in all_output
|
||||
assert "Config file:" in all_output
|
||||
|
||||
def test_show_config_no_path_when_auto_detected(self, python_src_layout, monkeypatch):
|
||||
"""Should not display config file path when config is auto-detected."""
|
||||
monkeypatch.chdir(python_src_layout)
|
||||
|
||||
# Track what gets printed
|
||||
printed_messages = []
|
||||
|
||||
def mock_print(msg="", *args, **kwargs):
|
||||
printed_messages.append(str(msg))
|
||||
|
||||
from codeflash.cli_cmds import console
|
||||
monkeypatch.setattr(console.console, "print", mock_print)
|
||||
|
||||
from codeflash.cli_cmds.cli import _handle_show_config
|
||||
_handle_show_config()
|
||||
|
||||
# Verify no config path line is displayed
|
||||
all_output = "\n".join(printed_messages)
|
||||
assert "Config file:" not in all_output
|
||||
assert "Auto-detected" in all_output
|
||||
|
||||
def test_reset_config_removes_from_pyproject(self, project_with_existing_config, monkeypatch):
|
||||
"""Should remove codeflash config from pyproject.toml."""
|
||||
monkeypatch.chdir(project_with_existing_config)
|
||||
|
|
|
|||
Loading…
Reference in a new issue