Conversation
🦋 Changeset detectedLatest commit: 4a94be7 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 |
github-actions
bot
added
A-Project
|
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:
WalkthroughAdds PNPM workspace catalog support end-to-end: new Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
crates/biome_package/src/node_js_package/package_json.rs (1)
149-162: Potential issue with values containing colons.Using
split_once(':')will incorrectly parse entries where the value contains a colon (e.g.,pkg: npm:alias@1.0.0or custom registry URLs). Since this is documented as a minimal parser, this may be acceptable, but worth noting.If you'd like to handle this edge case:
- if let Some((raw_key, raw_value)) = trimmed_start.split_once(':') { + if let Some(colon_pos) = trimmed_start.find(':') { + let raw_key = &trimmed_start[..colon_pos]; + let raw_value = &trimmed_start[colon_pos + 1..]; let key = raw_key.trim().trim_matches(['\'', '"']); let mut value = raw_value.trim();This wouldn't change the behaviour, but adding a test case for
npm:protocol versions would confirm the current behaviour is intentional.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
crates/biome_js_analyze/tests/specs/nursery/noReactForwardRef/react19-catalog/catalog.js.snapis excluded by!**/*.snapand included by**
📒 Files selected for processing (5)
crates/biome_js_analyze/tests/specs/nursery/noReactForwardRef/react19-catalog/catalog.js(1 hunks)crates/biome_js_analyze/tests/specs/nursery/noReactForwardRef/react19-catalog/catalog.package.json(1 hunks)crates/biome_js_analyze/tests/specs/nursery/noReactForwardRef/react19-catalog/pnpm-workspace.yaml(1 hunks)crates/biome_package/src/node_js_package/package_json.rs(4 hunks)crates/biome_test_utils/src/lib.rs(3 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.rs
📄 CodeRabbit inference engine (CONTRIBUTING.md)
**/*.rs: Use thedbg!()macro for debugging output during testing, and pass the--show-outputflag tocargoto view debug output
Usecargo torcargo testto run tests; for a single test, pass the test name after thetestcommand
Use snapshot testing with theinstacrate; runcargo insta accept,cargo insta reject, orcargo insta reviewto manage snapshot changes
Write doctests as doc comments with code blocks; the code inside code blocks will be run during the testing phase
Usejust f(alias forjust format) to format Rust and TOML files before committing
Files:
crates/biome_package/src/node_js_package/package_json.rscrates/biome_test_utils/src/lib.rs
🧠 Learnings (19)
📓 Common learnings
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
📚 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_package/src/node_js_package/package_json.rscrates/biome_test_utils/src/lib.rs
📚 Learning: 2025-11-27T23:04:02.022Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-11-27T23:04:02.022Z
Learning: Applies to crates/biome_analyze/**/tests/specs/**/*.jsonc : Use `.jsonc` files in test specs with code snippets as array of strings to test rules in script environment (no import/export syntax)
Applied to files:
crates/biome_js_analyze/tests/specs/nursery/noReactForwardRef/react19-catalog/catalog.package.jsoncrates/biome_js_analyze/tests/specs/nursery/noReactForwardRef/react19-catalog/catalog.jscrates/biome_js_analyze/tests/specs/nursery/noReactForwardRef/react19-catalog/pnpm-workspace.yaml
📚 Learning: 2025-11-28T09:08:10.091Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: CONTRIBUTING.md:0-0
Timestamp: 2025-11-28T09:08:10.091Z
Learning: Applies to **/*.tsx : For Node.js package development, build WebAssembly bindings and JSON-RPC bindings; run tests against compiled files after implementing features or bug fixes
Applied to files:
crates/biome_js_analyze/tests/specs/nursery/noReactForwardRef/react19-catalog/catalog.package.json
📚 Learning: 2025-11-27T23:04:02.022Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-11-27T23:04:02.022Z
Learning: Applies to crates/biome_analyze/**/tests/specs/**/*.{js,ts,tsx,jsx,json,css} : Test rules using snapshot tests via the `insta` library with test cases in `tests/specs/<group>/<rule_name>/` directories prefixed by `invalid` or `valid`
Applied to files:
crates/biome_js_analyze/tests/specs/nursery/noReactForwardRef/react19-catalog/catalog.jscrates/biome_js_analyze/tests/specs/nursery/noReactForwardRef/react19-catalog/pnpm-workspace.yaml
📚 Learning: 2025-11-24T18:05:20.371Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_formatter/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:05:20.371Z
Learning: Applies to crates/biome_formatter/**/biome_*_formatter/Cargo.toml : Include development dependencies in `Cargo.toml` for formatter tests: `biome_formatter_test`, `biome_<language>_factory`, `biome_<language>_parser`, `biome_parser`, `biome_service`, `countme`, `iai`, `quickcheck`, `quickcheck_macros`, and `tests_macros`
Applied to files:
crates/biome_test_utils/src/lib.rs
📚 Learning: 2025-11-28T09:08:10.091Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: CONTRIBUTING.md:0-0
Timestamp: 2025-11-28T09:08:10.091Z
Learning: Applies to Cargo.toml : Use workspace dependencies in the root Cargo.toml; internal crates should use `workspace = true` for dependencies and path dependencies for dev-dependencies
Applied to files:
crates/biome_test_utils/src/lib.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*.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_test_utils/src/lib.rs
📚 Learning: 2025-11-24T18:05:42.356Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_js_type_info/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:05:42.356Z
Learning: Applies to crates/biome_js_type_info/**/*.rs : Use `TypeReference` variants (`Qualifier`, `Resolved`, `Import`, `Unknown`) to represent different phases of type resolution
Applied to files:
crates/biome_test_utils/src/lib.rs
📚 Learning: 2025-11-24T18:05:42.356Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_js_type_info/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:05:42.356Z
Learning: Applies to crates/biome_js_type_info/**/*.rs : Use `TypeReference` instead of `Arc` for types that reference other types to avoid stale cache issues when modules are replaced
Applied to files:
crates/biome_test_utils/src/lib.rs
📚 Learning: 2025-11-24T18:05:42.356Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_js_type_info/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:05:42.356Z
Learning: Applies to crates/biome_js_type_info/**/*.rs : Use `TypeData::Unknown` to indicate when type inference falls short or is not implemented
Applied to files:
crates/biome_test_utils/src/lib.rs
📚 Learning: 2025-11-24T18:04:57.309Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_diagnostics/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:04:57.309Z
Learning: Applies to crates/biome_diagnostics/**/*.rs : Use helper types from the biome_diagnostics::v2 module (CodeFrameAdvice, CommandAdvice, DiffAdvice, LogAdvice) or implement the Advices trait yourself for custom advice handling
Applied to files:
crates/biome_test_utils/src/lib.rs
📚 Learning: 2025-11-24T18:05:42.356Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_js_type_info/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:05:42.356Z
Learning: Applies to crates/biome_js_type_info/**/*.rs : Always apply the correct `ResolverId` when retrieving raw type data from `ResolvedTypeData.as_raw_data()` to prevent panics during subsequent resolution calls
Applied to files:
crates/biome_test_utils/src/lib.rs
📚 Learning: 2025-11-24T18:05:27.810Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_js_formatter/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:05:27.810Z
Learning: Applies to crates/biome_js_formatter/**/*.rs : Import the `FormatNode` trait and implement it for your Node when creating formatters in biome_js_formatter
Applied to files:
crates/biome_test_utils/src/lib.rs
📚 Learning: 2025-08-20T16:24:59.781Z
Learnt from: arendjr
Repo: biomejs/biome PR: 7266
File: crates/biome_js_type_info/src/type.rs:94-102
Timestamp: 2025-08-20T16:24:59.781Z
Learning: In crates/biome_js_type_info/src/type.rs, the flattened_union_variants() method returns TypeReference instances that already have the correct module IDs applied to them. These references should be used directly with resolver.resolve_reference() without applying additional module ID transformations, as variant references may originate from nested unions in different modules.
Applied to files:
crates/biome_test_utils/src/lib.rs
📚 Learning: 2025-11-24T18:05:42.356Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_js_type_info/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:05:42.356Z
Learning: Applies to crates/biome_js_type_info/**/*.rs : When using `ResolvedTypeData`, track the `ResolverId` to ensure subsequent resolver calls use the correct context
Applied to files:
crates/biome_test_utils/src/lib.rs
📚 Learning: 2025-11-24T18:05:42.356Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_js_type_info/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:05:42.356Z
Learning: Applies to crates/biome_js_type_info/**/*.rs : Distinguish between `TypeData::Unknown` and `TypeData::UnknownKeyword` to measure inference effectiveness versus explicit user-provided unknown types
Applied to files:
crates/biome_test_utils/src/lib.rs
📚 Learning: 2025-11-24T18:05:42.356Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_js_type_info/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:05:42.356Z
Learning: Applies to crates/biome_js_type_info/**/scoped_resolver.rs : Implement full inference to resolve `TypeReference::Import` variants across the entire module graph
Applied to files:
crates/biome_test_utils/src/lib.rs
📚 Learning: 2025-11-27T23:04:02.022Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-11-27T23:04:02.022Z
Learning: Applies to crates/biome_analyze/**/*analyze/src/lint/nursery/**/*.rs : New rules must be placed inside the `nursery` group as an incubation space exempt from semantic versioning, with promotion to appropriate groups done during minor/major releases
Applied to files:
crates/biome_js_analyze/tests/specs/nursery/noReactForwardRef/react19-catalog/pnpm-workspace.yaml
🧬 Code graph analysis (1)
crates/biome_test_utils/src/lib.rs (1)
crates/biome_package/src/node_js_package/package_json.rs (2)
parse_pnpm_workspace_catalog(122-170)new(49-55)
⏰ 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). (1)
- GitHub Check: Validate PR title
🔇 Additional comments (11)
crates/biome_package/src/node_js_package/package_json.rs (5)
36-36: LGTM!The optional
catalogfield is a sensible addition — it won't be deserialised frompackage.jsondirectly but allows external population from pnpm workspace files.
89-109: LGTM!Nice integration of catalog support into the existing dependency matching logic without changing the public API.
173-204: LGTM!Clean handling of both
catalog:(implicit key from package name) andcatalog:explicit-keyformats. The fallback to the original version when catalog lookup fails is a sensible default.
292-299: LGTM!Linear search is fine here — dependency lists are typically small enough that the simplicity outweighs any performance concerns.
502-529: LGTM!Good test coverage for the happy paths. The test for
parse_pnpm_workspace_catalog_minimalnicely demonstrates handling of both quoted and unquoted values.crates/biome_js_analyze/tests/specs/nursery/noReactForwardRef/react19-catalog/pnpm-workspace.yaml (1)
1-5: LGTM!Minimal test fixture that correctly maps React to version 19.0.0 for the
noReactForwardRefdiagnostic test.crates/biome_js_analyze/tests/specs/nursery/noReactForwardRef/react19-catalog/catalog.package.json (1)
1-5: LGTM!Clean test fixture using the
catalog:placeholder. The naming convention aligns with the test harness expectations.crates/biome_js_analyze/tests/specs/nursery/noReactForwardRef/react19-catalog/catalog.js (1)
1-7: LGTM!Test case correctly uses
forwardRefwhich should triggernoReactForwardRefdiagnostics when React 19 is detected via the catalog resolution.crates/biome_test_utils/src/lib.rs (3)
19-19: LGTM!Import addition for the new
Dependenciestype used in catalog handling.
252-264: LGTM!The ancestor traversal correctly finds the nearest
pnpm-workspace.yamland parses its catalog. Usingstd::fsdirectly is fine for test utilities.
272-302: LGTM!Clean integration — the catalog is computed once and injected only when the manifest doesn't already have one. The
.clone()is necessary here and acceptable for test utilities.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
crates/biome_package/src/node_js_package/package_json.rs (1)
147-163: Potential edge case: lines at section boundaries may be skipped.When dedenting out of the
in_defaultsection (line 149) or exiting a named catalog (lines 157-163), the current line is unconditionally skipped viacontinue. If a new section header appears on the same dedent line, it won't be processed.For typical
pnpm-workspace.yamlfiles this is unlikely to matter, but worth noting. If you want to be defensive, you could re-process the current line after changing state instead of continuing.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
crates/biome_js_analyze/tests/specs/nursery/noReactForwardRef/react19-named-catalog/catalog.js.snapis excluded by!**/*.snapand included by**
📒 Files selected for processing (7)
crates/biome_js_analyze/tests/specs/nursery/noReactForwardRef/react19-named-catalog/catalog.js(1 hunks)crates/biome_js_analyze/tests/specs/nursery/noReactForwardRef/react19-named-catalog/catalog.package.json(1 hunks)crates/biome_js_analyze/tests/specs/nursery/noReactForwardRef/react19-named-catalog/pnpm-workspace.yaml(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/package_json.rs(5 hunks)crates/biome_test_utils/src/lib.rs(3 hunks)
✅ Files skipped from review due to trivial changes (1)
- crates/biome_js_analyze/tests/specs/nursery/noReactForwardRef/react19-named-catalog/catalog.package.json
🚧 Files skipped from review as they are similar to previous changes (1)
- crates/biome_test_utils/src/lib.rs
🧰 Additional context used
📓 Path-based instructions (1)
**/*.rs
📄 CodeRabbit inference engine (CONTRIBUTING.md)
**/*.rs: Use thedbg!()macro for debugging output during testing, and pass the--show-outputflag tocargoto view debug output
Usecargo torcargo testto run tests; for a single test, pass the test name after thetestcommand
Use snapshot testing with theinstacrate; runcargo insta accept,cargo insta reject, orcargo insta reviewto manage snapshot changes
Write doctests as doc comments with code blocks; the code inside code blocks will be run during the testing phase
Usejust f(alias forjust format) to format Rust and TOML files before committing
Files:
crates/biome_package/src/node_js_package/mod.rscrates/biome_package/src/node_js_package/package_json.rscrates/biome_package/src/lib.rs
🧠 Learnings (7)
📚 Learning: 2025-11-28T09:08:10.091Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: CONTRIBUTING.md:0-0
Timestamp: 2025-11-28T09:08:10.091Z
Learning: Applies to **/*.ts : For Node.js package development, build WebAssembly bindings and JSON-RPC bindings; run tests against compiled files after implementing features or bug fixes
Applied to files:
crates/biome_package/src/node_js_package/mod.rs
📚 Learning: 2025-11-28T09:08:10.091Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: CONTRIBUTING.md:0-0
Timestamp: 2025-11-28T09:08:10.091Z
Learning: Applies to **/*.tsx : For Node.js package development, build WebAssembly bindings and JSON-RPC bindings; run tests against compiled files after implementing features or bug fixes
Applied to files:
crates/biome_package/src/node_js_package/mod.rs
📚 Learning: 2025-11-27T23:04:02.022Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-11-27T23:04:02.022Z
Learning: Applies to crates/biome_analyze/**/*analyze/src/lint/**/*.rs : Use language tags in documentation code blocks (js, ts, tsx, json, css) and order properties consistently as: language, then `expect_diagnostic`, then options modifiers, then `ignore`, then `file=path`
Applied to files:
crates/biome_package/src/node_js_package/mod.rs
📚 Learning: 2025-11-24T18:05:42.356Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_js_type_info/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:05:42.356Z
Learning: Applies to crates/biome_js_type_info/**/*.rs : Use `TypeReference` instead of `Arc` for types that reference other types to avoid stale cache issues when modules are replaced
Applied to files:
crates/biome_package/src/node_js_package/mod.rscrates/biome_package/src/lib.rs
📚 Learning: 2025-11-24T18:05:27.810Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_js_formatter/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:05:27.810Z
Learning: Applies to crates/biome_js_formatter/**/*.rs : Import the `FormatNode` trait and implement it for your Node when creating formatters in biome_js_formatter
Applied to files:
crates/biome_package/src/node_js_package/package_json.rscrates/biome_package/src/lib.rs
📚 Learning: 2025-11-24T18:05:42.356Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_js_type_info/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:05:42.356Z
Learning: Applies to crates/biome_js_type_info/**/*.rs : Store type data in linear vectors instead of using recursive data structures with `Arc` for improved data locality and performance
Applied to files:
crates/biome_package/src/node_js_package/package_json.rscrates/biome_package/src/lib.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_package/src/node_js_package/package_json.rs
🔇 Additional comments (11)
crates/biome_package/src/node_js_package/mod.rs (1)
5-5: LGTM!Clean addition to the existing re-export list, consistent with the module's pattern.
crates/biome_package/src/node_js_package/package_json.rs (7)
37-37: LGTM!Good addition of the catalog field. The
Option<Catalogs>type correctly models that a package may or may not have catalog information available.
235-251: LGTM!Sensible parsing for the expected YAML subset. The quote handling covers the common cases.
264-284: LGTM!Clean catalog resolution logic. The fallback to the original version string when catalog lookup fails is a sensible defensive choice.
221-233: LGTM!The lookup semantics are correct: named catalog lookups are strict (no fallback to default), which matches pnpm's behaviour.
372-378: LGTM!Clean accessor method, consistent with the existing
containspattern.
583-649: Solid test coverage for the happy paths.The tests cover both default and named catalog scenarios for both parsing and matching. Consider adding a test for the fallback case (when a
catalog:reference doesn't resolve) if you want extra confidence, but this is optional.
96-106: LGTM!Clean integration of catalog-aware resolution into the existing dependency matching flow.
crates/biome_package/src/lib.rs (1)
12-15: LGTM!Correct re-export to make
Catalogspart of the public API.crates/biome_js_analyze/tests/specs/nursery/noReactForwardRef/react19-named-catalog/pnpm-workspace.yaml (1)
1-7: LGTM!Valid test fixture for the named catalog scenario. Works nicely with the corresponding
catalog.package.json.crates/biome_js_analyze/tests/specs/nursery/noReactForwardRef/react19-named-catalog/catalog.js (1)
1-7: LGTM!Clean test case for verifying that the rule triggers diagnostics when React 19 is resolved via a named pnpm catalog.
apple-yagi
changed the title
ematipico
left a comment
ematipico
left a comment
There was a problem hiding this comment.
This feels like more a feature than a fix, don't you agree? I am going to block it so you can prepare the PR accordingly:
- changeset for the feature
- more tests: we haven't added any tests for the "parsing" of
pnpm-workspace.yml. - implement the feature according to our architecture:
- use our files system
- resolve the pnpm workspace file correctly: it doesn't take into account the root of the project
| pub struct Catalogs { | ||
| pub default: Option<Dependencies>, | ||
| pub named: FxHashMap<Box<str>, Dependencies>, | ||
| } |
There was a problem hiding this comment.
Can you document each field and how it's mapped?
| if let Some(catalogs) = catalog | ||
| && let Some(rest) = version.strip_prefix("catalog:") |
There was a problem hiding this comment.
That's a new usage of if-let chain I see. Why don't you use catalogs in the second chain? I would argue that it reads better if you use a match of catalog and version.strip_prefix, and we don't use the let chain since it isn't needed
if let (Some(catalog), Some(rest)) = (catalog, version.strip_prefix()) {
}There was a problem hiding this comment.
Yes, I've made the correction.
| } | ||
| } | ||
|
|
||
| fn parse_entry(value: &str) -> Option<(Box<str>, Box<str>)> { |
There was a problem hiding this comment.
nit: This function is exclusively used inside parse_pnpm_workspace_catalog, so please give a more specialized name. parse_entry is too generic. Plus, add some docs that explain the business logic of the function
There was a problem hiding this comment.
After reorganizing the logic in various ways, that function has been removed. The new function I've created uses a name that's not overly generic.
crates/biome_test_utils/src/lib.rs
Outdated
| if let Ok(content) = std::fs::read_to_string(&workspace_file) { | ||
| if let Some(catalog) = PackageJson::parse_pnpm_workspace_catalog(&content) { | ||
| return Some(catalog); | ||
| } | ||
| } |
There was a problem hiding this comment.
We can't read from FS like this, but we need to use our fs, so we can test it
There was a problem hiding this comment.
I've modified it to use our own file system.
Merging this PR will not alter performance
Comparing 
Footnotes
|
|
@ematipico My apologies. I intended to create the PR as a PoC. I agree with adding the feature. |
apple-yagi
changed the title
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
crates/biome_package/src/node_js_package/package_json.rs (2)
278-288: Consider usingmatchfor better readability.The if-let chain works, but a
matchon the tuple might read more clearly, addressing previous review feedback.- if let (Some(catalogs), Some(rest)) = (catalog, version.strip_prefix("catalog:")) { - let (catalog_name, package_name) = if rest.is_empty() { - (None, specifier) - } else { - (Some(rest), specifier) - }; - - if let Some(mapped_version) = catalogs.lookup(package_name, catalog_name) { - return mapped_version; - } - } + match (catalog, version.strip_prefix("catalog:")) { + (Some(catalogs), Some(rest)) => { + let catalog_name = if rest.is_empty() { None } else { Some(rest) }; + if let Some(mapped_version) = catalogs.lookup(specifier, catalog_name) { + return mapped_version; + } + } + _ => {} + }Based on past review feedback from ematipico.
591-723: Consider adding edge case tests.The existing tests are solid, but a few additional cases would strengthen coverage:
- Empty workspace file (no catalog sections)
catalog:reference when package isn't in the default catalogcatalog:nonexistentreference with an unknown named catalogmatches_dependencywhencatalogfield isNoneThese scenarios test fallback behaviour and error handling.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
crates/biome_package/src/node_js_package/package_json.rs(5 hunks)crates/biome_test_utils/src/lib.rs(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- crates/biome_test_utils/src/lib.rs
🧰 Additional context used
📓 Path-based instructions (1)
**/*.rs
📄 CodeRabbit inference engine (CONTRIBUTING.md)
**/*.rs: Use thedbg!()macro for debugging output during testing, and pass the--show-outputflag tocargoto view debug output
Usecargo torcargo testto run tests; for a single test, pass the test name after thetestcommand
Use snapshot testing with theinstacrate; runcargo insta accept,cargo insta reject, orcargo insta reviewto manage snapshot changes
Write doctests as doc comments with code blocks; the code inside code blocks will be run during the testing phase
Usejust f(alias forjust format) to format Rust and TOML files before committing
Files:
crates/biome_package/src/node_js_package/package_json.rs
🧠 Learnings (10)
📓 Common learnings
Learnt from: CR
Repo: biomejs/biome PR: 0
File: CONTRIBUTING.md:0-0
Timestamp: 2025-11-28T09:08:10.091Z
Learning: Applies to Cargo.toml : Use workspace dependencies in the root Cargo.toml; internal crates should use `workspace = true` for dependencies and path dependencies for dev-dependencies
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
📚 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_package/src/node_js_package/package_json.rs
📚 Learning: 2025-11-28T09:08:10.091Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: CONTRIBUTING.md:0-0
Timestamp: 2025-11-28T09:08:10.091Z
Learning: Applies to Cargo.toml : Use workspace dependencies in the root Cargo.toml; internal crates should use `workspace = true` for dependencies and path dependencies for dev-dependencies
Applied to files:
crates/biome_package/src/node_js_package/package_json.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:
crates/biome_package/src/node_js_package/package_json.rs
📚 Learning: 2025-09-25T12:32:59.003Z
Learnt from: arendjr
Repo: biomejs/biome PR: 7593
File: crates/biome_service/src/workspace/server.rs:1306-1306
Timestamp: 2025-09-25T12:32:59.003Z
Learning: In the biomejs/biome project, do not flag compilation errors during code review as they are handled by the existing test infrastructure and CI. Focus on other code quality aspects instead.
Applied to files:
crates/biome_package/src/node_js_package/package_json.rs
📚 Learning: 2025-11-27T23:04:02.022Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-11-27T23:04:02.022Z
Learning: Commit rule changes with message format: `feat(biome_<crate>): <ruleName>` to follow Biome's conventional commit style
Applied to files:
crates/biome_package/src/node_js_package/package_json.rs
📚 Learning: 2025-11-27T23:04:02.022Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-11-27T23:04:02.022Z
Learning: Applies to crates/biome_analyze/**/*analyze/src/lint/**/*.rs : Avoid using `unwrap()` or `expect()` on `Result` and `Option` types; instead use helper functions like `map`, `filter`, `and_then` to maintain code clarity and avoid panics
Applied to files:
crates/biome_package/src/node_js_package/package_json.rs
📚 Learning: 2025-11-24T18:05:27.810Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_js_formatter/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:05:27.810Z
Learning: Applies to crates/biome_js_formatter/**/*.rs : Import the `FormatNode` trait and implement it for your Node when creating formatters in biome_js_formatter
Applied to files:
crates/biome_package/src/node_js_package/package_json.rs
📚 Learning: 2025-11-24T18:05:42.356Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_js_type_info/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:05:42.356Z
Learning: Applies to crates/biome_js_type_info/**/*.rs : Store type data in linear vectors instead of using recursive data structures with `Arc` for improved data locality and performance
Applied to files:
crates/biome_package/src/node_js_package/package_json.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_package/src/node_js_package/package_json.rs
🔇 Additional comments (2)
crates/biome_package/src/node_js_package/package_json.rs (2)
380-386: LGTM!Clean implementation with proper documentation.
112-212: Verify whether this minimal YAML parser covers pnpm workspace file requirements.This indentation-based parser handles a subset of YAML but doesn't account for:
- Multi-line values
- Comments after values (
react: 19.0.0 # latest)- Mixed indentation
- YAML arrays in catalog entries
- Escaped quotes in quoted keys
Confirm that target pnpm workspace files don't use these features, or implement handling for them.
apple-yagi
force-pushed
the
fix/no-react-forward-ref-pnpm-catalog
branch
from
9dee366 to
ed6fc55
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)
.changeset/nasty-cities-check.md(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
.changeset/**/*.md
📄 CodeRabbit inference engine (CONTRIBUTING.md)
.changeset/**/*.md: Create changesets for user-facing changes usingjust new-changeset; use headers with####or#####only; keep descriptions concise (1-3 sentences) and focus on user-facing changes
Use past tense when describing what was done ('Added new feature'), present tense when describing Biome behavior ('Biome now supports'); end sentences with a full stop
For new lint rules, show an example of an invalid case in an inline code snippet or code block; for rule changes, demonstrate what is now invalid or valid; for formatter changes, use adiffcode block
Files:
.changeset/nasty-cities-check.md
|
@ematipico I've made corrections.
My understanding of this comment might be insufficient. Please let me know if there's anything I'm missing. |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
crates/biome_package/src/node_js_package/package_json.rs (1)
118-220: Consider using an established YAML library.Whilst this custom parser works for the current use case, YAML is a complex format with numerous edge cases. Manual indentation tracking across 100+ lines is fragile and hard to maintain. Libraries like
serde_yamlwould handle this more robustly.If you prefer to avoid the dependency, at least consider breaking this function into smaller, focused helpers (e.g.,
parse_default_catalog_section,parse_named_catalogs_section) to improve readability and testability.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
crates/biome_package/src/node_js_package/package_json.rs(5 hunks)crates/biome_test_utils/src/lib.rs(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- crates/biome_test_utils/src/lib.rs
🧰 Additional context used
📓 Path-based instructions (1)
**/*.rs
📄 CodeRabbit inference engine (CONTRIBUTING.md)
**/*.rs: Use thedbg!()macro for debugging output during testing, and pass the--show-outputflag tocargoto view debug output
Usecargo torcargo testto run tests; for a single test, pass the test name after thetestcommand
Use snapshot testing with theinstacrate; runcargo insta accept,cargo insta reject, orcargo insta reviewto manage snapshot changes
Write doctests as doc comments with code blocks; the code inside code blocks will be run during the testing phase
Usejust f(alias forjust format) to format Rust and TOML files before committing
Files:
crates/biome_package/src/node_js_package/package_json.rs
🧠 Learnings (8)
📓 Common learnings
Learnt from: CR
Repo: biomejs/biome PR: 0
File: CONTRIBUTING.md:0-0
Timestamp: 2025-11-28T09:08:10.091Z
Learning: Applies to Cargo.toml : Use workspace dependencies in the root Cargo.toml; internal crates should use `workspace = true` for dependencies and path dependencies for dev-dependencies
📚 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:
crates/biome_package/src/node_js_package/package_json.rs
📚 Learning: 2025-09-25T12:32:59.003Z
Learnt from: arendjr
Repo: biomejs/biome PR: 7593
File: crates/biome_service/src/workspace/server.rs:1306-1306
Timestamp: 2025-09-25T12:32:59.003Z
Learning: In the biomejs/biome project, do not flag compilation errors during code review as they are handled by the existing test infrastructure and CI. Focus on other code quality aspects instead.
Applied to files:
crates/biome_package/src/node_js_package/package_json.rs
📚 Learning: 2025-11-27T23:04:02.022Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-11-27T23:04:02.022Z
Learning: Commit rule changes with message format: `feat(biome_<crate>): <ruleName>` to follow Biome's conventional commit style
Applied to files:
crates/biome_package/src/node_js_package/package_json.rs
📚 Learning: 2025-11-24T18:05:27.810Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_js_formatter/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:05:27.810Z
Learning: Applies to crates/biome_js_formatter/**/*.rs : Import the `FormatNode` trait and implement it for your Node when creating formatters in biome_js_formatter
Applied to files:
crates/biome_package/src/node_js_package/package_json.rs
📚 Learning: 2025-11-24T18:05:42.356Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_js_type_info/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:05:42.356Z
Learning: Applies to crates/biome_js_type_info/**/*.rs : Store type data in linear vectors instead of using recursive data structures with `Arc` for improved data locality and performance
Applied to files:
crates/biome_package/src/node_js_package/package_json.rs
📚 Learning: 2025-11-28T09:08:10.091Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: CONTRIBUTING.md:0-0
Timestamp: 2025-11-28T09:08:10.091Z
Learning: Applies to Cargo.toml : Use workspace dependencies in the root Cargo.toml; internal crates should use `workspace = true` for dependencies and path dependencies for dev-dependencies
Applied to files:
crates/biome_package/src/node_js_package/package_json.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_package/src/node_js_package/package_json.rs
🔇 Additional comments (7)
crates/biome_package/src/node_js_package/package_json.rs (7)
14-14: LGTM – documentation addresses previous feedback.The
catalogfield is now well-documented, clearly explaining its purpose and when it's populated.Also applies to: 37-43
104-109: LGTM – clean integration of catalog support.The catalog resolution is properly wired through to
dependency_satisfieswhilst maintaining backward compatibility.
222-242: LGTM – clean catalog abstraction.The
Catalogsstruct is well-documented and thelookuplogic correctly prioritises named catalogs over the default.
244-262: LGTM – focused helper with proper validation.The entry parsing correctly handles quoted values and rejects malformed input.
264-298: LGTM – catalog resolution logic is sound.The functions correctly handle both default (
catalog:) and named (catalog:<name>) catalog references, with appropriate fallback to literal versions when no match is found.
387-393: LGTM – straightforward accessor.Clean implementation with appropriate documentation.
598-730: LGTM – comprehensive test coverage.The tests validate catalog parsing (minimal, named, mixed), resolution logic, and entry parsing edge cases. Well done on covering both positive and negative test cases.
dyc3
left a comment
dyc3
left a comment
There was a problem hiding this comment.
I couldn't find anything that actually loads the pnpm-workspace.yaml file at runtime.
There needs to be at least one test in biome_cli that shows the file gets read.
crates/biome_js_analyze/tests/specs/nursery/noReactForwardRef/react19-catalog/catalog.js
Outdated
Show resolved
Hide resolved
crates/biome_js_analyze/tests/specs/nursery/noReactForwardRef/react19-named-catalog/catalog.js
Outdated
Show resolved
Hide resolved
| /// Extract catalog entries from a pnpm workspace file, supporting both the | ||
| /// default `catalog:` and named catalogs under `catalogs:`. | ||
| pub fn parse_pnpm_workspace_catalog(source: &str) -> Option<Catalogs> { | ||
| let mut catalogs = Catalogs::default(); | ||
|
|
||
| let mut in_default = false; | ||
| let mut default_indent = 0usize; | ||
| let mut default_entries: Vec<(Box<str>, Box<str>)> = Vec::new(); |
There was a problem hiding this comment.
This is parsing yaml. It should probably use biome's existing yaml parser instead. See biome_yaml_parser.
crates/biome_test_utils/src/lib.rs
Outdated
| for dir in working_directory.ancestors() { | ||
| let workspace_file = dir.join("pnpm-workspace.yaml"); | ||
| if !fs.path_is_file(&workspace_file) { | ||
| continue; | ||
| } |
There was a problem hiding this comment.
For tests, we need them to be as isolated as possible, and going up the file tree all the way searching for files breaks that.
We need to search for the pnpm-workspace.yaml that is adjacent to the file being tested. For example, for grabbing package.json for a test case, we have:
let package_json_file = input_file.with_extension("package.json");
Also, I think it needs to support the yml file extension too.
There was a problem hiding this comment.
Also, I think it needs to support the yml file extension too.
I don't think so, pnpm only supports pnpm-workspace.yaml (exact match): https://pnpm.io/pnpm-workspace_yaml
There was a problem hiding this comment.
I was checking the internal implementation of pnpm, and it seemed to be fixed in pnpm-workspace.yaml.
|
@dyc3 I've made corrections. |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
crates/biome_test_utils/src/lib.rs (1)
19-19: Catalogue propagation intoProjectLayoutlooks spot onWiring
find_pnpm_workspace_catalogintoproject_layout_for_test_fileand only fillingmanifest.catalogwhen it’sNoneis a nice, conservative way to let the workspace drivecatalog:resolution without trampling any manually‑set catalog data. The additional import is exactly what you need here.If you ever feel like shaving a microsecond, you could avoid the clone by moving the option in:
if manifest.catalog.is_none() { manifest.catalog = pnpm_catalog; }Not urgent at all though; the current version is perfectly fine for test harness usage.
Also applies to: 274-274, 293-297, 303-303
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lockand included by**
📒 Files selected for processing (3)
crates/biome_package/Cargo.toml(1 hunks)crates/biome_package/src/node_js_package/package_json.rs(5 hunks)crates/biome_test_utils/src/lib.rs(4 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.toml
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Use
just f(alias forjust format) to format TOML files before committing
Files:
crates/biome_package/Cargo.toml
**/*.rs
📄 CodeRabbit inference engine (CONTRIBUTING.md)
**/*.rs: Use thedbg!()macro for debugging output during testing, and pass the--show-outputflag tocargoto view debug output
Usecargo torcargo testto run tests; for a single test, pass the test name after thetestcommand
Use snapshot testing with theinstacrate; runcargo insta accept,cargo insta reject, orcargo insta reviewto manage snapshot changes
Write doctests as doc comments with code blocks; the code inside code blocks will be run during the testing phase
Usejust f(alias forjust format) to format Rust and TOML files before committing
Files:
crates/biome_package/src/node_js_package/package_json.rscrates/biome_test_utils/src/lib.rs
🧠 Learnings (37)
📚 Learning: 2025-11-24T18:05:20.371Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_formatter/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:05:20.371Z
Learning: Applies to crates/biome_formatter/**/biome_*_formatter/Cargo.toml : Include development dependencies in `Cargo.toml` for formatter tests: `biome_formatter_test`, `biome_<language>_factory`, `biome_<language>_parser`, `biome_parser`, `biome_service`, `countme`, `iai`, `quickcheck`, `quickcheck_macros`, and `tests_macros`
Applied to files:
crates/biome_package/Cargo.tomlcrates/biome_test_utils/src/lib.rs
📚 Learning: 2025-11-24T18:06:03.545Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_parser/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:06:03.545Z
Learning: Create two new crates `biome_{language}_syntax` and `biome_{language}_factory` using `cargo new --lib` for new language parsers
Applied to files:
crates/biome_package/Cargo.toml
📚 Learning: 2025-11-28T09:08:10.091Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: CONTRIBUTING.md:0-0
Timestamp: 2025-11-28T09:08:10.091Z
Learning: Applies to Cargo.toml : Use workspace dependencies in the root Cargo.toml; internal crates should use `workspace = true` for dependencies and path dependencies for dev-dependencies
Applied to files:
crates/biome_package/Cargo.tomlcrates/biome_package/src/node_js_package/package_json.rscrates/biome_test_utils/src/lib.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*.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_package/Cargo.tomlcrates/biome_test_utils/src/lib.rs
📚 Learning: 2025-11-24T18:06:03.545Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_parser/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:06:03.545Z
Learning: Applies to crates/biome_parser/**/language_kind.rs : Add a new language prefix to the `LANGUAGE_PREFIXES` constant in `language_kind.rs` file
Applied to files:
crates/biome_package/Cargo.toml
📚 Learning: 2025-11-24T18:04:57.309Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_diagnostics/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:04:57.309Z
Learning: Applies to crates/biome_diagnostics/**/*.rs : Use helper types from the biome_diagnostics::v2 module (CodeFrameAdvice, CommandAdvice, DiffAdvice, LogAdvice) or implement the Advices trait yourself for custom advice handling
Applied to files:
crates/biome_package/Cargo.tomlcrates/biome_test_utils/src/lib.rs
📚 Learning: 2025-11-24T18:06:03.545Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_parser/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:06:03.545Z
Learning: Applies to crates/biome_parser/**/language_kind.rs : Add a new variant to `LanguageKind` enum in `language_kind.rs` file and implement all methods for the new language variant
Applied to files:
crates/biome_package/Cargo.toml
📚 Learning: 2025-11-27T23:04:02.022Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-11-27T23:04:02.022Z
Learning: Commit rule changes with message format: `feat(biome_<crate>): <ruleName>` to follow Biome's conventional commit style
Applied to files:
crates/biome_package/Cargo.tomlcrates/biome_package/src/node_js_package/package_json.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_package/Cargo.tomlcrates/biome_package/src/node_js_package/package_json.rscrates/biome_test_utils/src/lib.rs
📚 Learning: 2025-11-24T18:06:03.545Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_parser/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:06:03.545Z
Learning: Applies to crates/biome_parser/**/src/**/*.rs : Use `ParseSeparatedList` and `ParseNodeList` for parsing lists with error recovery to avoid infinite loops
Applied to files:
crates/biome_package/Cargo.tomlcrates/biome_package/src/node_js_package/package_json.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: Debug the WorkspaceWatcher by starting the daemon with cargo run --bin=biome -- start and running commands such as cargo run --bin=biome -- lint --use-server <path>
Applied to files:
crates/biome_package/Cargo.toml
📚 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_package/Cargo.toml
📚 Learning: 2025-11-27T23:04:02.022Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-11-27T23:04:02.022Z
Learning: Applies to crates/biome_analyze/**/biome_rule_options/src/**/*.rs : Wrap optional rule option fields in `Option<_>` to properly track set vs unset options during configuration merging
Applied to files:
crates/biome_package/Cargo.toml
📚 Learning: 2025-11-27T23:04:02.022Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-11-27T23:04:02.022Z
Learning: Applies to crates/biome_analyze/**/biome_rule_options/src/**/*.rs : Use `Box<[T]>` instead of `Vec<T>` for rule options array fields to save memory (boxed slices and boxed str use 2 words instead of three words)
Applied to files:
crates/biome_package/Cargo.toml
📚 Learning: 2025-11-27T23:04:02.022Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-11-27T23:04:02.022Z
Learning: Applies to crates/biome_analyze/**/biome_rule_options/src/**/*.rs : Rule options must be defined in the `biome_rule_options` crate and implement traits: `Deserializable`, `Merge`, `Serialize`, `Deserialize`, and `JsonSchema`
Applied to files:
crates/biome_package/Cargo.toml
📚 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:
crates/biome_package/src/node_js_package/package_json.rs
📚 Learning: 2025-09-25T12:32:59.003Z
Learnt from: arendjr
Repo: biomejs/biome PR: 7593
File: crates/biome_service/src/workspace/server.rs:1306-1306
Timestamp: 2025-09-25T12:32:59.003Z
Learning: In the biomejs/biome project, do not flag compilation errors during code review as they are handled by the existing test infrastructure and CI. Focus on other code quality aspects instead.
Applied to files:
crates/biome_package/src/node_js_package/package_json.rs
📚 Learning: 2025-11-24T18:05:27.810Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_js_formatter/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:05:27.810Z
Learning: Applies to crates/biome_js_formatter/**/*.rs : Import the `FormatNode` trait and implement it for your Node when creating formatters in biome_js_formatter
Applied to files:
crates/biome_package/src/node_js_package/package_json.rscrates/biome_test_utils/src/lib.rs
📚 Learning: 2025-11-24T18:05:20.371Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_formatter/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:05:20.371Z
Learning: Applies to crates/biome_formatter/**/biome_*_formatter/src/lib.rs : Expose a public `format_node` function that accepts formatting options and a root syntax node, returning a `FormatResult<Formatted<Context>>` with appropriate documentation
Applied to files:
crates/biome_package/src/node_js_package/package_json.rs
📚 Learning: 2025-11-24T18:06:03.545Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_parser/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:06:03.545Z
Learning: Applies to crates/biome_parser/**/src/**/*.rs : A parser struct must implement the `Parser` trait and save the token source, parser context, and optional parser options
Applied to files:
crates/biome_package/src/node_js_package/package_json.rs
📚 Learning: 2025-11-24T18:06:03.545Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_parser/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:06:03.545Z
Learning: Applies to crates/biome_parser/**/src/**/*.rs : Use `p.eat(token)` for optional tokens, `p.expect(token)` for required tokens, `parse_rule(p).ok(p)` for optional nodes, and `parse_rule(p).or_add_diagnostic(p, error)` for required nodes
Applied to files:
crates/biome_package/src/node_js_package/package_json.rs
📚 Learning: 2025-11-24T18:06:03.545Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_parser/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:06:03.545Z
Learning: Applies to crates/biome_parser/**/src/**/*.rs : Use `ConditionalParsedSyntax` for syntax that is only valid in specific contexts (e.g., strict mode, file types, language versions) and call `or_invalid_to_bogus()` to convert to a bogus node if not supported
Applied to files:
crates/biome_package/src/node_js_package/package_json.rs
📚 Learning: 2025-11-24T18:05:27.810Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_js_formatter/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:05:27.810Z
Learning: Applies to crates/biome_js_formatter/**/*.rs : Do not attempt to 'fix' the code; if a token/node is known to be mandatory but is missing, return `None` instead
Applied to files:
crates/biome_package/src/node_js_package/package_json.rs
📚 Learning: 2025-11-24T18:06:03.545Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_parser/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:06:03.545Z
Learning: Applies to crates/biome_parser/**/src/**/*.rs : Implement error recovery in list parsing using `or_recover()` to wrap unparseable tokens in a `BOGUS_*` node and consume tokens until a recovery token is found
Applied to files:
crates/biome_package/src/node_js_package/package_json.rs
📚 Learning: 2025-11-24T18:04:57.309Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_diagnostics/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:04:57.309Z
Learning: Applies to crates/biome_diagnostics/crates/biome_diagnostics_categories/src/categories.rs : Register all new diagnostic categories in crates/biome_diagnostics_categories/src/categories.rs
Applied to files:
crates/biome_test_utils/src/lib.rs
📚 Learning: 2025-11-24T18:05:20.371Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_formatter/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:05:20.371Z
Learning: Applies to crates/biome_formatter/**/biome_*_formatter/tests/spec_tests.rs : Use the `tests_macros::gen_tests!` macro in `spec_tests.rs` to generate test functions for each specification file matching the pattern `tests/specs/<language>/**/*.<ext>`
Applied to files:
crates/biome_test_utils/src/lib.rs
📚 Learning: 2025-11-24T18:05:42.356Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_js_type_info/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:05:42.356Z
Learning: Applies to crates/biome_js_type_info/**/*.rs : Use `TypeReference` variants (`Qualifier`, `Resolved`, `Import`, `Unknown`) to represent different phases of type resolution
Applied to files:
crates/biome_test_utils/src/lib.rs
📚 Learning: 2025-11-24T18:05:42.356Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_js_type_info/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:05:42.356Z
Learning: Applies to crates/biome_js_type_info/**/*.rs : Use `TypeReference` instead of `Arc` for types that reference other types to avoid stale cache issues when modules are replaced
Applied to files:
crates/biome_test_utils/src/lib.rs
📚 Learning: 2025-11-24T18:05:42.356Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_js_type_info/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:05:42.356Z
Learning: Applies to crates/biome_js_type_info/**/*.rs : Use `TypeData::Unknown` to indicate when type inference falls short or is not implemented
Applied to files:
crates/biome_test_utils/src/lib.rs
📚 Learning: 2025-11-24T18:05:42.356Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_js_type_info/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:05:42.356Z
Learning: Applies to crates/biome_js_type_info/**/*.rs : Always apply the correct `ResolverId` when retrieving raw type data from `ResolvedTypeData.as_raw_data()` to prevent panics during subsequent resolution calls
Applied to files:
crates/biome_test_utils/src/lib.rs
📚 Learning: 2025-11-24T18:05:42.356Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_js_type_info/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:05:42.356Z
Learning: Applies to crates/biome_js_type_info/**/*.rs : When using `ResolvedTypeData`, track the `ResolverId` to ensure subsequent resolver calls use the correct context
Applied to files:
crates/biome_test_utils/src/lib.rs
📚 Learning: 2025-11-24T18:05:42.356Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_js_type_info/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:05:42.356Z
Learning: Applies to crates/biome_js_type_info/**/*.rs : Distinguish between `TypeData::Unknown` and `TypeData::UnknownKeyword` to measure inference effectiveness versus explicit user-provided unknown types
Applied to files:
crates/biome_test_utils/src/lib.rs
📚 Learning: 2025-08-20T16:24:59.781Z
Learnt from: arendjr
Repo: biomejs/biome PR: 7266
File: crates/biome_js_type_info/src/type.rs:94-102
Timestamp: 2025-08-20T16:24:59.781Z
Learning: In crates/biome_js_type_info/src/type.rs, the flattened_union_variants() method returns TypeReference instances that already have the correct module IDs applied to them. These references should be used directly with resolver.resolve_reference() without applying additional module ID transformations, as variant references may originate from nested unions in different modules.
Applied to files:
crates/biome_test_utils/src/lib.rs
📚 Learning: 2025-11-24T18:05:42.356Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_js_type_info/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:05:42.356Z
Learning: Applies to crates/biome_js_type_info/**/scoped_resolver.rs : Implement full inference to resolve `TypeReference::Import` variants across the entire module graph
Applied to files:
crates/biome_test_utils/src/lib.rs
📚 Learning: 2025-11-24T18:04:57.309Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_diagnostics/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:04:57.309Z
Learning: Applies to crates/biome_diagnostics/**/*.rs : Ensure the type implementing Diagnostic derives Debug
Applied to files:
crates/biome_test_utils/src/lib.rs
📚 Learning: 2025-11-24T18:04:57.309Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_diagnostics/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:04:57.309Z
Learning: Applies to crates/biome_diagnostics/**/*.rs : Implement the Diagnostic trait on types, or use the #[derive(Diagnostic)] procedural macro to implement the trait. Configure category, severity, description, message, location, and tags using the #[diagnostic] attribute
Applied to files:
crates/biome_test_utils/src/lib.rs
📚 Learning: 2025-11-27T23:04:02.022Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-11-27T23:04:02.022Z
Learning: Applies to crates/biome_analyze/**/*analyze/src/lint/**/*.rs : The `diagnostic` function must return a `RuleDiagnostic` that defines the message reported to the user using the `markup!` macro
Applied to files:
crates/biome_test_utils/src/lib.rs
🧬 Code graph analysis (2)
crates/biome_package/src/node_js_package/package_json.rs (1)
crates/biome_yaml_parser/src/lib.rs (1)
parse_yaml(14-17)
crates/biome_test_utils/src/lib.rs (1)
crates/biome_package/src/node_js_package/package_json.rs (1)
parse_pnpm_workspace_catalog(126-181)
🔇 Additional comments (4)
crates/biome_package/Cargo.toml (1)
28-29: Integration looks good.The new YAML dependencies are correctly added with
workspace = trueand maintain proper alphabetical ordering within the biome_* group. Before merging, ensure the TOML file has been formatted usingjust fper the project's guidelines.crates/biome_test_utils/src/lib.rs (1)
252-266: Helper + tests give a tidy, isolated PNPM workspace story
find_pnpm_workspace_catalogkeeping the search local topnpm-workspace.yamlnext to the test file matches how the rest of the harness handlespackage.json, and theMemoryFileSystemtests nicely cover both the “workspace present” and “workspace missing” cases.This gives the rest of the stack a clean
Option<Catalogs>to work with, without any surprising I/O.Also applies to: 334-365
crates/biome_package/src/node_js_package/package_json.rs (2)
43-49:catalog:integration into dependency matching is well-shapedThe new
catalogfield onPackageJson, plus thedependency_satisfies/resolve_dependency_versionflow, gives you exactly the behaviour you want:
- Plain semver dependencies still go through unchanged.
catalog:uses the default workspace catalogue.catalog:<name>resolves via named catalogues and correctly overrides the default when explicitly requested.- Unresolved
catalog:entries degrade to a non‑matching literal, which is a sensible failure mode for the linter.
Dependencies::getkeeps the lookup code simple, and the tests here (“with_pnpm_catalog”, “with_named_pnpm_catalog”, and “prefers_named_catalog”) cover the important edge cases for React 19 and similar rules.Nothing I’d block on here. Looks ready for the linter to consume.
Also applies to: 103-122, 352-386, 475-481, 686-715, 777-794
12-20: This review comment references non-existent code and cannot be applied.The functions, structs, and test scenarios described (
parse_pnpm_workspace_catalog,parse_catalog_mapping_entry,extract_*_scalar_*,collect_catalog_dependencies,Catalogs::is_empty) do not exist in the codebase. The suggested test commandcargo test -p biome_package parse_pnpm_workspace_catalog_will not find any matching tests. This feedback appears to be written for a different codebase or a version of the code that was not committed.Likely an incorrect or invalid review comment.
Ah, I apologize. I forgot to handle this. I will do it now. |
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
crates/biome_cli/tests/snapshots/main_cases_diagnostics/reads_pnpm_workspace_catalog.snapis excluded by!**/*.snapand included by**
📒 Files selected for processing (3)
crates/biome_cli/tests/cases/diagnostics.rs(1 hunks)crates/biome_project_layout/src/project_layout.rs(1 hunks)crates/biome_service/src/workspace/server.rs(4 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.rs
📄 CodeRabbit inference engine (CONTRIBUTING.md)
**/*.rs: Use thedbg!()macro for debugging output during testing, and pass the--show-outputflag tocargoto view debug output
Usecargo torcargo testto run tests; for a single test, pass the test name after thetestcommand
Use snapshot testing with theinstacrate; runcargo insta accept,cargo insta reject, orcargo insta reviewto manage snapshot changes
Write doctests as doc comments with code blocks; the code inside code blocks will be run during the testing phase
Usejust f(alias forjust format) to format Rust and TOML files before committing
Files:
crates/biome_project_layout/src/project_layout.rscrates/biome_cli/tests/cases/diagnostics.rscrates/biome_service/src/workspace/server.rs
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
🧠 Learnings (20)
📚 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_project_layout/src/project_layout.rscrates/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_cli/tests/cases/diagnostics.rscrates/biome_service/src/workspace/server.rs
📚 Learning: 2025-11-27T23:04:02.022Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-11-27T23:04:02.022Z
Learning: Applies to crates/biome_analyze/**/tests/specs/**/*.{js,ts,tsx,jsx,json,css} : Test rules using snapshot tests via the `insta` library with test cases in `tests/specs/<group>/<rule_name>/` directories prefixed by `invalid` or `valid`
Applied to files:
crates/biome_cli/tests/cases/diagnostics.rs
📚 Learning: 2025-11-27T23:04:02.022Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-11-27T23:04:02.022Z
Learning: Applies to crates/biome_analyze/**/*analyze/src/lint/**/*.rs : In rule documentation code blocks, mark invalid examples with the `expect_diagnostic` property and valid examples without it; each invalid example must emit exactly one diagnostic
Applied to files:
crates/biome_cli/tests/cases/diagnostics.rs
📚 Learning: 2025-11-27T23:04:02.022Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-11-27T23:04:02.022Z
Learning: Applies to crates/biome_analyze/**/*analyze/src/lint/**/*.rs : Use language tags in documentation code blocks (js, ts, tsx, json, css) and order properties consistently as: language, then `expect_diagnostic`, then options modifiers, then `ignore`, then `file=path`
Applied to files:
crates/biome_cli/tests/cases/diagnostics.rs
📚 Learning: 2025-11-27T23:04:02.022Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-11-27T23:04:02.022Z
Learning: Applies to crates/biome_analyze/**/tests/specs/**/*.jsonc : Use `.jsonc` files in test specs with code snippets as array of strings to test rules in script environment (no import/export syntax)
Applied to files:
crates/biome_cli/tests/cases/diagnostics.rs
📚 Learning: 2025-11-27T23:04:02.022Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-11-27T23:04:02.022Z
Learning: Applies to crates/biome_analyze/**/*analyze/src/lint/**/*.rs : For rules ported from other ecosystems like ESLint or Clippy, add a `sources` field with `RuleSource` metadata using `.same()` for identical behavior or `.inspired()` for different behavior
Applied to files:
crates/biome_cli/tests/cases/diagnostics.rs
📚 Learning: 2025-11-27T23:04:02.022Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-11-27T23:04:02.022Z
Learning: Applies to crates/biome_analyze/**/*analyze/src/lint/**/*.rs : Use `rule_category!()` macro instead of dynamic string parsing to refer to rule diagnostic categories for compile-time validation
Applied to files:
crates/biome_cli/tests/cases/diagnostics.rs
📚 Learning: 2025-11-27T23:04:02.022Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-11-27T23:04:02.022Z
Learning: Applies to crates/biome_analyze/**/*analyze/src/lint/**/*.rs : The `diagnostic` function must return a `RuleDiagnostic` that defines the message reported to the user using the `markup!` macro
Applied to files:
crates/biome_cli/tests/cases/diagnostics.rs
📚 Learning: 2025-11-24T18:04:57.309Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_diagnostics/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:04:57.309Z
Learning: Applies to crates/biome_diagnostics/crates/biome_diagnostics_categories/src/categories.rs : Register all new diagnostic categories in crates/biome_diagnostics_categories/src/categories.rs
Applied to files:
crates/biome_cli/tests/cases/diagnostics.rs
📚 Learning: 2025-11-24T18:04:57.309Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_diagnostics/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:04:57.309Z
Learning: Applies to crates/biome_diagnostics/**/*.rs : Ensure the type implementing Diagnostic derives Debug
Applied to files:
crates/biome_cli/tests/cases/diagnostics.rs
📚 Learning: 2025-11-24T18:05:20.371Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_formatter/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:05:20.371Z
Learning: Create test infrastructure with `tests/specs` folder structure and `spec_test.rs`, `spec_tests.rs`, and `language.rs` files in test directories
Applied to files:
crates/biome_cli/tests/cases/diagnostics.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-11-28T09:08:10.091Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: CONTRIBUTING.md:0-0
Timestamp: 2025-11-28T09:08:10.091Z
Learning: Applies to Cargo.toml : Use workspace dependencies in the root Cargo.toml; internal crates should use `workspace = true` for dependencies and path dependencies for dev-dependencies
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/client.rs : Use WorkspaceClient implementation for creating connections to the daemon and communicating with WorkspaceServer
Applied to files:
crates/biome_service/src/workspace/server.rs
📚 Learning: 2025-11-27T23:04:02.022Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-11-27T23:04:02.022Z
Learning: Applies to crates/biome_analyze/**/*analyze/src/lint/**/*.rs : Check if a variable is global before banning it to avoid false positives when the variable is redeclared in local scope; use the semantic model to verify global scope
Applied to files:
crates/biome_service/src/workspace/server.rs
📚 Learning: 2025-11-27T23:04:02.022Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-11-27T23:04:02.022Z
Learning: Applies to crates/biome_analyze/**/*analyze/src/lint/nursery/**/*.rs : New rules must be placed inside the `nursery` group as an incubation space exempt from semantic versioning, with promotion to appropriate groups done during minor/major releases
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.rs : Use WorkspaceWatcher to keep workspace state in sync with the filesystem, and only activate it in daemon mode
Applied to files:
crates/biome_service/src/workspace/server.rs
📚 Learning: 2025-11-24T18:05:42.356Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_js_type_info/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:05:42.356Z
Learning: Applies to crates/biome_js_type_info/**/*.rs : Use `TypeReference` instead of `Arc` for types that reference other types to avoid stale cache issues when modules are replaced
Applied to files:
crates/biome_service/src/workspace/server.rs
📚 Learning: 2025-11-24T18:06:03.545Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_parser/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:06:03.545Z
Learning: Applies to crates/biome_parser/**/src/**/*.rs : A parser struct must implement the `Parser` trait and save the token source, parser context, and optional parser options
Applied to files:
crates/biome_service/src/workspace/server.rs
🧬 Code graph analysis (1)
crates/biome_service/src/workspace/server.rs (1)
crates/biome_package/src/node_js_package/package_json.rs (1)
parse_pnpm_workspace_catalog(126-181)
🔇 Additional comments (4)
crates/biome_project_layout/src/project_layout.rs (1)
253-256: LGTM!The implementation correctly exposes tracked package paths. The approach of collecting and cloning keys is appropriate for the catalog propagation use case described in the PR.
crates/biome_cli/tests/cases/diagnostics.rs (1)
101-174: New pnpm workspace catalogue CLI test looks solidThe setup mirrors a realistic pnpm workspace + catalogue scenario and cleanly exercises
noReactForwardRefvia the CLI with an insta snapshot. Naming, fixtures, and expectations all line up with the intended regression coverage; I do not see any changes needed here.crates/biome_service/src/workspace/server.rs (2)
38-38: LGTM!Import addition for
Catalogsis correctly placed alongside the existingPackageJsonandPackageTypeimports.
857-876: LGTM!The logic correctly loads the pnpm catalog and attaches it only when the manifest doesn't already have one, which properly respects inline catalog definitions in the package.json itself.
|
@dyc3 I've added the implementation to load pnpm-workspace.yaml. |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
crates/biome_test_utils/src/lib.rs (1)
271-301: Catalogue injection is correct; consider de-duplicating file reads.You read
package_json_fileviastd::fs::read_to_string(Line 274) and then again viaPackageJson::read_manifest(Line 275). If you want to trim I/O, consider reusing thejsonstring as the parse input (or otherwise centralising the read), while still keeping thefs-based path for consistency.crates/biome_service/src/workspace/server.rs (2)
857-873: Avoid the “insert then re-fetch then re-insert” dance if you can.Current flow: insert the serialised manifest, then fetch the deserialised manifest, then potentially re-insert after setting
catalog. Ifinsert_serialized_node_manifestalready yields / stores enough to avoid the second lookup, consider streamlining to a single insertion path for less churn (and fewer chances for races when updates are rapid).
895-915: Please confirmpackage_paths()is a snapshot before mutatingproject_layoutinside the loop.If
project_layout.package_paths()is a live iterator over internal state, re-inserting manifests while iterating could cause missed/duplicated paths or (worse) internal borrow/concurrency surprises. A safe pattern is to collect first, then mutate:- for package_path in self.project_layout.package_paths() { + let package_paths: Vec<_> = self.project_layout.package_paths().collect(); + for package_path in package_paths { ... - self.project_layout - .insert_node_manifest(package_path.clone(), manifest); + self.project_layout.insert_node_manifest(package_path.clone(), manifest); } }crates/biome_package/src/node_js_package/package_json.rs (1)
354-388:catalog:resolution logic is tidy; consider trimmingrestfor resilience.If someone writes
catalog: legacy(accidental whitespace),strip_prefix("catalog:")leaves" legacy", which won’t match. You may want alet rest = rest.trim();before lookup.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
crates/biome_package/src/node_js_package/package_json.rs(5 hunks)crates/biome_service/src/workspace/server.rs(4 hunks)crates/biome_test_utils/src/lib.rs(4 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.rs
📄 CodeRabbit inference engine (CONTRIBUTING.md)
**/*.rs: Use thedbg!()macro for debugging output during testing, and pass the--show-outputflag tocargoto view debug output
Usecargo torcargo testto run tests; for a single test, pass the test name after thetestcommand
Use snapshot testing with theinstacrate; runcargo insta accept,cargo insta reject, orcargo insta reviewto manage snapshot changes
Write doctests as doc comments with code blocks; the code inside code blocks will be run during the testing phase
Usejust f(alias forjust format) to format Rust and TOML files before committing
Files:
crates/biome_package/src/node_js_package/package_json.rscrates/biome_test_utils/src/lib.rscrates/biome_service/src/workspace/server.rs
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
🧠 Learnings (34)
📚 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:
crates/biome_package/src/node_js_package/package_json.rscrates/biome_service/src/workspace/server.rs
📚 Learning: 2025-09-25T12:32:59.003Z
Learnt from: arendjr
Repo: biomejs/biome PR: 7593
File: crates/biome_service/src/workspace/server.rs:1306-1306
Timestamp: 2025-09-25T12:32:59.003Z
Learning: In the biomejs/biome project, do not flag compilation errors during code review as they are handled by the existing test infrastructure and CI. Focus on other code quality aspects instead.
Applied to files:
crates/biome_package/src/node_js_package/package_json.rscrates/biome_service/src/workspace/server.rs
📚 Learning: 2025-11-27T23:04:02.022Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-11-27T23:04:02.022Z
Learning: Commit rule changes with message format: `feat(biome_<crate>): <ruleName>` to follow Biome's conventional commit style
Applied to files:
crates/biome_package/src/node_js_package/package_json.rs
📚 Learning: 2025-11-27T23:04:02.022Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-11-27T23:04:02.022Z
Learning: Applies to crates/biome_analyze/**/*analyze/src/lint/**/*.rs : Avoid using `unwrap()` or `expect()` on `Result` and `Option` types; instead use helper functions like `map`, `filter`, `and_then` to maintain code clarity and avoid panics
Applied to files:
crates/biome_package/src/node_js_package/package_json.rs
📚 Learning: 2025-11-24T18:05:27.810Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_js_formatter/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:05:27.810Z
Learning: Applies to crates/biome_js_formatter/**/*.rs : Import the `FormatNode` trait and implement it for your Node when creating formatters in biome_js_formatter
Applied to files:
crates/biome_package/src/node_js_package/package_json.rscrates/biome_test_utils/src/lib.rs
📚 Learning: 2025-11-24T18:06:03.545Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_parser/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:06:03.545Z
Learning: Applies to crates/biome_parser/**/src/**/*.rs : Use `ParseSeparatedList` and `ParseNodeList` for parsing lists with error recovery to avoid infinite loops
Applied to files:
crates/biome_package/src/node_js_package/package_json.rscrates/biome_test_utils/src/lib.rs
📚 Learning: 2025-11-24T18:05:20.371Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_formatter/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:05:20.371Z
Learning: Applies to crates/biome_formatter/**/biome_*_formatter/src/lib.rs : Expose a public `format_node` function that accepts formatting options and a root syntax node, returning a `FormatResult<Formatted<Context>>` with appropriate documentation
Applied to files:
crates/biome_package/src/node_js_package/package_json.rs
📚 Learning: 2025-11-24T18:06:03.545Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_parser/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:06:03.545Z
Learning: Applies to crates/biome_parser/**/src/**/*.rs : A parser struct must implement the `Parser` trait and save the token source, parser context, and optional parser options
Applied to files:
crates/biome_package/src/node_js_package/package_json.rscrates/biome_service/src/workspace/server.rs
📚 Learning: 2025-11-24T18:06:03.545Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_parser/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:06:03.545Z
Learning: Applies to crates/biome_parser/**/src/**/*.rs : Use `ConditionalParsedSyntax` for syntax that is only valid in specific contexts (e.g., strict mode, file types, language versions) and call `or_invalid_to_bogus()` to convert to a bogus node if not supported
Applied to files:
crates/biome_package/src/node_js_package/package_json.rs
📚 Learning: 2025-11-24T18:05:27.810Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_js_formatter/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:05:27.810Z
Learning: Applies to crates/biome_js_formatter/**/*.rs : Do not attempt to 'fix' the code; if a token/node is known to be mandatory but is missing, return `None` instead
Applied to files:
crates/biome_package/src/node_js_package/package_json.rs
📚 Learning: 2025-11-24T18:06:03.545Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_parser/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:06:03.545Z
Learning: Applies to crates/biome_parser/**/src/**/*.rs : Use `p.eat(token)` for optional tokens, `p.expect(token)` for required tokens, `parse_rule(p).ok(p)` for optional nodes, and `parse_rule(p).or_add_diagnostic(p, error)` for required nodes
Applied to files:
crates/biome_package/src/node_js_package/package_json.rs
📚 Learning: 2025-11-28T09:08:10.091Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: CONTRIBUTING.md:0-0
Timestamp: 2025-11-28T09:08:10.091Z
Learning: Applies to Cargo.toml : Use workspace dependencies in the root Cargo.toml; internal crates should use `workspace = true` for dependencies and path dependencies for dev-dependencies
Applied to files:
crates/biome_package/src/node_js_package/package_json.rscrates/biome_test_utils/src/lib.rscrates/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_package/src/node_js_package/package_json.rscrates/biome_test_utils/src/lib.rscrates/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*.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_test_utils/src/lib.rscrates/biome_service/src/workspace/server.rs
📚 Learning: 2025-11-24T18:05:20.371Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_formatter/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:05:20.371Z
Learning: Applies to crates/biome_formatter/**/biome_*_formatter/Cargo.toml : Include development dependencies in `Cargo.toml` for formatter tests: `biome_formatter_test`, `biome_<language>_factory`, `biome_<language>_parser`, `biome_parser`, `biome_service`, `countme`, `iai`, `quickcheck`, `quickcheck_macros`, and `tests_macros`
Applied to files:
crates/biome_test_utils/src/lib.rs
📚 Learning: 2025-11-24T18:05:20.371Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_formatter/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:05:20.371Z
Learning: Applies to crates/biome_formatter/**/biome_*_formatter/tests/spec_tests.rs : Use the `tests_macros::gen_tests!` macro in `spec_tests.rs` to generate test functions for each specification file matching the pattern `tests/specs/<language>/**/*.<ext>`
Applied to files:
crates/biome_test_utils/src/lib.rs
📚 Learning: 2025-11-28T09:08:10.091Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: CONTRIBUTING.md:0-0
Timestamp: 2025-11-28T09:08:10.091Z
Learning: Internal crate changes that don't affect user-facing behavior do not require changesets; changesets are only for user-facing changes
Applied to files:
crates/biome_test_utils/src/lib.rs
📚 Learning: 2025-11-24T18:05:42.356Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_js_type_info/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:05:42.356Z
Learning: Applies to crates/biome_js_type_info/**/*.rs : Store type data in linear vectors instead of using recursive data structures with `Arc` for improved data locality and performance
Applied to files:
crates/biome_test_utils/src/lib.rs
📚 Learning: 2025-11-24T18:04:57.309Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_diagnostics/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:04:57.309Z
Learning: Applies to crates/biome_diagnostics/crates/biome_diagnostics_categories/src/categories.rs : Register all new diagnostic categories in crates/biome_diagnostics_categories/src/categories.rs
Applied to files:
crates/biome_test_utils/src/lib.rs
📚 Learning: 2025-11-24T18:05:42.356Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_js_type_info/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:05:42.356Z
Learning: Applies to crates/biome_js_type_info/**/*.rs : Use `TypeReference` instead of `Arc` for types that reference other types to avoid stale cache issues when modules are replaced
Applied to files:
crates/biome_test_utils/src/lib.rs
📚 Learning: 2025-11-24T18:05:42.356Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_js_type_info/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:05:42.356Z
Learning: Applies to crates/biome_js_type_info/**/*.rs : Use `TypeReference` variants (`Qualifier`, `Resolved`, `Import`, `Unknown`) to represent different phases of type resolution
Applied to files:
crates/biome_test_utils/src/lib.rs
📚 Learning: 2025-11-24T18:05:42.356Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_js_type_info/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:05:42.356Z
Learning: Applies to crates/biome_js_type_info/**/*.rs : Use `TypeData::Unknown` to indicate when type inference falls short or is not implemented
Applied to files:
crates/biome_test_utils/src/lib.rs
📚 Learning: 2025-11-24T18:05:42.356Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_js_type_info/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:05:42.356Z
Learning: Applies to crates/biome_js_type_info/**/*.rs : Always apply the correct `ResolverId` when retrieving raw type data from `ResolvedTypeData.as_raw_data()` to prevent panics during subsequent resolution calls
Applied to files:
crates/biome_test_utils/src/lib.rs
📚 Learning: 2025-11-24T18:05:42.356Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_js_type_info/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:05:42.356Z
Learning: Applies to crates/biome_js_type_info/**/*.rs : When using `ResolvedTypeData`, track the `ResolverId` to ensure subsequent resolver calls use the correct context
Applied to files:
crates/biome_test_utils/src/lib.rs
📚 Learning: 2025-11-24T18:05:42.356Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_js_type_info/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:05:42.356Z
Learning: Applies to crates/biome_js_type_info/**/*.rs : Distinguish between `TypeData::Unknown` and `TypeData::UnknownKeyword` to measure inference effectiveness versus explicit user-provided unknown types
Applied to files:
crates/biome_test_utils/src/lib.rs
📚 Learning: 2025-08-20T16:24:59.781Z
Learnt from: arendjr
Repo: biomejs/biome PR: 7266
File: crates/biome_js_type_info/src/type.rs:94-102
Timestamp: 2025-08-20T16:24:59.781Z
Learning: In crates/biome_js_type_info/src/type.rs, the flattened_union_variants() method returns TypeReference instances that already have the correct module IDs applied to them. These references should be used directly with resolver.resolve_reference() without applying additional module ID transformations, as variant references may originate from nested unions in different modules.
Applied to files:
crates/biome_test_utils/src/lib.rs
📚 Learning: 2025-11-24T18:04:57.309Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_diagnostics/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:04:57.309Z
Learning: Applies to crates/biome_diagnostics/**/*.rs : Use helper types from the biome_diagnostics::v2 module (CodeFrameAdvice, CommandAdvice, DiffAdvice, LogAdvice) or implement the Advices trait yourself for custom advice handling
Applied to files:
crates/biome_test_utils/src/lib.rs
📚 Learning: 2025-11-24T18:05:42.356Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_js_type_info/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:05:42.356Z
Learning: Applies to crates/biome_js_type_info/**/scoped_resolver.rs : Implement full inference to resolve `TypeReference::Import` variants across the entire module graph
Applied to files:
crates/biome_test_utils/src/lib.rs
📚 Learning: 2025-11-24T18:04:57.309Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_diagnostics/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:04:57.309Z
Learning: Applies to crates/biome_diagnostics/**/*.rs : Ensure the type implementing Diagnostic derives Debug
Applied to files:
crates/biome_test_utils/src/lib.rs
📚 Learning: 2025-11-24T18:04:57.309Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_diagnostics/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:04:57.309Z
Learning: Applies to crates/biome_diagnostics/**/*.rs : Implement the Diagnostic trait on types, or use the #[derive(Diagnostic)] procedural macro to implement the trait. Configure category, severity, description, message, location, and tags using the #[diagnostic] attribute
Applied to files:
crates/biome_test_utils/src/lib.rs
📚 Learning: 2025-11-27T23:04:02.022Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-11-27T23:04:02.022Z
Learning: Applies to crates/biome_analyze/**/*analyze/src/lint/**/*.rs : The `diagnostic` function must return a `RuleDiagnostic` that defines the message reported to the user using the `markup!` macro
Applied to files:
crates/biome_test_utils/src/lib.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-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/client.rs : Use WorkspaceClient implementation for creating connections to the daemon and communicating with WorkspaceServer
Applied to files:
crates/biome_service/src/workspace/server.rs
📚 Learning: 2025-11-27T23:04:02.022Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-11-27T23:04:02.022Z
Learning: Applies to crates/biome_analyze/**/*analyze/src/lint/**/*.rs : Use language tags in documentation code blocks (js, ts, tsx, json, css) and order properties consistently as: language, then `expect_diagnostic`, then options modifiers, then `ignore`, then `file=path`
Applied to files:
crates/biome_service/src/workspace/server.rs
🧬 Code graph analysis (2)
crates/biome_test_utils/src/lib.rs (1)
crates/biome_package/src/node_js_package/package_json.rs (1)
parse_pnpm_workspace_catalog(126-182)
crates/biome_service/src/workspace/server.rs (1)
crates/biome_package/src/node_js_package/package_json.rs (1)
parse_pnpm_workspace_catalog(126-182)
🔇 Additional comments (5)
crates/biome_test_utils/src/lib.rs (2)
252-263: Nice: test-harness lookup is now isolated (no ancestor crawl).This keeps catalogue discovery predictable for fixtures and aligns with the “adjacent manifest” style used elsewhere in the harness.
331-356: Good coverage for presence/absence.The
MemoryFileSystemtests validate the intended behaviour without touching the real FS.crates/biome_package/src/node_js_package/package_json.rs (3)
43-49: Docs forPackageJson::catalogare clear and practical.Explains provenance (“from
pnpm-workspace.yaml”) and the resolution behaviour succinctly.
124-182: Parsing approach is robust (error-tolerant + skips complex YAML).The “only accept scalar key/value pairs” rule is a sensible guardrail and should prevent surprising crashes on real-world workspace files.
Also applies to: 207-352
688-797: Tests cover the important bits (default + named + preference).Nice set of small, targeted cases that lock the behaviour in without overfitting.
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_test_utils/src/lib.rs(4 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
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_test_utils/src/lib.rs
🧠 Learnings (21)
📚 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_test_utils/src/lib.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*.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_test_utils/src/lib.rs
📚 Learning: 2025-11-24T18:05:20.371Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_formatter/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:05:20.371Z
Learning: Applies to crates/biome_formatter/**/biome_*_formatter/Cargo.toml : Include development dependencies in `Cargo.toml` for formatter tests: `biome_formatter_test`, `biome_<language>_factory`, `biome_<language>_parser`, `biome_parser`, `biome_service`, `countme`, `iai`, `quickcheck`, `quickcheck_macros`, and `tests_macros`
Applied to files:
crates/biome_test_utils/src/lib.rs
📚 Learning: 2025-12-12T10:11:05.549Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: CONTRIBUTING.md:0-0
Timestamp: 2025-12-12T10:11:05.549Z
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_test_utils/src/lib.rs
📚 Learning: 2025-11-24T18:05:20.371Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_formatter/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:05:20.371Z
Learning: Applies to crates/biome_formatter/**/biome_*_formatter/tests/spec_tests.rs : Use the `tests_macros::gen_tests!` macro in `spec_tests.rs` to generate test functions for each specification file matching the pattern `tests/specs/<language>/**/*.<ext>`
Applied to files:
crates/biome_test_utils/src/lib.rs
📚 Learning: 2025-11-24T18:06:03.545Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_parser/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:06:03.545Z
Learning: Applies to crates/biome_parser/**/src/**/*.rs : Use `ParseSeparatedList` and `ParseNodeList` for parsing lists with error recovery to avoid infinite loops
Applied to files:
crates/biome_test_utils/src/lib.rs
📚 Learning: 2025-11-24T18:04:57.309Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_diagnostics/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:04:57.309Z
Learning: Applies to crates/biome_diagnostics/crates/biome_diagnostics_categories/src/categories.rs : Register all new diagnostic categories in crates/biome_diagnostics_categories/src/categories.rs
Applied to files:
crates/biome_test_utils/src/lib.rs
📚 Learning: 2025-12-12T10:11:05.549Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: CONTRIBUTING.md:0-0
Timestamp: 2025-12-12T10:11:05.549Z
Learning: Applies to crates/**/*.rs : Update inline rustdoc documentation for rules, assists, and their options when adding new features or changing existing features in Rust crates
Applied to files:
crates/biome_test_utils/src/lib.rs
📚 Learning: 2025-12-04T13:29:49.287Z
Learnt from: dyc3
Repo: biomejs/biome PR: 8291
File: crates/biome_html_formatter/tests/specs/prettier/vue/html-vue/elastic-header.html:10-10
Timestamp: 2025-12-04T13:29:49.287Z
Learning: Files under `crates/biome_html_formatter/tests/specs/prettier` are test fixtures synced from Prettier and should not receive detailed code quality reviews (e.g., HTTP vs HTTPS, formatting suggestions, etc.). These files are test data meant to validate formatter behavior and should be preserved as-is.
Applied to files:
crates/biome_test_utils/src/lib.rs
📚 Learning: 2025-11-24T18:05:42.356Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_js_type_info/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:05:42.356Z
Learning: Applies to crates/biome_js_type_info/**/*.rs : Store type data in linear vectors instead of using recursive data structures with `Arc` for improved data locality and performance
Applied to files:
crates/biome_test_utils/src/lib.rs
📚 Learning: 2025-11-24T18:05:42.356Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_js_type_info/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:05:42.356Z
Learning: Applies to crates/biome_js_type_info/**/*.rs : Use `TypeReference` variants (`Qualifier`, `Resolved`, `Import`, `Unknown`) to represent different phases of type resolution
Applied to files:
crates/biome_test_utils/src/lib.rs
📚 Learning: 2025-11-24T18:05:42.356Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_js_type_info/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:05:42.356Z
Learning: Applies to crates/biome_js_type_info/**/*.rs : Use `TypeReference` instead of `Arc` for types that reference other types to avoid stale cache issues when modules are replaced
Applied to files:
crates/biome_test_utils/src/lib.rs
📚 Learning: 2025-11-24T18:05:42.356Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_js_type_info/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:05:42.356Z
Learning: Applies to crates/biome_js_type_info/**/*.rs : Use `TypeData::Unknown` to indicate when type inference falls short or is not implemented
Applied to files:
crates/biome_test_utils/src/lib.rs
📚 Learning: 2025-11-24T18:05:42.356Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_js_type_info/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:05:42.356Z
Learning: Applies to crates/biome_js_type_info/**/*.rs : Always apply the correct `ResolverId` when retrieving raw type data from `ResolvedTypeData.as_raw_data()` to prevent panics during subsequent resolution calls
Applied to files:
crates/biome_test_utils/src/lib.rs
📚 Learning: 2025-11-24T18:05:42.356Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_js_type_info/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:05:42.356Z
Learning: Applies to crates/biome_js_type_info/**/*.rs : When using `ResolvedTypeData`, track the `ResolverId` to ensure subsequent resolver calls use the correct context
Applied to files:
crates/biome_test_utils/src/lib.rs
📚 Learning: 2025-11-24T18:05:42.356Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_js_type_info/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:05:42.356Z
Learning: Applies to crates/biome_js_type_info/**/*.rs : Distinguish between `TypeData::Unknown` and `TypeData::UnknownKeyword` to measure inference effectiveness versus explicit user-provided unknown types
Applied to files:
crates/biome_test_utils/src/lib.rs
📚 Learning: 2025-08-20T16:24:59.781Z
Learnt from: arendjr
Repo: biomejs/biome PR: 7266
File: crates/biome_js_type_info/src/type.rs:94-102
Timestamp: 2025-08-20T16:24:59.781Z
Learning: In crates/biome_js_type_info/src/type.rs, the flattened_union_variants() method returns TypeReference instances that already have the correct module IDs applied to them. These references should be used directly with resolver.resolve_reference() without applying additional module ID transformations, as variant references may originate from nested unions in different modules.
Applied to files:
crates/biome_test_utils/src/lib.rs
📚 Learning: 2025-11-24T18:04:57.309Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_diagnostics/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:04:57.309Z
Learning: Applies to crates/biome_diagnostics/**/*.rs : Use helper types from the biome_diagnostics::v2 module (CodeFrameAdvice, CommandAdvice, DiffAdvice, LogAdvice) or implement the Advices trait yourself for custom advice handling
Applied to files:
crates/biome_test_utils/src/lib.rs
📚 Learning: 2025-11-24T18:05:27.810Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_js_formatter/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:05:27.810Z
Learning: Applies to crates/biome_js_formatter/**/*.rs : Import the `FormatNode` trait and implement it for your Node when creating formatters in biome_js_formatter
Applied to files:
crates/biome_test_utils/src/lib.rs
📚 Learning: 2025-11-24T18:05:42.356Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_js_type_info/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:05:42.356Z
Learning: Applies to crates/biome_js_type_info/**/scoped_resolver.rs : Implement full inference to resolve `TypeReference::Import` variants across the entire module graph
Applied to files:
crates/biome_test_utils/src/lib.rs
📚 Learning: 2025-11-24T18:05:20.371Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_formatter/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:05:20.371Z
Learning: Create test infrastructure with `tests/specs` folder structure and `spec_test.rs`, `spec_tests.rs`, and `language.rs` files in test directories
Applied to files:
crates/biome_test_utils/src/lib.rs
🧬 Code graph analysis (1)
crates/biome_test_utils/src/lib.rs (1)
crates/biome_package/src/node_js_package/package_json.rs (1)
parse_pnpm_workspace_catalog(126-182)
🔇 Additional comments (3)
crates/biome_test_utils/src/lib.rs (3)
19-19: LGTM!The import addition is necessary for the new PNPM catalog support.
271-271: LGTM!The integration correctly discovers the PNPM workspace catalog and applies it to the manifest when not already present. The logic respects any existing catalog and only augments manifests that lack catalog information.
Also applies to: 290-293, 300-300
679-703: LGTM!The tests properly validate both the happy path (catalog found and parsed) and the negative case (workspace file missing). Using
MemoryFileSystemensures test isolation as per best practices.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
crates/biome_service/src/workspace/server.rs (1)
821-839: Supportpnpm-workspace.ymlas well as.yaml(currently missed).
Right now both the ancestor search (Line 826) and the file-event branch (Line 893) only matchpnpm-workspace.yaml, so the common.ymlvariant won’t update/clear catalogs.fn load_pnpm_workspace_catalog(&self, start_dir: &Utf8Path) -> Option<Catalogs> { for dir in start_dir.ancestors() { - let workspace_file = dir.join("pnpm-workspace.yaml"); - if !self.fs.path_is_file(&workspace_file) { - continue; - } + let workspace_file_yaml = dir.join("pnpm-workspace.yaml"); + let workspace_file_yml = dir.join("pnpm-workspace.yml"); + let workspace_file = if self.fs.path_is_file(&workspace_file_yaml) { + workspace_file_yaml + } else if self.fs.path_is_file(&workspace_file_yml) { + workspace_file_yml + } else { + continue; + }; if let Ok(content) = self.fs.read_file_from_path(&workspace_file) && let Some(catalog) = PackageJson::parse_pnpm_workspace_catalog(&content) { return Some(catalog); } } None } @@ - } else if filename.is_some_and(|filename| filename == "pnpm-workspace.yaml") { + } else if filename.is_some_and(|filename| { + filename == "pnpm-workspace.yaml" || filename == "pnpm-workspace.yml" + }) { let workspace_root = path .parent() .map(|parent| parent.to_path_buf()) .unwrap_or_default(); let pnpm_catalog = self.load_pnpm_workspace_catalog(&workspace_root);Also applies to: 893-914
🧹 Nitpick comments (1)
crates/biome_service/src/workspace/server.rs (1)
855-871: Optional: avoid filesystem walk when the manifest already has a catalog.
You loadpnpm_catalogunconditionally (Line 855), but only apply it whenmanifest.catalog.is_none()(Line 866); you could check the manifest first and skip the ancestor scan most of the time.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lockand included by**
📒 Files selected for processing (1)
crates/biome_service/src/workspace/server.rs(4 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
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
🧠 Learnings (10)
📚 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-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-12T10:11:05.549Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: CONTRIBUTING.md:0-0
Timestamp: 2025-12-12T10:11:05.549Z
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/client.rs : Use WorkspaceClient implementation for creating connections to the daemon and communicating with WorkspaceServer
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:
crates/biome_service/src/workspace/server.rs
📚 Learning: 2025-09-25T12:32:59.003Z
Learnt from: arendjr
Repo: biomejs/biome PR: 7593
File: crates/biome_service/src/workspace/server.rs:1306-1306
Timestamp: 2025-09-25T12:32:59.003Z
Learning: In the biomejs/biome project, do not flag compilation errors during code review as they are handled by the existing test infrastructure and CI. Focus on other code quality aspects instead.
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.rs : Use WorkspaceWatcher to keep workspace state in sync with the filesystem, and only activate it in daemon mode
Applied to files:
crates/biome_service/src/workspace/server.rs
📚 Learning: 2025-11-27T23:04:02.022Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-11-27T23:04:02.022Z
Learning: Commit rule changes with message format: `feat(biome_<crate>): <ruleName>` to follow Biome's conventional commit style
Applied to files:
crates/biome_service/src/workspace/server.rs
📚 Learning: 2025-11-24T18:06:03.545Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_parser/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:06:03.545Z
Learning: Applies to crates/biome_parser/**/src/**/*.rs : A parser struct must implement the `Parser` trait and save the token source, parser context, and optional parser options
Applied to files:
crates/biome_service/src/workspace/server.rs
🧬 Code graph analysis (1)
crates/biome_service/src/workspace/server.rs (1)
crates/biome_package/src/node_js_package/package_json.rs (1)
parse_pnpm_workspace_catalog(126-182)
🔇 Additional comments (1)
crates/biome_service/src/workspace/server.rs (1)
38-38: Nice: keeping PNPM-catalog logic encapsulated viabiome_packagetypes.
KeepsWorkspaceServerfrom growing its own YAML parsing habits.
apple-yagi
force-pushed
the
fix/no-react-forward-ref-pnpm-catalog
branch
from
5e0dc88 to
e4384d2
Compare
ematipico
left a comment
ematipico
left a comment
There was a problem hiding this comment.
So, I believe code wise the feature is fine, however I'm a bit concerned about the stability.
The feature relies on the reading of the yaml lock file, which has its own format and conventions, which can change from version to another of pnpm. It's not the first time that happens. If that happens, this feature breaks and Biome doesn't work anymore.
Have you thought about it? How do you plan to address that?
|
Ah one more thing: this must be documented somewhere! We can't add a feature and not document it somewhere |
|
@ematipico Thanks, that’s a very fair concern. I agree that relying on I also considered an alternative approach: emulating Node.js So my intention is to keep this feature fail-safe with the current approach:
I’ll add tests for those failure scenarios and document this behavior so the stability guarantees are explicit. |
I think relying on pnpm-lock.yaml and pnpm-workspace.yaml is different; spec of pnpm-workspace.yaml will be unlikely changed between pnpm versions. The latter is intended to be edited by a user by hand, while the former is intended to be auto-generated. So I don't think relying on pnpm-workspace.yaml will be fragile at this point. |
Not that fragile, but things might be added in minors, and break in majors. I'm happy to make the system fail-safe, with a good messaging from our end. If possible, I'd prefer to take same road we took with editor config. Opt-in via configuration. |
Thanks for the feedback. I’d like to check whether we really need a flag for this. My reason is that when Loading So I think we may not need a flag. I will add tests that guarantee fail-safe behavior and add documentation for That said, I can also implement it as opt-in if you prefer. |
|
Sorry I thought we were reading the lock file! I have mixed feelings. From one hand I understand your argument @apple-yagi where doing things automatically is best, however we tried the same approach with editorconfig (true by default), and it was a mess DX wise because it turned out that people were using things wrong. Still, I would like to make opt-in for now for two reasons:
So here my proposal, having a brand new config named What do you @siketyan ? |
I agree, the two reasons make sense for me.
Maybe |
|
@ematipico Thanks for the background and explanation. |
apple-yagi
force-pushed
the
fix/no-react-forward-ref-pnpm-catalog
branch
from
bada5b9 to
de1cccd
Compare
|
I changed the implementation to support pnpm catalogs as an opt-in feature. I also updated it to detect changes to pnpm-workspace.yaml in real time. |
5 participants
Summary
resolved #8393
noReactForwardRefresolve React versions through pnpm catalogs(catalog:/catalog:<name>) whenjavascript.resolver.experimentalPnpmCatalogsis enabledpnpm-workspace.yamlchangesTest Plan
cargo testcargo build --releasejavascript.resolver.experimentalPnpmCatalogs: true,noReactForwardReftriggers for React 19 resolved viacatalog:/catalog:<name>pnpm-workspace.yamlupdates diagnostics without restartingDocs
AI Assistance