feat(resolver): resolve extension alias #7551
Conversation
🦋 Changeset detectedLatest commit: f3b2fc7 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 |
WalkthroughThis PR adds case-insensitive enum deserialization support in biome_deserialize_macros; introduces Module and ModuleResolution (and adds module/moduleResolution to CompilerOptions) in biome_package; expands the public resolver API with ResolveOptions.extension_aliases and a builder, threads extension aliasing into resolution flows (including new resolve_extension_aliases and passing base_dir through resolve_module_with_package_json); wires extension aliases into the module-graph visitor; and adds fixtures and tests exercising .js→.ts/.tsx/.d.ts/.mts alias resolution for Node16/NodeNext-style configs. No public ABI removals. Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro 📒 Files selected for processing (4)
🚧 Files skipped from review as they are similar to previous changes (3)
🧰 Additional context used📓 Path-based instructions (2)crates/biome_*/**📄 CodeRabbit inference engine (CLAUDE.md)
Files:
**/*.rs📄 CodeRabbit inference engine (CONTRIBUTING.md)
Files:
🧠 Learnings (3)📓 Common learnings📚 Learning: 2025-08-11T11:50:12.090ZApplied to files:
📚 Learning: 2025-08-11T11:48:52.001ZApplied to files:
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (20)
🔇 Additional comments (9)
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (7)
crates/biome_deserialize_macros/src/deserializable_derive/container_attrs.rs (3)
23-24: Enum-only flag: add validation (don’t silently noop on structs).Field addition looks good. Given it’s “only supported in enums”, please enforce this upstream (e.g. abort when used on structs) to avoid confusing no-ops.
Happy to wire an abort in the struct branch of
deserializable_derive.rsif you want.
27-27: Typo in doc comment.“unkinown” → “unknown”.
-/// Attributes for struct that control how unkinown fields are handled. +/// Attributes for struct that control how unknown fields are handled.
42-42: User-facing error string typo.“amomg” → “among”.
- _ => Err("unknown_fields takes a value amomg `deny`, `warn`, and `allow`."), + _ => Err("unknown_fields takes a value among `deny`, `warn`, and `allow`."),crates/biome_deserialize_macros/src/deserializable_derive.rs (1)
73-76: Gate misuse: error ifcase_insensitiveis set on structs.Currently it’s silently ignored on structs. Abort early to guide users.
Data::Struct(data) => { + if attrs.case_insensitive { + abort!( + data.fields, + "#[deserializable(case_insensitive)] is only supported on enums" + ) + }crates/biome_resolver/tests/fixtures/resolver_cases_8/dir2/index.js (1)
1-1: Empty fixture is fine; make the intent explicit.Add a one‑liner so future you doesn’t “clean it up” during a refactor.
+// Intentionally empty: resolver fixture..changeset/petite-waves-love.md (1)
5-5: Changeset voice: start with past‑tense summary; minor grammar tweak.Begin with what you did (past tense), then state current behaviour (present). Also fix “if exists” → “if it exists”.
-The resolver can now correctly resolve `.ts`, `.tsx`, `.d.ts`, `.js` files by `.js` extension if exists, based on [the file extension substitution in TypeScript](https://www.typescriptlang.org/docs/handbook/modules/reference.html#file-extension-substitution), if the `moduleResolution` option is set to `Node16` or `NodeNext`. +Added support for resolving `.js` specifiers to `.ts`, `.tsx`, `.d.ts`, or `.js` files (TypeScript’s file‑extension substitution) when `moduleResolution` is `Node16` or `NodeNext`. Biome now resolves these imports accordingly.crates/biome_resolver/tests/fixtures/resolver_cases_8/package.json (1)
1-3: Confirm ESM/CJS intent.If tests assume ESM semantics for
.js, consider adding"type": "module"; otherwise this is fine as CJS.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
.changeset/petite-waves-love.md(1 hunks)crates/biome_deserialize_macros/src/deserializable_derive.rs(6 hunks)crates/biome_deserialize_macros/src/deserializable_derive/container_attrs.rs(2 hunks)crates/biome_module_graph/src/js_module_info/visitor.rs(3 hunks)crates/biome_package/src/lib.rs(1 hunks)crates/biome_package/src/node_js_package/mod.rs(1 hunks)crates/biome_package/src/node_js_package/tsconfig_json.rs(1 hunks)crates/biome_resolver/src/lib.rs(11 hunks)crates/biome_resolver/tests/fixtures/resolver_cases_8/dir2/index.js(1 hunks)crates/biome_resolver/tests/fixtures/resolver_cases_8/package.json(1 hunks)crates/biome_resolver/tests/fixtures/resolver_cases_8/tsconfig.json(1 hunks)crates/biome_resolver/tests/spec_tests.rs(1 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
crates/biome_*/**
📄 CodeRabbit inference engine (CLAUDE.md)
Place core crates under /crates/biome_*/
Files:
crates/biome_resolver/tests/fixtures/resolver_cases_8/package.jsoncrates/biome_resolver/tests/fixtures/resolver_cases_8/dir2/index.jscrates/biome_deserialize_macros/src/deserializable_derive.rscrates/biome_package/src/node_js_package/mod.rscrates/biome_deserialize_macros/src/deserializable_derive/container_attrs.rscrates/biome_package/src/node_js_package/tsconfig_json.rscrates/biome_resolver/tests/fixtures/resolver_cases_8/tsconfig.jsoncrates/biome_resolver/src/lib.rscrates/biome_resolver/tests/spec_tests.rscrates/biome_module_graph/src/js_module_info/visitor.rscrates/biome_package/src/lib.rs
**/tests/**
📄 CodeRabbit inference engine (CLAUDE.md)
Place test files under a tests/ directory in each crate
Files:
crates/biome_resolver/tests/fixtures/resolver_cases_8/package.jsoncrates/biome_resolver/tests/fixtures/resolver_cases_8/dir2/index.jscrates/biome_resolver/tests/fixtures/resolver_cases_8/tsconfig.jsoncrates/biome_resolver/tests/spec_tests.rs
**/*.rs
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Format all Rust source files before committing (just f)
Files:
crates/biome_deserialize_macros/src/deserializable_derive.rscrates/biome_package/src/node_js_package/mod.rscrates/biome_deserialize_macros/src/deserializable_derive/container_attrs.rscrates/biome_package/src/node_js_package/tsconfig_json.rscrates/biome_resolver/src/lib.rscrates/biome_resolver/tests/spec_tests.rscrates/biome_module_graph/src/js_module_info/visitor.rscrates/biome_package/src/lib.rs
.changeset/*.md
📄 CodeRabbit inference engine (CONTRIBUTING.md)
.changeset/*.md: In changeset files, only use #### or ##### headers; avoid other header levels
Changeset descriptions should use past tense for what you did (e.g., "Added...")
Describe current Biome behavior in present tense within changesets (e.g., "Biome now supports...")
For bug fixes in changesets, start with a link to the issue (e.g., "Fixed #1234: ...")
When referencing rules or assists in changesets, include links to their documentation pages
Include a minimal code block in the changeset when applicable to demonstrate the change
End every sentence in the changeset description with a period
Files:
.changeset/petite-waves-love.md
🧠 Learnings (10)
📚 Learning: 2025-09-10T08:05:22.867Z
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-09-10T08:05:22.867Z
Learning: Applies to crates/biome_analyze/crates/**/tests/specs/**/*.jsonc : Snapshot test `.jsonc` files must contain an array of code snippets (strings); these run in script mode (no ESM syntax)
Applied to files:
crates/biome_resolver/tests/fixtures/resolver_cases_8/package.jsoncrates/biome_resolver/tests/fixtures/resolver_cases_8/tsconfig.json
📚 Learning: 2025-08-11T11:48:52.001Z
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_js_formatter/CONTRIBUTING.md:0-0
Timestamp: 2025-08-11T11:48:52.001Z
Learning: Applies to crates/biome_js_formatter/**/Cargo.toml : Add biome_js_formatter as a path dependency in Cargo.toml: biome_js_formatter = { version = "0.0.1", path = "../biome_js_formatter" }
Applied to files:
crates/biome_package/src/node_js_package/mod.rscrates/biome_resolver/src/lib.rs
📚 Learning: 2025-08-11T11:50:12.090Z
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_js_type_info/CONTRIBUTING.md:0-0
Timestamp: 2025-08-11T11:50:12.090Z
Learning: Applies to crates/biome_js_type_info/{src,resolver,biome_module_graph/src}/**/*.rs : Implement and use type resolution via the TypeResolver trait; resolvers own TypeStore vectors and provide fast by-id and hashed lookups
Applied to files:
crates/biome_package/src/node_js_package/tsconfig_json.rscrates/biome_resolver/tests/fixtures/resolver_cases_8/tsconfig.jsoncrates/biome_resolver/src/lib.rscrates/biome_resolver/tests/spec_tests.rscrates/biome_module_graph/src/js_module_info/visitor.rscrates/biome_package/src/lib.rs
📚 Learning: 2025-08-11T11:48:27.774Z
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_formatter/CONTRIBUTING.md:0-0
Timestamp: 2025-08-11T11:48:27.774Z
Learning: Applies to crates/biome_formatter/biome_html_formatter/tests/spec_test.rs : Create tests/spec_test.rs implementing the run(spec_input_file, _expected_file, test_directory, _file_type) function as shown and include!("language.rs")
Applied to files:
crates/biome_resolver/tests/spec_tests.rs
📚 Learning: 2025-08-11T11:53:15.299Z
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_service/CONTRIBUTING.md:0-0
Timestamp: 2025-08-11T11:53:15.299Z
Learning: Applies to crates/biome_service/../biome_lsp/src/server.tests.rs : Keep end-to-end LSP tests in biome_lsp’s server.tests.rs
Applied to files:
crates/biome_resolver/tests/spec_tests.rs
📚 Learning: 2025-08-11T11:48:27.774Z
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_formatter/CONTRIBUTING.md:0-0
Timestamp: 2025-08-11T11:48:27.774Z
Learning: Applies to crates/biome_formatter/biome_html_formatter/tests/spec_tests.rs : Create tests/spec_tests.rs in the biome_html_formatter crate that generates tests via tests_macros::gen_tests! for all HTML files at tests/specs/html/**/*.html
Applied to files:
crates/biome_resolver/tests/spec_tests.rs
📚 Learning: 2025-08-11T11:53:15.299Z
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_service/CONTRIBUTING.md:0-0
Timestamp: 2025-08-11T11:53:15.299Z
Learning: Applies to crates/biome_service/src/workspace/watcher.tests.rs : Place watcher tests for workspace methods in src/workspace/watcher.tests.rs
Applied to files:
crates/biome_resolver/tests/spec_tests.rs
📚 Learning: 2025-08-11T11:50:12.090Z
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_js_type_info/CONTRIBUTING.md:0-0
Timestamp: 2025-08-11T11:50:12.090Z
Learning: Applies to crates/biome_js_type_info/{src,biome_module_graph/src}/**/*.rs : When pattern-matching on ResolvedTypeData via as_raw_data(), ensure any nested TypeReferences are subsequently resolved using the correct ResolverId; never use the raw data with a resolver without applying the right ResolverId to avoid panics
Applied to files:
crates/biome_resolver/tests/spec_tests.rs
📚 Learning: 2025-09-10T08:05:22.867Z
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-09-10T08:05:22.867Z
Learning: Applies to crates/biome_analyze/crates/biome_js_analyze/tests/quick_test.rs : Quick test: edit `tests/quick_test.rs`, remove or comment `#[ignore]`, set `SOURCE`, and adjust `RuleFilter`
Applied to files:
crates/biome_resolver/tests/spec_tests.rs
📚 Learning: 2025-08-11T11:50:12.090Z
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_js_type_info/CONTRIBUTING.md:0-0
Timestamp: 2025-08-11T11:50:12.090Z
Learning: Applies to crates/biome_js_type_info/biome_module_graph/src/js_module_info/collector.rs : Thin (module-level) inference may resolve qualifiers to local declarations or globals (TypeReference::Resolved), mark imported bindings as TypeReference::Import, and set unresolved to TypeReference::Unknown; it must not look beyond the current module
Applied to files:
crates/biome_module_graph/src/js_module_info/visitor.rs
🧬 Code graph analysis (2)
crates/biome_resolver/tests/spec_tests.rs (2)
crates/biome_resolver/src/lib.rs (7)
new(905-918)new(1146-1148)default(898-900)resolve(31-62)from(1128-1130)from(1134-1136)from(1140-1142)crates/biome_fs/src/fs/os.rs (1)
default(41-46)
crates/biome_package/src/lib.rs (1)
packages/@biomejs/js-api/src/wasm.ts (1)
Module(110-117)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (21)
- GitHub Check: Bench (biome_package)
- GitHub Check: autofix
- GitHub Check: Bench (biome_html_parser)
- GitHub Check: Bench (biome_configuration)
- GitHub Check: Documentation
- GitHub Check: Bench (biome_css_analyze)
- GitHub Check: Test (depot-windows-2022-16)
- GitHub Check: Bench (biome_module_graph)
- GitHub Check: Bench (biome_json_formatter)
- GitHub Check: Bench (biome_graphql_parser)
- GitHub Check: Bench (biome_css_parser)
- GitHub Check: Check Dependencies
- GitHub Check: Bench (biome_js_analyze)
- GitHub Check: Bench (biome_graphql_formatter)
- GitHub Check: Bench (biome_json_analyze)
- GitHub Check: Bench (biome_css_formatter)
- GitHub Check: Bench (biome_html_formatter)
- GitHub Check: Bench (biome_json_parser)
- GitHub Check: Bench (biome_js_formatter)
- GitHub Check: Bench (biome_js_parser)
- GitHub Check: Test (depot-ubuntu-24.04-arm-16)
🔇 Additional comments (24)
crates/biome_deserialize_macros/src/deserializable_derive/container_attrs.rs (1)
79-81: Attribute parsing LGTM.
deserializable(case_insensitive)is parsed correctly and defaults remain intact.crates/biome_deserialize_macros/src/deserializable_derive.rs (4)
69-71: Wiring through the flag: looks good.
case_insensitiveis correctly propagated into enum data.
171-173: Struct shape update LGTM.Adding
case_insensitive: booltoDeserializableEnumDatais straightforward and non-breaking here.
245-251: Nice touch: lowercase allowed variants for diagnostics.Ensures suggestions match the normalised comparison set.
262-266: Key normalisation for matching LGTM.Per-variant keys are lowercased when enabled; matches the diagnostics behaviour.
crates/biome_resolver/tests/fixtures/resolver_cases_8/tsconfig.json (1)
1-5: LGTM: focused Node16 fixture.Reads clean and minimal for the test intent.
crates/biome_package/src/node_js_package/tsconfig_json.rs (2)
140-142: Case‑insensitive enum deserialisation: nice touch.This matches real‑world tsconfig usage variability.
153-156:ModuleincludesNode18/Node20which TypeScript doesn’t accept.TS
compilerOptions.modulesupportsNode16/NodeNextbut notNode18/Node20. Keeping unsupported variants invites invalid configs.ESNext, Node16, - Node18, - Node20, NodeNext, Preserve,Likely an incorrect or invalid review comment.
.changeset/petite-waves-love.md (1)
7-7: Add docs link for noFloatingPromisesIn .changeset/petite-waves-love.md (line 7) include the Biome rule docs URL: https://biomejs.dev/linter/rules/no-floating-promises/
crates/biome_module_graph/src/js_module_info/visitor.rs (3)
21-27: LGTM! Extension alias mapping properly configured.The constant correctly maps the TypeScript module resolution extension alias behaviour, enabling
.jsimports to resolve to.ts/.tsx/.d.ts/.js/.jsxfiles under Node16/NodeNext resolution.
453-453: Extension alias correctly wired into resolve options.The extension alias is properly integrated into the ResolveOptions, enabling the module resolution to use TypeScript extension aliasing.
404-404: Minor formatting alignment - no functional change.This is just a whitespace adjustment to maintain consistent indentation.
crates/biome_package/src/lib.rs (1)
13-14: Module and ModuleResolution types now publicly exposed.These new exports allow consumers to access the Module and ModuleResolution enums needed for TypeScript extension alias resolution configuration.
crates/biome_resolver/tests/spec_tests.rs (2)
688-723: Comprehensive extension alias testing.Good coverage of extension alias behaviour including:
- Fully specified files with alias fallbacks
- Multiple alias options with priority ordering
- Different extension mappings (js → ts/js, mjs → mts)
The test assertions properly verify both successful resolutions and fallback behaviour.
725-748: Edge case coverage for non-applicable scenarios.Properly tests that extension aliases don't apply to directory resolution or extension-less file resolution, ensuring the feature only affects fully-specified imports as intended.
crates/biome_package/src/node_js_package/mod.rs (1)
6-6: Module and ModuleResolution added to public exports.The new types are correctly exposed for consumers requiring TypeScript module resolution configuration.
crates/biome_resolver/src/lib.rs (8)
11-11: TypeScript module resolution types imported.Module and ModuleResolution imports enable the extension alias logic to check tsconfig module resolution settings.
48-51: Graceful fallback on relative path resolution failure.The change allows the resolver to continue to module resolution when relative path resolution fails, improving resilience.
163-170: Base directory propagated for extension alias support.The base_dir parameter is now correctly passed through to enable extension alias resolution logic in the module resolution flow.
Also applies to: 176-183
258-262: Extension alias resolution integrated into module flow.The extension alias resolution is correctly positioned after path mapping but before dependency resolution, providing the right precedence order.
424-479: Robust extension alias implementation.The implementation correctly:
- Validates Node16/NodeNext module resolution requirements
- Only applies to relative specifiers with extensions
- Tries each alias in order with proper error handling
- Returns early on first successful resolution
832-840: Clear documentation for extension alias option.The documentation properly explains the purpose, behaviour, and constraints of the extension_alias field, including the Node16/NodeNext requirement.
911-911: Extension alias field and builder method added.The ResolveOptions struct is properly extended with the extension_alias field and corresponding builder method for configuration.
Also applies to: 950-957
998-998: Extension alias propagated in options constructors.The extension_alias field is correctly copied in internal option constructor methods, ensuring consistency across resolution contexts.
Also applies to: 1013-1013
278ac33 to
7713368
Compare
CodSpeed Performance ReportMerging #7551 will not alter performanceComparing Summary
Footnotes
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (5)
crates/biome_deserialize_macros/src/deserializable_derive.rs (1)
245-251: Avoid extra allocations; emit string literals explicitly; pre-empt normalisation collisions.
- Drop the unnecessary to_string() and build a string literal via LitStr to avoid repeated parsing by the quote machinery.
- Minor alloc win and clearer intent (ties in with our prior learning to avoid avoidable string allocations).
- Optional but worthwhile: detect duplicate keys after lowercasing and abort with a friendly message.
Apply this diff:
- .map(|DeserializableVariantData { key, .. }| { - let mut key = key.to_string(); - if data.case_insensitive { - key = key.to_ascii_lowercase(); - } - quote! { #key } - }) + .map(|DeserializableVariantData { key, .. }| { + let key_str = if data.case_insensitive { + key.to_ascii_lowercase() + } else { + key.clone() + }; + let key_lit = syn::LitStr::new(&key_str, proc_macro2::Span::call_site()); + quote! { #key_lit } + })- |DeserializableVariantData { - ident: variant_ident, - key, - }| { - let mut key = key.to_string(); - if data.case_insensitive { - key = key.to_ascii_lowercase(); - } - quote! { #key => Self::#variant_ident } - }, + |DeserializableVariantData { ident: variant_ident, key }| { + let key_str = if data.case_insensitive { + key.to_ascii_lowercase() + } else { + key.clone() + }; + let key_lit = syn::LitStr::new(&key_str, proc_macro2::Span::call_site()); + quote! { #key_lit => Self::#variant_ident } + },Add the missing imports (outside this hunk):
use proc_macro2::Span; use syn::LitStr;Optionally detect collisions (outside this hunk, near the start of generate_deserializable_enum):
if data.case_insensitive { use std::collections::{BTreeSet, HashSet}; let mut seen = HashSet::new(); let mut dupes = BTreeSet::new(); for v in &data.variants { let k = v.key.to_ascii_lowercase(); if !seen.insert(k.clone()) { dupes.insert(k); } } if !dupes.is_empty() { abort_call_site!( "case_insensitive enum deserialisation produces duplicate keys after normalisation: {}", dupes.into_iter().collect::<Vec<_>>().join(", ") ); } }Also applies to: 262-266
crates/biome_resolver/src/lib.rs (4)
449-454: Alias applies only to relative specifiers — clarify docs or broaden scope.Functionally fine, but it contradicts the field docs (which mention absolute paths). Either update the docs (preferred) or support absolute paths here.
456-468: Avoid redundant re‑resolves when aliasing doesn’t apply.resolve_absolute_path() was already tried earlier; rerunning it here repeats filesystem work.
Apply this diff to just signal “no alias match” and let callers continue:
- // Skip if no extension is in the path. - let Some(extension) = path.extension() else { - return resolve_absolute_path(path, fs, options); - }; + // Skip if no extension is in the path. + let Some(extension) = path.extension() else { + return Err(ResolveError::NotFound); + }; @@ - else { - return resolve_absolute_path(path, fs, options); - }; + else { + return Err(ResolveError::NotFound); + };
832-841: Doc fix: alias applies to relative specifiers only (and not within exports/imports).Small tweak to match behaviour and avoid surprises.
Apply this doc update:
- /// List of extension aliases to search for in absolute or relative paths. + /// List of extension aliases to try for relative specifiers. @@ - /// Note that this option is applicable only if the `moduleResolution` option in tsconfig.json - /// is set to `Node16` or `NodeNext`. + /// Applied only when `tsconfig.compilerOptions.moduleResolution` is `Node16` or `NodeNext` + /// (or when that field is omitted and `compilerOptions.module` is `Node16`/`NodeNext`). + /// Not used for `package.json` `exports`/`imports` resolutions.
432-445: Tighten applicability check — drop Node18/Node20TypeScript defaults moduleResolution to node16/nodenext when module is Node16/NodeNext, so only Node16/NodeNext should imply the Node resolution mode.
- let is_applicable = matches!( - ( - tsconfig_json.compiler_options.module, - tsconfig_json.compiler_options.module_resolution, - ), - ( - Some(Module::Node16 | Module::Node18 | Module::Node20 | Module::NodeNext), - None, - ) | ( - _, - Some(ModuleResolution::Node16 | ModuleResolution::NodeNext), - ) - ); + let is_applicable = match tsconfig_json.compiler_options.module_resolution { + Some(ModuleResolution::Node16 | ModuleResolution::NodeNext) => true, + None => matches!( + tsconfig_json.compiler_options.module, + Some(Module::Node16 | Module::NodeNext) + ), + _ => false, + };
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
.changeset/petite-waves-love.md(1 hunks)crates/biome_deserialize_macros/src/deserializable_derive.rs(6 hunks)crates/biome_deserialize_macros/src/deserializable_derive/container_attrs.rs(2 hunks)crates/biome_module_graph/src/js_module_info/visitor.rs(3 hunks)crates/biome_package/src/lib.rs(1 hunks)crates/biome_package/src/node_js_package/mod.rs(1 hunks)crates/biome_package/src/node_js_package/tsconfig_json.rs(1 hunks)crates/biome_resolver/src/lib.rs(11 hunks)crates/biome_resolver/tests/fixtures/resolver_cases_8/dir2/index.js(1 hunks)crates/biome_resolver/tests/fixtures/resolver_cases_8/package.json(1 hunks)crates/biome_resolver/tests/fixtures/resolver_cases_8/tsconfig.json(1 hunks)crates/biome_resolver/tests/spec_tests.rs(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- crates/biome_resolver/tests/fixtures/resolver_cases_8/package.json
🚧 Files skipped from review as they are similar to previous changes (8)
- crates/biome_package/src/node_js_package/mod.rs
- crates/biome_deserialize_macros/src/deserializable_derive/container_attrs.rs
- .changeset/petite-waves-love.md
- crates/biome_package/src/lib.rs
- crates/biome_resolver/tests/fixtures/resolver_cases_8/dir2/index.js
- crates/biome_module_graph/src/js_module_info/visitor.rs
- crates/biome_resolver/tests/fixtures/resolver_cases_8/tsconfig.json
- crates/biome_resolver/tests/spec_tests.rs
🧰 Additional context used
📓 Path-based instructions (2)
crates/biome_*/**
📄 CodeRabbit inference engine (CLAUDE.md)
Place core crates under /crates/biome_*/
Files:
crates/biome_package/src/node_js_package/tsconfig_json.rscrates/biome_deserialize_macros/src/deserializable_derive.rscrates/biome_resolver/src/lib.rs
**/*.rs
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Format all Rust source files before committing (just f)
Files:
crates/biome_package/src/node_js_package/tsconfig_json.rscrates/biome_deserialize_macros/src/deserializable_derive.rscrates/biome_resolver/src/lib.rs
🧠 Learnings (4)
📚 Learning: 2025-08-11T11:50:12.090Z
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_js_type_info/CONTRIBUTING.md:0-0
Timestamp: 2025-08-11T11:50:12.090Z
Learning: Applies to crates/biome_js_type_info/{src,resolver,biome_module_graph/src}/**/*.rs : Implement and use type resolution via the TypeResolver trait; resolvers own TypeStore vectors and provide fast by-id and hashed lookups
Applied to files:
crates/biome_package/src/node_js_package/tsconfig_json.rscrates/biome_resolver/src/lib.rs
📚 Learning: 2025-09-10T08:05:22.867Z
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-09-10T08:05:22.867Z
Learning: Applies to crates/biome_analyze/crates/biome_rule_options/lib/**/*.rs : Options types must implement serialization/deserialization and schema support using derives: `Serialize`, `Deserialize`, `Deserializable`, and `#[cfg_attr(feature = "schema", derive(schemars::JsonSchema))]`
Applied to files:
crates/biome_package/src/node_js_package/tsconfig_json.rs
📚 Learning: 2025-09-10T08:05:22.867Z
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-09-10T08:05:22.867Z
Learning: Applies to crates/biome_analyze/**/src/{lint,assist}/**/*.rs : Avoid avoidable string allocations: prefer comparing `&str` or using `TokenText` over calling `to_string()`
Applied to files:
crates/biome_deserialize_macros/src/deserializable_derive.rs
📚 Learning: 2025-08-11T11:48:52.001Z
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_js_formatter/CONTRIBUTING.md:0-0
Timestamp: 2025-08-11T11:48:52.001Z
Learning: Applies to crates/biome_js_formatter/**/Cargo.toml : Add biome_js_formatter as a path dependency in Cargo.toml: biome_js_formatter = { version = "0.0.1", path = "../biome_js_formatter" }
Applied to files:
crates/biome_resolver/src/lib.rs
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (20)
- GitHub Check: Bench (biome_css_formatter)
- GitHub Check: Bench (biome_json_formatter)
- GitHub Check: Bench (biome_module_graph)
- GitHub Check: Bench (biome_json_parser)
- GitHub Check: Bench (biome_configuration)
- GitHub Check: Bench (biome_html_formatter)
- GitHub Check: Bench (biome_js_parser)
- GitHub Check: Bench (biome_js_formatter)
- GitHub Check: Bench (biome_json_analyze)
- GitHub Check: Bench (biome_graphql_parser)
- GitHub Check: Bench (biome_graphql_formatter)
- GitHub Check: Bench (biome_html_parser)
- GitHub Check: Bench (biome_js_analyze)
- GitHub Check: Bench (biome_css_analyze)
- GitHub Check: Bench (biome_css_parser)
- GitHub Check: Documentation
- GitHub Check: Test (depot-windows-2022-16)
- GitHub Check: Test (depot-ubuntu-24.04-arm-16)
- GitHub Check: Check Dependencies
- GitHub Check: autofix
🔇 Additional comments (12)
crates/biome_deserialize_macros/src/deserializable_derive.rs (3)
67-71: Enum wiring for case_insensitive looks sound.Plumbs the attr cleanly into the enum data. No blocking issues here.
Please confirm the new #[deserializable(case_insensitive)] is documented and only applied to enums (struct/newtype paths ignore it).
169-173: Struct field addition: OK.Private bool is fine and carried through correctly.
281-288: Don’t rely on Deref quirks; match on an explicit &str and preserve the original for diagnostics.Shadowing text and then matching via Deref is brittle and loses the user’s original input when case_insensitive is on.
Apply this diff:
- let to_lowercase = if data.case_insensitive { - quote! { - let text = text.text().to_ascii_lowercase(); - } - } else { - quote! {} - }; + let prepare_needle = if data.case_insensitive { + quote! { + let original = text.text(); + let lowered_owned = original.to_ascii_lowercase(); + let needle: &str = &lowered_owned; + } + } else { + quote! { + let original = text.text(); + let needle: &str = original; + } + };- let text = biome_deserialize::Text::deserialize(ctx, value, name)?; - #to_lowercase - let mut result = match std::ops::Deref::deref(&text) { + let text = biome_deserialize::Text::deserialize(ctx, value, name)?; + #prepare_needle + let mut result = match needle {Optional polish (outside this hunk): in the unknown arm, report original rather than the lowercased needle.
- unknown_variant => { + _ => { const ALLOWED_VARIANTS: &[&str] = &[#(#allowed_variants),*]; - ctx.report(biome_deserialize::DeserializationDiagnostic::new_unknown_value( - unknown_variant, + ctx.report(biome_deserialize::DeserializationDiagnostic::new_unknown_value( + original, value.range(), ALLOWED_VARIANTS, )); return None; }Also applies to: 298-301
crates/biome_package/src/node_js_package/tsconfig_json.rs (3)
135-136: Serde rename for moduleResolution: spot‑on.This addresses the earlier deserialisation mismatch; field will populate correctly from tsconfig.
161-170: moduleResolution enum looks good.Case‑insensitive deserialisation matches TS; set is complete.
141-159: Keep Node18/Node20 — they’re valid TS module options (TS 5.8 / 5.9)
TypeScript added "node18" in 5.8 and "node20" in 5.9, so these are valid compilerOptions.module values; retain the enum variants but ensure serialization emits the lowercase TS names and that downstream gating explicitly handles Node16/Node18/Node20 (or document a minimum TS version).Likely an incorrect or invalid review comment.
crates/biome_resolver/src/lib.rs (6)
11-11: Re‑export of Module/ModuleResolution: good integration point.
48-51: Graceful fallback on relative NotFound is 👍.Ensures we still try package/tsconfig resolution before bailing.
258-263: Extension‑alias hook is placed sensibly.Runs after tsconfig paths/baseUrl checks and before deps.
Please confirm we have tests for both:
- moduleResolution omitted but module=Node16/NodeNext → alias applies
- moduleResolution=Classic/Node/Node10 → alias does not apply
950-958: Builder for extension_alias looks tidy.API is consistent with existing builders.
998-999: Nice: carry extension_alias across type‑roots cloning.
1013-1014: Good call to zero extension_alias when disabling manifests.Prevents unintended aliasing inside
exports/imports.
7713368 to
6390529
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
crates/biome_resolver/src/lib.rs(11 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
crates/biome_*/**
📄 CodeRabbit inference engine (CLAUDE.md)
Place core crates under /crates/biome_*/
Files:
crates/biome_resolver/src/lib.rs
**/*.rs
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Format all Rust source files before committing (just f)
Files:
crates/biome_resolver/src/lib.rs
🧠 Learnings (2)
📚 Learning: 2025-08-11T11:50:12.090Z
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_js_type_info/CONTRIBUTING.md:0-0
Timestamp: 2025-08-11T11:50:12.090Z
Learning: Applies to crates/biome_js_type_info/{src,resolver,biome_module_graph/src}/**/*.rs : Implement and use type resolution via the TypeResolver trait; resolvers own TypeStore vectors and provide fast by-id and hashed lookups
Applied to files:
crates/biome_resolver/src/lib.rs
📚 Learning: 2025-08-11T11:48:52.001Z
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_js_formatter/CONTRIBUTING.md:0-0
Timestamp: 2025-08-11T11:48:52.001Z
Learning: Applies to crates/biome_js_formatter/**/Cargo.toml : Add biome_js_formatter as a path dependency in Cargo.toml: biome_js_formatter = { version = "0.0.1", path = "../biome_js_formatter" }
Applied to files:
crates/biome_resolver/src/lib.rs
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Lint project (depot-windows-2022)
- GitHub Check: Bench (biome_js_parser)
- GitHub Check: Bench (biome_js_formatter)
🔇 Additional comments (8)
crates/biome_resolver/src/lib.rs (8)
11-11: LGTM! Clean extension of public imports.The imports align with the new public re-exports in the package system and support the extension alias functionality.
163-170: Function signature updated correctly.The addition of
base_dirparameter toresolve_module_with_package_jsonis properly propagated to all call sites.Also applies to: 176-183
258-263: Extension alias integration looks solid.The integration point is well-placed after paths mapping but before dependency resolution, maintaining the correct precedence order.
424-479: Well-implemented extension alias resolution.The logic correctly:
- Checks for Node16/NodeNext module resolution
- Handles only relative specifiers
- Tries each alias extension in order
- Falls back gracefully on NotFound
The implementation matches the TypeScript behaviour described in the PR objectives.
829-829: Minor documentation correction.Good catch on the wording - "without a leading dot" is clearer than "without the leading dot".
832-840: Extension alias API design is consistent.The new field and builder method follow the same patterns as existing options. The documentation clearly explains the Node16/NodeNext requirement.
Also applies to: 950-957
911-911: Extension alias properly propagated through option transformations.The field is correctly included in all option transformation methods, ensuring consistency across the resolver's internal state transitions.
Also applies to: 998-998, 1013-1013
48-52: Verify fallback logic for relative specifier resolution (crates/biome_resolver/src/lib.rs:48–56)resolve_relative_path merely delegates to resolve_absolute_path(base_dir.join(path)), so Err(ResolveError::NotFound) is intentionally allowed to fall through into tsconfig/alias/extension/module resolution — the same pattern is used elsewhere in this module. Confirm no tests or external callers rely on an immediate NotFound for relative specifiers and add/update tests if this change alters observable behaviour.
Co-authored-by: Arend van Beelen jr. <arend@arendjr.nl>
bde7fcf to
f3b2fc7
Compare
| return resolve_import_alias(specifier, package_path, package_json, fs, options); | ||
| } | ||
|
|
||
| if is_relative_specifier(specifier) { |
There was a problem hiding this comment.
I don't think this is_relative_specifier() check belongs here. It may break the use of extension aliases in combination with path aliases, I believe. Just in case I would suggest adding a test for that scenario.
Summary
Revamps #7158 and enables it only if the
moduleResolutionoption is set toNode16orNodeNextin tsconfig.json.Test Plan
Added tests.
Docs
N/A