diff --git a/graphify/extract.py b/graphify/extract.py index f68757a85..8526f68ef 100644 --- a/graphify/extract.py +++ b/graphify/extract.py @@ -1366,7 +1366,7 @@ def _rescue_js_dynamic_imports(path: Path, result: dict) -> None: resolution = _resolve_rescued_specifier(path, raw, aliases, base_url) if resolution is None: continue - node_id, _stub_sf, resolved_file = resolution + node_id, _stub_sf, resolved_file, _is_external = resolution # AST-captured already: same resolved target id, same resolved # on-disk file, or the engine's ref-namespaced external id. if node_id in deferred_ids or _make_id("ref", raw) in deferred_ids: @@ -1508,10 +1508,10 @@ def _resolve_rescued_specifier( raw: str, aliases, base_url, -) -> "tuple[str, str, Path | None] | None": +) -> "tuple[str, str, Path | None, bool] | None": """Resolve a regex-rescued import specifier the way ``_import_js`` does. - Returns ``(node_id, stub_source_file, resolved_file)`` — ``resolved_file`` + Returns ``(node_id, stub_source_file, resolved_file, is_external)`` — ``resolved_file`` is the target as a real on-disk file, or None when the specifier is external or dangling. Returns None when no target can be minted at all (empty bare-import segment). Split out of :func:`_emit_rescued_import` so @@ -1524,7 +1524,7 @@ def _resolve_rescued_specifier( Path(os.path.normpath(path.parent / raw)) ) resolved_file = resolved if resolved is not None and resolved.is_file() else None - return _make_id(str(resolved)), str(resolved), resolved_file + return _make_id(str(resolved)), str(resolved), resolved_file, False # Check tsconfig.json path aliases (e.g. "$lib/" -> "src/lib/", # "@/" -> "src/") before treating as external. Mirrors _import_js # logic so alias imports resolve to the same file node IDs the @@ -1534,13 +1534,15 @@ def _resolve_rescued_specifier( resolved_alias = _resolve_js_module_path(resolved_alias) resolved_file = (resolved_alias if resolved_alias is not None and resolved_alias.is_file() else None) - return _make_id(str(resolved_alias)), str(resolved_alias), resolved_file - # Bare/scoped import (node_modules) - use last segment; - # build_from_json drops as external if no matching node exists. - module_name = raw.split("/")[-1] + return _make_id(str(resolved_alias)), str(resolved_alias), resolved_file, False + # Bare/scoped import (node_modules) - anchor to the PACKAGE, not the path + # leaf: `lodash/fp/map` is a dependency on `lodash`, and `@a/utils` and + # `@b/utils` are different packages that share a leaf. + parts = raw.split("/") + module_name = "/".join(parts[:2]) if raw.startswith("@") and len(parts) >= 2 else parts[0] if not module_name: return None - return _make_id(module_name), raw, None + return _make_id(module_name), module_name, None, True def _emit_rescued_import( @@ -1573,7 +1575,7 @@ def _emit_rescued_import( resolution = _resolve_rescued_specifier(path, raw, aliases, base_url) if resolution is None: return - node_id, stub_source_file, resolved_file = resolution + node_id, stub_source_file, resolved_file, is_external = resolution edge = { "source": file_node_id, "target": node_id, "relation": relation, "confidence": "EXTRACTED", @@ -1589,11 +1591,28 @@ def _emit_rescued_import( # Edge target already a real node - just add the edge, don't add a node. result.setdefault("edges", []).append(edge) return - result.setdefault("nodes", []).append({ + stub: dict = { "id": node_id, "label": raw, "file_type": "code", "source_file": stub_source_file, "confidence": "EXTRACTED", - }) + } + if is_external: + # A bare/scoped specifier names a MODULE, not a path: the same package + # imported from N files is one node, and it is the same package a + # manifest in the corpus declares under that name. Mark it the way Swift + # module anchors are marked (#1327) — `file_type=code` keeps build.py + # validation happy, `type=module` exempts it from id-disambiguation — so + # it collapses with that declaration instead of being salted apart from + # it. Without this the salt renames the node and every importing edge is + # left on the dead id, so a declared dependency silently loses all of its + # inbound edges while an undeclared one keeps them (#3084). + stub["type"] = "module" + # For an external, `stub_source_file` is the package name (see + # _resolve_rescued_specifier) — and the node IS that package, so name it + # that way. `lodash/fp/map` and `lodash/debounce` are one `lodash` node, + # and neither subpath should end up titling it. + stub["label"] = stub_source_file + result.setdefault("nodes", []).append(stub) result.setdefault("edges", []).append(edge) existing_ids.add(node_id) diff --git a/tests/test_js_dynamic_imports.py b/tests/test_js_dynamic_imports.py index 422c38f92..c74be8223 100644 --- a/tests/test_js_dynamic_imports.py +++ b/tests/test_js_dynamic_imports.py @@ -275,3 +275,127 @@ def test_dynamic_import_is_traversed_by_affected(): g.add_edge("importer", "dep", relation="dynamic_import") hits = affected_nodes(g, "dep", depth=1) assert any(h.node_id == "importer" for h in hits) + + +def test_declared_npm_dependency_keeps_its_import_edge(tmp_path, monkeypatch): + """A package declared in package.json must not lose its inbound import edges. + + The bare specifier mints an external stub whose id is the package name; the + manifest in the same corpus produces a node under that same name. Two nodes, + one id, different source_files -> _disambiguate_colliding_node_ids salted both + apart, and the importing edge — which carries neither salt's source key — + was left on the now-dead id and dropped at build. The perverse result was + that an UNDECLARED package kept its edge (nothing to collide with) while a + properly declared dependency became invisible (#3084). + + The stub is a module anchor, like the Swift ones (#1327), so it collapses onto + the declaration instead of being salted away from it. + """ + _write(tmp_path / "package.json", + '{"name": "mre", "version": "1.0.0", "dependencies": {"postgres": "^3.4.0"}}\n') + _write(tmp_path / "app.mts", + "const postgres = (await import('postgres')).default;\n" + "export async function connect(url: string): Promise {\n" + " postgres(url, { max: 1 });\n" + "}\n") + + monkeypatch.chdir(tmp_path) + result = extract([Path("app.mts"), Path("package.json")], cache_root=tmp_path / ".cache") + + node_ids = {n["id"] for n in result["nodes"]} + pg_edges = [e for e in result["edges"] + if e["relation"] in ("dynamic_import", "imports_from") + and "postgres" in e["target"]] + assert pg_edges, "the dynamic import of a declared dependency must emit an edge" + for edge in pg_edges: + assert edge["target"] in node_ids, ( + f"edge target {edge['target']!r} has no node — it would be dropped at build" + ) + + +def test_undeclared_npm_dependency_still_keeps_its_edge(tmp_path, monkeypatch): + """The no-manifest case must keep working: nothing to collapse onto, but the + stub still anchors the edge.""" + _write(tmp_path / "app.mts", + "const postgres = (await import('postgres')).default;\n" + "export async function connect(url: string): Promise {\n" + " postgres(url, { max: 1 });\n" + "}\n") + + monkeypatch.chdir(tmp_path) + result = extract([Path("app.mts")], cache_root=tmp_path / ".cache") + + node_ids = {n["id"] for n in result["nodes"]} + pg_edges = [e for e in result["edges"] + if e["relation"] in ("dynamic_import", "imports_from") + and "postgres" in e["target"]] + assert pg_edges + for edge in pg_edges: + assert edge["target"] in node_ids + + +def test_unresolved_relative_import_is_not_marked_a_module(tmp_path, monkeypatch): + """Only bare/scoped specifiers are module anchors. + + An unresolved RELATIVE specifier names a path, not a module, so it must stay + subject to id-disambiguation — two `./helper` stubs in different directories + are different files and must not collapse onto one node. + """ + _write(tmp_path / "app.mts", "const h = (await import('./missing-helper')).default;\n") + + monkeypatch.chdir(tmp_path) + result = extract([Path("app.mts")], cache_root=tmp_path / ".cache") + + for node in result["nodes"]: + if "missing" in str(node.get("label", "")) or "missing" in node["id"]: + assert node.get("type") != "module", ( + "an unresolved relative import must not be exempted from disambiguation" + ) + + +def test_scoped_packages_sharing_a_leaf_stay_distinct(tmp_path, monkeypatch): + """`@a/utils` and `@b/utils` are different packages that share a path leaf. + + The external stub is anchored to the package name, not the last segment. + Anchoring to the leaf gave both the id `utils`, and because a module anchor + is exempt from id-disambiguation they then merged into one node — so an + import of `@scope-a/utils` resolved to a node labelled `@scope-b/utils`. + """ + _write(tmp_path / "a.mts", "const x = (await import('@scope-a/utils')).default;\nexport const A = 1;\n") + _write(tmp_path / "b.mts", "const y = (await import('@scope-b/utils')).default;\nexport const B = 2;\n") + + monkeypatch.chdir(tmp_path) + result = extract([Path("a.mts"), Path("b.mts")], cache_root=tmp_path / ".cache") + + stubs = {n["id"]: n for n in result["nodes"] if n.get("type") == "module"} + assert len(stubs) == 2, f"the two packages must not share a node: {sorted(stubs)}" + assert {n["label"] for n in stubs.values()} == {"@scope-a/utils", "@scope-b/utils"} + + node_ids = {n["id"] for n in result["nodes"]} + targets = {e["target"] for e in result["edges"] if e["relation"] == "dynamic_import"} + assert len(targets) == 2, f"each import must reach its own package: {targets}" + for t in targets: + assert t in node_ids + + +def test_subpath_imports_collapse_onto_their_package(tmp_path, monkeypatch): + """`lodash/fp/map` is a dependency on `lodash`, not on `map`. + + Every subpath of one package is one package node, and it is named for the + package — not for whichever specifier happened to be extracted first. + """ + _write(tmp_path / "a.mts", "const x = (await import('lodash/fp/map')).default;\nexport const A = 1;\n") + _write(tmp_path / "b.mts", "const y = (await import('lodash/debounce')).default;\nexport const B = 2;\n") + + monkeypatch.chdir(tmp_path) + result = extract([Path("a.mts"), Path("b.mts")], cache_root=tmp_path / ".cache") + + # One stub dict is emitted per importing file; they carry the same id and so + # collapse to a single node at build. What matters is that the id and the + # name are the package's, whichever subpath minted them. + stubs = [n for n in result["nodes"] if n.get("type") == "module"] + assert {n["id"] for n in stubs} == {"lodash"}, "both subpaths are the same package" + assert {n["label"] for n in stubs} == {"lodash"}, "the node is the package, not one of its subpaths" + + targets = {e["target"] for e in result["edges"] if e["relation"] == "dynamic_import"} + assert targets == {"lodash"}