From ba8b4c863cd63416e5abe819be672f3bda1dddde Mon Sep 17 00:00:00 2001 From: rohit-jsfreaky Date: Wed, 26 Aug 2026 01:17:09 +0530 Subject: [PATCH] fix(js): keep import edges to declared npm packages (#3084) (cherry picked from commit 70bdd8a6dc49444b350af09f4b978c7015fe5073) --- graphify/extract.py | 32 ++++++--- tests/test_js_dynamic_imports.py | 76 ++++++++++++++++++++++ tests/test_unmapped_at_alias_resolution.py | 3 +- 3 files changed, 100 insertions(+), 11 deletions(-) diff --git a/graphify/extract.py b/graphify/extract.py index d31c89326..fe0535023 100644 --- a/graphify/extract.py +++ b/graphify/extract.py @@ -2276,7 +2276,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: @@ -2418,10 +2418,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 @@ -2434,7 +2434,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 @@ -2444,18 +2444,18 @@ 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 + return _make_id(str(resolved_alias)), str(resolved_alias), resolved_file, False # Unmapped `@/` project-root convention fallback (#3357). if raw.startswith("@/"): resolved_unmapped = _resolve_js_module_path(raw, path.parent) if resolved_unmapped is not None and resolved_unmapped.is_file(): - return _make_id(str(resolved_unmapped)), str(resolved_unmapped), resolved_unmapped + return _make_id(str(resolved_unmapped)), str(resolved_unmapped), resolved_unmapped, False # Bare/scoped import (node_modules) - use last segment; # build_from_json drops as external if no matching node exists. module_name = raw.split("/")[-1] if not module_name: return None - return _make_id(module_name), raw, None + return _make_id(module_name), raw, None, True def _emit_rescued_import( @@ -2488,7 +2488,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", @@ -2504,11 +2504,23 @@ 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" + 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..b8926e2cb 100644 --- a/tests/test_js_dynamic_imports.py +++ b/tests/test_js_dynamic_imports.py @@ -275,3 +275,79 @@ 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" + ) diff --git a/tests/test_unmapped_at_alias_resolution.py b/tests/test_unmapped_at_alias_resolution.py index 7ad230afc..a27e533c7 100644 --- a/tests/test_unmapped_at_alias_resolution.py +++ b/tests/test_unmapped_at_alias_resolution.py @@ -253,8 +253,9 @@ def test_regex_rescue_unmapped_at_alias_resolves(tmp_path: Path): ) resolution = _resolve_rescued_specifier(svelte_file, "@/lib.js", aliases={}, base_url=None) assert resolution is not None - node_id, stub_sf, resolved_file = resolution + node_id, stub_sf, resolved_file, is_external = resolution assert resolved_file == lib + assert is_external is False def test_unresolved_at_alias_does_not_infer_call_to_unrelated_definition(tmp_path: Path):