fix(ts): type-only imports no longer manufacture Import Cycles (#3123) - #3172
fix(ts): type-only imports no longer manufacture Import Cycles (#3123)#3172abhay-codes07 wants to merge 1 commit into
Conversation
…ify-Labs#3123) `import type { X } from` and `export type { X } from` are erased by the TypeScript compiler - no runtime emit, no module-graph edge - yet the extractor emitted ordinary imports_from/re_exports edges for them, and every one of the reporter's Import Cycles (3 of 3 on a ~7,900-node repo) closed only through such edges. The report even flagged code whose comment said the `export type` form was chosen for "zero runtime emit"; the mitigation was reported as the defect. The edges are kept - the type dependency is real for "what references this type" - and stamped `type_only`, at both producers: _import_js (the module edge and the symbol-level re-exports) and the module-resolution facts pass (the export facts carry a defaulted type_only field through to add_edge). find_import_cycles excludes stamped edges exactly the way it already excludes deferred `import(...)` (Graphify-Labs#1241). The statement-level `type` keyword is a bare anonymous child in the grammar, which keeps the two look-alikes straight: `import type from './x'` (a default binding NAMED type) has the keyword inside the import_clause and stays runtime, and a mixed `import { type B, C }` stays runtime because C is a runtime import.
There was a problem hiding this comment.
Graphify reviewed this change.
Worth a look — the grounded gate found no coupling regressions or blocking issues, but 2 advisory finding(s) below merit a look before merge.
Formal verification. 1 change(s) tested, no difference found (not proven).
Graphify review — findings
Excludes type-only imports and re-exports (import type / export type … from) from import-cycle detection, since they are erased at compile time and can't form a runtime cycle. Such edges are stamped with type_only in the JS/TS extractor rather than dropped—so they still count for "what references this type"—and find_import_cycles skips them the same way it skips deferred dynamic import(...). Distinguishes the erased forms from a default binding literally named type and from mixed specifier lists like import { type B, C }, which keep their runtime edges.
Worth a look
- find_import_cycles now silently omits import cycles when an edge has type_only —
graphify/analyze.py:685· Escalate · medium- agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review
- Edge dedupe ignores type_only —
graphify/extractors/resolution.py:877· Escalate · medium- agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review
Analysis details — impact, health, verification
Impact & health
Graphify review
Impact — 2179 functions depend on the 397 functions this change touches.
Health — this change adds coupling hotspots:
- new:
extract()— 502 callers, 42 callees - new:
_rebuild_code()— 98 callers, 50 callees - new:
to_obsidian()— 36 callers, 13 callees - new:
_extract_generic()— 18 callers, 24 callees - new:
to_json()— 54 callers, 7 callees - new:
extract_xaml()— 19 callers, 17 callees - new:
extract_js()— 85 callers, 3 callees - new:
dispatch_command()— 2 callers, 122 callees - …and 60 more — each is listed as a finding
Verification — 2179 functions in the blast radius were not formally verified this run (proofs are advisory here).
Gate & verification
graphify gate
PASS — objectively clean (no health regressions, tests not run — proofs not run this pass (advisory)). Grounded, not self-assessed.
Advisory (not blocking):
- verification_scope: 1983 function(s) in the blast radius were not formally verified this run
Formal verification
No difference found (not proven): No behavior difference found in find\_import\_cycles (not a proof).
The verifier ran both versions of find\_import\_cycles on many inputs and saw identical behavior every time. Strong evidence the change is safe, but evidence, not a proof.
Guarantee: Empirical: differential testing (both versions run on many generated inputs). A divergence on an untested input remains possible, so this is 'no counterexample found', not 'proven equivalent'.
Note: An input the sampler did not try could still differ.
Could not verify: Could not verify \_import\_js.
The verifier did not have enough to check \_import\_js, so it is saying so rather than guessing. No false assurance is the whole point.
Guarantee: No guarantee either way, this is an honest abstention, not a pass.
Note: Reason: not verifiable: all 200 sampled inputs raised on both versions — the function never executed, so 'no divergence' would be vacuous (mostly AttributeError — names the real obstacle, not a sampling gap)
Could not verify: Could not verify \_apply\_symbol\_resolution\_facts.
The verifier did not have enough to check \_apply\_symbol\_resolution\_facts, so it is saying so rather than guessing. No false assurance is the whole point.
Guarantee: No guarantee either way, this is an honest abstention, not a pass.
Note: Reason: parameter `root` is annotated `Path` — outside the synthesizable primitive/collection set
Could not verify: Could not verify \_collect\_js\_symbol\_resolution\_facts.
The verifier did not have enough to check \_collect\_js\_symbol\_resolution\_facts, so it is saying so rather than guessing. No false assurance is the whole point.
Guarantee: No guarantee either way, this is an honest abstention, not a pass.
Note: Reason: parameter `facts` is annotated `_SymbolResolutionFacts` — outside the synthesizable primitive/collection set
· 68 more finding(s) on lines outside this diff (see the check run).
Closes #3123.
The problem
import type { X } fromandexport type { X } fromare erased by the TypeScript compiler — no runtime emit, no module-graph edge — yet the extractor emitted ordinaryimports_from/re_exportsedges for them. On the reporter's ~7,900-node repo, all 3 reported Import Cycles closed only through such edges, and one of the flagged files carried a comment saying theexport typeform was chosen precisely for "zero runtime emit — no module graph cost". The mitigation was reported as the defect, in a headline section ofGRAPH_REPORT.md.The change
The issue's own suggestion: tag, don't drop. The type dependency is real and useful for "what references this type", so the edges stay in the graph, stamped
type_only, andfind_import_cyclesexcludes them exactly the way it already excludes deferredimport(...)edges (#1241 established the pattern one line above).Both producers stamp:
_import_js— the module-levelimports_fromedge and the symbol-level re-exports of a type-only statement;type_onlyfield, carried from the export-statement walk through toadd_edge, so the file-levelre_exportsedges it synthesizes agree with the extractor's. The facts still participate in symbol resolution unchanged: a type import resolved through a type-only barrel is itself type-only.Precision comes from the grammar: the statement-level
typeis a bare anonymous child, which keeps the two look-alikes straight —import type from './x'(a default binding namedtype, erased by nothing) has the word inside theimport_clauseand stays runtime, and a mixedimport { type B, C }stays runtime becauseCis a runtime import.Tests
tests/test_type_only_import_cycles.py— 8 tests: the stamp onimport typeand onexport type … from(all three edges, including the resolution pass's), the default-binding-named-typeand mixed-specifier look-alikes staying runtime, plain imports carrying no stamp at all; and at cycle level the reporter's exact two-file repro producing no cycle, a genuine runtime cycle still reported, and a mixed one-runtime-leg/one-type-leg pair producing none. With the fix reverted, 4 of 8 fail. The JS/TS, re-export/barrel, analyze and cycle suites are unchanged (244 passed); the full suite matches the freshv8(0.9.51) baseline.