Skip to content

fix(parsers): keep JSX members of bare package imports external when the package name matches a project path - #2806

Merged
vitali87 merged 5 commits into
mainfrom
claude/jsx-package-collision-2535
Oct 4, 2026
Merged

vitali87 merged 5 commits into
mainfrom
claude/jsx-package-collision-2535

Conversation

@vitali87

@vitali87 vitali87 commented Oct 3, 2026 •

Copy link
Copy Markdown
Owner

Summary

  • <React.Fragment /> under import * as React from "react" no longer gets a REFERENCES edge to an unrelated local Fragment when the project itself is named react, or when a repo-root react.tsx exists. The same holds for default, named and subpath imports (react/jsx-runtime) and for require("react").
  • Root cause: 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 named react, and once project-prefixed it is the same path as a repo-root react.tsx (proj.react). Both checks then called the package first-party, and the tag fell back to the bare-name trie.
  • Fix: the import processor now flags a specifier as a package at the point where it resolves it, because only there does it still know whether a tsconfig alias or a workspace package mapped the specifier to a project file. _resolve_js_import_target returns the module qn together with an is_package flag (JsImportTarget). Local names bound by a package import or require go into a new per-module js_ts_package_imports set, which is cleared on re-parse like js_ts_bare_imports. js_member_head_is_external now checks only that set and never compares the qn with project paths. Relative, tsconfig-alias, workspace and aliased-scheme (deno ext:) 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 react run against a fresh Memgraph 3.3.0. The repo has src/App.tsx (<React.Fragment> from "react" wrapping <Local.Card /> from ./widgets) and an unrelated src/fragment.tsx that exports Fragment. On main, App gets REFERENCES ... exact to react.src.fragment.Fragment and to react.src.widgets.Card. On this branch only the Card edge remains, still exact.

Type of Change

  • Bug fix
  • New feature
  • Performance improvement
  • Refactoring (no functional changes)
  • Documentation
  • CI/CD or tooling
  • Dependencies

Related Issues

Follow-up to #2577 (#2535). Refs #2535

Greptile's finding at codebase_rag/parsers/call_resolver.py:2000-2004 on main, verified with a T-Rex reproduction ("the import-root equality check returns False, which lets JSX path fallback to the local symbol"):

If a project is named react, import * as React from "react" is treated as a first-party import because the package name matches the project name. Indexing <React.Fragment /> then creates a reference to an unrelated local Fragment function.

A variant from another run:

If a local module shares an external package's name, this check treats the package import as first-party. With React imported from react, adding an unrelated local react.tsx makes <React.Fragment /> reference that local module's Fragment.

Test Plan

  • Unit tests pass (make test-parallel or uv 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.py and test_slots_lazy_logger.py.
  • New tests added: codebase_rag/tests/test_jsx_package_name_collision.py
  • Integration tests pass (make test-integration, requires Docker)
  • Manual testing: the before/after cgr start --update-graph + Cypher run in the Demo

Red tests. All six failed on main with a REFERENCES ... exact edge to the local Fragment:

  • test_package_named_like_the_project_binds_no_local_symbol[namespace-import], [subpath-import], [default-import], [require]: project react, edge to react.src.frag.Fragment
  • test_package_named_like_a_local_module_binds_no_local_symbol[namespace-import], [named-import]: repo-root react.tsx, edge to proj.react.Fragment

Negative 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 /> exact to the local module
  • test_workspace_package_import_still_links: a first-party workspace package that is itself named react is resolved by the import processor and still links
  • test_tsconfig_alias_named_like_the_project_still_links: a react/* tsconfig alias in a project named react still links
  • All 11 tests in test_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 format and ty check are 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 in JsImportTarget.

Merge and Sonar follow-up (22ce459)

  • Merged main. Both sides now clear their per-module state on a re-parse: this PR's js_ts_package_imports and fix(calls): stop binding generic Python member calls to an arbitrary same-named method #2495's python_import_rebinds.
  • S1192, ".json" appeared three times: fixed. It is now named once, as EXT_JSON in the EXT_* block of constants/languages.py. CONTRACT_JSON_EXTENSION, CONTRACT_SPEC_EXTENSIONS and TSCONFIG_EXTENSION use it, and their values are unchanged.
  • Results: 326 passed on the merge and 89 after the refactor.

Checklist

  • PR title follows Conventional Commits format
  • All pre-commit checks pass (make pre-commit)
  • No hardcoded strings in non-config/non-constants files
  • No # type: ignore, cast(), Any, or object type hints
  • New comments and docstrings explain why, not what (see Comment Policy)

Review follow-ups (2d8b1c6)

  • baseUrl-only imports lost their JSX reference (Greptile): the import processor now reads every tsconfig baseUrl, including configs without paths. A bare specifier counts as a package only when no baseUrl resolves it to a file on disk. import * as Widgets from "widgets" with baseUrl: "." and a root widgets.tsx links <Widgets.Card /> again, as on main. react stays 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_mapping is 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] and test_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[...] and test_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 baseUrl now applies only to files its tsconfig governs. Each config directory is recorded with its effective baseUrl, and an importing module uses the nearest enclosing config. A baseUrl inherited through a relative extends (e.g. an app extending the root tsconfig.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) and test_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: the paths alias 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

  • Bug Fixes
    • Improved JavaScript and TypeScript import handling when a package name matches a local project symbol, preventing package references from being incorrectly linked to local code.
    • Preserved local code resolution for relative imports, workspace packages, and configured aliases, including base URL mappings.
    • Updated reference handling when an imported or required name is reassigned, so links reflect its final binding.

Generated by Claude Code

@coderabbitai

coderabbitai Bot commented Oct 3, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

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
  • Configuration used: Repository: vitali87/code-graph-rag/.coderabbit.yaml
  • Review profile: QUIET
  • Plan: Advanced
  • Run ID: 4ffe3d43-a540-446f-b46e-4a1010d8c796
📥 Commits

Reviewing files that changed from the base of the PR and between 69d2a3d and 2d8b1c6.

📒 Files selected for processing (3)
  • codebase_rag/parsers/call_resolver.py
  • codebase_rag/parsers/import_processor.py
  • codebase_rag/tests/test_jsx_package_name_collision.py

Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 7 remain after this review.


📝 Walkthrough

Walkthrough

JS/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.

Changes

JS/TS Import Classification

Layer / File(s) Summary
Resolve JS/TS import targets
codebase_rag/parsers/import_processor.py
The resolver collects tsconfig baseUrl prefixes and returns module targets with package classification.
Track package status through bindings
codebase_rag/parsers/import_processor.py, codebase_rag/parsers/call_resolver.py
Import and require bindings retain package status, including when a later binding replaces an earlier one. Member-call resolution uses the tracked package names.
Verify package and project-file references
codebase_rag/tests/test_jsx_package_name_collision.py
Tests cover bare-package collisions, project-file resolutions, baseUrl behavior, and repeated require bindings.

Priority: ⬇️ Low

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Bug fix

Suggested reviewers: claude

Merge Risk: ⚪ Minimal · up to 2d8b1

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 Review

Security architecture risk: 🔵 Low · up to 2d8b1

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
No architecture-level concerns identified.

Security review details

Security Blast Radius

  • observed — The directly traced downstream outcome is the indexed reference graph: a package-bound JSX head returns before component resolution and relationship emission. The identified high-fanout range belongs to fixture helpers that run GraphUpdater, rather than a production authority boundary.

Trust Boundaries and Controls

  • inferred — Package status is a static-analysis ownership distinction, not an authentication or authorization decision. Repository-controlled imports influence reference selection, but the inspected consumer does not invoke the selected component or grant package code additional privileges.

Resilience and Maintainability Implications

  • observed — Normal updater execution completes definition and import processing before the reference-analysis phase. Import parsing nevertheless retains the pre-existing catch-and-continue behavior after in-place state updates. The new package set adds another state item to that lifecycle; complete failure rollback and external concurrent-use guarantees were not established.
🚥 Pre-merge checks | ✅ 4
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Title check ✅ Passed The title clearly identifies the JSX import collision fix and follows the Conventional Commits format.
Description check ✅ Passed The description covers the change, demo, type, related issues, testing, and checklist. It reports targeted tests and manual testing, and clearly notes that the full integration suite was not run.
✨ Finishing Touches
📝 Generate docstrings
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@greptile-apps

greptile-apps Bot commented Oct 3, 2026 •

Copy link
Copy Markdown
Contributor

RetriggerConfidence Score: 5/5

[Medium risk] Fixes JSX import resolution logic for package name collisions.

No outstanding findings block merging.

Summary

The PR distinguishes external JavaScript and TypeScript package imports from project imports when resolving JSX references. The latest commit also adds safeguards against misleading Python call links and consolidates the JSON extension constant. No new findings were identified.

Reviews (4) · Last reviewed commit: "refactor(constants): name the .json exte..."

vitali87 commented Oct 3, 2026

Copy link
Copy Markdown
Owner Author

@greptileai please review the latest commit (2d8b1c6).


Generated by Claude Code

vitali87 commented Oct 3, 2026

Copy link
Copy Markdown
Owner Author

@greptileai please review the latest commit (07ea445).


Generated by Claude Code

vitali87 commented Oct 3, 2026

Copy link
Copy Markdown
Owner Author

@greptileai please review the latest commit (22ce459).


Generated by Claude Code

@sonarqubecloud

sonarqubecloud Bot commented Oct 3, 2026

Copy link
Copy Markdown

@vitali87
vitali87 merged commit 57f365c into main Oct 4, 2026
34 checks passed
@vitali87
vitali87 deleted the claude/jsx-package-collision-2535 branch October 4, 2026 21:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

1 participant