mirror of
https://github.com/safishamsi/graphify.git
synced 2026-08-30 02:07:15 +00:00
fix(extract): TS bare-path / .svelte.ts / index.ts import resolution
_import_js previously only rewrote .js→.ts and .jsx→.tsx, leaving every
other common TypeScript / SvelteKit / Vite import shape unresolved. The
resulting node id wouldn't match the target file's own _make_id, so
build_from_json dropped the edge as external.
Three missed shapes:
1. Bare paths (no extension) — TS convention:
`import { foo } from './foo'` → real file is foo.ts
2. .svelte → .svelte.ts (Svelte 5 rune-only files):
`import { x } from './x.svelte'` → real file is x.svelte.ts
3. Directory imports / barrel index files:
`import { x } from './queue'` → real file is queue/index.ts
Fix
---
New helper _resolve_with_extensions(p: Path) -> Path mirrors Vite/TS
resolver order:
1. exact path (file)
2. .js→.ts, .jsx→.tsx (existing TS-ESM convention)
3. bare path → .ts/.tsx/.svelte/.js/.jsx/.mjs
4. bare path → directory's index.{ts,tsx,js,jsx}
5. .svelte → .svelte.ts (Svelte 5 rune file)
Falls back to the original path on no match — preserves pre-fix behaviour
for genuinely external modules (build_from_json drops them as phantoms).
Wired into _import_js (relative + alias branches) and extract_svelte's
regex pass for dynamic_import so static and dynamic imports both benefit.
Subtle: uses .is_file() / .is_dir() rather than .exists(). When the
import is a directory, .exists() returns True and would short-circuit
before the index.ts lookup ever ran.
Tests
-----
20 new tests in tests/test_import_extension_resolution.py:
Resolver unit tests (12):
- existing path returned unchanged
- bare path → .ts / .tsx / .svelte
- .ts wins over .svelte for ambiguous bare paths (Vite order)
- directory → index.ts
- directory prefers index.ts over index.js
- .svelte → .svelte.ts (Svelte 5 rune file)
- .js → .ts (TS ESM convention)
- .jsx → .tsx
- real .js stays .js when .ts doesn't exist
- unresolvable returns input unchanged
End-to-end (8):
- bare-path import resolves in TS file
- directory import resolves to index.ts
- .svelte import resolves to .svelte.ts rune file
- explicit .ts/.svelte imports still work (regression guard)
- external module specifiers unchanged
- alias + bare path resolves
- dynamic_import bare path resolves
This commit is contained in:
+77
-10
@@ -188,6 +188,72 @@ class LanguageConfig:
|
||||
|
||||
# ── Generic helpers ───────────────────────────────────────────────────────────
|
||||
|
||||
# Vite/TS resolver order. Used by _resolve_with_extensions() to map TypeScript
|
||||
# bare-path imports onto real files on disk, so the resulting node id matches
|
||||
# the one _extract_generic creates for the target file (#716).
|
||||
_TS_RESOLVE_EXTS = (".ts", ".tsx", ".svelte", ".js", ".jsx", ".mjs")
|
||||
_TS_INDEX_FILES = ("index.ts", "index.tsx", "index.js", "index.jsx")
|
||||
|
||||
|
||||
def _resolve_with_extensions(p: Path) -> Path:
|
||||
"""Resolve a TypeScript-style import path to an actual file on disk.
|
||||
|
||||
TS / SvelteKit / Vite let you write imports without a file extension and
|
||||
auto-resolve via a fixed extension order. The pre-existing .js→.ts and
|
||||
.jsx→.tsx rewrites only covered the TS-ESM-via-.js convention; everything
|
||||
else dropped to a phantom node id and the edge was lost in build_from_json.
|
||||
|
||||
Order, mirroring Vite's resolver:
|
||||
1. exact path (if it exists)
|
||||
2. .js → .ts (TS ESM convention; written as .js, file is .ts)
|
||||
3. .jsx → .tsx
|
||||
4. bare path → try .ts/.tsx/.svelte/.js/.jsx/.mjs
|
||||
5. bare path → try directory's index.{ts,tsx,js,jsx}
|
||||
6. .svelte path that isn't a real .svelte file → try the same name
|
||||
with .ts appended (Svelte 5 rune-only files like foo.svelte.ts —
|
||||
imports are written as './foo.svelte' but the file is .svelte.ts)
|
||||
|
||||
Falls back to the original path on no match — the edge will be dropped
|
||||
as external by build_from_json, matching pre-#716 behaviour for cases
|
||||
we genuinely can't resolve (truly external modules).
|
||||
"""
|
||||
# Existing FILE wins — directory matches must fall through to index lookup,
|
||||
# otherwise `from './queue'` (where queue/ is a real directory) would
|
||||
# short-circuit and never resolve to queue/index.ts.
|
||||
if p.is_file():
|
||||
return p
|
||||
# Directory imports: try index.{ts,tsx,js,jsx}
|
||||
if p.is_dir():
|
||||
for idx in _TS_INDEX_FILES:
|
||||
c = p / idx
|
||||
if c.is_file():
|
||||
return c
|
||||
return p
|
||||
if p.suffix == ".js":
|
||||
c = p.with_suffix(".ts")
|
||||
if c.is_file():
|
||||
return c
|
||||
if p.suffix == ".jsx":
|
||||
c = p.with_suffix(".tsx")
|
||||
if c.is_file():
|
||||
return c
|
||||
if p.suffix == "":
|
||||
for ext in _TS_RESOLVE_EXTS:
|
||||
c = p.with_suffix(ext)
|
||||
if c.is_file():
|
||||
return c
|
||||
if p.suffix == ".svelte":
|
||||
# SvelteKit imports written as `from './foo.svelte'` may actually point
|
||||
# at `foo.svelte.ts` (a Svelte 5 rune file). Append .ts to the FULL
|
||||
# filename rather than swapping the suffix — `with_suffix(".svelte.ts")`
|
||||
# would replace `.svelte` with `.svelte.ts`, but `with_suffix` only
|
||||
# replaces the final segment.
|
||||
c = p.parent / (p.name + ".ts")
|
||||
if c.is_file():
|
||||
return c
|
||||
return p
|
||||
|
||||
|
||||
def _read_text(node, source: bytes) -> str:
|
||||
return source[node.start_byte:node.end_byte].decode("utf-8", errors="replace")
|
||||
|
||||
@@ -275,11 +341,9 @@ def _import_js(node, source: bytes, file_nid: str, stem: str, edges: list, str_p
|
||||
# Relative import - resolve to full path so IDs match file node IDs
|
||||
# normpath removes ".." segments so the ID matches the target file's own node ID
|
||||
resolved = Path(os.path.normpath(Path(str_path).parent / raw))
|
||||
# TypeScript ESM: imports written as .js but actual file is .ts/.tsx
|
||||
if resolved.suffix == ".js":
|
||||
resolved = resolved.with_suffix(".ts")
|
||||
elif resolved.suffix == ".jsx":
|
||||
resolved = resolved.with_suffix(".tsx")
|
||||
# TS / SvelteKit resolver: try .ts/.tsx/.svelte/.svelte.ts/index.{ts,…}
|
||||
# so bare-path and Svelte-5-rune imports land on the right node id (#716)
|
||||
resolved = _resolve_with_extensions(resolved)
|
||||
tgt_nid = _make_id(str(resolved))
|
||||
resolved_path = resolved
|
||||
else:
|
||||
@@ -292,6 +356,9 @@ def _import_js(node, source: bytes, file_nid: str, stem: str, edges: list, str_p
|
||||
resolved_alias = Path(os.path.normpath(Path(alias_base) / rest))
|
||||
break
|
||||
if resolved_alias is not None:
|
||||
# Same resolver fixups as the relative branch — alias targets
|
||||
# are equally likely to be bare paths / .svelte.ts / index.ts (#716)
|
||||
resolved_alias = _resolve_with_extensions(resolved_alias)
|
||||
tgt_nid = _make_id(str(resolved_alias))
|
||||
resolved_path = resolved_alias
|
||||
else:
|
||||
@@ -1761,11 +1828,10 @@ def extract_svelte(path: Path) -> dict:
|
||||
if raw.startswith("."):
|
||||
# Relative import - resolve to full path so IDs match file node IDs.
|
||||
resolved = Path(os.path.normpath(path.parent / raw))
|
||||
# TypeScript ESM: imports written as .js but actual file is .ts/.tsx
|
||||
if resolved.suffix == ".js":
|
||||
resolved = resolved.with_suffix(".ts")
|
||||
elif resolved.suffix == ".jsx":
|
||||
resolved = resolved.with_suffix(".tsx")
|
||||
# Apply same TS/Svelte resolver fixups as static imports so dynamic
|
||||
# imports of bare paths and .svelte.ts rune files land on real
|
||||
# file nodes instead of phantom ids (#716).
|
||||
resolved = _resolve_with_extensions(resolved)
|
||||
node_id = _make_id(str(resolved))
|
||||
else:
|
||||
# Check tsconfig.json path aliases (e.g. "$lib/" -> "src/lib/", "@/" -> "src/")
|
||||
@@ -1778,6 +1844,7 @@ def extract_svelte(path: Path) -> dict:
|
||||
resolved_alias = Path(os.path.normpath(Path(alias_base) / rest))
|
||||
break
|
||||
if resolved_alias is not None:
|
||||
resolved_alias = _resolve_with_extensions(resolved_alias)
|
||||
node_id = _make_id(str(resolved_alias))
|
||||
else:
|
||||
# Bare/scoped import (node_modules) - use last segment;
|
||||
|
||||
@@ -0,0 +1,258 @@
|
||||
"""Tests for #716 — TypeScript bare-path imports, Svelte 5 rune file imports
|
||||
(`from './foo.svelte'` for a `.svelte.ts` file), and directory/index.ts
|
||||
imports must resolve to the actual file's node id, not a phantom.
|
||||
|
||||
Before #716, `_import_js` only rewrote `.js → .ts` and `.jsx → .tsx`. Every
|
||||
other shape (bare path, `.svelte → .svelte.ts`, `./foo` directory imports)
|
||||
produced an id like `..._foo` while the real file's node id was `..._foo_ts`,
|
||||
so `build_from_json` dropped the edge as external.
|
||||
"""
|
||||
|
||||
from pathlib import Path
|
||||
|
||||
from graphify.extract import (
|
||||
_make_id,
|
||||
_resolve_with_extensions,
|
||||
extract_js,
|
||||
extract_svelte,
|
||||
)
|
||||
|
||||
|
||||
def _write(path: Path, body: str) -> Path:
|
||||
path.parent.mkdir(parents=True, exist_ok=True)
|
||||
path.write_text(body, encoding="utf-8")
|
||||
return path
|
||||
|
||||
|
||||
def _import_targets(result: dict) -> set[str]:
|
||||
return {str(e.get("target") or "") for e in result["edges"]
|
||||
if e.get("relation") in ("imports", "imports_from")}
|
||||
|
||||
|
||||
# ── _resolve_with_extensions unit tests ──────────────────────────────────────
|
||||
|
||||
|
||||
def test_resolve_returns_existing_path_unchanged(tmp_path):
|
||||
p = _write(tmp_path / "foo.ts", "export const x = 1")
|
||||
assert _resolve_with_extensions(p) == p
|
||||
|
||||
|
||||
def test_resolve_bare_path_to_ts(tmp_path):
|
||||
target = _write(tmp_path / "foo.ts", "export const x = 1")
|
||||
bare = tmp_path / "foo"
|
||||
assert _resolve_with_extensions(bare) == target
|
||||
|
||||
|
||||
def test_resolve_bare_path_to_tsx(tmp_path):
|
||||
target = _write(tmp_path / "Component.tsx", "export const x = 1")
|
||||
bare = tmp_path / "Component"
|
||||
assert _resolve_with_extensions(bare) == target
|
||||
|
||||
|
||||
def test_resolve_bare_path_to_svelte(tmp_path):
|
||||
target = _write(tmp_path / "Card.svelte", "<div></div>")
|
||||
bare = tmp_path / "Card"
|
||||
assert _resolve_with_extensions(bare) == target
|
||||
|
||||
|
||||
def test_resolve_prefers_ts_over_svelte_when_both_exist(tmp_path):
|
||||
"""Vite resolver order: .ts wins over .svelte for ambiguous bare paths."""
|
||||
ts_target = _write(tmp_path / "foo.ts", "export const x = 1")
|
||||
_write(tmp_path / "foo.svelte", "<div></div>")
|
||||
bare = tmp_path / "foo"
|
||||
assert _resolve_with_extensions(bare) == ts_target
|
||||
|
||||
|
||||
def test_resolve_directory_to_index_ts(tmp_path):
|
||||
pkg = tmp_path / "queue"
|
||||
target = _write(pkg / "index.ts", "export const x = 1")
|
||||
assert _resolve_with_extensions(pkg) == target
|
||||
|
||||
|
||||
def test_resolve_directory_prefers_index_ts_over_index_js(tmp_path):
|
||||
pkg = tmp_path / "queue"
|
||||
target = _write(pkg / "index.ts", "export const x = 1")
|
||||
_write(pkg / "index.js", "module.exports = {}")
|
||||
assert _resolve_with_extensions(pkg) == target
|
||||
|
||||
|
||||
def test_resolve_svelte_to_svelte_ts_for_rune_files(tmp_path):
|
||||
"""Svelte 5: `from './foo.svelte'` may actually point at `foo.svelte.ts`
|
||||
(a rune-only TypeScript file with no .svelte file). The resolver must
|
||||
APPEND .ts to the full filename, not swap suffixes."""
|
||||
target = _write(tmp_path / "is-mobile.svelte.ts",
|
||||
"export const isMobile = () => true")
|
||||
written_as = tmp_path / "is-mobile.svelte"
|
||||
resolved = _resolve_with_extensions(written_as)
|
||||
assert resolved == target, (
|
||||
f"Expected resolution to is-mobile.svelte.ts; got {resolved}"
|
||||
)
|
||||
|
||||
|
||||
def test_resolve_js_to_ts_when_real_file_is_ts(tmp_path):
|
||||
"""TS ESM convention: imports written as .js but the actual file is .ts."""
|
||||
target = _write(tmp_path / "foo.ts", "export const x = 1")
|
||||
written_as = tmp_path / "foo.js"
|
||||
assert _resolve_with_extensions(written_as) == target
|
||||
|
||||
|
||||
def test_resolve_jsx_to_tsx_when_real_file_is_tsx(tmp_path):
|
||||
target = _write(tmp_path / "Component.tsx", "export const x = 1")
|
||||
written_as = tmp_path / "Component.jsx"
|
||||
assert _resolve_with_extensions(written_as) == target
|
||||
|
||||
|
||||
def test_resolve_returns_unchanged_when_nothing_matches(tmp_path):
|
||||
"""External / truly missing paths fall back to the input — preserves
|
||||
pre-#716 behavior of becoming an external phantom edge."""
|
||||
nothing = tmp_path / "does_not_exist"
|
||||
assert _resolve_with_extensions(nothing) == nothing
|
||||
|
||||
|
||||
def test_resolve_real_js_stays_js_when_ts_does_not_exist(tmp_path):
|
||||
"""If `.js` exists and `.ts` does not, keep the `.js` rewrite from
|
||||
triggering — return the existing file."""
|
||||
target = _write(tmp_path / "foo.js", "module.exports = 1")
|
||||
assert _resolve_with_extensions(target) == target
|
||||
|
||||
|
||||
# ── End-to-end: bare-path imports in pure TS files ───────────────────────────
|
||||
|
||||
|
||||
def test_bare_path_import_resolves_in_ts_file(tmp_path):
|
||||
"""The #716 reproducer: TS file imports a sibling without an extension."""
|
||||
target = _write(tmp_path / "type-helpers.ts",
|
||||
"export type GetNestedType<T> = T")
|
||||
importer = _write(tmp_path / "page.ts",
|
||||
"import type { GetNestedType } from './type-helpers'\n")
|
||||
result = extract_js(importer)
|
||||
expected = _make_id(str(target))
|
||||
assert expected in _import_targets(result), (
|
||||
f"Bare-path .ts import must resolve to target node id; "
|
||||
f"expected {expected}; got {_import_targets(result)}"
|
||||
)
|
||||
|
||||
|
||||
def test_directory_import_resolves_to_index_ts(tmp_path):
|
||||
"""`from './queue'` must resolve to `./queue/index.ts`."""
|
||||
target = _write(tmp_path / "queue" / "index.ts",
|
||||
"export const enqueue = () => {}")
|
||||
importer = _write(tmp_path / "page.ts",
|
||||
"import { enqueue } from './queue'\n")
|
||||
result = extract_js(importer)
|
||||
expected = _make_id(str(target))
|
||||
assert expected in _import_targets(result), (
|
||||
f"Directory import must resolve to ./queue/index.ts; "
|
||||
f"expected {expected}; got {_import_targets(result)}"
|
||||
)
|
||||
|
||||
|
||||
# ── End-to-end: .svelte → .svelte.ts (Svelte 5 rune files) ───────────────────
|
||||
|
||||
|
||||
def test_dot_svelte_import_resolves_to_dot_svelte_ts(tmp_path):
|
||||
"""Svelte 5 rune file: import written as .svelte, real file is .svelte.ts."""
|
||||
target = _write(tmp_path / "is-mobile.svelte.ts",
|
||||
"export const isMobile = () => true")
|
||||
importer = _write(tmp_path / "page.ts",
|
||||
"import { isMobile } from './is-mobile.svelte'\n")
|
||||
result = extract_js(importer)
|
||||
expected = _make_id(str(target))
|
||||
assert expected in _import_targets(result), (
|
||||
f".svelte → .svelte.ts resolution failed; "
|
||||
f"expected {expected}; got {_import_targets(result)}"
|
||||
)
|
||||
|
||||
|
||||
# ── Regression guards: existing behavior preserved ───────────────────────────
|
||||
|
||||
|
||||
def test_explicit_ts_import_still_works(tmp_path):
|
||||
"""The most common case — import with explicit .ts extension — must
|
||||
continue to work after the resolver change."""
|
||||
target = _write(tmp_path / "foo.ts", "export const x = 1")
|
||||
importer = _write(tmp_path / "page.ts",
|
||||
"import { x } from './foo.ts'\n")
|
||||
result = extract_js(importer)
|
||||
expected = _make_id(str(target))
|
||||
assert expected in _import_targets(result), (
|
||||
f"Explicit .ts imports must still resolve; "
|
||||
f"expected {expected}; got {_import_targets(result)}"
|
||||
)
|
||||
|
||||
|
||||
def test_explicit_svelte_import_still_works(tmp_path):
|
||||
"""Real .svelte file imports must still resolve when the .svelte file
|
||||
exists (i.e. don't accidentally redirect to a non-existent .svelte.ts)."""
|
||||
target = _write(tmp_path / "Card.svelte", "<div></div>")
|
||||
importer = _write(tmp_path / "page.ts",
|
||||
"import Card from './Card.svelte'\n")
|
||||
result = extract_js(importer)
|
||||
expected = _make_id(str(target))
|
||||
assert expected in _import_targets(result), (
|
||||
f"Existing .svelte imports must resolve to the .svelte node, "
|
||||
f"not get redirected; expected {expected}; "
|
||||
f"got {_import_targets(result)}"
|
||||
)
|
||||
|
||||
|
||||
def test_external_module_unchanged(tmp_path):
|
||||
"""Bare module specifiers (no leading dot, no alias match) must still
|
||||
fall through to the external/last-segment path — don't accidentally
|
||||
treat 'lodash' as a relative path."""
|
||||
importer = _write(tmp_path / "page.ts",
|
||||
"import _ from 'lodash-es'\n")
|
||||
result = extract_js(importer)
|
||||
targets = _import_targets(result)
|
||||
# The target should be the bare module name, not a resolved file path
|
||||
assert "lodash_es" in targets or any("lodash" in t for t in targets), (
|
||||
f"External module specifier should still produce an external "
|
||||
f"reference; got {targets}"
|
||||
)
|
||||
|
||||
|
||||
# ── End-to-end: alias-resolved imports go through the same resolver ─────────
|
||||
|
||||
|
||||
def test_alias_import_with_bare_path_resolves(tmp_path):
|
||||
"""`$lib/foo` (alias + bare path) — both layers must work together."""
|
||||
src = tmp_path / "src"
|
||||
target = _write(src / "lib" / "type-helpers.ts",
|
||||
"export type X = string")
|
||||
_write(tmp_path / "tsconfig.json",
|
||||
'{"compilerOptions":{"paths":{"$lib":["./src/lib"],'
|
||||
'"$lib/*":["./src/lib/*"]}}}')
|
||||
importer_dir = src / "routes"
|
||||
importer = _write(importer_dir / "page.ts",
|
||||
"import type { X } from '$lib/type-helpers'\n")
|
||||
result = extract_js(importer)
|
||||
expected = _make_id(str(target))
|
||||
assert expected in _import_targets(result), (
|
||||
f"Alias + bare-path resolution failed; "
|
||||
f"expected {expected}; got {_import_targets(result)}"
|
||||
)
|
||||
|
||||
|
||||
# ── End-to-end: dynamic_import in .svelte regex pass uses resolver ──────────
|
||||
|
||||
|
||||
def test_dynamic_import_bare_path_resolves(tmp_path):
|
||||
"""The regex pass for `import('...')` in .svelte files must also use
|
||||
the new resolver — otherwise dynamic imports of bare paths still
|
||||
produce phantom edges."""
|
||||
target = _write(tmp_path / "Heavy.svelte.ts",
|
||||
"export const heavy = () => 1")
|
||||
importer = _write(tmp_path / "page.svelte", """\
|
||||
<script>
|
||||
const lazy = () => import('./Heavy.svelte')
|
||||
</script>
|
||||
""")
|
||||
result = extract_svelte(importer)
|
||||
dyn_targets = {str(e.get("target") or "") for e in result["edges"]
|
||||
if e.get("relation") == "dynamic_import"}
|
||||
expected = _make_id(str(target))
|
||||
assert expected in dyn_targets, (
|
||||
f"dynamic_import of .svelte that's actually .svelte.ts must "
|
||||
f"resolve through the new resolver; "
|
||||
f"expected {expected}; got {dyn_targets}"
|
||||
)
|
||||
Reference in New Issue
Block a user