Repository navigation
fix(parsers): keep JSX members of bare package imports external when the package name matches a project path - #2806
Conversation
…the package name matches a project path (#2535)
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configuration
📒 Files selected for processing (3)
Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 7 remain after this review. 📝 WalkthroughWalkthroughJS/TS import resolution now distinguishes package imports from project-file imports, including relative, workspace, tsconfig alias, and baseUrl resolutions. The classification is tracked through import and require bindings and used to identify external member-call heads. New tests cover package-name collisions and related resolutions. ChangesJS/TS Import Classification
Priority: ⬇️ Low Estimated code review effort: 3 (Moderate) | ~20 minutes Change: Bug fix Suggested reviewers: Merge Risk: ⚪ Minimal · up to This change keeps JSX members of bare package imports external when the package name matches a project path. No outstanding merge-blocking issues were found. The full unit and integration suites were not reported as run, which is normal residual uncertainty. Security Architecture ReviewSecurity architecture risk: 🔵 Low · up to The change corrects how package references are distinguished from project code. No new execution or access-control path was identified. Confidence is limited by incomplete evidence about interrupted updates and concurrent reuse. Retained concerns Security review detailsSecurity Blast Radius
Trust Boundaries and Controls
Resilience and Maintainability Implications
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
|
@greptileai please review the latest commit (2d8b1c6). Generated by Claude Code |
|
@greptileai please review the latest commit (07ea445). Generated by Claude Code |
|
@greptileai please review the latest commit (22ce459). Generated by Claude Code |
|



Summary
<React.Fragment />underimport * as React from "react"no longer gets aREFERENCESedge to an unrelated localFragmentwhen the project itself is namedreact, or when a repo-rootreact.tsxexists. The same holds for default, named and subpath imports (react/jsx-runtime) and forrequire("react").CallResolver.js_member_head_is_external(added in fix(parsers): resolve JSX member tags of external namespace imports as external #2577) decided whether the tag's head was external by comparing the head's import target qn with project paths. A bare specifier resolves to the specifier itself (react). That qn has the same root as a project namedreact, and once project-prefixed it is the same path as a repo-rootreact.tsx(proj.react). Both checks then called the package first-party, and the tag fell back to the bare-name trie._resolve_js_import_targetreturns the module qn together with anis_packageflag (JsImportTarget). Local names bound by a package import orrequirego into a new per-modulejs_ts_package_importsset, which is cleared on re-parse likejs_ts_bare_imports.js_member_head_is_externalnow checks only that set and never compares the qn with project paths. Relative, tsconfig-alias, workspace and aliased-scheme (denoext:) imports are never flagged, so they keep today's resolution.The parser fingerprint covers
parsers/, so an existing graph re-parses once after upgrading.Demo
cgr --project-name react, before vs after: <React.Fragment/> no longer links to a local Fragment
Animated runs
cgr --project-name react, before vs after: <React.Fragment/> no longer links to a local Fragment, animated
The screenshot comes from a real
cgr start --update-graph --project-name reactrun against a fresh Memgraph 3.3.0. The repo hassrc/App.tsx(<React.Fragment>from"react"wrapping<Local.Card />from./widgets) and an unrelatedsrc/fragment.tsxthat exportsFragment. On main,AppgetsREFERENCES ... exacttoreact.src.fragment.Fragmentand toreact.src.widgets.Card. On this branch only theCardedge remains, stillexact.Type of Change
Related Issues
Follow-up to #2577 (#2535). Refs #2535
Greptile's finding at
codebase_rag/parsers/call_resolver.py:2000-2004on main, verified with a T-Rex reproduction ("the import-root equality check returns False, which lets JSX path fallback to the local symbol"):A variant from another run:
Test Plan
make test-paralleloruv run pytest -n auto -m "not integration"): CI runs the full suite on every push. Locally I ran a targeted scope of 26 files, 353 passed: the new file,test_jsx_external_namespace_member.py,test_jsx_component_references.py,test_tsx_jsx_calls.py,test_ts_jsx_ternary_handler_ref.py,test_import_parsing.py,test_javascript_path_resolution.py,test_javascript_imports.py,test_javascript_modules.py,test_js_ts_module_system.py,test_js_ts_module_system_unit.py,test_js_ts_module_extension_parity.py,test_js_module_extension_variants.py,test_js_ts_aliased_import_calls.py,test_ts_tsconfig_paths.py,test_ts_workspace_package_imports.py,test_ts_workspace_require_resolution.py,test_js_type_inference_unit.py,test_js_builtin_call_no_phantom.py,test_js_untyped_receiver_member_call.py,test_js_commonjs_direct_function_export_call.py,test_io_access_js_imports.py,test_typescript_namespaces_modules.py,test_dead_code_js_symbol_roots.py,test_slots_and_optimizations.pyandtest_slots_lazy_logger.py.codebase_rag/tests/test_jsx_package_name_collision.pymake test-integration, requires Docker)cgr start --update-graph+ Cypher run in the DemoRed tests. All six failed on main with a
REFERENCES ... exactedge to the localFragment:test_package_named_like_the_project_binds_no_local_symbol[namespace-import],[subpath-import],[default-import],[require]: projectreact, edge toreact.src.frag.Fragmenttest_package_named_like_a_local_module_binds_no_local_symbol[namespace-import],[named-import]: repo-rootreact.tsx, edge toproj.react.FragmentNegative tests. These pass on main and on this branch:
test_relative_import_of_same_named_module_still_links[project-named-react]and[root-react-module]:import * as R from "./react"still resolves<R.Fragment />exactto the local moduletest_workspace_package_import_still_links: a first-party workspace package that is itself namedreactis resolved by the import processor and still linkstest_tsconfig_alias_named_like_the_project_still_links: areact/*tsconfig alias in a project namedreactstill linkstest_jsx_external_namespace_member.py(fix(parsers): resolve JSX member tags of external namespace imports as external #2577) are unchanged and green.ruff check,ruff formatandty checkare clean, and the pre-commit hooks pass.Merge check (
git merge-tree --write-tree): #2743 and #2790 merge cleanly. #2625 has a small textual conflict in_resolve_js_module_path: it rewrites the relative-path tail, and this PR wraps that function's return values inJsImportTarget.Merge and Sonar follow-up (22ce459)
main. Both sides now clear their per-module state on a re-parse: this PR'sjs_ts_package_importsand fix(calls): stop binding generic Python member calls to an arbitrary same-named method #2495'spython_import_rebinds.".json"appeared three times: fixed. It is now named once, asEXT_JSONin theEXT_*block ofconstants/languages.py.CONTRACT_JSON_EXTENSION,CONTRACT_SPEC_EXTENSIONSandTSCONFIG_EXTENSIONuse it, and their values are unchanged.Checklist
make pre-commit)# type: ignore,cast(),Any, orobjecttype hintsReview follow-ups (2d8b1c6)
baseUrl-only imports lost their JSX reference (Greptile): the import processor now reads every tsconfig
baseUrl, including configs withoutpaths. A bare specifier counts as a package only when nobaseUrlresolves it to a file on disk.import * as Widgets from "widgets"withbaseUrl: "."and a rootwidgets.tsxlinks<Widgets.Card />again, as on main.reactstays external when no<baseUrl>/react.*exists. The earlier baseUrl trade-off no longer applies.Rebound names stayed external (Greptile): last binding wins, the same way
import_mappingis overwritten. A later binding of the name to a project module removes it from the package set.New tests:
test_base_url_import_of_a_project_file_still_links[local-module|shadows-package-name]andtest_name_rebound_to_a_project_module_links(pass on main, failed on f290452). Negatives:test_base_url_that_names_no_file_keeps_the_package_external[...]andtest_name_rebound_to_a_package_stays_external. Targeted scope: 359 passed across 26 JS/TS/JSX/import/workspace files.Sibling configs created false links (Greptile; 07ea445): a
baseUrlnow applies only to files its tsconfig governs. Each config directory is recorded with its effectivebaseUrl, and an importing module uses the nearest enclosing config. AbaseUrlinherited through a relativeextends(e.g. an app extending the roottsconfig.base.json) counts as the config's own, resolved against the declaring config, as TypeScript does. Package-shipped bases are not followed. New tests:test_sibling_app_base_url_keeps_the_package_external(passes on main, failed on 2d8b1c6) andtest_base_url_applies_to_the_files_its_config_governs[file-under-the-config|nearest-config-wins|inherited-through-extends]. Targeted scope: 363 passed. Unchanged from main: thepathsalias loader does not scope by importing file either.🤖 Generated with Claude Code
https://claude.ai/code/session_01QjpNsgSNyL5gsmAFqfuoD6
Generated by Claude Code
Summary by CodeRabbit
Generated by Claude Code