feat(biome_js_analyze): custom jsxFactory and jsxFragmentFactory #7847
Conversation
🦋 Changeset detectedLatest commit: fc4e4d6 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 |
WalkthroughThis PR extends Biome to respect custom JSX factory identifiers from tsconfig.json when using the classic JSX runtime. It adds jsx_factory and jsx_fragment_factory configuration options that flow through AnalyzerConfiguration → AnalyzerOptions → RuleContext, with automatic population from tsconfig during analysis. The noUnusedImports rule now recognises custom factory imports (e.g., Preact's Possibly related PRs
Suggested labels
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (1)
🧰 Additional context used📓 Path-based instructions (3)**/*.{rs,toml}📄 CodeRabbit inference engine (CONTRIBUTING.md)
Files:
**/*.rs📄 CodeRabbit inference engine (CONTRIBUTING.md)
Files:
.changeset/*.md📄 CodeRabbit inference engine (CONTRIBUTING.md)
Files:
🔇 Additional comments (6)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
crates/biome_service/src/settings.rs (1)
1796-1819: Propagate factory settings in overrides tooOverrides currently merge only jsx_runtime; jsx_factory/jsx_fragment_factory are dropped, which can surprise users overriding per‑path settings.
Consider:
- language_setting.environment.jsx_runtime = - conf.jsx_runtime.or(parent_settings.environment.jsx_runtime); + language_setting.environment.jsx_runtime = + conf.jsx_runtime.or(parent_settings.environment.jsx_runtime); + language_setting.environment.jsx_factory = + conf.jsx_factory.or_else(|| parent_settings.environment.jsx_factory.clone()); + language_setting.environment.jsx_fragment_factory = + conf.jsx_fragment_factory.or_else(|| parent_settings.environment.jsx_fragment_factory.clone());
🧹 Nitpick comments (9)
crates/biome_js_analyze/src/react.rs (1)
338-345: Consider validating import source.Unlike
is_global_react_import(which checks declaration type and import source), this function only checks the binding name. This might match local variables with the same name as the factory.If the JSX factory configuration expects imports from specific modules (e.g., 'preact'), consider adding source validation similar to
is_global_react_import.crates/biome_configuration/src/javascript/mod.rs (1)
48-60: JSX factory fields: good addition; consider normalising inputsTwo minor polish items:
- Trim whitespace on values at read time to avoid config nits.
- Clarify in rustdoc that these should be base identifiers (before any dot), since downstream code often expects that.
Happy path as-is; these are non-blocking.
crates/biome_js_analyze/tests/spec_tests.rs (1)
182-200: Avoid double tsconfig lookup and trim identifiers
- Query tsconfig once and reuse for factory + fragment.
- Trim the identifiers to be safe.
Example tweak:
- if options.jsx_runtime() == Some(JsxRuntime::ReactClassic) { - if options.jsx_factory().is_none() { - let factory = project_layout.query_tsconfig_for_path(input_file, |tsconfig| { - tsconfig.jsx_factory_identifier().map(|s| s.to_string()) - }).flatten(); - options.set_jsx_factory(factory.map(|s| s.into())); - } - if options.jsx_fragment_factory().is_none() { - let fragment_factory = project_layout.query_tsconfig_for_path(input_file, |tsconfig| { - tsconfig.jsx_fragment_factory_identifier().map(|s| s.to_string()) - }).flatten(); - options.set_jsx_fragment_factory(fragment_factory.map(|s| s.into())); - } - } + if options.jsx_runtime() == Some(JsxRuntime::ReactClassic) { + if options.jsx_factory().is_none() || options.jsx_fragment_factory().is_none() { + if let Some(tsconfig) = project_layout.find_tsconfig_for_path(input_file) { + if options.jsx_factory().is_none() { + let f = tsconfig.jsx_factory_identifier().map(|s| s.trim().to_string()); + options.set_jsx_factory(f.map(Into::into)); + } + if options.jsx_fragment_factory().is_none() { + let ff = tsconfig.jsx_fragment_factory_identifier().map(|s| s.trim().to_string()); + options.set_jsx_fragment_factory(ff.map(Into::into)); + } + } + } + }crates/biome_js_analyze/src/lint/correctness/no_unused_imports.rs (1)
626-642: Nice exemption for custom JSX factories; consider sharing helperLogic is sound and fixes the false positives. To DRY with use_import_type, extract a shared helper (e.g. react::is_jsx_factory_binding(binding, ctx)) so both rules stay in lockstep when this grows.
crates/biome_analyze/src/context.rs (1)
164-172: Doc nudge: clarify expected shape of identifiersPlease mention these are base identifiers (pre‑dot), e.g. “React” not “React.createElement”, matching what services/options pass in. Keeps invariants obvious for rule authors.
crates/biome_service/src/file_handlers/javascript.rs (1)
346-359: Minor normalisation: trim and gate by runtimeTwo tiny tweaks:
- Trim before split to avoid stray spaces.
- Optionally only forward factories when runtime is ReactClassic (purely to reduce surprise).
Example:
- let jsx_factory = environment + let jsx_factory = (matches!(jsx_runtime, biome_analyze::options::JsxRuntime::ReactClassic)) + .then_some(environment) .and_then(|env| env.jsx_factory.as_ref()) - .and_then(|factory| factory.split('.').next()) + .map(|s| s.trim()) + .and_then(|factory| factory.split('.').next()) .map(|s| s.into()); - let jsx_fragment_factory = environment + let jsx_fragment_factory = (matches!(jsx_runtime, biome_analyze::options::JsxRuntime::ReactClassic)) + .then_some(environment) .and_then(|env| env.jsx_fragment_factory.as_ref()) - .and_then(|factory| factory.split('.').next()) + .map(|s| s.trim()) + .and_then(|factory| factory.split('.').next()) .map(|s| s.into());crates/biome_package/src/node_js_package/tsconfig_json.rs (1)
120-136: Trim before splitting to be tolerant of whitespaceA tiny guard improves resilience:
- pub fn jsx_factory_identifier(&self) -> Option<&str> { - self.compiler_options - .jsx_factory - .as_ref() - .map(|factory| factory.split('.').next().unwrap_or(factory.as_str())) - } + pub fn jsx_factory_identifier(&self) -> Option<&str> { + self.compiler_options.jsx_factory.as_deref().map(|factory| { + let f = factory.trim(); + f.split('.').next().unwrap_or(f) + }) + }Apply the same to jsx_fragment_factory_identifier.
crates/biome_js_analyze/src/lint/style/use_import_type.rs (1)
1101-1129: Centraliseis_jsx_factory_bindingto avoid driftGreat extraction. Consider moving this to
crate::react(public helper) so both this rule andnoUnusedImportsshare one source of truth for JSX‑factory recognition. Less chance of future divergence.crates/biome_analyze/src/options.rs (1)
179-193: Consider adding rustdoc and reordering methods.Two suggestions to improve maintainability:
Missing rustdoc comments: Per the coding guidelines, "Document rules, assists, and options with inline rustdoc in source". These public API methods should have doc comments explaining their purpose and return values.
Method ordering: Grouping each field's getter and setter together would improve readability. Current order (jsx_factory getter → factory setter → fragment setter → fragment getter) is a bit unusual.
Example improvement:
+ /// Returns the configured JSX factory identifier, if any (e.g., "h" for Preact). pub fn jsx_factory(&self) -> Option<&str> { self.configuration.jsx_factory.as_deref() } + /// Sets the JSX factory identifier for the classic JSX runtime. pub fn set_jsx_factory(&mut self, jsx_factory: Option<Box<str>>) { self.configuration.jsx_factory = jsx_factory; } - pub fn set_jsx_fragment_factory(&mut self, jsx_fragment_factory: Option<Box<str>>) { - self.configuration.jsx_fragment_factory = jsx_fragment_factory; - } - + /// Returns the configured JSX fragment factory identifier, if any (e.g., "Fragment"). pub fn jsx_fragment_factory(&self) -> Option<&str> { self.configuration.jsx_fragment_factory.as_deref() } + + /// Sets the JSX fragment factory identifier for the classic JSX runtime. + pub fn set_jsx_fragment_factory(&mut self, jsx_fragment_factory: Option<Box<str>>) { + self.configuration.jsx_fragment_factory = jsx_fragment_factory; + }
📜 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/correctness/noUnusedImports/jsxFactoryPreact.js.snapis excluded by!**/*.snapand included by**
📒 Files selected for processing (17)
crates/biome_analyze/src/context.rs(4 hunks)crates/biome_analyze/src/options.rs(3 hunks)crates/biome_analyze/src/registry.rs(2 hunks)crates/biome_analyze/src/signals.rs(3 hunks)crates/biome_configuration/src/javascript/mod.rs(1 hunks)crates/biome_js_analyze/src/lint/correctness/no_unused_imports.rs(2 hunks)crates/biome_js_analyze/src/lint/style/use_import_type.rs(6 hunks)crates/biome_js_analyze/src/react.rs(1 hunks)crates/biome_js_analyze/tests/quick_test.rs(1 hunks)crates/biome_js_analyze/tests/spec_tests.rs(1 hunks)crates/biome_js_analyze/tests/specs/correctness/noUnusedImports/jsxFactoryPreact.js(1 hunks)crates/biome_js_analyze/tests/specs/correctness/noUnusedImports/jsxFactoryPreact.options.json(1 hunks)crates/biome_js_analyze/tests/specs/correctness/noUnusedImports/jsxFactoryPreact.tsconfig.json(1 hunks)crates/biome_package/src/node_js_package/tsconfig_json.rs(2 hunks)crates/biome_service/src/file_handlers/javascript.rs(2 hunks)crates/biome_service/src/file_handlers/mod.rs(1 hunks)crates/biome_service/src/settings.rs(1 hunks)
🧰 Additional context used
📓 Path-based instructions (5)
crates/biome_*/**
📄 CodeRabbit inference engine (CLAUDE.md)
Place core crates under /crates/biome_*/
Files:
crates/biome_analyze/src/registry.rscrates/biome_js_analyze/src/react.rscrates/biome_service/src/settings.rscrates/biome_js_analyze/tests/specs/correctness/noUnusedImports/jsxFactoryPreact.tsconfig.jsoncrates/biome_js_analyze/src/lint/correctness/no_unused_imports.rscrates/biome_analyze/src/signals.rscrates/biome_configuration/src/javascript/mod.rscrates/biome_package/src/node_js_package/tsconfig_json.rscrates/biome_js_analyze/tests/specs/correctness/noUnusedImports/jsxFactoryPreact.jscrates/biome_service/src/file_handlers/mod.rscrates/biome_js_analyze/tests/specs/correctness/noUnusedImports/jsxFactoryPreact.options.jsoncrates/biome_js_analyze/tests/spec_tests.rscrates/biome_js_analyze/tests/quick_test.rscrates/biome_analyze/src/options.rscrates/biome_analyze/src/context.rscrates/biome_service/src/file_handlers/javascript.rscrates/biome_js_analyze/src/lint/style/use_import_type.rs
**/*.rs
📄 CodeRabbit inference engine (CONTRIBUTING.md)
**/*.rs: Format Rust files before committing (e.g., viajust fwhich formats Rust)
Document rules, assists, and options with inline rustdoc in source
Files:
crates/biome_analyze/src/registry.rscrates/biome_js_analyze/src/react.rscrates/biome_service/src/settings.rscrates/biome_js_analyze/src/lint/correctness/no_unused_imports.rscrates/biome_analyze/src/signals.rscrates/biome_configuration/src/javascript/mod.rscrates/biome_package/src/node_js_package/tsconfig_json.rscrates/biome_service/src/file_handlers/mod.rscrates/biome_js_analyze/tests/spec_tests.rscrates/biome_js_analyze/tests/quick_test.rscrates/biome_analyze/src/options.rscrates/biome_analyze/src/context.rscrates/biome_service/src/file_handlers/javascript.rscrates/biome_js_analyze/src/lint/style/use_import_type.rs
crates/biome_*_{syntax,parser,formatter,analyze,factory,semantic}/**
📄 CodeRabbit inference engine (CLAUDE.md)
Maintain the per-language crate structure: biome_{lang}_{syntax,parser,formatter,analyze,factory,semantic}
Files:
crates/biome_js_analyze/src/react.rscrates/biome_js_analyze/tests/specs/correctness/noUnusedImports/jsxFactoryPreact.tsconfig.jsoncrates/biome_js_analyze/src/lint/correctness/no_unused_imports.rscrates/biome_js_analyze/tests/specs/correctness/noUnusedImports/jsxFactoryPreact.jscrates/biome_js_analyze/tests/specs/correctness/noUnusedImports/jsxFactoryPreact.options.jsoncrates/biome_js_analyze/tests/spec_tests.rscrates/biome_js_analyze/tests/quick_test.rscrates/biome_js_analyze/src/lint/style/use_import_type.rs
**/tests/**
📄 CodeRabbit inference engine (CLAUDE.md)
Place test files under a tests/ directory in each crate
Files:
crates/biome_js_analyze/tests/specs/correctness/noUnusedImports/jsxFactoryPreact.tsconfig.jsoncrates/biome_js_analyze/tests/specs/correctness/noUnusedImports/jsxFactoryPreact.jscrates/biome_js_analyze/tests/specs/correctness/noUnusedImports/jsxFactoryPreact.options.jsoncrates/biome_js_analyze/tests/spec_tests.rscrates/biome_js_analyze/tests/quick_test.rs
crates/biome_configuration/src/**
📄 CodeRabbit inference engine (CLAUDE.md)
Keep configuration sources under biome_configuration/src/
Files:
crates/biome_configuration/src/javascript/mod.rs
🧠 Learnings (3)
📚 Learning: 2025-10-15T09:20:19.139Z
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-10-15T09:20:19.139Z
Learning: Applies to crates/biome_analyze/**/lib/src/{lint,assist}/**/*.rs : Retrieve rule options via ctx.options() inside rules; do not manually handle overrides/extends resolution
Applied to files:
crates/biome_analyze/src/registry.rs
📚 Learning: 2025-10-15T09:20:19.139Z
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-10-15T09:20:19.139Z
Learning: Applies to crates/biome_analyze/**/lib/src/{lint,assist}/**/*.rs : Set the language field in declare_lint_rule! to the most specific applicable language (js, jsx, ts, or tsx)
Applied to files:
crates/biome_service/src/settings.rscrates/biome_service/src/file_handlers/mod.rs
📚 Learning: 2025-10-15T09:20:19.139Z
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-10-15T09:20:19.139Z
Learning: Applies to crates/biome_analyze/**/tests/quick_test.rs : Use the quick test at biome_js_analyze/tests/quick_test.rs by un-ignoring and adjusting SOURCE and RuleFilter for ad-hoc checks
Applied to files:
crates/biome_js_analyze/tests/quick_test.rs
🧬 Code graph analysis (10)
crates/biome_analyze/src/registry.rs (2)
crates/biome_analyze/src/context.rs (2)
jsx_factory(165-167)jsx_fragment_factory(170-172)crates/biome_analyze/src/options.rs (2)
jsx_factory(179-181)jsx_fragment_factory(191-193)
crates/biome_service/src/settings.rs (2)
crates/biome_analyze/src/context.rs (3)
jsx_runtime(160-162)jsx_factory(165-167)jsx_fragment_factory(170-172)crates/biome_analyze/src/options.rs (3)
jsx_runtime(175-177)jsx_factory(179-181)jsx_fragment_factory(191-193)
crates/biome_js_analyze/src/lint/correctness/no_unused_imports.rs (2)
crates/biome_js_analyze/src/react.rs (2)
is_global_react_import(305-336)is_jsx_factory_import(341-345)crates/biome_analyze/src/context.rs (2)
jsx_factory(165-167)jsx_fragment_factory(170-172)
crates/biome_service/src/file_handlers/mod.rs (3)
crates/biome_service/src/settings.rs (1)
analyzer_options(1119-1139)crates/biome_analyze/src/context.rs (1)
options(155-157)packages/@biomejs/backend-jsonrpc/src/workspace.ts (1)
JsxRuntime(701-701)
crates/biome_js_analyze/tests/spec_tests.rs (2)
packages/@biomejs/backend-jsonrpc/src/workspace.ts (1)
JsxRuntime(701-701)crates/biome_js_analyze/src/services/module_graph.rs (1)
project_layout(23-25)
crates/biome_js_analyze/tests/quick_test.rs (5)
packages/@biomejs/backend-jsonrpc/src/workspace.ts (4)
JsFileSource(9483-9492)TextRange(9327-9327)JsxRuntime(701-701)Severity(9308-9308)crates/biome_js_syntax/src/file_source.rs (1)
jsx(168-170)crates/biome_analyze/src/context.rs (3)
file_path(187-189)new(33-65)options(155-157)crates/biome_js_analyze/src/lib.rs (1)
analyze(188-209)crates/biome_diagnostics/src/lib.rs (1)
print_diagnostic_to_string(85-103)
crates/biome_analyze/src/options.rs (1)
crates/biome_analyze/src/context.rs (2)
jsx_factory(165-167)jsx_fragment_factory(170-172)
crates/biome_analyze/src/context.rs (1)
crates/biome_analyze/src/options.rs (2)
jsx_factory(179-181)jsx_fragment_factory(191-193)
crates/biome_service/src/file_handlers/javascript.rs (3)
packages/@biomejs/backend-jsonrpc/src/workspace.ts (1)
JsxRuntime(701-701)crates/biome_analyze/src/context.rs (3)
jsx_runtime(160-162)jsx_factory(165-167)jsx_fragment_factory(170-172)crates/biome_analyze/src/options.rs (4)
jsx_runtime(175-177)jsx_factory(179-181)jsx_fragment_factory(191-193)configuration(199-201)
crates/biome_js_analyze/src/lint/style/use_import_type.rs (3)
crates/biome_js_analyze/src/react.rs (2)
is_global_react_import(305-336)is_jsx_factory_import(341-345)packages/@biomejs/backend-jsonrpc/src/workspace.ts (1)
JsxRuntime(701-701)crates/biome_analyze/src/context.rs (2)
jsx_factory(165-167)jsx_fragment_factory(170-172)
🔇 Additional comments (13)
crates/biome_service/src/file_handlers/mod.rs (1)
1630-1647: Well-structured tsconfig integration.The logic correctly queries tsconfig.json for JSX factory settings only when needed (ReactClassic runtime, factories not already set). The defensive checks and use of
flatten()are appropriate.crates/biome_analyze/src/registry.rs (2)
409-410: LGTM.Consistent with the existing pattern for extracting options.
424-425: LGTM.Correctly threads JSX factory options through to RuleContext.
crates/biome_js_analyze/tests/specs/correctness/noUnusedImports/jsxFactoryPreact.js (1)
1-10: Good test fixture for Preact JSX support.The test case appropriately demonstrates custom JSX factory usage with Preact's
handFragment.crates/biome_js_analyze/tests/specs/correctness/noUnusedImports/jsxFactoryPreact.options.json (1)
1-6: LGTM.Correctly configures ReactClassic runtime for the test scenario.
crates/biome_js_analyze/tests/specs/correctness/noUnusedImports/jsxFactoryPreact.tsconfig.json (1)
1-8: LGTM.Standard Preact configuration demonstrating the feature.
crates/biome_analyze/src/signals.rs (3)
373-374: LGTM.Correctly propagates JSX factory options to RuleContext in the diagnostic path.
412-413: LGTM.Consistent with the diagnostic path.
479-480: LGTM.Completes the JSX factory propagation across all execution paths.
crates/biome_js_analyze/tests/quick_test.rs (1)
99-157: Solid test coverage for JSX factory support.The test clearly demonstrates that custom JSX factories (h, Fragment) are correctly recognised as used when configured via ReactClassic runtime options.
crates/biome_service/src/settings.rs (1)
500-510: Environment wiring for JSX factories looks rightThe environment now carries jsx_runtime + factory fields; aligns with downstream usage. No blockers here.
crates/biome_analyze/src/options.rs (2)
78-82: LGTM!The field declarations are well-commented and use appropriate types for optional JSX factory configuration.
104-112: LGTM!The builder methods follow the established pattern and enable fluent configuration chaining.
There was a problem hiding this comment.
Ah, remember to:
- point this PR to
next - create a changeset
https://github.com/biomejs/biome?tab=contributing-ov-file#creating-pull-requests
CodSpeed Performance ReportMerging #7847 will not alter performanceComparing Summary
Footnotes
|
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
crates/biome_service/src/file_handlers/mod.rs (1)
1637-1653: Minor allocs: avoid intermediate String if the setters accept SmolStr/Cow.You’re creating a fresh String per analysis via to_string(). If set_jsx_factory/set_jsx_fragment_factory accept SmolStr or Cow<'static, str>, you can map directly from &str and skip the extra hop. If not, feel free to ignore — this was raised before and deemed acceptable.
Run to confirm the setter signatures:
#!/bin/bash # Inspect setter types so we can decide if we can map &str -> SmolStr/Cow directly. rg -nP -C2 'set_jsx_(fragment_)?factory\s*\(' --type=rust
🧹 Nitpick comments (4)
crates/biome_js_analyze/tests/quick_test.rs (3)
100-100: Consider clarifying the test name.The name
test_jsx_factory_from_tsconfigmight suggest the test reads from an actual tsconfig file, when it actually validates the analyzer behaviour after JSX factory options are set programmatically (simulating what would happen when tsconfig is parsed). The current name is acceptable but could be more precise.
99-157: Consider adding a negative test case.The test validates that with JSX factory configuration, imports are correctly preserved. For a more robust test, consider adding a companion test that proves WITHOUT the configuration, the same imports WOULD be flagged as unused by
noUnusedImports. This would demonstrate the configuration actually changes behaviour.
99-157: Edge cases to consider for future tests.Whilst the current test covers the primary use case well, additional test cases could be valuable:
- Only
jsx_factoryset (no fragment factory)- JSX factory configured but import uses a different identifier
- Non-ReactClassic runtime with factory configuration
These can be addressed in follow-up work if needed.
crates/biome_service/src/file_handlers/mod.rs (1)
1636-1655: Avoid double tsconfig queries; fetch both identifiers in one go.Call query_tsconfig_for_path once and pull both factory and fragment to reduce overhead (IO/cache lookup/locks). Keeps the hot path lean.
Apply this diff within the current block:
- if analyzer_options.jsx_factory().is_none() { - let factory = self - .project_layout - .query_tsconfig_for_path(path, |tsconfig| { - tsconfig.jsx_factory_identifier().map(|s| s.to_string()) - }) - .flatten(); - analyzer_options.set_jsx_factory(factory.map(|s| s.into())); - } - if analyzer_options.jsx_fragment_factory().is_none() { - let fragment_factory = self - .project_layout - .query_tsconfig_for_path(path, |tsconfig| { - tsconfig - .jsx_fragment_factory_identifier() - .map(|s| s.to_string()) - }) - .flatten(); - analyzer_options.set_jsx_fragment_factory(fragment_factory.map(|s| s.into())); - } + let need_factory = analyzer_options.jsx_factory().is_none(); + let need_fragment = analyzer_options.jsx_fragment_factory().is_none(); + if need_factory || need_fragment { + if let Some((factory, fragment)) = self.project_layout.query_tsconfig_for_path(path, |tsconfig| { + ( + tsconfig.jsx_factory_identifier().map(|s| s.to_string()), + tsconfig.jsx_fragment_factory_identifier().map(|s| s.to_string()), + ) + }) { + if need_factory { + analyzer_options.set_jsx_factory(factory.map(|s| s.into())); + } + if need_fragment { + analyzer_options.set_jsx_fragment_factory(fragment.map(|s| s.into())); + } + } + }
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
packages/@biomejs/backend-jsonrpc/src/workspace.tsis excluded by!**/backend-jsonrpc/src/workspace.tsand included by**packages/@biomejs/biome/configuration_schema.jsonis excluded by!**/configuration_schema.jsonand included by**
📒 Files selected for processing (4)
crates/biome_js_analyze/src/lint/style/use_import_type.rs(6 hunks)crates/biome_js_analyze/tests/quick_test.rs(1 hunks)crates/biome_js_analyze/tests/spec_tests.rs(1 hunks)crates/biome_service/src/file_handlers/mod.rs(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- crates/biome_js_analyze/src/lint/style/use_import_type.rs
- crates/biome_js_analyze/tests/spec_tests.rs
🧰 Additional context used
📓 Path-based instructions (4)
crates/biome_*_{syntax,parser,formatter,analyze,factory,semantic}/**
📄 CodeRabbit inference engine (CLAUDE.md)
Maintain the per-language crate structure: biome_{lang}_{syntax,parser,formatter,analyze,factory,semantic}
Files:
crates/biome_js_analyze/tests/quick_test.rs
crates/biome_*/**
📄 CodeRabbit inference engine (CLAUDE.md)
Place core crates under /crates/biome_*/
Files:
crates/biome_js_analyze/tests/quick_test.rscrates/biome_service/src/file_handlers/mod.rs
**/tests/**
📄 CodeRabbit inference engine (CLAUDE.md)
Place test files under a tests/ directory in each crate
Files:
crates/biome_js_analyze/tests/quick_test.rs
**/*.rs
📄 CodeRabbit inference engine (CONTRIBUTING.md)
**/*.rs: Format Rust files before committing (e.g., viajust fwhich formats Rust)
Document rules, assists, and options with inline rustdoc in source
Files:
crates/biome_js_analyze/tests/quick_test.rscrates/biome_service/src/file_handlers/mod.rs
🧠 Learnings (2)
📚 Learning: 2025-10-24T21:24:58.631Z
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-10-24T21:24:58.631Z
Learning: Applies to crates/biome_analyze/crates/*_analyze/tests/quick_test.rs : Quick test lives in biome_js_analyze/tests/quick_test.rs; unignore #[ignore] and set rule filter and SOURCE for ad-hoc runs
Applied to files:
crates/biome_js_analyze/tests/quick_test.rs
📚 Learning: 2025-10-24T21:24:58.631Z
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-10-24T21:24:58.631Z
Learning: Applies to crates/biome_analyze/crates/*_analyze/**/src/**/lint/**/*.rs : Set the language field in declare_lint_rule! to the most appropriate dialect (js/jsx/ts/tsx)
Applied to files:
crates/biome_service/src/file_handlers/mod.rs
🧬 Code graph analysis (2)
crates/biome_js_analyze/tests/quick_test.rs (4)
packages/@biomejs/backend-jsonrpc/src/workspace.ts (3)
JsFileSource(9491-9500)TextRange(9335-9335)JsxRuntime(709-709)crates/biome_js_syntax/src/file_source.rs (1)
jsx(168-170)crates/biome_analyze/src/context.rs (3)
file_path(187-189)new(33-65)options(155-157)crates/biome_js_analyze/src/lib.rs (1)
analyze(188-209)
crates/biome_service/src/file_handlers/mod.rs (2)
crates/biome_service/src/settings.rs (1)
analyzer_options(1119-1139)packages/@biomejs/backend-jsonrpc/src/workspace.ts (1)
JsxRuntime(709-709)
🔇 Additional comments (1)
crates/biome_service/src/file_handlers/mod.rs (1)
1630-1657: Good guardrails and non-overriding defaults.Nice: only read tsconfig when jsx_runtime is ReactClassic and existing options are unset. This preserves explicit config and fixes the false positives.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
crates/biome_package/src/node_js_package/tsconfig_json.rs (1)
350-365: Consider renaming this test.The test name suggests it verifies that normalization happens during deserialization (not on every access), but it only validates correctness by calling the accessor multiple times. Both approaches (normalize once vs normalize on every call) would pass this test. Consider renaming to
test_jsx_factory_repeated_accessor adding a comment clarifying it's a stress test rather than an efficiency verification.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (3)
crates/biome_cli/tests/snapshots/main_cases_handle_astro_files/format_astro_with_typescript_script_tag.snapis excluded by!**/*.snapand included by**crates/biome_package/tests/tsconfig/valid/tsconfig.valid.baseUrl.json.snapis excluded by!**/*.snapand included by**crates/biome_package/tests/tsconfig/valid/tsconfig.valid.paths.json.snapis excluded by!**/*.snapand included by**
📒 Files selected for processing (9)
.changeset/eight-eels-pull.md(1 hunks).changeset/ten-hoops-unite.md(1 hunks)crates/biome_cli/tests/cases/handle_astro_files.rs(2 hunks)crates/biome_js_analyze/tests/quick_test.rs(1 hunks)crates/biome_package/src/node_js_package/tsconfig_json.rs(3 hunks)crates/biome_package/tests/tsconfig/valid/tsconfig.valid.baseUrl.json.snap.new(1 hunks)crates/biome_package/tests/tsconfig/valid/tsconfig.valid.paths.json.snap.new(1 hunks)crates/biome_service/src/file_handlers/html.rs(1 hunks)crates/biome_service/src/settings.rs(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- crates/biome_package/tests/tsconfig/valid/tsconfig.valid.baseUrl.json.snap.new
🚧 Files skipped from review as they are similar to previous changes (2)
- crates/biome_service/src/settings.rs
- crates/biome_js_analyze/tests/quick_test.rs
🧰 Additional context used
📓 Path-based instructions (4)
.changeset/*.md
📄 CodeRabbit inference engine (CONTRIBUTING.md)
.changeset/*.md: In changesets, only use #### or ##### headers; other header levels are not allowed
Changesets should cover user-facing changes only; internal changes do not need changesets
Use past tense for what you did and present tense for current Biome behavior in changesets
When fixing a bug in a changeset, start with an issue link (e.g., “Fixed #1234: …”)
When referencing a rule or assist in a changeset, include a link to its page on the website
Include code blocks in changesets when applicable to illustrate changes
End every sentence in a changeset with a period
Files:
.changeset/ten-hoops-unite.md.changeset/eight-eels-pull.md
crates/biome_*/**
📄 CodeRabbit inference engine (CLAUDE.md)
Place core crates under /crates/biome_*/
Files:
crates/biome_package/src/node_js_package/tsconfig_json.rscrates/biome_package/tests/tsconfig/valid/tsconfig.valid.paths.json.snap.newcrates/biome_service/src/file_handlers/html.rscrates/biome_cli/tests/cases/handle_astro_files.rs
**/*.rs
📄 CodeRabbit inference engine (CONTRIBUTING.md)
**/*.rs: Format Rust files before committing (e.g., viajust fwhich formats Rust)
Document rules, assists, and options with inline rustdoc in source
Files:
crates/biome_package/src/node_js_package/tsconfig_json.rscrates/biome_service/src/file_handlers/html.rscrates/biome_cli/tests/cases/handle_astro_files.rs
**/tests/**
📄 CodeRabbit inference engine (CLAUDE.md)
Place test files under a tests/ directory in each crate
Files:
crates/biome_package/tests/tsconfig/valid/tsconfig.valid.paths.json.snap.newcrates/biome_cli/tests/cases/handle_astro_files.rs
🧬 Code graph analysis (2)
crates/biome_package/src/node_js_package/tsconfig_json.rs (2)
crates/biome_analyze/src/context.rs (1)
new(33-65)crates/biome_package/src/node_js_package/package_json.rs (3)
deserialize(128-168)deserialize(229-235)deserialize(326-336)
crates/biome_cli/tests/cases/handle_astro_files.rs (2)
crates/biome_fs/src/fs/memory.rs (1)
default(37-49)crates/biome_cli/tests/main.rs (1)
run_cli(332-347)
🔇 Additional comments (9)
.changeset/ten-hoops-unite.md (1)
1-5: LGTM!The changeset follows all guidelines: references the issue, uses appropriate tense, and ends with a period.
.changeset/eight-eels-pull.md (1)
1-6: LGTM!The changeset clearly describes the new behavior and follows all guidelines.
crates/biome_service/src/file_handlers/html.rs (1)
491-492: LGTM!The Astro branch follows the established pattern for Svelte and Vue, correctly creating a TypeScript file source with Astro embedding kind.
crates/biome_cli/tests/cases/handle_astro_files.rs (2)
59-72: LGTM!The test constant appropriately represents an Astro file with TypeScript type annotations in a script tag.
638-669: LGTM!The test follows the established pattern and properly validates TypeScript script tag formatting in Astro files.
crates/biome_package/tests/tsconfig/valid/tsconfig.valid.paths.json.snap.new (1)
1-45: LGTM!The snapshot correctly validates the TsConfigJson structure with the new optional JSX factory fields.
crates/biome_package/src/node_js_package/tsconfig_json.rs (3)
120-128: LGTM!The accessors cleanly expose the normalized JSX factory identifiers.
151-163: LGTM!The new fields are well-documented and use the JsxFactoryIdentifier type for automatic normalization.
193-231: LGTM!The implementation correctly normalizes during deserialization (addressing the previous review concern), making the operation efficient. The Deref implementation provides ergonomic access.
Based on learnings
db4c897 to
73a45a9
Compare
|
@ematipico should I rebase on "next" as well? Too many extra changes showed up on "main" based branch |
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
crates/biome_package/src/node_js_package/tsconfig_json.rs (1)
193-205: Bold move normalising at deserialisation — this addresses the earlier concern about repeated string work and the added tests are spot on. LGTM.
🧹 Nitpick comments (6)
crates/biome_package/src/node_js_package/tsconfig_json.rs (2)
216-231: Trim whitespace during deserialisation; simplify split handling.
Prevents “ React.createElement ”-style values from leaking spaces and removes the redundant unwrap_or.Apply this diff:
- let full_name = String::deserialize(ctx, value, name)?; - // Extract the base identifier (everything before the first dot) - let base_identifier = full_name - .split('.') - .next() - .unwrap_or(&full_name) - .to_string(); + let full_name = String::deserialize(ctx, value, name)?; + let trimmed = full_name.trim(); + // Extract the base identifier (everything before the first dot) + let base_identifier = trimmed + .split('.') + .next() + .unwrap_or("") + .to_string(); Some(Self(base_identifier))
233-366: Add tests for whitespace and empty-string inputs.
Covers edge cases and guards regressions.Apply this diff:
@@ fn test_jsx_factory_efficiency() { @@ } + + #[test] + fn test_jsx_factory_whitespace_and_empty() { + let json = r#"{ + "compilerOptions": { + "jsxFactory": " React.createElement ", + "jsxFragmentFactory": " Fragment " + } + }"#; + let (tsconfig, _) = TsConfigJson::parse(Utf8Path::new("/test/tsconfig.json"), json); + assert_eq!(tsconfig.jsx_factory_identifier(), Some("React")); + assert_eq!(tsconfig.jsx_fragment_factory_identifier(), Some("Fragment")); + + let json_empty = r#"{ + "compilerOptions": { + "jsxFactory": "", + "jsxFragmentFactory": "" + } + }"#; + let (tsconfig_empty, _) = + TsConfigJson::parse(Utf8Path::new("/test/tsconfig.json"), json_empty); + assert_eq!(tsconfig_empty.jsx_factory_identifier(), None); + assert_eq!(tsconfig_empty.jsx_fragment_factory_identifier(), None); + }.changeset/eight-eels-pull.md (1)
1-6: Add issue links and a rule link per changeset guidelines.
Start with “Fixed #...”, and reference the rule page.Apply this diff:
--- "@biomejs/backend-jsonrpc": patch "@biomejs/biome": patch --- -Biome now respects jsxFactory and jsxFragmentFactory settings from tsconfig.json when using the classic JSX runtime, preventing false positive noUnusedImports errors for custom JSX libraries like Preact. +Fixed #6438 and #3682: Biome respects jsxFactory and jsxFragmentFactory from tsconfig.json when using the classic JSX runtime. +This prevents false-positive correctness/noUnusedImports diagnostics for custom JSX libraries (e.g., Preact). See https://biomejs.dev/lint/rules/no-unused-imports/.As per coding guidelines.
crates/biome_js_analyze/tests/quick_test.rs (1)
99-156: Solid targeted test; exercises ReactClassic with custom factory/fragment and no diagnostics.Consider an extra case using a namespace/default import (e.g., React + React.Fragment) to cover the classic React path too.
crates/biome_js_analyze/src/lint/style/use_import_type.rs (1)
1100-1129: Factor the helper into crate::react and update the rule docs.
Multiple rules now need this predicate; moving it avoids drift and makes testing easier. Also amend the rule’s docstring to note exceptions for custom jsxFactory/jsxFragmentFactory, not only React globals.Happy to extract a shared helper and update docs in a follow-up PR.
crates/biome_analyze/src/context.rs (1)
164-172: Accessors correctly implemented.The methods properly expose the optional JSX factory identifiers. Documentation is adequate, though you might consider elaborating slightly on when these are
None(e.g., "ReturnsNoneunless custom factories are configured in tsconfig.json") to help rule implementers.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (5)
crates/biome_js_analyze/tests/specs/correctness/noUnusedImports/jsxFactoryPreact.js.snapis excluded by!**/*.snapand included by**crates/biome_package/tests/tsconfig/valid/tsconfig.valid.baseUrl.json.snapis excluded by!**/*.snapand included by**crates/biome_package/tests/tsconfig/valid/tsconfig.valid.paths.json.snapis excluded by!**/*.snapand included by**packages/@biomejs/backend-jsonrpc/src/workspace.tsis excluded by!**/backend-jsonrpc/src/workspace.tsand included by**packages/@biomejs/biome/configuration_schema.jsonis excluded by!**/configuration_schema.jsonand included by**
📒 Files selected for processing (18)
.changeset/eight-eels-pull.md(1 hunks)crates/biome_analyze/src/context.rs(4 hunks)crates/biome_analyze/src/options.rs(3 hunks)crates/biome_analyze/src/registry.rs(2 hunks)crates/biome_analyze/src/signals.rs(3 hunks)crates/biome_js_analyze/src/lint/correctness/no_unused_imports.rs(2 hunks)crates/biome_js_analyze/src/lint/style/use_import_type.rs(6 hunks)crates/biome_js_analyze/src/react.rs(1 hunks)crates/biome_js_analyze/tests/quick_test.rs(1 hunks)crates/biome_js_analyze/tests/spec_tests.rs(1 hunks)crates/biome_js_analyze/tests/specs/correctness/noUnusedImports/jsxFactoryPreact.js(1 hunks)crates/biome_js_analyze/tests/specs/correctness/noUnusedImports/jsxFactoryPreact.options.json(1 hunks)crates/biome_js_analyze/tests/specs/correctness/noUnusedImports/jsxFactoryPreact.tsconfig.json(1 hunks)crates/biome_package/src/node_js_package/tsconfig_json.rs(3 hunks)crates/biome_package/tests/tsconfig/valid/tsconfig.valid.baseUrl.json.snap.new(1 hunks)crates/biome_package/tests/tsconfig/valid/tsconfig.valid.paths.json.snap.new(1 hunks)crates/biome_service/src/file_handlers/mod.rs(1 hunks)crates/biome_service/src/settings.rs(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (11)
- crates/biome_package/tests/tsconfig/valid/tsconfig.valid.baseUrl.json.snap.new
- crates/biome_js_analyze/src/react.rs
- crates/biome_js_analyze/tests/specs/correctness/noUnusedImports/jsxFactoryPreact.tsconfig.json
- crates/biome_service/src/settings.rs
- crates/biome_service/src/file_handlers/mod.rs
- crates/biome_js_analyze/src/lint/correctness/no_unused_imports.rs
- crates/biome_package/tests/tsconfig/valid/tsconfig.valid.paths.json.snap.new
- crates/biome_analyze/src/options.rs
- crates/biome_analyze/src/registry.rs
- crates/biome_js_analyze/tests/spec_tests.rs
- crates/biome_js_analyze/tests/specs/correctness/noUnusedImports/jsxFactoryPreact.js
🧰 Additional context used
📓 Path-based instructions (5)
.changeset/*.md
📄 CodeRabbit inference engine (CONTRIBUTING.md)
.changeset/*.md: In changesets, only use #### or ##### headers; other header levels are not allowed
Changesets should cover user-facing changes only; internal changes do not need changesets
Use past tense for what you did and present tense for current Biome behavior in changesets
When fixing a bug in a changeset, start with an issue link (e.g., “Fixed #1234: …”)
When referencing a rule or assist in a changeset, include a link to its page on the website
Include code blocks in changesets when applicable to illustrate changes
End every sentence in a changeset with a period
Files:
.changeset/eight-eels-pull.md
crates/biome_*_{syntax,parser,formatter,analyze,factory,semantic}/**
📄 CodeRabbit inference engine (CLAUDE.md)
Maintain the per-language crate structure: biome_{lang}_{syntax,parser,formatter,analyze,factory,semantic}
Files:
crates/biome_js_analyze/tests/specs/correctness/noUnusedImports/jsxFactoryPreact.options.jsoncrates/biome_js_analyze/tests/quick_test.rscrates/biome_js_analyze/src/lint/style/use_import_type.rs
crates/biome_*/**
📄 CodeRabbit inference engine (CLAUDE.md)
Place core crates under /crates/biome_*/
Files:
crates/biome_js_analyze/tests/specs/correctness/noUnusedImports/jsxFactoryPreact.options.jsoncrates/biome_js_analyze/tests/quick_test.rscrates/biome_analyze/src/signals.rscrates/biome_package/src/node_js_package/tsconfig_json.rscrates/biome_js_analyze/src/lint/style/use_import_type.rscrates/biome_analyze/src/context.rs
**/tests/**
📄 CodeRabbit inference engine (CLAUDE.md)
Place test files under a tests/ directory in each crate
Files:
crates/biome_js_analyze/tests/specs/correctness/noUnusedImports/jsxFactoryPreact.options.jsoncrates/biome_js_analyze/tests/quick_test.rs
**/*.rs
📄 CodeRabbit inference engine (CONTRIBUTING.md)
**/*.rs: Format Rust files before committing (e.g., viajust fwhich formats Rust)
Document rules, assists, and options with inline rustdoc in source
Files:
crates/biome_js_analyze/tests/quick_test.rscrates/biome_analyze/src/signals.rscrates/biome_package/src/node_js_package/tsconfig_json.rscrates/biome_js_analyze/src/lint/style/use_import_type.rscrates/biome_analyze/src/context.rs
🧠 Learnings (2)
📚 Learning: 2025-10-24T21:24:58.631Z
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-10-24T21:24:58.631Z
Learning: Applies to crates/biome_analyze/crates/*_analyze/tests/specs/**/*.jsonc : .jsonc snapshot test files must contain an array of code strings and are interpreted as script (no ESM import/export)
Applied to files:
crates/biome_js_analyze/tests/specs/correctness/noUnusedImports/jsxFactoryPreact.options.json
📚 Learning: 2025-10-24T21:24:58.631Z
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-10-24T21:24:58.631Z
Learning: Applies to crates/biome_analyze/crates/*_analyze/tests/quick_test.rs : Quick test lives in biome_js_analyze/tests/quick_test.rs; unignore #[ignore] and set rule filter and SOURCE for ad-hoc runs
Applied to files:
crates/biome_js_analyze/tests/quick_test.rs
🧬 Code graph analysis (4)
crates/biome_js_analyze/tests/quick_test.rs (3)
crates/biome_js_syntax/src/file_source.rs (1)
jsx(168-170)crates/biome_analyze/src/context.rs (3)
file_path(187-189)new(33-65)options(155-157)crates/biome_js_analyze/src/lib.rs (1)
analyze(188-209)
crates/biome_package/src/node_js_package/tsconfig_json.rs (2)
crates/biome_analyze/src/context.rs (1)
new(33-65)crates/biome_analyze/src/registry.rs (2)
new(298-300)new(382-461)
crates/biome_js_analyze/src/lint/style/use_import_type.rs (4)
crates/biome_js_analyze/src/react.rs (2)
is_global_react_import(305-336)is_jsx_factory_import(341-345)packages/@biomejs/backend-jsonrpc/src/workspace.ts (1)
JsxRuntime(709-709)crates/biome_analyze/src/context.rs (2)
jsx_factory(165-167)jsx_fragment_factory(170-172)crates/biome_analyze/src/options.rs (2)
jsx_factory(179-181)jsx_fragment_factory(191-193)
crates/biome_analyze/src/context.rs (1)
crates/biome_analyze/src/options.rs (2)
jsx_factory(179-181)jsx_fragment_factory(191-193)
🔇 Additional comments (5)
crates/biome_js_analyze/tests/specs/correctness/noUnusedImports/jsxFactoryPreact.options.json (1)
1-6: Looks good. Correct runtime for the spec.crates/biome_js_analyze/src/lint/style/use_import_type.rs (1)
195-199: Nice consolidation via is_jsx_factory_binding — fewer branches, clearer intent, same behaviour under ReactClassic.Also applies to: 251-253, 277-279, 343-345
crates/biome_analyze/src/signals.rs (1)
372-376: All RuleContext::new call sites correctly updated.Verification confirms all four RuleContext::new instantiations (three in signals.rs, one in registry.rs) now properly pass jsx_factory and jsx_fragment_factory with consistent parameter ordering.
The optional refactor suggestion to extract a helper remains valid for reducing duplication, but the current changes are correct and complete.
crates/biome_analyze/src/context.rs (2)
23-24: LGTM – JSX factory fields correctly added.The fields use the appropriate lifetime
'aand are sensibly positioned alongside other JSX configuration.
33-65: Constructor properly extended.The new parameters are correctly threaded through to field initialization, maintaining consistency with the existing parameter-heavy signature.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (5)
crates/biome_js_analyze/tests/specs/correctness/noUnusedImports/jsxFactoryPreact.js.snapis excluded by!**/*.snapand included by**crates/biome_package/tests/tsconfig/valid/tsconfig.valid.baseUrl.json.snapis excluded by!**/*.snapand included by**crates/biome_package/tests/tsconfig/valid/tsconfig.valid.paths.json.snapis excluded by!**/*.snapand included by**packages/@biomejs/backend-jsonrpc/src/workspace.tsis excluded by!**/backend-jsonrpc/src/workspace.tsand included by**packages/@biomejs/biome/configuration_schema.jsonis excluded by!**/configuration_schema.jsonand included by**
📒 Files selected for processing (18)
.changeset/eight-eels-pull.md(1 hunks)crates/biome_analyze/src/context.rs(4 hunks)crates/biome_analyze/src/options.rs(3 hunks)crates/biome_analyze/src/registry.rs(2 hunks)crates/biome_analyze/src/signals.rs(3 hunks)crates/biome_js_analyze/src/lint/correctness/no_unused_imports.rs(2 hunks)crates/biome_js_analyze/src/lint/style/use_import_type.rs(6 hunks)crates/biome_js_analyze/src/react.rs(1 hunks)crates/biome_js_analyze/tests/quick_test.rs(1 hunks)crates/biome_js_analyze/tests/spec_tests.rs(1 hunks)crates/biome_js_analyze/tests/specs/correctness/noUnusedImports/jsxFactoryPreact.js(1 hunks)crates/biome_js_analyze/tests/specs/correctness/noUnusedImports/jsxFactoryPreact.options.json(1 hunks)crates/biome_js_analyze/tests/specs/correctness/noUnusedImports/jsxFactoryPreact.tsconfig.json(1 hunks)crates/biome_package/src/node_js_package/tsconfig_json.rs(3 hunks)crates/biome_package/tests/tsconfig/valid/tsconfig.valid.baseUrl.json.snap.new(1 hunks)crates/biome_package/tests/tsconfig/valid/tsconfig.valid.paths.json.snap.new(1 hunks)crates/biome_service/src/file_handlers/mod.rs(1 hunks)crates/biome_service/src/settings.rs(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- crates/biome_package/tests/tsconfig/valid/tsconfig.valid.baseUrl.json.snap.new
🚧 Files skipped from review as they are similar to previous changes (11)
- crates/biome_js_analyze/tests/specs/correctness/noUnusedImports/jsxFactoryPreact.options.json
- crates/biome_service/src/file_handlers/mod.rs
- crates/biome_service/src/settings.rs
- crates/biome_js_analyze/src/react.rs
- crates/biome_js_analyze/tests/quick_test.rs
- crates/biome_analyze/src/signals.rs
- crates/biome_analyze/src/options.rs
- crates/biome_js_analyze/src/lint/correctness/no_unused_imports.rs
- crates/biome_js_analyze/tests/spec_tests.rs
- crates/biome_analyze/src/registry.rs
- crates/biome_js_analyze/tests/specs/correctness/noUnusedImports/jsxFactoryPreact.js
🧰 Additional context used
📓 Path-based instructions (5)
.changeset/*.md
📄 CodeRabbit inference engine (CONTRIBUTING.md)
.changeset/*.md: In changesets, only use #### or ##### headers; other header levels are not allowed
Changesets should cover user-facing changes only; internal changes do not need changesets
Use past tense for what you did and present tense for current Biome behavior in changesets
When fixing a bug in a changeset, start with an issue link (e.g., “Fixed #1234: …”)
When referencing a rule or assist in a changeset, include a link to its page on the website
Include code blocks in changesets when applicable to illustrate changes
End every sentence in a changeset with a period
Files:
.changeset/eight-eels-pull.md
crates/biome_*_{syntax,parser,formatter,analyze,factory,semantic}/**
📄 CodeRabbit inference engine (CLAUDE.md)
Maintain the per-language crate structure: biome_{lang}_{syntax,parser,formatter,analyze,factory,semantic}
Files:
crates/biome_js_analyze/tests/specs/correctness/noUnusedImports/jsxFactoryPreact.tsconfig.jsoncrates/biome_js_analyze/src/lint/style/use_import_type.rs
crates/biome_*/**
📄 CodeRabbit inference engine (CLAUDE.md)
Place core crates under /crates/biome_*/
Files:
crates/biome_js_analyze/tests/specs/correctness/noUnusedImports/jsxFactoryPreact.tsconfig.jsoncrates/biome_package/src/node_js_package/tsconfig_json.rscrates/biome_js_analyze/src/lint/style/use_import_type.rscrates/biome_package/tests/tsconfig/valid/tsconfig.valid.paths.json.snap.newcrates/biome_analyze/src/context.rs
**/tests/**
📄 CodeRabbit inference engine (CLAUDE.md)
Place test files under a tests/ directory in each crate
Files:
crates/biome_js_analyze/tests/specs/correctness/noUnusedImports/jsxFactoryPreact.tsconfig.jsoncrates/biome_package/tests/tsconfig/valid/tsconfig.valid.paths.json.snap.new
**/*.rs
📄 CodeRabbit inference engine (CONTRIBUTING.md)
**/*.rs: Format Rust files before committing (e.g., viajust fwhich formats Rust)
Document rules, assists, and options with inline rustdoc in source
Files:
crates/biome_package/src/node_js_package/tsconfig_json.rscrates/biome_js_analyze/src/lint/style/use_import_type.rscrates/biome_analyze/src/context.rs
🧬 Code graph analysis (3)
crates/biome_package/src/node_js_package/tsconfig_json.rs (1)
crates/biome_package/src/node_js_package/package_json.rs (3)
deserialize(128-168)deserialize(229-235)deserialize(326-336)
crates/biome_js_analyze/src/lint/style/use_import_type.rs (4)
crates/biome_js_analyze/src/react.rs (2)
is_global_react_import(305-336)is_jsx_factory_import(341-345)packages/@biomejs/backend-jsonrpc/src/workspace.ts (1)
JsxRuntime(709-709)crates/biome_analyze/src/context.rs (2)
jsx_factory(165-167)jsx_fragment_factory(170-172)crates/biome_analyze/src/options.rs (2)
jsx_factory(179-181)jsx_fragment_factory(191-193)
crates/biome_analyze/src/context.rs (1)
crates/biome_analyze/src/options.rs (2)
jsx_factory(179-181)jsx_fragment_factory(191-193)
🔇 Additional comments (12)
crates/biome_js_analyze/tests/specs/correctness/noUnusedImports/jsxFactoryPreact.tsconfig.json (1)
1-8: LGTM!Valid tsconfig test fixture for Preact-style JSX configuration.
crates/biome_analyze/src/context.rs (3)
23-24: LGTM!Fields are appropriately typed and follow the existing pattern.
44-45: LGTM!Constructor correctly accepts and assigns the new parameters.
Also applies to: 61-62
164-172: LGTM!Accessors are well-documented and follow established conventions.
crates/biome_js_analyze/src/lint/style/use_import_type.rs (2)
1100-1129: LGTM!Helper function consolidates JSX factory detection logic cleanly. The early return for non-ReactClassic runtime is efficient, and the three-check pattern (standard React, custom factory, custom fragment) is clear and correct.
195-195: LGTM!Consistent use of the helper across all relevant call sites eliminates duplication and improves maintainability.
Also applies to: 251-251, 277-277, 343-343
crates/biome_package/src/node_js_package/tsconfig_json.rs (5)
120-128: LGTM!The accessors are correctly implemented. Empty identifier filtering is handled during deserialization (lines 230-234), which is more efficient than filtering on every accessor call.
151-163: LGTM!Fields are well-documented with clear explanation of normalization behaviour. The rename attributes correctly handle TypeScript's camelCase convention.
193-238: LGTM!The deserialization logic correctly extracts the base identifier, trims whitespace, and filters out empty/unusable values. Normalising during deserialization is efficient, and the
Derefimplementation provides ergonomic access.
244-372: LGTM!Comprehensive test coverage for basic functionality including normalisation, simple identifiers, namespaced identifiers, missing fields, deeply nested paths, and efficiency verification.
374-468: LGTM!Excellent edge case coverage including empty strings, whitespace-only, dot-only, and surrounding whitespace scenarios. These tests verify the filtering behaviour that prevents "configured but unusable" states.
crates/biome_package/tests/tsconfig/valid/tsconfig.valid.paths.json.snap.new (1)
1-45: LGTM!Snapshot correctly documents the new
jsx_factoryandjsx_fragment_factoryfields in theCompilerOptionsstructure. TheNonevalues are expected since the test input doesn't configure these options.
567a82d to
8c256e5
Compare
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (3)
.changeset/small-sides-teach.md (1)
1-5: Consider adding a code example.The changeset follows the guidelines well, but could benefit from a code block showing the
@pluginsyntax with options that is now supported.Example addition:
Fixed [#7860](https://github.com/biomejs/biome/issues/7860): The css parser, with `tailwindDirectives` enabled, will now accept `@plugin` options. + +```css +@plugin "my-plugin" { + debug: false; + threshold: 0.5; +} +```As per coding guidelines
.changeset/thirty-readers-cross.md (1)
1-9: Consider linking the related issue if one exists.The changeset is well-written and clear. If there's an associated issue for this bug fix, consider linking it at the start as per the coding guidelines.
As per coding guidelines
crates/biome_css_formatter/src/tailwind/statements/plugin_at_rule.rs (1)
16-26: Correctly handles both plugin syntax variants.The branching logic appropriately formats plugins with and without option blocks, removing trailing semicolons when a block is present.
One minor note: Line 23 formats
semicolon_token(anOption<SyntaxToken>) directly. Whilst the parser ensures this should always beSomewhen there's no block, consider whether error-recovery cases could produceNonehere. If so, an explicit check might be safer:} else if let Some(token) = semicolon_token { write!(f, [token.format()])?; }Only raise this if the formatter needs to handle malformed AST gracefully—otherwise, the current code is fine.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (26)
crates/biome_cli/tests/snapshots/main_cases_html/should_error_when_interpolation_is_disabled.snapis excluded by!**/*.snapand included by**crates/biome_css_factory/src/generated/node_factory.rsis excluded by!**/generated/**,!**/generated/**and included by**crates/biome_css_factory/src/generated/syntax_factory.rsis excluded by!**/generated/**,!**/generated/**and included by**crates/biome_css_formatter/tests/specs/css/tailwind/plugin-no-options.css.snapis excluded by!**/*.snapand included by**crates/biome_css_formatter/tests/specs/css/tailwind/plugin-with-options.css.snapis excluded by!**/*.snapand included by**crates/biome_css_parser/tests/css_test_suite/error/tailwind/when-enabled/plugin-with-invalid-options-2.css.snapis excluded by!**/*.snapand included by**crates/biome_css_parser/tests/css_test_suite/error/tailwind/when-enabled/plugin-with-invalid-options.css.snapis excluded by!**/*.snapand included by**crates/biome_css_parser/tests/css_test_suite/ok/tailwind/plugin-with-options-2.css.snapis excluded by!**/*.snapand included by**crates/biome_css_parser/tests/css_test_suite/ok/tailwind/plugin-with-options.css.snapis excluded by!**/*.snapand included by**crates/biome_css_parser/tests/css_test_suite/ok/tailwind/plugin.css.snapis excluded by!**/*.snapand included by**crates/biome_css_parser/tests/css_test_suite/ok/tailwind/shadcn-default.css.snapis excluded by!**/*.snapand included by**crates/biome_css_syntax/src/generated/nodes.rsis excluded by!**/generated/**,!**/generated/**and included by**crates/biome_css_syntax/src/generated/nodes_mut.rsis excluded by!**/generated/**,!**/generated/**and included by**crates/biome_html_formatter/tests/specs/prettier/html/interpolation/example.html.snapis excluded by!**/*.snapand included by**crates/biome_html_parser/tests/html_specs/error/interpolation-attributes.html.snapis excluded by!**/*.snapand included by**crates/biome_html_parser/tests/html_specs/error/interpolation.html.snapis excluded by!**/*.snapand included by**crates/biome_html_parser/tests/html_specs/error/template-langs/django/issue-5450.html.snapis excluded by!**/*.snapand included by**crates/biome_html_parser/tests/html_specs/ok/svelte/dynamic-prop.svelte.snapis excluded by!**/*.snapand included by**crates/biome_html_parser/tests/html_specs/ok/svelte/shorthand-prop.svelte.snapis excluded by!**/*.snapand included by**crates/biome_html_parser/tests/html_specs/ok/svelte/shorthand-spread-props.svelte.snapis excluded by!**/*.snapand included by**crates/biome_html_syntax/src/generated/nodes.rsis excluded by!**/generated/**,!**/generated/**and included by**crates/biome_js_analyze/tests/specs/correctness/noUnusedImports/jsxFactoryPreact.js.snapis excluded by!**/*.snapand included by**crates/biome_package/tests/tsconfig/valid/tsconfig.valid.baseUrl.json.snapis excluded by!**/*.snapand included by**crates/biome_package/tests/tsconfig/valid/tsconfig.valid.paths.json.snapis excluded by!**/*.snapand included by**packages/@biomejs/backend-jsonrpc/src/workspace.tsis excluded by!**/backend-jsonrpc/src/workspace.tsand included by**packages/@biomejs/biome/configuration_schema.jsonis excluded by!**/configuration_schema.jsonand included by**
📒 Files selected for processing (46)
.changeset/eight-eels-pull.md(1 hunks).changeset/odd-lizards-build.md(1 hunks).changeset/small-sides-teach.md(1 hunks).changeset/thirty-readers-cross.md(1 hunks).github/PULL_REQUEST_TEMPLATE.md(1 hunks)AGENTS..md(1 hunks)CLAUDE.md(0 hunks)CLAUDE.md(1 hunks)CONTRIBUTING.md(2 hunks)crates/biome_analyze/src/context.rs(4 hunks)crates/biome_analyze/src/options.rs(3 hunks)crates/biome_analyze/src/registry.rs(2 hunks)crates/biome_analyze/src/signals.rs(3 hunks)crates/biome_css_formatter/src/tailwind/statements/plugin_at_rule.rs(1 hunks)crates/biome_css_formatter/tests/specs/css/tailwind/plugin-no-options.css(1 hunks)crates/biome_css_formatter/tests/specs/css/tailwind/plugin-with-options.css(1 hunks)crates/biome_css_parser/src/syntax/at_rule/tailwind.rs(4 hunks)crates/biome_css_parser/tests/css_test_suite/error/tailwind/when-enabled/plugin-with-invalid-options-2.css(1 hunks)crates/biome_css_parser/tests/css_test_suite/error/tailwind/when-enabled/plugin-with-invalid-options.css(1 hunks)crates/biome_css_parser/tests/css_test_suite/ok/tailwind/plugin-with-options-2.css(1 hunks)crates/biome_css_parser/tests/css_test_suite/ok/tailwind/plugin-with-options.css(1 hunks)crates/biome_grit_patterns/src/grit_target_language/css_target_language/constants.rs(1 hunks)crates/biome_html_formatter/src/html/any/attribute.rs(1 hunks)crates/biome_html_formatter/src/html/lists/attribute_list.rs(1 hunks)crates/biome_html_parser/src/syntax/mod.rs(2 hunks)crates/biome_html_parser/src/syntax/parse_error.rs(1 hunks)crates/biome_html_parser/tests/html_specs/error/interpolation-attributes.html(1 hunks)crates/biome_html_parser/tests/html_specs/ok/svelte/dynamic-prop.svelte(1 hunks)crates/biome_html_parser/tests/html_specs/ok/svelte/shorthand-prop.svelte(1 hunks)crates/biome_html_parser/tests/html_specs/ok/svelte/shorthand-spread-props.svelte(1 hunks)crates/biome_js_analyze/src/lint/correctness/no_unused_imports.rs(2 hunks)crates/biome_js_analyze/src/lint/style/use_import_type.rs(6 hunks)crates/biome_js_analyze/src/react.rs(1 hunks)crates/biome_js_analyze/tests/quick_test.rs(1 hunks)crates/biome_js_analyze/tests/spec_tests.rs(1 hunks)crates/biome_js_analyze/tests/specs/correctness/noUnusedImports/jsxFactoryPreact.js(1 hunks)crates/biome_js_analyze/tests/specs/correctness/noUnusedImports/jsxFactoryPreact.options.json(1 hunks)crates/biome_js_analyze/tests/specs/correctness/noUnusedImports/jsxFactoryPreact.tsconfig.json(1 hunks)crates/biome_package/src/node_js_package/tsconfig_json.rs(3 hunks)crates/biome_package/tests/tsconfig/valid/tsconfig.valid.baseUrl.json.snap.new(1 hunks)crates/biome_package/tests/tsconfig/valid/tsconfig.valid.paths.json.snap.new(1 hunks)crates/biome_parser/src/lib.rs(3 hunks)crates/biome_service/src/file_handlers/mod.rs(1 hunks)crates/biome_service/src/settings.rs(1 hunks)xtask/codegen/css.ungram(3 hunks)xtask/codegen/html.ungram(1 hunks)
✅ Files skipped from review due to trivial changes (4)
- CLAUDE.md
- crates/biome_css_parser/tests/css_test_suite/ok/tailwind/plugin-with-options-2.css
- crates/biome_css_formatter/tests/specs/css/tailwind/plugin-with-options.css
- crates/biome_package/tests/tsconfig/valid/tsconfig.valid.baseUrl.json.snap.new
🚧 Files skipped from review as they are similar to previous changes (12)
- crates/biome_js_analyze/tests/spec_tests.rs
- crates/biome_js_analyze/src/react.rs
- crates/biome_service/src/file_handlers/mod.rs
- crates/biome_js_analyze/tests/specs/correctness/noUnusedImports/jsxFactoryPreact.js
- crates/biome_analyze/src/context.rs
- crates/biome_analyze/src/registry.rs
- crates/biome_js_analyze/tests/specs/correctness/noUnusedImports/jsxFactoryPreact.tsconfig.json
- crates/biome_service/src/settings.rs
- crates/biome_js_analyze/tests/specs/correctness/noUnusedImports/jsxFactoryPreact.options.json
- crates/biome_js_analyze/tests/quick_test.rs
- crates/biome_js_analyze/src/lint/style/use_import_type.rs
- .changeset/eight-eels-pull.md
🧰 Additional context used
📓 Path-based instructions (3)
.changeset/*.md
📄 CodeRabbit inference engine (CONTRIBUTING.md)
.changeset/*.md: In changeset files, only use #### or ##### headers
Changesets should describe user-facing changes; internal-only changes do not need changesets
Use past tense for what you did in the changeset description and present tense for current behavior
For bug fixes, start the changeset description with a link to the issue (e.g., Fixed #1234: ...)
When referencing a rule or assist in a changeset, include a link to the rule/assist page on the website
Include a code block in the changeset when applicable to illustrate the change
End every sentence in a changeset with a full stop (.)
Files:
.changeset/odd-lizards-build.md.changeset/small-sides-teach.md.changeset/thirty-readers-cross.md
**/*.{rs,toml}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Format Rust and TOML files before committing (e.g., via
just f)
Files:
crates/biome_html_parser/src/syntax/parse_error.rscrates/biome_html_formatter/src/html/lists/attribute_list.rscrates/biome_parser/src/lib.rscrates/biome_css_formatter/src/tailwind/statements/plugin_at_rule.rscrates/biome_analyze/src/signals.rscrates/biome_html_parser/src/syntax/mod.rscrates/biome_grit_patterns/src/grit_target_language/css_target_language/constants.rscrates/biome_analyze/src/options.rscrates/biome_html_formatter/src/html/any/attribute.rscrates/biome_js_analyze/src/lint/correctness/no_unused_imports.rscrates/biome_package/src/node_js_package/tsconfig_json.rscrates/biome_css_parser/src/syntax/at_rule/tailwind.rs
**/*.rs
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Document rules, assists, and their options with inline rustdoc in the Rust source
Files:
crates/biome_html_parser/src/syntax/parse_error.rscrates/biome_html_formatter/src/html/lists/attribute_list.rscrates/biome_parser/src/lib.rscrates/biome_css_formatter/src/tailwind/statements/plugin_at_rule.rscrates/biome_analyze/src/signals.rscrates/biome_html_parser/src/syntax/mod.rscrates/biome_grit_patterns/src/grit_target_language/css_target_language/constants.rscrates/biome_analyze/src/options.rscrates/biome_html_formatter/src/html/any/attribute.rscrates/biome_js_analyze/src/lint/correctness/no_unused_imports.rscrates/biome_package/src/node_js_package/tsconfig_json.rscrates/biome_css_parser/src/syntax/at_rule/tailwind.rs
🧠 Learnings (5)
📚 Learning: 2025-10-26T15:28:00.941Z
Learnt from: CR
PR: biomejs/biome#0
File: CONTRIBUTING.md:0-0
Timestamp: 2025-10-26T15:28:00.941Z
Learning: Disclose any AI assistance used in creating a pull request, including scope of usage, in the PR description
Applied to files:
.github/PULL_REQUEST_TEMPLATE.mdCONTRIBUTING.md
📚 Learning: 2025-10-15T09:24:31.042Z
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_parser/CONTRIBUTING.md:0-0
Timestamp: 2025-10-15T09:24:31.042Z
Learning: Applies to crates/biome_parser/xtask/codegen/*.ungram : Unions of nodes must start with Any* (e.g., AnyHtmlAttribute)
Applied to files:
xtask/codegen/html.ungram
📚 Learning: 2025-10-24T21:24:58.631Z
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-10-24T21:24:58.631Z
Learning: Applies to crates/biome_analyze/crates/*_analyze/tests/specs/**/*.jsonc : .jsonc snapshot test files must contain an array of code strings and are interpreted as script (no ESM import/export)
Applied to files:
crates/biome_package/tests/tsconfig/valid/tsconfig.valid.paths.json.snap.new
📚 Learning: 2025-10-15T09:22:15.851Z
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_formatter/CONTRIBUTING.md:0-0
Timestamp: 2025-10-15T09:22:15.851Z
Learning: Applies to crates/biome_formatter/src/lib.rs : Implement FormatLanguage for HtmlFormatLanguage with associated types: SyntaxLanguage=HtmlLanguage, Context=HtmlFormatContext, FormatRule=FormatHtmlSyntaxNode
Applied to files:
crates/biome_html_formatter/src/html/any/attribute.rs
📚 Learning: 2025-10-15T09:22:15.851Z
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_formatter/CONTRIBUTING.md:0-0
Timestamp: 2025-10-15T09:22:15.851Z
Learning: Applies to crates/biome_formatter/src/cst.rs : Create FormatHtmlSyntaxNode in cst.rs implementing FormatRule<HtmlSyntaxNode> and AsFormat/IntoFormat for HtmlSyntaxNode using the provided plumbing
Applied to files:
crates/biome_html_formatter/src/html/any/attribute.rs
🧬 Code graph analysis (7)
crates/biome_html_parser/src/syntax/parse_error.rs (1)
crates/biome_service/src/workspace.rs (1)
markup(1149-1151)
crates/biome_html_formatter/src/html/lists/attribute_list.rs (1)
crates/biome_html_formatter/src/html/any/attribute.rs (1)
fmt(9-16)
crates/biome_css_formatter/src/tailwind/statements/plugin_at_rule.rs (2)
crates/biome_css_syntax/src/generated/nodes.rs (15)
plugin_token(7137-7139)semicolon_token(347-349)semicolon_token(1553-1555)semicolon_token(1684-1686)semicolon_token(2234-2236)semicolon_token(2788-2790)semicolon_token(3444-3446)semicolon_token(6489-6491)semicolon_token(6624-6626)semicolon_token(6912-6914)semicolon_token(6957-6959)semicolon_token(7051-7053)semicolon_token(7146-7148)semicolon_token(7192-7194)semicolon_token(7241-7243)crates/biome_formatter/src/builders.rs (1)
space(606-608)
crates/biome_html_parser/src/syntax/mod.rs (1)
crates/biome_html_parser/src/syntax/parse_error.rs (1)
disabled_svelte_prop(16-18)
crates/biome_analyze/src/options.rs (1)
crates/biome_analyze/src/context.rs (2)
jsx_factory(165-167)jsx_fragment_factory(170-172)
crates/biome_js_analyze/src/lint/correctness/no_unused_imports.rs (3)
crates/biome_js_analyze/src/react.rs (2)
is_global_react_import(305-336)is_jsx_factory_import(341-345)crates/biome_analyze/src/context.rs (2)
jsx_factory(165-167)jsx_fragment_factory(170-172)crates/biome_analyze/src/options.rs (2)
jsx_factory(179-181)jsx_fragment_factory(191-193)
crates/biome_package/src/node_js_package/tsconfig_json.rs (2)
crates/biome_analyze/src/context.rs (1)
new(33-65)crates/biome_analyze/src/registry.rs (2)
new(298-300)new(382-461)
🪛 LanguageTool
CONTRIBUTING.md
[uncategorized] ~60-~60: Possible missing comma found.
Context: ...ulted ChatGPT to understand the codebase but the solution > was fully authored man...
(AI_HYDRA_LEO_MISSING_COMMA)
🔇 Additional comments (34)
.changeset/odd-lizards-build.md (1)
1-6: Changeset describes fix for #7861 (Svelte HTML), but PR objectives focus on #6438 & #6682 (JSX factory).This changeset documents a fix for Svelte attribute shorthand syntax, but the PR's stated objectives are about custom JSX factory and jsxFragmentFactory support. These appear to be unrelated changes.
Is this the correct changeset for this PR, or was it accidentally included? The PR should have a changeset documenting the custom JSX factory changes (issues #6438 and #3682).
crates/biome_html_parser/tests/html_specs/ok/svelte/shorthand-prop.svelte (1)
1-1: This file appears unrelated to the PR objectives.This Svelte HTML parser test fixture doesn't seem connected to the JSX factory configuration changes described in the PR. Given your comment about extra changes appearing on your branch, this may have been pulled in during the rebase. Consider whether this file should be included in this PR.
crates/biome_parser/src/lib.rs (1)
391-392: Nice documentation improvement!These doc comments clarify the return behaviour for the token consumption methods, making the API easier to use.
Also applies to: 404-405, 417-418
crates/biome_grit_patterns/src/grit_target_language/css_target_language/constants.rs (1)
1-9: No action required—change is properly generated.The verification confirms
TW_PLUGIN_AT_RULEis defined in the codegen source (xtask/codegen/src/css_kinds_src.rs) and consistently regenerated across all dependent files (syntax factories, kind definitions, parser, etc.). This is a legitimate Tailwind CSS plugin support addition that was correctly regenerated via the codegen system. The file was not manually edited.crates/biome_css_parser/tests/css_test_suite/ok/tailwind/plugin-with-options.css (1)
1-9: LGTM!Valid test fixture for Tailwind plugin options, covering both with and without trailing semicolon.
crates/biome_html_parser/tests/html_specs/ok/svelte/shorthand-spread-props.svelte (1)
1-1: LGTM!Valid Svelte spread props test case.
crates/biome_html_formatter/src/html/any/attribute.rs (1)
14-14: LGTM!The new match arm correctly handles
HtmlSingleTextExpressionfollowing the same pattern as existing attribute variants.xtask/codegen/html.ungram (1)
162-162: LGTM!Correctly extends
AnyHtmlAttributeto includeHtmlSingleTextExpression, aligning with the formatter changes.CONTRIBUTING.md (1)
47-70: LGTM!The AI assistance disclosure section is clear, well-structured, and promotes transparency in contributions. The guidance strikes a good balance between disclosure requirements and respectful tone.
CLAUDE.md (1)
1-1: Clarify the purpose of this file.This file contains only "CONTRIBUTING.md" as a single line. Is this a placeholder, a reference file, or was it accidentally committed? If it's not needed, please remove it.
.github/PULL_REQUEST_TEMPLATE.md (1)
1-5: LGTM!The AI assistance disclosure is properly positioned and links to the contributing guidelines as expected.
Based on learnings.
crates/biome_html_parser/src/syntax/mod.rs (2)
315-321: Verify these HTML parser changes belong in this PR.These changes add support for single text expressions in HTML attributes, but the PR objectives focus on JSX factory configuration from tsconfig.json. Are these HTML changes intentionally included, or were they accidentally merged from another branch?
334-334: LGTM!The token set correctly includes the new
T!['{']token alongside existing attribute start tokens.crates/biome_html_formatter/src/html/lists/attribute_list.rs (1)
66-68: Implementation looks correct, but verify PR scope.The formatting logic correctly mirrors the
HtmlDoubleTextExpressionpattern. However, like the parser changes, this appears unrelated to the JSX factory configuration objectives. Please confirm these HTML formatting changes are intentionally included in this PR.crates/biome_js_analyze/src/lint/correctness/no_unused_imports.rs (2)
4-4: LGTM!The new import
is_jsx_factory_importis correctly added to support custom JSX factory detection.
626-642: Excellent implementation of custom JSX factory support!The logic correctly handles three scenarios when using the ReactClassic runtime:
- Standard React imports
- Custom JSX factory imports from
ctx.jsx_factory()- Custom JSX fragment factory imports from
ctx.jsx_fragment_factory()This properly addresses the PR objectives to prevent false-positive unused import warnings for custom JSX factories configured in tsconfig.json.
crates/biome_html_parser/src/syntax/parse_error.rs (2)
12-14: Option name update looks correct, but verify PR scope.The hint now references "html.parser.interpolation" instead of "html.parser.textExpression", which appears to be a configuration option rename. However, this change seems unrelated to JSX factory support. Please confirm this is intentionally included.
16-18: New Svelte diagnostic function is well-structured.The function follows the existing pattern and provides helpful guidance. Same question about PR scope applies here.
crates/biome_package/tests/tsconfig/valid/tsconfig.valid.paths.json.snap.new (1)
1-44: LGTM!The snapshot correctly shows the new
jsx_factoryandjsx_fragment_factoryfields in theCompilerOptionsdata structure. Since the test input doesn't specify these options,Nonevalues are expected.crates/biome_package/src/node_js_package/tsconfig_json.rs (4)
120-128: LGTM!The accessors correctly expose the normalized base identifiers. The deserialization logic prevents empty strings from being stored, so no additional filtering is needed here (contrary to the past review comment—the concern is already addressed at deserialization time).
151-163: LGTM!The new fields are properly documented with TypeScript documentation links and clear descriptions of the normalization behaviour. The serde rename attributes correctly map to the camelCase tsconfig.json property names.
193-238: Excellent normalization implementation!The
JsxFactoryIdentifiertype elegantly handles:
- Normalization during deserialization (efficient—happens once)
- Extraction of base identifier before the first dot
- Trimming whitespace
- Filtering empty/whitespace-only identifiers to return
NoneThe
Derefimplementation provides ergonomic access. This directly addresses the past review comment about efficiency and filtering empty identifiers.
240-469: Comprehensive test coverage!The test suite thoroughly validates:
- Standard normalization cases (namespaced → base)
- Simple identifiers (passthrough)
- Missing configuration
- Edge cases (deeply nested, empty, whitespace-only, dot-only)
- Efficiency guarantee (deserialization-time normalization)
- Whitespace handling
This gives high confidence in the implementation's correctness and robustness.
crates/biome_css_formatter/tests/specs/css/tailwind/plugin-no-options.css (1)
1-1: LGTM!Clean test fixture for the no-options plugin case.
crates/biome_html_parser/tests/html_specs/error/interpolation-attributes.html (1)
1-1: LGTM!Appropriate test case for interpolation attribute error handling.
crates/biome_html_parser/tests/html_specs/ok/svelte/dynamic-prop.svelte (1)
1-1: LGTM!Valid Svelte dynamic prop test case.
crates/biome_css_parser/src/syntax/at_rule/tailwind.rs (3)
3-5: LGTM!Required import for the new declaration block parsing in
parse_plugin_at_rule.
212-215: LGTM!Sensible extension allowing plugin blocks with optional trailing semicolons. The logic correctly handles both the classic semicolon-only case and the new block variant.
228-230: LGTM!Clean addition of the optional
notmodifier for source at-rules.crates/biome_css_parser/tests/css_test_suite/error/tailwind/when-enabled/plugin-with-invalid-options.css (1)
1-5: LGTM!Good negative test case ensuring object literals are properly rejected in plugin options.
crates/biome_css_parser/tests/css_test_suite/error/tailwind/when-enabled/plugin-with-invalid-options-2.css (1)
1-8: LGTM!Good negative test verifying that selectors within plugin blocks are rejected.
crates/biome_analyze/src/signals.rs (1)
373-374: LGTM!Consistent propagation of JSX factory configuration through all RuleContext creation paths (diagnostic, action, and transformation). The mechanical threading is correct.
Also applies to: 412-413, 479-480
xtask/codegen/css.ungram (1)
881-884: Are these Tailwind CSS changes intended for this PR?The PR objectives focus on JSX factory support for Preact/Stencil, but this file contains Tailwind CSS grammar changes (optional dash in theme references,
notqualifier for@source, optional blocks for@plugin). These seem unrelated to the PR's purpose.Were these changes inadvertently included during the rebase onto
next, or is there a connection I'm missing?crates/biome_analyze/src/options.rs (1)
104-112: Implementation follows existing patterns nicely.The builder and accessor methods are consistent with the rest of the codebase (e.g.,
with_jsx_runtime,globals). The use ofOption<Box<str>>and.as_deref()matches the established conventions.Also applies to: 179-193
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
crates/biome_package/src/node_js_package/tsconfig_json.rs (1)
193-234: Solid implementation with excellent defensive coding.The normalization-during-deserialization approach addresses the maintainer's efficiency concern perfectly.
Minor note: Line 224's
.unwrap_or(&full_name)is technically redundant sincesplit('.').next()always returnsSomefor any string, but it makes the intent crystal-clear and costs nothing.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
crates/biome_package/src/node_js_package/tsconfig_json.rs(3 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{rs,toml}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Format Rust and TOML files before committing (e.g., via
just f)
Files:
crates/biome_package/src/node_js_package/tsconfig_json.rs
**/*.rs
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Document rules, assists, and their options with inline rustdoc in the Rust source
Files:
crates/biome_package/src/node_js_package/tsconfig_json.rs
🧬 Code graph analysis (1)
crates/biome_package/src/node_js_package/tsconfig_json.rs (2)
crates/biome_analyze/src/context.rs (1)
new(33-65)crates/biome_analyze/src/registry.rs (2)
new(298-300)new(382-461)
🔇 Additional comments (2)
crates/biome_package/src/node_js_package/tsconfig_json.rs (2)
120-128: Empty string filtering already handled—accessors look good.The bot's earlier concern about empty strings is addressed in the
JsxFactoryIdentifierdeserializer (lines 226-230), which returnsNonefor empty or whitespace-only values. Filtering at construction time is actually better than filtering at every accessor call.
236-465: Excellent test coverage—maintainer's request thoroughly addressed.The test suite covers all the important cases: normalization, simple identifiers, namespaced factories, edge cases (empty, whitespace, dots), and even an efficiency check. Well done.
(cherry picked from commit a2663c7)
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
crates/biome_package/src/node_js_package/tsconfig_json.rs (1)
120-128: Empty identifier filtering is correctly handled upstream.The previous review comment suggested filtering empty strings here, but this is now handled more efficiently during deserialization (lines 228-230), where
JsxFactoryIdentifier::deserializereturnsNonefor empty or whitespace-only values. This ensures the accessors never returnSome(""), making additional checks here unnecessary.
🧹 Nitpick comments (2)
crates/biome_js_analyze/src/lint/style/use_import_type.rs (1)
1100-1129: Nice refactoring!Extracting the repeated JSX factory detection logic into a helper function is a good application of DRY principles. The function is well-structured and clearly documents its purpose.
Minor suggestion: Consider adding a rustdoc comment to explain when this function returns true, e.g.:
/// Returns `true` if the binding is a JSX factory or fragment factory import /// when using the ReactClassic runtime. This includes: /// - Standard React imports (e.g., `import React from 'react'`) /// - Custom JSX factory imports matching `ctx.jsx_factory()` /// - Custom JSX fragment factory imports matching `ctx.jsx_fragment_factory()` fn is_jsx_factory_binding(crates/biome_package/src/node_js_package/tsconfig_json.rs (1)
216-234: Minor: Unreachableunwrap_oron line 224.The
unwrap_or(&full_name)is unnecessary sincesplit('.').next()always returnsSomefor any string input (even empty strings yieldSome("")). The fallback will never be used.Apply this diff to simplify:
- let base_identifier = full_name.split('.').next().unwrap_or(&full_name).trim(); + let base_identifier = full_name.split('.').next().unwrap().trim();Otherwise, the deserialization logic is sound—it correctly handles all edge cases (empty, whitespace, dots, nested identifiers) and normalises once for efficiency.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (3)
crates/biome_js_analyze/tests/specs/correctness/noUnusedImports/jsxFactoryPreact.js.snapis excluded by!**/*.snapand included by**crates/biome_package/tests/tsconfig/valid/tsconfig.valid.baseUrl.json.snapis excluded by!**/*.snapand included by**crates/biome_package/tests/tsconfig/valid/tsconfig.valid.paths.json.snapis excluded by!**/*.snapand included by**
📒 Files selected for processing (18)
.changeset/eight-eels-pull.md(1 hunks)crates/biome_analyze/src/context.rs(4 hunks)crates/biome_analyze/src/options.rs(3 hunks)crates/biome_analyze/src/registry.rs(2 hunks)crates/biome_analyze/src/signals.rs(3 hunks)crates/biome_js_analyze/src/lint/correctness/no_unused_imports.rs(2 hunks)crates/biome_js_analyze/src/lint/style/use_import_type.rs(6 hunks)crates/biome_js_analyze/src/react.rs(1 hunks)crates/biome_js_analyze/tests/quick_test.rs(1 hunks)crates/biome_js_analyze/tests/spec_tests.rs(1 hunks)crates/biome_js_analyze/tests/specs/correctness/noUnusedImports/jsxFactoryPreact.js(1 hunks)crates/biome_js_analyze/tests/specs/correctness/noUnusedImports/jsxFactoryPreact.options.json(1 hunks)crates/biome_js_analyze/tests/specs/correctness/noUnusedImports/jsxFactoryPreact.tsconfig.json(1 hunks)crates/biome_package/src/node_js_package/tsconfig_json.rs(3 hunks)crates/biome_package/tests/tsconfig/valid/tsconfig.valid.baseUrl.json.snap.new(1 hunks)crates/biome_package/tests/tsconfig/valid/tsconfig.valid.paths.json.snap.new(1 hunks)crates/biome_service/src/file_handlers/mod.rs(1 hunks)crates/biome_service/src/settings.rs(1 hunks)
✅ Files skipped from review due to trivial changes (2)
- crates/biome_package/tests/tsconfig/valid/tsconfig.valid.baseUrl.json.snap.new
- crates/biome_package/tests/tsconfig/valid/tsconfig.valid.paths.json.snap.new
🚧 Files skipped from review as they are similar to previous changes (8)
- crates/biome_js_analyze/tests/spec_tests.rs
- crates/biome_service/src/file_handlers/mod.rs
- crates/biome_analyze/src/registry.rs
- crates/biome_analyze/src/context.rs
- crates/biome_js_analyze/tests/quick_test.rs
- crates/biome_js_analyze/tests/specs/correctness/noUnusedImports/jsxFactoryPreact.js
- crates/biome_js_analyze/tests/specs/correctness/noUnusedImports/jsxFactoryPreact.options.json
- crates/biome_analyze/src/options.rs
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{rs,toml}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Format Rust and TOML files before committing (e.g., via
just f)
Files:
crates/biome_js_analyze/src/react.rscrates/biome_js_analyze/src/lint/correctness/no_unused_imports.rscrates/biome_service/src/settings.rscrates/biome_js_analyze/src/lint/style/use_import_type.rscrates/biome_package/src/node_js_package/tsconfig_json.rscrates/biome_analyze/src/signals.rs
**/*.rs
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Document rules, assists, and their options with inline rustdoc in the Rust source
Files:
crates/biome_js_analyze/src/react.rscrates/biome_js_analyze/src/lint/correctness/no_unused_imports.rscrates/biome_service/src/settings.rscrates/biome_js_analyze/src/lint/style/use_import_type.rscrates/biome_package/src/node_js_package/tsconfig_json.rscrates/biome_analyze/src/signals.rs
.changeset/*.md
📄 CodeRabbit inference engine (CONTRIBUTING.md)
.changeset/*.md: In changeset files, only use #### or ##### headers
Changesets should describe user-facing changes; internal-only changes do not need changesets
Use past tense for what you did in the changeset description and present tense for current behavior
For bug fixes, start the changeset description with a link to the issue (e.g., Fixed #1234: ...)
When referencing a rule or assist in a changeset, include a link to the rule/assist page on the website
Include a code block in the changeset when applicable to illustrate the change
End every sentence in a changeset with a full stop (.)
Files:
.changeset/eight-eels-pull.md
🧠 Learnings (1)
📚 Learning: 2025-10-24T21:24:58.631Z
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-10-24T21:24:58.631Z
Learning: Applies to crates/biome_analyze/crates/*_analyze/**/src/**/lint/**/*.rs : Set the language field in declare_lint_rule! to the most appropriate dialect (js/jsx/ts/tsx)
Applied to files:
crates/biome_service/src/settings.rs
🧬 Code graph analysis (5)
crates/biome_js_analyze/src/react.rs (1)
crates/biome_js_analyze/src/react/components.rs (3)
name(428-435)name(465-489)name(531-546)
crates/biome_js_analyze/src/lint/correctness/no_unused_imports.rs (2)
crates/biome_js_analyze/src/react.rs (2)
is_global_react_import(305-336)is_jsx_factory_import(341-345)crates/biome_analyze/src/context.rs (2)
jsx_factory(165-167)jsx_fragment_factory(170-172)
crates/biome_service/src/settings.rs (2)
crates/biome_analyze/src/context.rs (1)
jsx_runtime(160-162)crates/biome_analyze/src/options.rs (1)
jsx_runtime(175-177)
crates/biome_js_analyze/src/lint/style/use_import_type.rs (4)
crates/biome_js_analyze/src/react.rs (2)
is_global_react_import(305-336)is_jsx_factory_import(341-345)packages/@biomejs/backend-jsonrpc/src/workspace.ts (1)
JsxRuntime(701-701)crates/biome_analyze/src/context.rs (2)
jsx_factory(165-167)jsx_fragment_factory(170-172)crates/biome_analyze/src/options.rs (2)
jsx_factory(179-181)jsx_fragment_factory(191-193)
crates/biome_package/src/node_js_package/tsconfig_json.rs (2)
crates/biome_analyze/src/context.rs (1)
new(33-65)crates/biome_analyze/src/registry.rs (2)
new(298-300)new(382-461)
⏰ 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). (5)
- GitHub Check: Check Dependencies
- GitHub Check: Bench (biome_js_formatter)
- GitHub Check: Bench (biome_js_parser)
- GitHub Check: Bench (biome_js_analyze)
- GitHub Check: autofix
🔇 Additional comments (14)
crates/biome_js_analyze/tests/specs/correctness/noUnusedImports/jsxFactoryPreact.tsconfig.json (1)
1-7: Test fixture looks good.The tsconfig.json is correctly configured for testing the Preact scenario with custom
jsxFactory("h") andjsxFragmentFactory("Fragment"). This aligns perfectly with the PR objectives for supporting custom JSX factories..changeset/eight-eels-pull.md (1)
6-6: LGTM!The changeset follows coding guidelines: includes issue links, rule documentation link, and proper formatting. Well done addressing the previous feedback.
crates/biome_js_analyze/src/react.rs (1)
338-345: Verify if name-only matching is sufficient for JSX factory detection.Unlike
is_global_react_import(lines 304-336), which validates the import source and declaration type, this function only checks the binding name. This could incorrectly treat a local variable namedhas a JSX factory import.Consider whether this is intentional (e.g., trusting tsconfig configuration) or if additional validation is needed to confirm the binding is actually an import.
crates/biome_service/src/settings.rs (1)
500-502: LGTM!Good change to preserve other environment fields rather than replacing the entire environment object. This allows jsx_factory and jsx_fragment_factory to coexist with jsx_runtime.
crates/biome_js_analyze/src/lint/correctness/no_unused_imports.rs (1)
626-642: LGTM!The logic clearly exempts custom JSX factory imports when using the classic runtime. The three-part check (standard React, jsx_factory, jsx_fragment_factory) is straightforward and correct.
crates/biome_analyze/src/signals.rs (3)
362-377: LGTM!Consistent threading of jsx_factory and jsx_fragment_factory parameters through the diagnostic context.
401-416: LGTM!Consistent threading of jsx_factory and jsx_fragment_factory parameters through the action context.
468-483: LGTM!Consistent threading of jsx_factory and jsx_fragment_factory parameters through the transformation context.
crates/biome_js_analyze/src/lint/style/use_import_type.rs (4)
195-199: LGTM!Good refactoring to use the centralised helper function.
251-253: LGTM!Consistent use of the helper function for namespace imports.
277-279: LGTM!Consistent use of the helper function for default imports.
343-345: LGTM!Consistent use of the helper function for namespace clause imports.
crates/biome_package/src/node_js_package/tsconfig_json.rs (2)
151-163: LGTM!The field declarations are properly documented with clear examples of the normalization behaviour, and the
#[deserializable(rename = ...)]attributes correctly map to TypeScript's camelCase naming.
236-465: Excellent test coverage!The test suite comprehensively covers normalization behaviour (simple, namespaced, deeply nested) and all relevant edge cases (empty strings, whitespace, dots). The efficiency test (lines 352-368) confirms that normalization happens during deserialization rather than on every access, addressing the reviewer's earlier concern about performance.
Closes #6438
Closes #3682
Summary
Problem: When using alternative JSX libraries (Preact, Stencil, or custom JSX implementations) with the classic JSX runtime, Biome incorrectly reports custom JSX factory functions as unused imports.
Root cause: Biome only recognized the hardcoded identifiers when jsxRuntime is set to "reactClassic". It didn't read or respect the jsxFactory and jsxFragmentFactory settings from tsconfig.json.
Changeset
Added support for custom JSX factory functions when using the classic React runtime.
Test Plan
Tests added into "biome/crates/biome_js_analyze/tests/specs/correctness/noUnusedImports"