fix: resolve glob patterns relative to project root when extending#8519
Conversation
🦋 Changeset detectedLatest commit: 45c011e 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 |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughThis change makes workspace settings resolution use Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
No actionable comments were generated in the recent review. 🎉 🧹 Recent nitpick comments
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
crates/biome_service/src/workspace/server.rs (1)
1016-1021: Consider reducing allocations from cloningresolution_directory.
resolution_directoryis cloned on both line 1016 and line 1019. Consider:
- Using
as_ref()where possible to avoid clones- On line 1019, the pattern
.clone().unwrap_or_default()could potentially be simplified depending onload_plugins' signature💡 Potential optimisation
If
load_pluginsaccepts&Utf8Pathinstead of&Utf8PathBuf, you could write:- let plugin_diagnostics = self.load_plugins( - &resolution_directory.clone().unwrap_or_default(), + let plugin_diagnostics = self.load_plugins( + resolution_directory.as_ref().map(|p| p.as_path()).unwrap_or(Utf8Path::new("")), &settings.as_all_plugins(), );And for line 1016, if
merge_with_configurationcan accept a reference:- settings.merge_with_configuration(configuration, resolution_directory.clone())?; + settings.merge_with_configuration(configuration, resolution_directory.as_ref())?;
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.changeset/fix-monorepo-glob-resolution.md(1 hunks)crates/biome_service/src/workspace/server.rs(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
crates/biome_service/src/workspace/server.rs
📄 CodeRabbit inference engine (crates/biome_service/CONTRIBUTING.md)
Use WorkspaceServer implementation for maintaining workspace state in daemon mode and CLI daemonless mode
Files:
crates/biome_service/src/workspace/server.rs
crates/**/*.rs
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Update inline rustdoc documentation for rules, assists, and their options when adding new features or changing existing features in Rust crates
Files:
crates/biome_service/src/workspace/server.rs
.changeset/*.md
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Write changesets that are concise (1-3 sentences), user-focused, use past tense for actions taken and present tense for Biome behavior, include code examples for rules, and end sentences with periods
Files:
.changeset/fix-monorepo-glob-resolution.md
🧠 Learnings (11)
📚 Learning: 2025-11-24T18:06:12.048Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_service/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:06:12.048Z
Learning: Applies to crates/biome_service/src/workspace*.rs : Implement the Workspace trait in the Biome Service to manage internal state of projects, including open documents, project layout instances, and module graph instances
Applied to files:
crates/biome_service/src/workspace/server.rs
📚 Learning: 2025-11-24T18:06:12.048Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_service/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:06:12.048Z
Learning: Applies to crates/biome_service/src/workspace/watcher.tests.rs : Implement watcher tests for workspace methods in watcher.tests.rs and end-to-end tests in LSP tests
Applied to files:
crates/biome_service/src/workspace/server.rs
📚 Learning: 2025-12-12T10:11:05.564Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: CONTRIBUTING.md:0-0
Timestamp: 2025-12-12T10:11:05.564Z
Learning: Applies to **/Cargo.toml : Use workspace dependencies defined in root `Cargo.toml` for internal crates with `workspace = true`, and use path dependencies for `dev-dependencies` to avoid requiring published versions
Applied to files:
crates/biome_service/src/workspace/server.rs
📚 Learning: 2025-11-24T18:06:12.048Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_service/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:06:12.048Z
Learning: Applies to crates/biome_service/src/workspace/server.rs : Use WorkspaceServer implementation for maintaining workspace state in daemon mode and CLI daemonless mode
Applied to files:
crates/biome_service/src/workspace/server.rs
📚 Learning: 2025-12-19T12:53:30.399Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-12-19T12:53:30.399Z
Learning: Applies to crates/biome_analyze/**/*analyze/src/**/*.rs : Assist rules should detect refactoring opportunities and emit code action signals
Applied to files:
crates/biome_service/src/workspace/server.rs
📚 Learning: 2025-12-19T12:53:30.399Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-12-19T12:53:30.399Z
Learning: Applies to crates/biome_analyze/**/biome_rule_options/lib/**/*.rs : Implement `Merge` trait for rule options to support configuration inheritance
Applied to files:
crates/biome_service/src/workspace/server.rs
📚 Learning: 2025-12-19T12:53:30.399Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-12-19T12:53:30.399Z
Learning: Applies to crates/biome_analyze/**/biome_rule_options/lib/**/*.rs : Wrap rule options fields in `Option<>` to properly track set and unset options during merge
Applied to files:
crates/biome_service/src/workspace/server.rs
📚 Learning: 2025-12-19T12:53:30.399Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-12-19T12:53:30.399Z
Learning: Applies to crates/biome_analyze/**/biome_rule_options/lib/**/*.rs : Use `rename_all = "camelCase"` in serde derive macro for rule options
Applied to files:
crates/biome_service/src/workspace/server.rs
📚 Learning: 2025-12-19T12:53:30.399Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-12-19T12:53:30.399Z
Learning: Applies to crates/biome_analyze/**/*analyze/src/**/*.rs : Check if a variable is global using the semantic model to avoid false positives
Applied to files:
crates/biome_service/src/workspace/server.rs
📚 Learning: 2025-12-19T12:53:30.399Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-12-19T12:53:30.399Z
Learning: Applies to crates/biome_analyze/**/*analyze/src/**/*.rs : Set `version` field to `next` in `declare_lint_rule!` macro
Applied to files:
crates/biome_service/src/workspace/server.rs
📚 Learning: 2025-08-05T14:43:29.581Z
Learnt from: dyc3
Repo: biomejs/biome PR: 7081
File: packages/@biomejs/biome/configuration_schema.json:7765-7781
Timestamp: 2025-08-05T14:43:29.581Z
Learning: The file `packages/biomejs/biome/configuration_schema.json` is auto-generated and should not be manually edited or reviewed for schema issues; any changes should be made at the code generation source.
Applied to files:
.changeset/fix-monorepo-glob-resolution.md
🧬 Code graph analysis (1)
crates/biome_service/src/workspace/server.rs (1)
crates/biome_configuration/src/lib.rs (1)
extends_root(275-277)
🔇 Additional comments (2)
.changeset/fix-monorepo-glob-resolution.md (1)
1-7: LGTM!The changeset follows all coding guidelines: concise, user-focused, uses proper tenses, and accurately describes the fix. Well done.
crates/biome_service/src/workspace/server.rs (1)
1010-1014: No issues identified. The code at lines 1010–1014 safely handles bothNoneandSomecases:merge_with_configurationexplicitly acceptsOption<Utf8PathBuf>, andload_pluginsuses.unwrap_or_default()to convertNoneto an empty path.
arendjr
left a comment
There was a problem hiding this comment.
I think you were also okay with this change, right, @ematipico ?
ematipico
left a comment
There was a problem hiding this comment.
Code looks ok and agree with the fix, but I would like to see a test too
…rectory instead of the current package directory.
9f272bf to
f22b68e
Compare
|
I think the docs need to be updated due to this change. https://biomejs.dev/guides/big-projects/#monorepo still says:
|
|
This change has been reverted for now |
Summary
Fixes #8518
When using
"extends": "//"in monorepo packages, glob patterns from the root config are now resolved relative to the project root instead of the package directory.Before:
"!packages/*/lib/**"in root config → evaluated from package dir → didn't workAfter:
"!packages/*/lib/**"in root config → evaluated from root dir → works correctlyAI Assistance Notice:
This PR was developed with assistance from Claude (Anthropic).
Test Plan
extends_rootDocs
No documentation changes needed - bug fix for existing behavior.