fix(js): keep import edges to declared npm packages (#3084)

(cherry picked from commit 70bdd8a6dc)

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)
 

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<void> {\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<void> {\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"
+            )

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):