feat(resolver): support baseUrl in tsconfig.json#7263
Conversation
🦋 Changeset detectedLatest commit: b99716b The changes in this PR will be included in the next version bump. This PR includes changesets to release 14 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 |
WalkthroughAdds tsconfig-aware resolution and baseUrl support to the module resolver. ResolveOptions gains Clone; DiscoverableManifest gets Clone/Debug/Default. The resolver now optionally loads tsconfig (Auto/Explicit/Off), attempts an early baseUrl-based resolution, and uses tsconfig.compiler_options.paths_base for path-alias resolution. Tsconfig parsing now initialises normalized paths_base/base_url and exposes CompilerOptions.paths_base publicly. Manifest deserialisers/insertors and many call sites now accept an explicit Utf8Path. Tests, fixtures and a changeset documenting baseUrl support were added. Assessment against linked issues
Out-of-scope changes
Possibly related PRs
Suggested reviewers
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. 📜 Recent review detailsConfiguration used: .coderabbit.yaml 💡 Knowledge Base configuration:
You can enable these sources in your CodeRabbit configuration. 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ 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). (24)
✨ Finishing Touches
🧪 Generate unit tests
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
Status, Documentation and Community
|
| let tsconfig = fs.read_tsconfig_json(&package_path.join("tsconfig.json")); | ||
| if let Some(path) = tsconfig.as_ref().ok().and_then(|ts_config| { | ||
| resolve_paths_mapping(specifier, ts_config, package_path, fs, options).ok() | ||
| let tsconfig = match &options.tsconfig { | ||
| DiscoverableManifest::Auto => fs | ||
| .read_tsconfig_json(&package_path.join("tsconfig.json")) | ||
| .map(Cow::Owned), | ||
| DiscoverableManifest::Explicit { manifest, .. } => Ok(Cow::Borrowed(*manifest)), | ||
| DiscoverableManifest::Off => Err(ResolveError::NotFound), | ||
| }; |
There was a problem hiding this comment.
This fixes a slightly unrelated oversight: If an explicit manifest was configured for tsconfig.json, it would be ignored in favour of the one on disk.
This came to light as I added tests.
| resolve_relative_path(specifier, base_dir, fs, options) | ||
| resolve_relative_path( | ||
| specifier, | ||
| &tsconfig_json.compiler_options.paths_base, |
There was a problem hiding this comment.
paths_base was explicitly intended for this purpose, although it was unused until now. I noticed this as I extended the documentation comments.
![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
🔭 Outside diff range comments (1)
crates/biome_package/src/node_js_package/tsconfig_json.rs (1)
72-75: Bug: base_url becomes None if the tsconfig path has no parentIf
path.parent()returnsNone, you drop a validbaseUrlby assigningNone. Keep the original value (normalised) when the directory is unavailable.Apply this diff:
- if let Some(base_url) = tsconfig.compiler_options.base_url { - tsconfig.compiler_options.base_url = - directory.map(|dir| normalize_path(&dir.join(base_url))); - } + if let Some(base_url) = tsconfig.compiler_options.base_url { + tsconfig.compiler_options.base_url = Some(if let Some(dir) = directory { + normalize_path(dir.join(&base_url).as_path()) + } else { + // Preserve the provided baseUrl (still normalised) if no parent directory is available + normalize_path(&base_url) + }); + }
🧹 Nitpick comments (7)
crates/biome_package/src/node_js_package/tsconfig_json.rs (2)
76-82: Normalise paths_base when falling back to the tsconfig directoryThe doc says this is an absolute, normalised path; the directory branch currently isn’t normalised. Normalising ensures consistent behaviour across relative invocations and platforms.
Apply this diff:
- tsconfig.compiler_options.paths_base = - tsconfig.compiler_options.base_url.as_ref().map_or_else( - || directory.map_or_else(Default::default, Utf8Path::to_path_buf), - Clone::clone, - ); + tsconfig.compiler_options.paths_base = tsconfig + .compiler_options + .base_url + .as_ref() + .map_or_else( + || directory.map_or_else(Default::default, |dir| normalize_path(dir)), + Clone::clone, + );
97-101: Clarify docs and invariant for paths_baseMinor: the comment references “base URL” but this field represents the base used for
pathsresolution. Also, consider initialising it unconditionally to a normalised tsconfig directory (so it’s always meaningful even whenpathsis absent), or keep it private to maintain the invariant that it’s a derived field.Apply this diff to the doc comment:
- /// The actual base from where path aliases are resolved. - /// - /// The base URL is normalised to an absolute path. + /// Base directory used to resolve entries in `compilerOptions.paths`. + /// + /// This is normalised to an absolute path after parsing.If you’d like, I can follow up with a PR to make this field private again and expose an accessor.
crates/biome_resolver/tests/fixtures/resolver_cases_7/tsconfig.json (1)
1-5: Fixture looks good; consider an additional absolute-path caseNice minimal fixture. Consider adding a companion case where baseUrl is absolute, and one where tsconfig lives in a subfolder, to guard path-join edge cases.
Happy to draft those fixtures and tests if helpful.
.changeset/based-bears-brawl.md (3)
5-8: Tighten prose to match changeset conventions (past tense + sentence endings).Use past tense for what changed and end sentences with a period. Suggested tweak below also removes the dangling “This …” line break.
-Biome's resolver now supports `baseUrl` if specified in `tsconfig.json`. This -means that the following now resolves: +Added resolver support for `baseUrl` in `tsconfig.json`. +The following now resolves.
9-15: Use jsonc for tsconfig examples (or remove trailing comma).tsconfig.json supports JSON with comments/trailing commas (JSONC). Label the block as jsonc to avoid confusion or remove the trailing comma. I’d go with jsonc:
-```json +```jsonc { "compilerOptions": { "baseUrl": "./src", } }--- `27-27`: **Ensure a trailing period at the end.** Changeset style guide asks every sentence to end with a period. If missing, add one. ```diff -Fixes [#6432](https://github.com/biomejs/biome/issues/6432) +Fixes [#6432](https://github.com/biomejs/biome/issues/6432).crates/biome_resolver/tests/spec_tests.rs (1)
537-616: Great coverage for baseUrl; minor readability nits.Solid assertions (explicit and auto tsconfig, different base dirs, subfolder, and no-baseUrl fallback). Two small nits:
- Prefer join() over format! for paths.
- Avoid a giant inline &TsConfigJson literal by introducing locals.
@@ fn test_resolve_from_base_url() { let base_dir = get_fixtures_path("resolver_cases_7"); let fs = OsFileSystem::new(base_dir.clone()); let options = ResolveOptions { condition_names: &["default"], extensions: &["js", "ts"], ..Default::default() }; + let tsconfig_path = base_dir.join("tsconfig.json"); + let tsconfig_with_base = TsConfigJson { + compiler_options: CompilerOptions { + base_url: Some(base_dir.join("src")), + ..Default::default() + }, + ..Default::default() + }; + // Make sure resolution works with explicitly specified `tsconfig.json`. assert_eq!( resolve( "bar", &base_dir.join("src"), &fs, - &options - .clone() - .with_tsconfig(DiscoverableManifest::Explicit { - package_path: Utf8PathBuf::from(format!("{base_dir}/tsconfig.json")), - manifest: &TsConfigJson { - compiler_options: CompilerOptions { - base_url: Some(base_dir.join("src")), - ..Default::default() - }, - ..Default::default() - } - }) + &options.clone().with_tsconfig(DiscoverableManifest::Explicit { + package_path: tsconfig_path.clone(), + manifest: &tsconfig_with_base, + }) ), - Ok(Utf8PathBuf::from(format!("{base_dir}/src/bar.ts"))) + Ok(base_dir.join("src/bar.ts")) ); @@ - assert_eq!( - resolve("bar", &base_dir.join("src"), &fs, &options), - Ok(Utf8PathBuf::from(format!("{base_dir}/src/bar.ts"))) - ); + assert_eq!( + resolve("bar", &base_dir.join("src"), &fs, &options), + Ok(base_dir.join("src/bar.ts")) + ); @@ - assert_eq!( - resolve("bar", &base_dir, &fs, &options), - Ok(Utf8PathBuf::from(format!("{base_dir}/src/bar.ts"))) - ); + assert_eq!(resolve("bar", &base_dir, &fs, &options), Ok(base_dir.join("src/bar.ts"))); assert_eq!( resolve("bar", &base_dir.join("src/foo"), &fs, &options), - Ok(Utf8PathBuf::from(format!("{base_dir}/src/bar.ts"))) + Ok(base_dir.join("src/bar.ts")) ); @@ - assert_eq!( - resolve("foo/foo", &base_dir.join("src"), &fs, &options), - Ok(Utf8PathBuf::from(format!("{base_dir}/src/foo/foo.ts"))) - ); + assert_eq!( + resolve("foo/foo", &base_dir.join("src"), &fs, &options), + Ok(base_dir.join("src/foo/foo.ts")) + ); @@ + let tsconfig_without_base = TsConfigJson { + compiler_options: CompilerOptions { + base_url: None, + ..Default::default() + }, + ..Default::default() + }; assert_eq!( resolve( "bar", &base_dir.join("src"), &fs, &options .clone() .with_tsconfig(DiscoverableManifest::Explicit { - package_path: Utf8PathBuf::from(format!("{base_dir}/tsconfig.json")), - manifest: &TsConfigJson { - compiler_options: CompilerOptions { - base_url: None, - ..Default::default() - }, - ..Default::default() - } + package_path: tsconfig_path, + manifest: &tsconfig_without_base, }) ), Ok(Utf8PathBuf::from(format!( "{base_dir}/node_modules/bar/index.js" ))) ); }
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (2)
crates/biome_resolver/tests/fixtures/resolver_cases_7/node_modules/bar/index.jsis excluded by!**/node_modules/**and included by**crates/biome_resolver/tests/fixtures/resolver_cases_7/node_modules/bar/package.jsonis excluded by!**/node_modules/**and included by**
📒 Files selected for processing (8)
.changeset/based-bears-brawl.md(1 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(5 hunks)crates/biome_resolver/tests/fixtures/resolver_cases_7/package.json(1 hunks)crates/biome_resolver/tests/fixtures/resolver_cases_7/tsconfig.json(1 hunks)crates/biome_resolver/tests/spec_tests.rs(2 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_7/package.jsoncrates/biome_resolver/tests/fixtures/resolver_cases_7/tsconfig.jsoncrates/biome_package/src/node_js_package/mod.rscrates/biome_package/src/lib.rscrates/biome_resolver/tests/spec_tests.rscrates/biome_package/src/node_js_package/tsconfig_json.rscrates/biome_resolver/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_7/package.jsoncrates/biome_resolver/tests/fixtures/resolver_cases_7/tsconfig.jsoncrates/biome_resolver/tests/spec_tests.rs
**/*.{rs,toml}
📄 CodeRabbit Inference Engine (CONTRIBUTING.md)
Format Rust and TOML files before committing (use
just f/just format).
Files:
crates/biome_package/src/node_js_package/mod.rscrates/biome_package/src/lib.rscrates/biome_resolver/tests/spec_tests.rscrates/biome_package/src/node_js_package/tsconfig_json.rscrates/biome_resolver/src/lib.rs
.changeset/*.md
📄 CodeRabbit Inference Engine (CONTRIBUTING.md)
.changeset/*.md: Create changesets withjust new-changeset; store them in.changeset/with correct frontmatter (package keys and change type).
In changeset descriptions, follow content conventions: user-facing changes only; past tense for what you did; present tense for current behavior; link issues for fixes; link rules/assists; include representative code blocks; end every sentence with a period.
When adding headers in a changeset, only use #### or ##### levels.
Files:
.changeset/based-bears-brawl.md
🧠 Learnings (13)
📚 Learning: 2025-08-17T08:56:30.822Z
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-08-17T08:56:30.822Z
Learning: Applies to crates/biome_analyze/crates/**/tests/specs/**/*.jsonc : `.jsonc` snapshot files must contain an array of code strings; they are interpreted in script mode (no ESM syntax)
Applied to files:
crates/biome_resolver/tests/fixtures/resolver_cases_7/package.jsoncrates/biome_resolver/tests/fixtures/resolver_cases_7/tsconfig.json
📚 Learning: 2025-08-17T08:56:30.822Z
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-08-17T08:56:30.822Z
Learning: Applies to crates/biome_analyze/crates/**/tests/specs/**/{invalid*,valid*}.{js,jsx,ts,tsx,css,graphql,jsonc} : Place snapshot test cases under `tests/specs/<group>/<ruleName>/` using files prefixed with `invalid` and `valid`
Applied to files:
crates/biome_resolver/tests/fixtures/resolver_cases_7/tsconfig.json
📚 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/tests/spec_tests.rscrates/biome_resolver/src/lib.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_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: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.rscrates/biome_resolver/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_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-17T08:56:30.822Z
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-08-17T08:56:30.822Z
Learning: Applies to crates/biome_analyze/crates/biome_js_analyze/tests/quick_test.rs : Use `biome_js_analyze/tests/quick_test.rs` for quick, ad-hoc testing; un-ignore the test and adjust the rule filter as needed
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/**/*.rs : Use TypeData::Unknown for unimplemented inference and TypeData::UnknownKeyword for the explicit TypeScript unknown keyword; treat them semantically the same but keep them distinct for measurement
Applied to files:
crates/biome_resolver/tests/spec_tests.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/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/**/*.rs : Represent links between types using TypeReference (not Arc) to avoid cross-module retention and recursive structures; store type data in linear vectors
Applied to files:
crates/biome_resolver/src/lib.rs
📚 Learning: 2025-08-17T08:56:30.822Z
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-08-17T08:56:30.822Z
Learning: Applies to crates/biome_analyze/crates/biome_rule_options/lib/**/*.rs : Define per-rule options in the `biome_rule_options` crate under `lib/`, with serde- and schemars-compatible derives and `#[serde(rename_all = "camelCase", deny_unknown_fields, default)]`
Applied to files:
crates/biome_resolver/src/lib.rs
🧬 Code Graph Analysis (1)
crates/biome_resolver/tests/spec_tests.rs (1)
crates/biome_resolver/src/lib.rs (8)
new(807-819)new(1036-1038)default(800-802)resolve(31-59)with_tsconfig(873-876)from(1018-1020)from(1024-1026)from(1030-1032)
⏰ 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). (27)
- GitHub Check: Documentation
- GitHub Check: End-to-end tests
- GitHub Check: Check Dependencies
- GitHub Check: Lint project (depot-windows-2022)
- GitHub Check: Lint project (depot-ubuntu-24.04-arm-16)
- GitHub Check: Test (depot-windows-2022-16)
- GitHub Check: Test (depot-ubuntu-24.04-arm-16)
- GitHub Check: Bench (biome_graphql_formatter)
- GitHub Check: Bench (biome_configuration)
- GitHub Check: Bench (biome_package)
- GitHub Check: Bench (biome_module_graph)
- GitHub Check: Bench (biome_html_parser)
- GitHub Check: Bench (biome_html_formatter)
- GitHub Check: Bench (biome_json_analyze)
- GitHub Check: Bench (biome_js_analyze)
- GitHub Check: Bench (biome_graphql_parser)
- GitHub Check: Bench (biome_css_analyze)
- GitHub Check: Bench (biome_json_parser)
- GitHub Check: Bench (biome_json_formatter)
- GitHub Check: Bench (biome_js_formatter)
- GitHub Check: Bench (biome_css_formatter)
- GitHub Check: Bench (biome_css_parser)
- GitHub Check: Bench (biome_js_parser)
- GitHub Check: Validate PR title
- GitHub Check: autofix
- GitHub Check: triage
- GitHub Check: lint
🔇 Additional comments (10)
crates/biome_package/src/node_js_package/mod.rs (1)
5-5: LGTM: re-exporting CompilerOptions is a sensible API additionMatches the resolver/tests changes and keeps consumers in one import path. Make sure it’s covered by a changeset (looks like it is).
crates/biome_package/src/lib.rs (1)
13-14: LGTM: crate-level re-export keeps the public surface coherentThis aligns with the module re-export and simplifies test imports.
crates/biome_resolver/tests/fixtures/resolver_cases_7/package.json (1)
1-3: LGTM: minimal package.json is sufficient for the fixtureNo issues spotted.
crates/biome_resolver/tests/spec_tests.rs (1)
2-2: Import exposure looks good.Re-export of CompilerOptions and TsConfigJson from biome_package is used correctly here.
crates/biome_resolver/src/lib.rs (6)
186-196: Neat: option-driven tsconfig handling and early paths mapping.Reading tsconfig via DiscoverableManifest and trying compilerOptions.paths first is clean and keeps behaviour well-scoped.
231-241: baseUrl pre-resolution ordering — please confirm precedence.You try package import aliases (“#…”) and self-exports before baseUrl. That’s likely sensible, but TypeScript’s baseUrl is a compile-time feature and some ecosystems expect it to precede package self-exports. Worth a quick confirmation to avoid surprising lookups.
Would you like me to add a spec asserting that a self-export doesn’t shadow a baseUrl file (or vice versa), documenting the intended precedence?
705-705: Deriving Clone on ResolveOptions is handy.Enables ergonomic test composition (as used) and builder chaining without surprising moves.
919-928: DiscoverableManifest gains Clone/Debug/Default — 👍Default to Auto and better debuggability are both welcome; aligns with test ergonomics.
173-193: Assumption: only resolve tsconfig when a package.json is present.The comment and code mean Auto tsconfig is never consulted if no package.json is found up the tree. Some setups have a tsconfig without a package.json (e.g. pure TS libs). If that’s intentional: great. If not, consider allowing tsconfig discovery independent of package.json.
Happy to add an additional discovery path and a spec (e.g. baseUrl works in a fixture without package.json) if you want to support that case.
355-363: No non-rooted tsconfig “paths” targets found—optional refactor not needed.I ran the scan across all
tsconfig.jsonfixtures and didn’t find any non-rooted path targets, so this change wouldn’t affect existing configs. Treating “src/*” as dependencies remains safe in our codebase today.
| /// https://www.typescriptlang.org/tsconfig/#baseUrl | ||
| /// | ||
| /// The base URL is normalised to an absolute path after parsing. | ||
| pub base_url: Option<Utf8PathBuf>, | ||
|
|
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Ensure JSON key maps to base_url (baseUrl) during deserialisation
Unless the derive macro auto-maps camelCase to snake_case, base_url won’t deserialize from baseUrl. Adding an explicit rename is cheap insurance.
Apply this diff:
/// https://www.typescriptlang.org/tsconfig/#baseUrl
///
/// The base URL is normalised to an absolute path after parsing.
- pub base_url: Option<Utf8PathBuf>,
+ #[deserializable(rename = "baseUrl")]
+ pub base_url: Option<Utf8PathBuf>,To confirm whether auto-mapping is already handled, run:
🏁 Script executed:
#!/bin/bash
# Inspect how camelCase keys are handled elsewhere and if baseUrl mapping exists
rg -n -C2 --type=rust 'Deserializable.*rename'
rg -n -C3 --type=rust '\bcompiler_options\b'
rg -n -C2 --type=rust '"baseUrl"|base_url'Length of output: 9285
I want to double-check how TsConfigJson is derived and whether there’s a rename_all = "camelCase" in place.
🏁 Script executed:
#!/bin/bash
# Inspect TsConfigJson derive and serde attributes
rg -n '#\[derive' crates/biome_package/src/node_js_package/tsconfig_json.rs
rg -n 'rename_all' -C2 --type=rust crates/biome_package/src/node_js_package/tsconfig_json.rs
rg -n 'serde' -C2 --type=rust crates/biome_package/src/node_js_package/tsconfig_json.rsLength of output: 458
Let’s peek at the top of tsconfig_json.rs to see which derive and import paths Deserializable comes from:
🏁 Script executed:
#!/bin/bash
sed -n '1,30p' crates/biome_package/src/node_js_package/tsconfig_json.rsLength of output: 1081
Explicitly rename base_url to match JSON baseUrl
The TsConfigJson derive has no rename_all, so base_url will map to "base_url", not "baseUrl". Add an explicit rename:
/// The base URL is normalised to an absolute path after parsing.
- pub base_url: Option<Utf8PathBuf>,
+ #[deserializable(rename = "baseUrl")]
+ pub base_url: Option<Utf8PathBuf>,📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| /// https://www.typescriptlang.org/tsconfig/#baseUrl | |
| /// | |
| /// The base URL is normalised to an absolute path after parsing. | |
| pub base_url: Option<Utf8PathBuf>, | |
| /// https://www.typescriptlang.org/tsconfig/#baseUrl | |
| /// | |
| /// The base URL is normalised to an absolute path after parsing. | |
| #[deserializable(rename = "baseUrl")] | |
| pub base_url: Option<Utf8PathBuf>, |
🤖 Prompt for AI Agents
In crates/biome_package/src/node_js_package/tsconfig_json.rs around lines 89 to
93, the struct field `base_url` will serialize/deserialize as "base_url" because
the derive has no rename_all; add an explicit serde rename to match the
TypeScript tsconfig key by annotating the field with #[serde(rename =
"baseUrl")] so it maps to "baseUrl" for both serialization and deserialization.
CodSpeed Performance ReportMerging #7263 will improve performances by ×2.7Comparing Summary
Benchmarks breakdown
Footnotes |
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
crates/biome_package/src/node_js_package/tsconfig_json.rs (1)
104-111: baseUrl won’t deserialize without an explicit rename
CompilerOptions.base_urlcurrently won’t map from"baseUrl"(no rename_all on the derive). This breaks the new feature end-to-end.Add an explicit rename so
"baseUrl"in tsconfig.json is deserialised:/// https://www.typescriptlang.org/tsconfig/#baseUrl /// /// The base URL is normalised to an absolute path after parsing. - pub base_url: Option<Utf8PathBuf>, + #[deserializable(rename = "baseUrl")] + pub base_url: Option<Utf8PathBuf>,To confirm coverage after the change, I can help add/adjust a test asserting
compiler_options.base_url.is_some()for a tsconfig containing"baseUrl".
🧹 Nitpick comments (2)
crates/biome_package/src/node_js_package/tsconfig_json.rs (2)
86-101: Avoid dropping baseUrl when the tsconfig has no parent directoryIf
path.parent()isNone, the current code overwrites a presentbase_urlwithNone. That’s surprising even if most call sites pass a file path with a parent.Apply this to preserve the value when there’s no parent:
- if let Some(base_url) = self.compiler_options.base_url.as_ref() { - self.compiler_options.base_url = - directory.map(|dir| normalize_path(&dir.join(base_url))); - } + if let Some(base_url) = self.compiler_options.base_url.as_ref() { + let resolved = directory + .map(|dir| normalize_path(&dir.join(base_url))) + .unwrap_or_else(|| base_url.clone()); + self.compiler_options.base_url = Some(resolved); + }
115-119: Doc string nit: wrong field referencedThis comment sits on
paths_basebut talks about “The base URL”.Tighten the wording:
- /// - /// The base URL is normalised to an absolute path. + /// + /// Normalised to an absolute path.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (6)
crates/biome_package/src/lib.rs(3 hunks)crates/biome_package/src/node_js_package/mod.rs(3 hunks)crates/biome_package/src/node_js_package/package_json.rs(1 hunks)crates/biome_package/src/node_js_package/tsconfig_json.rs(3 hunks)crates/biome_package/tests/manifest_spec_tests.rs(4 hunks)crates/biome_project_layout/src/project_layout.rs(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- crates/biome_package/src/node_js_package/mod.rs
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{rs,toml}
📄 CodeRabbit Inference Engine (CONTRIBUTING.md)
Format Rust and TOML files before committing (use
just f/just format).
Files:
crates/biome_package/src/node_js_package/package_json.rscrates/biome_project_layout/src/project_layout.rscrates/biome_package/tests/manifest_spec_tests.rscrates/biome_package/src/lib.rscrates/biome_package/src/node_js_package/tsconfig_json.rs
crates/biome_*/**
📄 CodeRabbit Inference Engine (CLAUDE.md)
Place core crates under /crates/biome_*/
Files:
crates/biome_package/src/node_js_package/package_json.rscrates/biome_project_layout/src/project_layout.rscrates/biome_package/tests/manifest_spec_tests.rscrates/biome_package/src/lib.rscrates/biome_package/src/node_js_package/tsconfig_json.rs
**/tests/**
📄 CodeRabbit Inference Engine (CLAUDE.md)
Place test files under a tests/ directory in each crate
Files:
crates/biome_package/tests/manifest_spec_tests.rs
🧠 Learnings (6)
📚 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_package/tests/manifest_spec_tests.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_package/tests/manifest_spec_tests.rscrates/biome_package/src/lib.rscrates/biome_package/src/node_js_package/tsconfig_json.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_package/tests/manifest_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_package/tests/manifest_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_package/tests/manifest_spec_tests.rs
📚 Learning: 2025-08-11T11:46:05.836Z
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_diagnostics/CONTRIBUTING.md:0-0
Timestamp: 2025-08-11T11:46:05.836Z
Learning: Applies to crates/biome_diagnostics/**/*.rs : Types implementing Diagnostic must also implement Debug (e.g., use #[derive(Debug, Diagnostic)])
Applied to files:
crates/biome_package/tests/manifest_spec_tests.rs
🧬 Code Graph Analysis (1)
crates/biome_package/tests/manifest_spec_tests.rs (1)
crates/biome_module_graph/src/js_module_info/visitor.rs (1)
new(28-34)
⏰ 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). (24)
- GitHub Check: Documentation
- GitHub Check: Check Dependencies
- GitHub Check: Test (depot-ubuntu-24.04-arm-16)
- GitHub Check: End-to-end tests
- GitHub Check: Test (depot-windows-2022-16)
- GitHub Check: Lint project (depot-windows-2022)
- GitHub Check: Lint project (depot-ubuntu-24.04-arm-16)
- GitHub Check: Bench (biome_json_analyze)
- GitHub Check: Bench (biome_configuration)
- GitHub Check: Bench (biome_graphql_formatter)
- GitHub Check: Bench (biome_html_parser)
- GitHub Check: Bench (biome_json_parser)
- GitHub Check: Bench (biome_html_formatter)
- GitHub Check: Bench (biome_css_analyze)
- GitHub Check: Bench (biome_js_formatter)
- GitHub Check: Bench (biome_package)
- GitHub Check: Bench (biome_graphql_parser)
- GitHub Check: Bench (biome_module_graph)
- GitHub Check: Bench (biome_json_formatter)
- GitHub Check: Bench (biome_css_formatter)
- GitHub Check: Bench (biome_js_parser)
- GitHub Check: Bench (biome_js_analyze)
- GitHub Check: Bench (biome_css_parser)
- GitHub Check: autofix
🔇 Additional comments (12)
crates/biome_package/src/node_js_package/package_json.rs (1)
109-113: Path-aware signature added: looks goodThe extra
_path: &Utf8Pathparameter aligns this manifest with the new path-aware flow. Keeping it underscore-prefixed is fine until it’s needed.crates/biome_project_layout/src/project_layout.rs (2)
129-129: Good: pass the concrete package.json file path into deserialisation
- Using
path.clone()for the map key while retainingpathfor the closures is the right move.- Passing
&path.join("package.json")ensures the manifest gets the real file path, which improves downstream diagnostics and path normalisation.Also applies to: 140-144, 151-155
167-167: Good: thread the concrete tsconfig.json path into insertionMirrors the manifest change; this will let
TsConfigJsoninitialisebaseUrl/paths_basecorrectly relative to the tsconfig’s directory.Also applies to: 178-182, 189-193
crates/biome_package/tests/manifest_spec_tests.rs (4)
4-4: Using Utf8Path in tests is appropriateCamino’s Utf8Path is consistent with the rest of the codebase and avoids encoding surprises on snapshots.
17-19: Manifest test pathing: all setConverting
inputtoUtf8Pathand passing it intoinsert_serialized_manifestis in line with the new API. Unwrappingfile_name()is safe in this harness.Also applies to: 23-27
71-73: Invalid tsconfig tests: correct propagation of the tsconfig pathForwarding
input_filetoinsert_serialized_tsconfigensures path-based initialisation happens during parsing. Comment/trailing comma options are preserved.Also applies to: 77-84
128-130: Valid tsconfig tests: correct usageSame as the invalid path: forwarding the concrete tsconfig path is essential for
baseUrlnormalisation. Looks good.Also applies to: 134-141
crates/biome_package/src/lib.rs (2)
13-14: Re-exporting CompilerOptions is a sensible public API additionThis allows resolver/tests to depend on the canonical type. No concerns.
33-37: Allinsert_serialized_*calls now receive full file pathsI’ve checked every call site—for both
insert_serialized_manifestandinsert_serialized_tsconfig—and they all pass a path that includes the filename (never just the directory):
- Manifest
- crates/biome_project_layout/src/project_layout.rs:140,151 →
&path.join("package.json")- crates/biome_package/tests/manifest_spec_tests.rs:26 →
input_file(contains filename)- TSConfig
- crates/biome_project_layout/src/project_layout.rs:178,189 →
&path.join("tsconfig.json")- crates/biome_package/tests/manifest_spec_tests.rs:83 & 140 →
input_file- crates/biome_module_graph/tests/spec_tests.rs:96 & 1503 → project layout joins internally
- crates/biome_service/src/workspace/server.rs:686 → passes package directory into project layout, which appends
"tsconfig.json"No directories-only slips found—this is good to go.
crates/biome_package/src/node_js_package/tsconfig_json.rs (3)
42-53: Path-aware deserialisation + post-pass initialisation: goodDeserialising from the AST and then invoking
initialise_paths(path)is the right layering for path-dependent fields.
58-60: read_manifest now defers to a path-aware parse: goodThis keeps a single code path for normalising fields and reduces drift between in-memory and on-disk parsing.
67-82: parse(path, json): shape and options look correctAllowing comments and trailing commas matches tsconfig reality. Using
path.file_name().unwrap_or("tsconfig.json")for the parse context is fine.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
crates/biome_package/tests/manifest_spec_tests.rs (3)
23-27: Prefer matching on "json" directly (ditch the byte conversion).
Utf8Path::extension()already returnsOption<&str>, so comparing bytes is unnecessary. This trims noise and reads better.Apply these diffs:
--- a +++ b @@ - match input_file.extension().map(str::as_bytes) { - Some(b"json") => { + match input_file.extension() { + Some("json") => { let parsed = parse_json(input_code.as_str(), JsonParserOptions::default()); package.insert_serialized_manifest(&parsed.tree(), input_file); } _ => { panic!("Extension not supported"); } };--- a +++ b @@ - match input_file.extension().map(str::as_bytes) { - Some(b"json") => { + match input_file.extension() { + Some("json") => { let parsed = parse_json( input_code.as_str(), JsonParserOptions::default().with_allow_comments(), ); project.insert_serialized_tsconfig(&parsed.tree(), input_file); } _ => { panic!("Extension not supported"); } };--- a +++ b @@ - match input_file.extension().map(str::as_bytes) { - Some(b"json") => { + match input_file.extension() { + Some("json") => { let parsed = parse_json( input_code.as_str(), JsonParserOptions::default().with_allow_comments(), ); project.insert_serialized_tsconfig(&parsed.tree(), input_file); } _ => { panic!("Extension not supported"); } };Also applies to: 77-85, 135-141
156-169: Make strip_prefix resilient to unexpected inputs.
assert!(path.is_absolute())will panic if a path slips through that isn’t absolute, which is brittle in a test harness. Downgrade to a debug assertion and early-return to avoid spurious panics. Behaviour remains the same for correct fixtures.- let strip_prefix = |path: &mut Utf8PathBuf| { - assert!(path.is_absolute()); + let strip_prefix = |path: &mut Utf8PathBuf| { + debug_assert!(path.is_absolute(), "expected absolute path in fixtures"); + if !path.is_absolute() { + return; + } let mut stripped_path = Utf8PathBuf::from("<PREFIX>"); let mut past_prefix = false; for component in path.components() { if past_prefix { stripped_path.push(component); } else if component == Utf8Component::Normal("tests") { past_prefix = true; } } *path = stripped_path; };
170-178: Prefer expect over unwrap; avoid using map for side-effects.
- unwrap() produces a vague panic; expect() documents intent.
- Using Option::map for mutation is a code smell;
if let Some(..)reads clearer.- let mut tsconfig = project.tsconfig.unwrap(); + let mut tsconfig = project + .tsconfig + .expect("expected tsconfig to be present for valid fixtures"); @@ - tsconfig - .compiler_options - .base_url - .as_mut() - .map(strip_prefix); + if let Some(base_url) = tsconfig.compiler_options.base_url.as_mut() { + strip_prefix(base_url); + }
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (2)
crates/biome_package/tests/tsconfig/valid/tsconfig.valid.baseUrl.json.snapis excluded by!**/*.snapand included by**crates/biome_package/tests/tsconfig/valid/tsconfig.valid.paths.json.snapis excluded by!**/*.snapand included by**
📒 Files selected for processing (1)
crates/biome_package/tests/manifest_spec_tests.rs(5 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{rs,toml}
📄 CodeRabbit Inference Engine (CONTRIBUTING.md)
Format Rust and TOML files before committing (use
just f/just format).
Files:
crates/biome_package/tests/manifest_spec_tests.rs
crates/biome_*/**
📄 CodeRabbit Inference Engine (CLAUDE.md)
Place core crates under /crates/biome_*/
Files:
crates/biome_package/tests/manifest_spec_tests.rs
**/tests/**
📄 CodeRabbit Inference Engine (CLAUDE.md)
Place test files under a tests/ directory in each crate
Files:
crates/biome_package/tests/manifest_spec_tests.rs
🧠 Learnings (6)
📚 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/tests/manifest_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_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_package/tests/manifest_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_package/tests/manifest_spec_tests.rs
📚 Learning: 2025-08-17T08:55:30.109Z
Learnt from: CR
PR: biomejs/biome#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-17T08:55:30.109Z
Learning: Applies to **/tests/specs/**/*.snap : Use snapshot spec tests stored as .snap files under tests/specs/ (via cargo insta)
Applied to files:
crates/biome_package/tests/manifest_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_package/tests/manifest_spec_tests.rs
📚 Learning: 2025-08-11T11:46:05.836Z
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_diagnostics/CONTRIBUTING.md:0-0
Timestamp: 2025-08-11T11:46:05.836Z
Learning: Applies to crates/biome_diagnostics/**/*.rs : Types implementing Diagnostic must also implement Debug (e.g., use #[derive(Debug, Diagnostic)])
Applied to files:
crates/biome_package/tests/manifest_spec_tests.rs
🧬 Code Graph Analysis (1)
crates/biome_package/tests/manifest_spec_tests.rs (4)
crates/biome_module_graph/src/js_module_info/visitor.rs (1)
new(28-34)crates/biome_module_graph/src/module_graph/fs_proxy.rs (1)
new(15-25)crates/biome_json_parser/src/lib.rs (1)
parse_json(23-26)crates/biome_module_graph/src/module_graph.rs (1)
from(251-253)
⏰ 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). (23)
- GitHub Check: Test (depot-ubuntu-24.04-arm-16)
- GitHub Check: Documentation
- GitHub Check: Test (depot-windows-2022-16)
- GitHub Check: Check Dependencies
- GitHub Check: Lint project (depot-ubuntu-24.04-arm-16)
- GitHub Check: Lint project (depot-windows-2022)
- GitHub Check: Bench (biome_package)
- GitHub Check: Bench (biome_graphql_formatter)
- GitHub Check: Bench (biome_configuration)
- GitHub Check: Bench (biome_html_parser)
- GitHub Check: Bench (biome_module_graph)
- GitHub Check: Bench (biome_css_parser)
- GitHub Check: Bench (biome_json_analyze)
- GitHub Check: Bench (biome_js_parser)
- GitHub Check: Bench (biome_html_formatter)
- GitHub Check: Bench (biome_json_parser)
- GitHub Check: Bench (biome_js_analyze)
- GitHub Check: Bench (biome_css_analyze)
- GitHub Check: Bench (biome_graphql_parser)
- GitHub Check: Bench (biome_json_formatter)
- GitHub Check: Bench (biome_css_formatter)
- GitHub Check: Bench (biome_js_formatter)
- GitHub Check: autofix
🔇 Additional comments (6)
crates/biome_package/tests/manifest_spec_tests.rs (6)
4-4: Good call switching to Camino’s Utf8 types.Using Utf8Path/Utf8PathBuf makes these tests cross-platform and avoids OsStr churn. Nice.
17-18: LGTM on path handling initialisation.Constructing Utf8Path and deriving file_name early keeps things tidy and avoids platform quirks.
26-26: API updates wired correctly.Passing the path into insert_serialized_* aligns with the new path-aware APIs. All good here.
Also applies to: 83-83, 140-140
71-73: LGTM on tsconfig invalid harness updates.Consistent with the manifest changes; keeps snapshots stable.
128-130: LGTM on tsconfig valid harness updates.Same tidy path initialisation; consistent across test suites.
183-183: Snapshotting the structured tsconfig is a win.Using pretty Debug output ensures stable, reviewable snapshots post path sanitisation.
arendjr
force-pushed
the
support-baseUrl
branch
from
ca80f42 to
afba8bc
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (4)
crates/biome_package/tests/manifest_spec_tests.rs (4)
71-72: Repeat: swap unwrap for expect here tooSame rationale as above; better panic message if something’s off in the generated path.
- let file_name = input_file.file_name().unwrap(); + let file_name = input_file + .file_name() + .expect("expected a file name in generated test path");
77-85: Repeat: simplify extension check here as wellStick to the &str returned by Utf8Path::extension.
- match input_file.extension().map(str::as_bytes) { - Some(b"json") => { + match input_file.extension() { + Some("json") => { let parsed = parse_json( input_code.as_str(), JsonParserOptions::default().with_allow_comments(), ); project.insert_serialized_tsconfig(&parsed.tree(), input_file); }
128-129: Repeat: favour expect over unwrapConsistent, readable test failures.
- let file_name = input_file.file_name().unwrap(); + let file_name = input_file + .file_name() + .expect("expected a file name in generated test path");
134-141: Repeat: extension check can stay in &str landMinor cleanup for consistency and readability.
- match input_file.extension().map(str::as_bytes) { - Some(b"json") => { + match input_file.extension() { + Some("json") => { let parsed = parse_json( input_code.as_str(), JsonParserOptions::default().with_allow_comments(), ); project.insert_serialized_tsconfig(&parsed.tree(), input_file); }
🧹 Nitpick comments (5)
crates/biome_package/tests/manifest_spec_tests.rs (5)
17-18: Prefer expect over unwrap for clearer failuresIf a test path ever misses a file name, this will panic without context. Expect reads better in test output.
- let file_name = input_file.file_name().unwrap(); + let file_name = input_file + .file_name() + .expect("expected a file name in generated test path");
23-27: Simplify extension check with Utf8PathUtf8Path::extension returns Option<&str>, so there’s no need to hop through bytes.
- match input_file.extension().map(str::as_bytes) { - Some(b"json") => { + match input_file.extension() { + Some("json") => { let parsed = parse_json(input_code.as_str(), JsonParserOptions::default()); package.insert_serialized_manifest(&parsed.tree(), input_file); }
156-160: Avoid allocation in empty-path checkto_string() allocates; you can query the underlying &str directly.
- if path.to_string().is_empty() { + if path.as_str().is_empty() { return; }
161-172: Consider a fallback when "tests" isn’t in the pathIf "tests" isn’t present, you end up with "" and lose useful context in the snapshot. Not urgent, but a small guard would make this future‑proof if paths ever change.
One approach:
- Detect if the "tests" component was found; if not, append the last 2 components so snapshots still show a stable, informative tail.
Sketch:
let mut found = false; for component in path.components() { if past_prefix { stripped_path.push(component); } else if component == Utf8Component::Normal("tests") { past_prefix = true; found = true; } } if !found { if let Some(parent) = path.parent() { stripped_path.push(parent.file_name().unwrap_or_default()); } if let Some(name) = path.file_name() { stripped_path.push(name); } }
178-181: Don’t use Option::map for side effectsmap is for transformations; if let reads better for mutation/side‑effects.
- tsconfig - .compiler_options - .base_url - .as_mut() - .map(strip_prefix); + if let Some(base) = tsconfig.compiler_options.base_url.as_mut() { + strip_prefix(base); + }
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (2)
crates/biome_package/tests/tsconfig/valid/tsconfig.valid.baseUrl.json.snapis excluded by!**/*.snapand included by**crates/biome_package/tests/tsconfig/valid/tsconfig.valid.paths.json.snapis excluded by!**/*.snapand included by**
📒 Files selected for processing (1)
crates/biome_package/tests/manifest_spec_tests.rs(5 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{rs,toml}
📄 CodeRabbit Inference Engine (CONTRIBUTING.md)
Format Rust and TOML files before committing (use
just f/just format).
Files:
crates/biome_package/tests/manifest_spec_tests.rs
crates/biome_*/**
📄 CodeRabbit Inference Engine (CLAUDE.md)
Place core crates under /crates/biome_*/
Files:
crates/biome_package/tests/manifest_spec_tests.rs
**/tests/**
📄 CodeRabbit Inference Engine (CLAUDE.md)
Place test files under a tests/ directory in each crate
Files:
crates/biome_package/tests/manifest_spec_tests.rs
🧠 Learnings (5)
📚 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_package/tests/manifest_spec_tests.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_package/tests/manifest_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_package/tests/manifest_spec_tests.rs
📚 Learning: 2025-08-11T11:46:05.836Z
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_diagnostics/CONTRIBUTING.md:0-0
Timestamp: 2025-08-11T11:46:05.836Z
Learning: Applies to crates/biome_diagnostics/**/*.rs : Types implementing Diagnostic must also implement Debug (e.g., use #[derive(Debug, Diagnostic)])
Applied to files:
crates/biome_package/tests/manifest_spec_tests.rs
📚 Learning: 2025-08-17T08:55:30.109Z
Learnt from: CR
PR: biomejs/biome#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-17T08:55:30.109Z
Learning: Applies to **/tests/specs/**/*.snap : Use snapshot spec tests stored as .snap files under tests/specs/ (via cargo insta)
Applied to files:
crates/biome_package/tests/manifest_spec_tests.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). (24)
- GitHub Check: Documentation
- GitHub Check: Lint project (depot-windows-2022)
- GitHub Check: End-to-end tests
- GitHub Check: Lint project (depot-ubuntu-24.04-arm-16)
- GitHub Check: Test (depot-ubuntu-24.04-arm-16)
- GitHub Check: Check Dependencies
- GitHub Check: Test (depot-windows-2022-16)
- GitHub Check: Bench (biome_module_graph)
- GitHub Check: Bench (biome_configuration)
- GitHub Check: Bench (biome_graphql_parser)
- GitHub Check: Bench (biome_json_analyze)
- GitHub Check: Bench (biome_package)
- GitHub Check: Bench (biome_graphql_formatter)
- GitHub Check: Bench (biome_html_formatter)
- GitHub Check: Bench (biome_html_parser)
- GitHub Check: Bench (biome_css_analyze)
- GitHub Check: Bench (biome_json_formatter)
- GitHub Check: Bench (biome_css_parser)
- GitHub Check: Bench (biome_js_analyze)
- GitHub Check: Bench (biome_json_parser)
- GitHub Check: Bench (biome_css_formatter)
- GitHub Check: Bench (biome_js_formatter)
- GitHub Check: Bench (biome_js_parser)
- GitHub Check: autofix
🔇 Additional comments (3)
crates/biome_package/tests/manifest_spec_tests.rs (3)
4-4: LGTM: Camino path types are a solid cross‑platform choiceNice move to Utf8Path/Utf8PathBuf/Utf8Component. This simplifies string handling and avoids OsStr pain, especially on Windows.
187-188: LGTM: Pretty Debug in snapshots improves DXSnapshotting the full tsconfig with {:#?} is great for reviewing path sanitisation and compiler options.
174-181: Verify resolver rule coverage for baseUrl + useImportExtensions autofixI couldn’t spot any spec test in
crates/biome_resolver/tests/spec_tests.rsthat enablescompilerOptions.baseUrl, imports a module without an extension, and asserts the autofix appends.ts/.tsx/.js. Please manually confirm (or add) a case that:
- Sets
baseUrlin the tsconfig fixture.- Imports a path with no extension.
- Includes an assertion or snapshot verifying the autofix appends the correct extension.
arendjr
force-pushed
the
support-baseUrl
branch
from
470c0dc to
bcace8e
Compare
|
|
||
| /// Initialises the paths stored in the manifest. | ||
| /// | ||
| /// `path` must be an absolute path to the `tsconfig.json` file itself. |
There was a problem hiding this comment.
What does it mean to be absolute to a file? Can we add a debug_assert! that checks it?
There was a problem hiding this comment.
It means the path is absolute, i.e. not relative.
Added a debug_assert!.
| } | ||
| } | ||
| ``` | ||
|
|
There was a problem hiding this comment.
Can you add a paragraph that explains the example below? That's the format that we try with the docs. E.g. "Given the following file structure ..., when importing a "foo" from index.ts, Biome will automatically pick up src/foo.ts"
|
Shouldn't we mention this somewhere in the docs once we land it? Since we're are slowly adding support for more TS features ( It will help users to understand the expectations of the tool and its interop with |
I wouldn't be opposed to it, but I don't think I have the bandwidth to maintain such a list. We don't do it for type features in general either, so I would prefer not to do it. Things should "just work", and if users report issues, we'll do our best to resolve it. |
2 participants
Summary
Biome's resolver now supports
baseUrlif specified intsconfig.json. Thismeans that the following now resolves:
tsconfig.json{ "compilerOptions": { "baseUrl": "./src", } }index.tssrc/foo.tsFixes #6432.
Test Plan
Tests added.
Docs
N/A