fix(noUnresolvedImports): keep export = namespace members in project scans#9634
fix(noUnresolvedImports): keep export = namespace members in project scans#9634raashish1601 wants to merge 5 commits intobiomejs:mainfrom
Conversation
🦋 Changeset detectedLatest commit: 78b8700 The changes in this PR will be included in the next version bump. This PR includes changesets to release 13 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
github-actions
bot
added
A-Project
Merging this PR will not alter performance
Comparing 
Footnotes
|
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (2)
📒 Files selected for processing (2)
✅ Files skipped from review due to trivial changes (2)
WalkthroughThe PR updates JS module export collection to surface named exports from Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
crates/biome_module_graph/src/js_module_info/collector.rs (1)
908-946:⚠️ Potential issue | 🟠 MajorThe no-inference path still drops members declared on the exported value type.
collect_namespace_exports_for_local_name()only hoists direct bindings from the namespace scope. That fixes the React-style shape, butexport = shareddeclarations whose public API comes from the exported binding’s declared type (declare const shared: { foo(): ... }) still losefoowheninfer_types == false, because theown_members()branch above has no populated binding type to inspect in this mode. Please derive members from the binding’s declared type in the thin path as well.Based on learnings: "Implement module-level (thin) inference to resolve
TypeReference::Qualifiervariants by looking up declarations in module scopes and handling import statements."Also applies to: 1019-1047
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@crates/biome_module_graph/tests/spec_tests.rs`:
- Around line 1632-1677: Add a new test mirroring
test_react_named_exports_are_visible_without_type_inference that uses an `@types`
module which uses the "export = shared" pattern (e.g., a declared binding
`declare const shared: { useState: ... }` followed by `export = shared`), call
ModuleGraph::update_graph_for_js_paths(&fs, &project_layout, &added_paths,
false) just like the existing test, and assert that
js_module_info_for_path(...).find_js_exported_symbol(&module_graph, "useState")
returns Some; ensure the test name clearly indicates it's for the export =
shared/no-inference case and reference the same helpers (MemoryFileSystem,
get_added_js_paths, ProjectLayout, update_graph_for_js_paths) so the
no-inference path is exercised for the exported-binding shape.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 5252f79c-9617-4813-950c-f261f2ebb091
⛔ Files ignored due to path filters (2)
crates/biome_js_analyze/tests/specs/correctness/noUnresolvedImports/export-equals-namespace.ts.snapis excluded by!**/*.snapand included by**crates/biome_js_analyze/tests/specs/correctness/noUnresolvedImports/export-equals-react.d.ts.snapis excluded by!**/*.snapand included by**
📒 Files selected for processing (5)
crates/biome_js_analyze/tests/specs/correctness/noUnresolvedImports/export-equals-namespace.tscrates/biome_js_analyze/tests/specs/correctness/noUnresolvedImports/export-equals-react.d.tscrates/biome_module_graph/src/js_module_info/collector.rscrates/biome_module_graph/src/js_module_info/visitor.rscrates/biome_module_graph/tests/spec_tests.rs
ematipico
added
the
M-Likely Agent
There was a problem hiding this comment.
🧹 Nitpick comments (1)
crates/biome_module_graph/src/js_module_info/collector.rs (1)
961-1003: Consider&selfinstead of&mut self.This method doesn't appear to mutate
self— it only reads viaget_by_resolved_idandresolve_reference, both of which take&self. The mutation is only onfinalised_exports.♻️ Proposed signature change
- fn collect_member_exports_for_resolved_type( - &mut self, + fn collect_member_exports_for_resolved_type( + &self, mut resolved: ResolvedTypeId, finalised_exports: &mut IndexMap<Text, JsExport>, ) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/biome_module_graph/src/js_module_info/collector.rs` around lines 961 - 1003, The method collect_member_exports_for_resolved_type only reads from the struct (calling get_by_resolved_id and resolve_reference) and mutates only the local seen_types and the passed-in finalised_exports, so change its receiver from &mut self to &self; update the function signature accordingly and ensure any callers use an immutable borrow (replace &mut collector with &collector where applicable). Confirm get_by_resolved_id and resolve_reference remain callable with &self and that no other internal mutations require &mut self; adjust caller sites to pass mutable finalised_exports as before.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@crates/biome_module_graph/src/js_module_info/collector.rs`:
- Around line 961-1003: The method collect_member_exports_for_resolved_type only
reads from the struct (calling get_by_resolved_id and resolve_reference) and
mutates only the local seen_types and the passed-in finalised_exports, so change
its receiver from &mut self to &self; update the function signature accordingly
and ensure any callers use an immutable borrow (replace &mut collector with
&collector where applicable). Confirm get_by_resolved_id and resolve_reference
remain callable with &self and that no other internal mutations require &mut
self; adjust caller sites to pass mutable finalised_exports as before.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: a42f6d37-d9a1-4961-8200-219733c7b9b6
📒 Files selected for processing (2)
crates/biome_module_graph/src/js_module_info/collector.rscrates/biome_module_graph/tests/spec_tests.rs
🚧 Files skipped from review as they are similar to previous changes (1)
- crates/biome_module_graph/tests/spec_tests.rs
2 participants
Summary
export =declarations during project scans even when type inference is disabledexport =assignments and add a non-type-inference regression against the React declaration shapenoUnresolvedImportsanalyzer fixtures provingimport { useState } from "react"stays resolved through the project-domain CLI pathTesting
Fixes #9626.