Repository navigation
fix(js): resolve split export { .. } from clauses and a barrel's default as X component re-export - #4630
fix(js): resolve split export { .. } from clauses and a barrel's default as X component re-export#4630if12is wants to merge 2 commits into
export { .. } from clauses and a barrel's default as X component re-export#4630Conversation
…t line
tree-sitter ends the statement at the closing brace of
'export {\n default as X\n}\nfrom "./X.vue"', so the module string is never
seen and the barrel gets no edges. Move the clause up byte-for-byte.
…to the component
import { Alert } from '@/ui/alert' where the barrel does
export { default as Alert } from './Alert.vue' left the imports edge on a
nonexistent symbol id. A single-file component's default export is the file,
so the edge now targets the .vue/.svelte file node when the barrel exports
that name from exactly one module. The split-from normalisation also feeds the
symbol-fact parse so multi-line barrels take part.
There was a problem hiding this comment.
Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.
An organization owner can choose the repository organization as the billing source for reviews requested by members with a paid Copilot license. Learn about Copilot code review billing.
|
Thanks for the pull request, @if12is. A maintainer will review it soon. Want to talk it through while it is in review? Come join us on our Discord server. For longer-form discussion there is also GitHub Discussions. A couple of things that speed up review: make sure the test suite passes on Python 3.10 and 3.13, and that the change keeps extraction deterministic. |
![graphify-labs[bot]](https://avatars.githubusercontent.com/in/4356270?s=60&v=4){kind=small}
There was a problem hiding this comment.
Graphify reviewed this change.
No issues found by static checks — no coupling regressions or blocking issues in the code graph. That is not the same as safe to merge: see what was not checked.
Not checked: tests were not run; formal verification proved 0 of 4 changed function(s) (2 sampled, not proven, 2 not verified).
Formal verification. PR-changed functions: 2/4 verified (0 proven, 2 may-equivalent, 0 distinguished) · 2 not verified (1 vacuous, 1 unsupported).
Not verified on this run: extract\_js (vacuous: never exercised), \_apply\_symbol\_resolution\_facts (unsupported).
Graphify review — findings
Adds parsing for JS/TS/Vue export { ... } and import { ... } clauses whose from '...' sits on the line after the closing }, so barrel files using that layout now get their import/re-export edges. _normalize_js_split_from_clause rewrites the source before parsing, moving the clause up onto the } line while keeping the byte length and line count unchanged. It runs in extract_js (now for plain JS too, not just TS), extract_vue, and _parse_js_tree. Also resolves a barrel's export { default as X } from './X.vue' (or .svelte) to the single-file component's file node, since no symbol is named default. If two re-exports of the same name make it ambiguous, the import is left unresolved. Bumps _AST_CACHE_SCHEMA to 21 so cached JS/TS/Vue extractions re-run.
No blocking issues surfaced. 1 lower-confidence candidate did not survive cross-model review.
Analysis details — impact, health, verification
Impact & health
Graphify review
Impact — 3861 functions depend on the 599 functions this change touches.
Health — this change adds coupling hotspots:
- worse:
extract()— 991 callers, 59 callees - worse:
extract_js()— 95 callers, 8 callees - worse:
extract_vue()— 10 callers, 9 callees - worse:
_apply_symbol_resolution_facts()— 5 callers, 11 callees - worse:
_parse_js_tree()— 8 callers, 5 callees
Verification — 3861 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: 3610 function(s) in the blast radius were not formally verified this run
Test selection
Test selection
231 of 438 test file(s) selected (53%) via static blast radius.
tests/test_affected_overrides.py— impacttests/test_astro_extraction.py— impacttests/test_astro_import_ids.py— impacttests/test_blade_extractor.py— impacttests/test_build.py— impacttests/test_builtin_global_type_refs.py— impacttests/test_c_include_resolution.py— impacttests/test_cache.py— impacttests/test_cache_stale_import_target.py— impacttests/test_case_sensitive_resolution.py— impacttests/test_charmap_encoding.py— impacttests/test_chunking.py— impacttests/test_cjs_module_extension.py— impacttests/test_cobol_extractor.py— impacttests/test_cpp_method_declarations.py— impacttests/test_cpp_nested_and_cli.py— impacttests/test_cpp_objc_cross_file_calls.py— impacttests/test_cpp_template_type_params.py— impacttests/test_cross_extension_reexport_self_cycle.py— impacttests/test_cross_language_call_resolution.py— impacttests/test_cross_repo_external_call_guards.py— impacttests/test_cross_repo_member_calls.py— impacttests/test_csharp_call_site_generic_args.py— impacttests/test_csharp_delegate_declarations.py— impacttests/test_csharp_enum_members.py— impacttests/test_csharp_field_generic_args.py— impacttests/test_csharp_foreach_loop_variable.py— impacttests/test_csharp_generic_callsites.py— impacttests/test_csharp_interface_dispatch.py— impacttests/test_csharp_member_calls.py— impacttests/test_csharp_member_chain_calls.py— impacttests/test_csharp_member_nodes.py— impacttests/test_csharp_object_creation.py— impacttests/test_csharp_partial_classes.py— impacttests/test_csharp_tuple_type_refs.py— impacttests/test_csharp_type_resolution.py— impacttests/test_csharp_underscore_collisions.py— impacttests/test_csharp_var_call_return_type.py— impacttests/test_csharp_verbatim_interpolation.py— impacttests/test_definition_file_portability.py— impacttests/test_detect.py— impacttests/test_dotnet.py— impacttests/test_duplicate_annotation_edges.py— impacttests/test_ejs_extractor.py— impacttests/test_elixir_defdelegate.py— impacttests/test_elixir_import_resolution.py— impacttests/test_elixir_keyword_def_calls.py— impacttests/test_elixir_qualified_calls.py— impacttests/test_elixir_unqualified_call_scope.py— impacttests/test_erlang_extractor.py— impact- … and 181 more
Selection is safe under the controlled-regression assumption; always-run tests + a periodic full run are the backstops. Advisory — it never changes the check verdict.
Formal verification
Could not verify: Could not verify extract\_js.
The verifier did not have enough to check extract\_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: no capturable inputs from the test suite; property tier: not verifiable: all 9 sampled inputs raised on both versions — the function never executed, so 'no divergence' would be vacuous (mostly NameError — names the real obstacle, not a sampling gap)
No difference found (not proven): No behavior difference found in extract\_vue (not a proof).
The verifier ran both versions of extract\_vue 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 \_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: no capturable inputs from the test suite; property tier: parameter `facts` is annotated `_SymbolResolutionFacts` — outside the synthesizable primitive/collection set
No difference found (not proven): No behavior difference found in \_parse\_js\_tree (not a proof).
The verifier ran both versions of \_parse\_js\_tree 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.
· 2 grounded finding(s) anchored inline below; 3 more finding(s) on lines outside this diff (see the check run).
| _SFC_SUFFIXES = frozenset({".vue", ".svelte"}) | ||
|
|
||
|
|
||
| def _apply_symbol_resolution_facts( |
There was a problem hiding this comment.
_apply_symbol_resolution_facts()
fans out to 11 callees (efferent coupling).
Grounded coupling-delta finding (deterministic), not an LLM guess.
There was a problem hiding this comment.
_parse_js_tree()
8 callers depend on it (afferent coupling).
Grounded coupling-delta finding (deterministic), not an LLM guess.
Branch:
upstream/js-barrel-default-reexport(base:upstream/v8at b90b9df). Two commits, oneconcern: a barrel file that re-exports single-file components must give its importers real edges.
I found no existing issue for this; related to the earlier barrel work around #2034 (ambiguous
default asre-exports) which this keeps intact.Problem
A component barrel such as
resources/js/ui/alert/index.js:has two defects.
}on one line,from '...'on the next) is valid JavaScript, buttree-sitter-javascript and tree-sitter-typescript end the statement at the
}: the clause becomestwo stray expression statements, so the module string is never seen and the barrel gets no
import / re-export edge at all. The same applies to
import { .. }\nfrom '...'.import { Alert } from '@/ui/alert', the importer's edge targeted a symbol id inside the barrelthat no real node has, so the
importsedge ended on a sourceless placeholder node. A single-file component's defaultexport is the file itself, so the edge should land on the
.vue/.sveltefile node.Implementation
graphify/extractors/resolution.py: new_normalize_js_split_from_clause, which moves afrom '...'clause that starts the line after a line holding only
}up onto the}line, byte for byte thesame length and with the same line count, so offsets and line numbers hold (
from 'x'is a syntaxerror anywhere else, so the pattern cannot match anything but this statement). The JS symbol-fact
parse applies it as well, so multi-line barrels take part in symbol resolution.
graphify/extract.py: the JS/TS parse path imports it and applies it before parsing (alongside theexisting TS
export type *normalisation) and when masking source.graphify/extractors/resolution.py(origin_node_id): adefaultexport of a.vue/.sveltefile resolves to that file's node. A named import through a barrel resolves there only when the
barrel exports that name from exactly one module; two
default as Alertre-exports stay ambiguousand the import is left unresolved.
graphify/cache.py: AST cache schema 20 -> 21 (cached JS/TS/Vue extractions lack the barrel edges).The first commit is the clause normalisation (with the schema bump); the second adds the component
resolution and moves the normaliser so the symbol-fact parse can share it.
Verification
New tests:
tests/test_js_split_export_from.py(3 tests: multi-line export clause withfromon itsown line; the split form keeps the symbol re-export; the same for an import clause) and
tests/test_js_barrel_default_reexport.py(2 tests: a named import through the barrel targets thecomponent file; an ambiguous default name is not guessed).
Fails before, passes after (unmodified
graphify/ofupstream/v8):Full checks on this branch:
Bare
upstream/v8(b90b9df) gives1 failed, 8469 passed, 15 skipped, ruff clean and the same641 errorsfrom pyright. The one failure is pre-existing and identical onv8:tests/test_skill_no_backtick_substitution.py::test_control_a_backticked_unknown_command_is_caught.Before/after on two real Laravel + Vue applications (private copies,
uv run graphify update <copy> --forcefrom this branch and from bareupstream/v8):imports/re_exportsedges that land on a real targetre_exportsedgesIn app A, 1432
importsedges changed target (from a sourceless placeholder node named after thebarrel symbol, e.g.
index.alert, to the.vuefile node) and 27re_exportsedges were added;the 79 nodes that disappeared are those placeholder nodes. App B has no such barrels, and its graph is unchanged. Edges whose endpoint is not a node: 0
before and after in both.
Limitations
}newlinefrom '...'split is normalised; other unusual line breaks inside animport/export statement are not.
.vueand.sveltefiles; other default exports (a default-exportedfunction or class in a
.jsfile) are unchanged.