fix(module_graph): properly index and resolve barrel exports#9267
Merged
Conversation
🦋 Changeset detectedLatest commit: 5372f16 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 |
Merging this PR will not alter performance
Comparing 
Footnotes
|
Contributor
|
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:
WalkthroughUpdates the no_unresolved_imports lint and module-graph to reduce false positives: accept Possibly related PRs
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)
Comment |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
crates/biome_resolver/src/lib.rs (1)
610-615: Please alignresolve_typesdocs with the new fallback behaviour.The implementation now falls back to
mainwhentypes/typingsis absent, but theResolveOptions::resolve_typesdocs still saymainis ignored.Doc tweak suggestion
- /// - The `package.json`'s `main` field will be ignored. + /// - The `package.json`'s `types`/`typings` field is preferred. + /// If neither is present, the `main` field is used as a fallback.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/biome_resolver/src/lib.rs` around lines 610 - 615, Update the ResolveOptions::resolve_types documentation to reflect that when resolve_types is true the resolver prefers package_json.types/typings but will fall back to package_json.main if neither types nor typings is present; change any wording that currently states "main is ignored" to indicate that main is used as a fallback and mention the exact precedence (types/typings first, then main). Ensure references to options.resolve_types and the behavior in the resolver (the if branch using package_json.types.as_ref().or(package_json.main.as_ref())) are consistent with the doc text.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.claude/skills/testing-codegen/SKILL.md:
- Line 306: The ordered list item "3. **Description**: What changed (used in
CHANGELOG)" is incorrectly numbered and fails markdownlint MD029; change the
leading "3." to "1." so the list restarts correctly (edit the line in
.claude/skills/testing-codegen/SKILL.md where the Description list item appears
and replace "3." with "1.").
- Around line 185-187: Fix the broken bold markdown spans in SKILL.md by merging
the split "**" markers so each bolded phrase is contained on a single line;
specifically update the line that currently breaks "Every JS/TS test spec file
**must ** begin..." (around the referenced block) and the similar broken span
near line 223–224 so the opening and closing "**" are on the same line and the
bold text renders correctly.
In `@crates/biome_package/src/node_js_package/package_json.rs`:
- Around line 309-313: The current match arm for "types" | "typings" lets the
first-seen key win, so if "typings" arrives before "types" it can incorrectly
block the canonical "types"; change the logic to inspect the actual key name:
when key_text == "types" always assign result.types = Some(value) (overwriting
any prior), but when key_text == "typings" only assign if
result.types.is_none(); keep using Deserializable::deserialize(...) to obtain
the value before assignment and reference the existing result.types and key_text
symbols to locate where to implement this conditional.
---
Nitpick comments:
In `@crates/biome_resolver/src/lib.rs`:
- Around line 610-615: Update the ResolveOptions::resolve_types documentation to
reflect that when resolve_types is true the resolver prefers
package_json.types/typings but will fall back to package_json.main if neither
types nor typings is present; change any wording that currently states "main is
ignored" to indicate that main is used as a fallback and mention the exact
precedence (types/typings first, then main). Ensure references to
options.resolve_types and the behavior in the resolver (the if branch using
package_json.types.as_ref().or(package_json.main.as_ref())) are consistent with
the doc text.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (9)
crates/biome_js_analyze/tests/specs/correctness/noUnresolvedImports/aliased-reexport-barrel.js.snapis excluded by!**/*.snapand included by**crates/biome_js_analyze/tests/specs/correctness/noUnresolvedImports/aliased-reexport-source.js.snapis excluded by!**/*.snapand included by**crates/biome_js_analyze/tests/specs/correctness/noUnresolvedImports/invalid.js.snapis excluded by!**/*.snapand included by**crates/biome_js_analyze/tests/specs/correctness/noUnresolvedImports/namespace-reexport-barrel.js.snapis excluded by!**/*.snapand included by**crates/biome_js_analyze/tests/specs/correctness/noUnresolvedImports/namespace-reexport-source.js.snapis excluded by!**/*.snapand included by**crates/biome_js_analyze/tests/specs/correctness/noUnresolvedImports/valid.js.snapis excluded by!**/*.snapand included by**crates/biome_js_analyze/tests/specs/correctness/noUnresolvedImports/validRegressions.js.snapis excluded by!**/*.snapand included by**crates/biome_module_graph/tests/snapshots/test_namespace_reexport_type_inference.snapis excluded by!**/*.snapand included by**crates/biome_module_graph/tests/snapshots/test_resolve_exports.snapis excluded by!**/*.snapand included by**
📒 Files selected for processing (20)
.changeset/fix-no-unresolved-imports-false-positives.md.claude/skills/lint-rule-development/SKILL.md.claude/skills/testing-codegen/SKILL.mdcrates/biome_js_analyze/src/lint/correctness/no_unresolved_imports.rscrates/biome_js_analyze/tests/specs/correctness/noUnresolvedImports/aliased-reexport-barrel.jscrates/biome_js_analyze/tests/specs/correctness/noUnresolvedImports/aliased-reexport-source.jscrates/biome_js_analyze/tests/specs/correctness/noUnresolvedImports/invalid.jscrates/biome_js_analyze/tests/specs/correctness/noUnresolvedImports/namespace-reexport-barrel.jscrates/biome_js_analyze/tests/specs/correctness/noUnresolvedImports/namespace-reexport-source.jscrates/biome_js_analyze/tests/specs/correctness/noUnresolvedImports/valid.jscrates/biome_js_analyze/tests/specs/correctness/noUnresolvedImports/validRegressions.jscrates/biome_module_graph/src/format_module_graph.rscrates/biome_module_graph/src/js_module_info.rscrates/biome_module_graph/src/js_module_info/collector.rscrates/biome_module_graph/src/js_module_info/module_resolver.rscrates/biome_module_graph/src/module_graph.rscrates/biome_module_graph/tests/snap/mod.rscrates/biome_module_graph/tests/spec_tests.rscrates/biome_package/src/node_js_package/package_json.rscrates/biome_resolver/src/lib.rs
5fcdcd8 to
b66f95f
Compare
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (1)
.claude/skills/testing-codegen/SKILL.md (1)
304-304:⚠️ Potential issue | 🟡 MinorFix CI blocker: use consistent "1." numbering for ordered list items.
The markdownlint check (MD029) enforces "1/1/1" style where every ordered list item should be numbered "1." and the renderer handles auto-numbering. Change line 304 from "3." to "1." to match the style.
🔧 Proposed fix
-3. **Description**: What changed (used in CHANGELOG) +1. **Description**: What changed (used in CHANGELOG)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.claude/skills/testing-codegen/SKILL.md at line 304, The ordered list item "3. **Description**: What changed (used in CHANGELOG)" should use the markdownlint MD029 "1." style; update that list marker from "3." to "1." (and verify other nearby ordered items also use "1." so the list is consistently auto-numbered), leaving the rest of the line text unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In @.claude/skills/testing-codegen/SKILL.md:
- Line 304: The ordered list item "3. **Description**: What changed (used in
CHANGELOG)" should use the markdownlint MD029 "1." style; update that list
marker from "3." to "1." (and verify other nearby ordered items also use "1." so
the list is consistently auto-numbered), leaving the rest of the line text
unchanged.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (9)
crates/biome_js_analyze/tests/specs/correctness/noUnresolvedImports/aliased-reexport-barrel.js.snapis excluded by!**/*.snapand included by**crates/biome_js_analyze/tests/specs/correctness/noUnresolvedImports/aliased-reexport-source.js.snapis excluded by!**/*.snapand included by**crates/biome_js_analyze/tests/specs/correctness/noUnresolvedImports/invalid.js.snapis excluded by!**/*.snapand included by**crates/biome_js_analyze/tests/specs/correctness/noUnresolvedImports/namespace-reexport-barrel.js.snapis excluded by!**/*.snapand included by**crates/biome_js_analyze/tests/specs/correctness/noUnresolvedImports/namespace-reexport-source.js.snapis excluded by!**/*.snapand included by**crates/biome_js_analyze/tests/specs/correctness/noUnresolvedImports/valid.js.snapis excluded by!**/*.snapand included by**crates/biome_js_analyze/tests/specs/correctness/noUnresolvedImports/validRegressions.js.snapis excluded by!**/*.snapand included by**crates/biome_module_graph/tests/snapshots/test_namespace_reexport_type_inference.snapis excluded by!**/*.snapand included by**crates/biome_module_graph/tests/snapshots/test_resolve_exports.snapis excluded by!**/*.snapand included by**
📒 Files selected for processing (19)
.changeset/fix-no-unresolved-imports-false-positives.md.claude/skills/lint-rule-development/SKILL.md.claude/skills/testing-codegen/SKILL.mdcrates/biome_js_analyze/src/lint/correctness/no_unresolved_imports.rscrates/biome_js_analyze/tests/specs/correctness/noUnresolvedImports/aliased-reexport-barrel.jscrates/biome_js_analyze/tests/specs/correctness/noUnresolvedImports/aliased-reexport-source.jscrates/biome_js_analyze/tests/specs/correctness/noUnresolvedImports/invalid.jscrates/biome_js_analyze/tests/specs/correctness/noUnresolvedImports/namespace-reexport-barrel.jscrates/biome_js_analyze/tests/specs/correctness/noUnresolvedImports/namespace-reexport-source.jscrates/biome_js_analyze/tests/specs/correctness/noUnresolvedImports/valid.jscrates/biome_js_analyze/tests/specs/correctness/noUnresolvedImports/validRegressions.jscrates/biome_module_graph/src/format_module_graph.rscrates/biome_module_graph/src/js_module_info.rscrates/biome_module_graph/src/js_module_info/collector.rscrates/biome_module_graph/src/js_module_info/module_resolver.rscrates/biome_module_graph/src/module_graph.rscrates/biome_module_graph/tests/snap/mod.rscrates/biome_module_graph/tests/spec_tests.rscrates/biome_package/src/node_js_package/package_json.rs
🚧 Files skipped from review as they are similar to previous changes (7)
- crates/biome_js_analyze/tests/specs/correctness/noUnresolvedImports/validRegressions.js
- crates/biome_module_graph/tests/snap/mod.rs
- crates/biome_module_graph/src/js_module_info.rs
- crates/biome_js_analyze/tests/specs/correctness/noUnresolvedImports/invalid.js
- .claude/skills/lint-rule-development/SKILL.md
- crates/biome_module_graph/src/js_module_info/module_resolver.rs
- crates/biome_js_analyze/tests/specs/correctness/noUnresolvedImports/valid.js
dyc3
approved these changes
Feb 27, 2026
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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_package/src/node_js_package/package_json.rs`:
- Around line 308-317: The code uses result.types.is_none() to decide if the
legacy "typings" key should override, but that reflects deserialization success
not presence; add a boolean flag (e.g., types_seen) in the parsing scope/struct
and set it in the "types" match arm (when handling "types" — where
Deserializable::deserialize(ctx, &value, &key_text) is called) to true
regardless of whether deserialization returned Some/None, then change the
"typings" guard to check !types_seen (instead of result.types.is_none()) so
"typings" only takes effect when "types" was not present at all.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.changeset/fix-no-unresolved-imports-false-positives.mdcrates/biome_package/src/node_js_package/package_json.rs
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (1)
crates/biome_package/src/node_js_package/package_json.rs (1)
313-316:⚠️ Potential issue | 🟡 Minor
typingscan still win whentypesis present but invalidOn Line 314, the guard checks deserialisation success (
result.types.is_none()), not whether"types"was present.
So"types": 123followed by valid"typings"still lets the alias override the canonical field.Consider tracking presence explicitly (e.g.
seen_types_field) and gating"typings"on that flag.Suggested fix
- let mut result = Self::Output::default(); + let mut result = Self::Output::default(); + let mut seen_types_field = false; @@ "types" => { + seen_types_field = true; result.types = Deserializable::deserialize(ctx, &value, &key_text); } @@ "typings" => { - if result.types.is_none() + if !seen_types_field && let Some(value) = Deserializable::deserialize(ctx, &value, &key_text) { result.types = Some(value); } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/biome_package/src/node_js_package/package_json.rs` around lines 313 - 316, The "typings" branch currently checks result.types.is_none() which only tests deserialization, not whether the "types" field was present, allowing invalid "types" values to be overridden by "typings"; modify the parsing logic to track explicit presence of the "types" field (e.g. add a seen_types_field boolean set when handling the "types" key) and change the guard in the "typings" arm to only allow aliasing when seen_types_field is false and Deserializable::deserialize(ctx, &value, &key_text) succeeds; update the code paths that set result.types to also set the seen flag so Deserializable::deserialize and the "typings" branch both consult that flag.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@crates/biome_package/src/node_js_package/package_json.rs`:
- Around line 313-316: The "typings" branch currently checks
result.types.is_none() which only tests deserialization, not whether the "types"
field was present, allowing invalid "types" values to be overridden by
"typings"; modify the parsing logic to track explicit presence of the "types"
field (e.g. add a seen_types_field boolean set when handling the "types" key)
and change the guard in the "typings" arm to only allow aliasing when
seen_types_field is false and Deserializable::deserialize(ctx, &value,
&key_text) succeeds; update the code paths that set result.types to also set the
seen flag so Deserializable::deserialize and the "typings" branch both consult
that flag.
6d03dc6 to
5372f16
Compare
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Closes #9143
Closes #8849
This was done with an AI agent, carefully steered towards a proper solution. I also made sure to add proper type inference for the new exports.
Our module graph didn't follow blanked exports and blanked re-exports properly when they were defined as namespaces.
I carefully reviwed all the code.
Test Plan
Manually tested against the libraries. Added many tests
Docs
N/A