Conversation
🦋 Changeset detectedLatest commit: cc98726 The changes in this PR will be included in the next version bump. This PR includes changesets to release 13 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Merging this PR will not alter performance
Comparing Footnotes
|
85810f3 to
302732f
Compare
|
Also, not sure if it's worth mentioning. But this is also caught by TS types, if the user makes use of Typescript & We could also add something similar for the style prop, like https://www.eslint-react.xyz/docs/rules/dom-no-string-style-prop (Same stuff really, caught by TS, but not JS) |
302732f to
1002d10
Compare
1002d10 to
7a2be39
Compare
WalkthroughAdds a new nursery lint rule Suggested reviewers
🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@crates/biome_js_analyze/src/lint/nursery/no_react_string_refs.rs`:
- Line 70: Replace the source parity marker for the rule by changing
RuleSource::EslintReact("no-string-refs").same() to use .inspired() instead of
.same(); locate the occurrence of RuleSource::EslintReact("no-string-refs") in
no_react_string_refs.rs and call .inspired() so the source reflects the
documented behavior divergence.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 98b3c36f-5ffc-44be-90ba-21c0d5881f81
⛔ Files ignored due to path filters (10)
crates/biome_cli/src/execute/migrate/eslint_any_rule_to_biome.rsis excluded by!**/migrate/eslint_any_rule_to_biome.rsand included by**crates/biome_configuration/src/analyzer/linter/rules.rsis excluded by!**/rules.rsand included by**crates/biome_configuration/src/generated/domain_selector.rsis excluded by!**/generated/**,!**/generated/**and included by**crates/biome_configuration/src/generated/linter_options_check.rsis excluded by!**/generated/**,!**/generated/**and included by**crates/biome_diagnostics_categories/src/categories.rsis excluded by!**/categories.rsand included by**crates/biome_js_analyze/tests/specs/nursery/noReactStringRefs/invalid.jsx.snapis excluded by!**/*.snapand included by**crates/biome_js_analyze/tests/specs/nursery/noReactStringRefs/react183/valid.jsx.snapis excluded by!**/*.snapand included by**crates/biome_js_analyze/tests/specs/nursery/noReactStringRefs/valid.jsx.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 (8)
.changeset/fifty-chicken-like.mdcrates/biome_js_analyze/src/lint/nursery/no_react_string_refs.rscrates/biome_js_analyze/tests/specs/nursery/noReactStringRefs/invalid.jsxcrates/biome_js_analyze/tests/specs/nursery/noReactStringRefs/react183/valid.jsxcrates/biome_js_analyze/tests/specs/nursery/noReactStringRefs/react183/valid.package.jsoncrates/biome_js_analyze/tests/specs/nursery/noReactStringRefs/valid.jsxcrates/biome_rule_options/src/lib.rscrates/biome_rule_options/src/no_react_string_refs.rs
| version: "next", | ||
| name: "noReactStringRefs", | ||
| language: "js", | ||
| sources: &[RuleSource::EslintReact("no-string-refs").same()], |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify the local inconsistency between documented behaviour and source parity marker.
sed -n '20,75p' crates/biome_js_analyze/src/lint/nursery/no_react_string_refs.rs
rg -n 'EslintReact\("no-string-refs"\)\.(same|inspired)\(' crates/biome_js_analyze/src/lintRepository: biomejs/biome
Length of output: 1622
🏁 Script executed:
sed -n '75,125p' crates/biome_js_analyze/src/lint/nursery/no_react_string_refs.rsRepository: biomejs/biome
Length of output: 2049
🏁 Script executed:
sed -n '125,180p' crates/biome_js_analyze/src/lint/nursery/no_react_string_refs.rsRepository: biomejs/biome
Length of output: 2409
Use .inspired() instead of .same() for source parity.
Line 65 marks this as behaviour-identical to ESLint, but the rule unconditionally flags template literal refs (lines 156–165), whereas ESLint's no-string-refs only does so via an optional setting. The documentation already calls this out (line 26), so the source marker should reflect the actual behaviour divergence.
Suggested fix
- sources: &[RuleSource::EslintReact("no-string-refs").same()],
+ sources: &[RuleSource::EslintReact("no-string-refs").inspired()],📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| sources: &[RuleSource::EslintReact("no-string-refs").same()], | |
| sources: &[RuleSource::EslintReact("no-string-refs").inspired()], |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@crates/biome_js_analyze/src/lint/nursery/no_react_string_refs.rs` at line 70,
Replace the source parity marker for the rule by changing
RuleSource::EslintReact("no-string-refs").same() to use .inspired() instead of
.same(); locate the occurrence of RuleSource::EslintReact("no-string-refs") in
no_react_string_refs.rs and call .inspired() so the source reflects the
documented behavior divergence.
7a2be39 to
cc98726
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (1)
crates/biome_js_analyze/src/lint/nursery/no_react_string_refs.rs (1)
151-168: The template-literal helper comment and predicate disagree.The comment says “only string chunks”, but the predicate currently accepts template elements too, so maintainers will trip over this later. Either tighten the predicate or simplify/comment it as “any template literal”.
One clear option (match current behaviour)
-/// Check if the attribute value is a string ref, which can be either a string literal or a template literal with only string chunks. +/// Check if the attribute value is a string ref, either a string literal or any template literal. fn is_string_ref_value(value: &AnyJsxAttributeValue) -> bool { @@ - Some(AnyJsExpression::JsTemplateExpression(template)) => { - template.elements().into_iter().any(|element| { - matches!( - element, - AnyJsTemplateElement::JsTemplateChunkElement(_) - | AnyJsTemplateElement::JsTemplateElement(_) - ) - }) - } + Some(AnyJsExpression::JsTemplateExpression(_)) => true,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/biome_js_analyze/src/lint/nursery/no_react_string_refs.rs` around lines 151 - 168, The comment on is_string_ref_value claims it should accept "template literal with only string chunks" but the current predicate accepts AnyJsTemplateElement::JsTemplateElement as well; update the predicate inside is_string_ref_value to only treat template literals as string refs when every element is a JsTemplateChunkElement (i.e., replace the current matches that includes JsTemplateElement with a check that all template.elements() are JsTemplateChunkElement), and adjust the comment to remain "only string chunks" if you enforce this, or alternatively change the comment to "any template literal" if you intend to keep the broader behavior—reference is_string_ref_value, AnyJsTemplateElement::JsTemplateChunkElement, and AnyJsTemplateElement::JsTemplateElement.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@crates/biome_js_analyze/src/lint/nursery/no_react_string_refs.rs`:
- Around line 151-168: The comment on is_string_ref_value claims it should
accept "template literal with only string chunks" but the current predicate
accepts AnyJsTemplateElement::JsTemplateElement as well; update the predicate
inside is_string_ref_value to only treat template literals as string refs when
every element is a JsTemplateChunkElement (i.e., replace the current matches
that includes JsTemplateElement with a check that all template.elements() are
JsTemplateChunkElement), and adjust the comment to remain "only string chunks"
if you enforce this, or alternatively change the comment to "any template
literal" if you intend to keep the broader behavior—reference
is_string_ref_value, AnyJsTemplateElement::JsTemplateChunkElement, and
AnyJsTemplateElement::JsTemplateElement.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 16e35be8-05e3-48aa-824f-8c3f7c331466
⛔ Files ignored due to path filters (10)
crates/biome_cli/src/execute/migrate/eslint_any_rule_to_biome.rsis excluded by!**/migrate/eslint_any_rule_to_biome.rsand included by**crates/biome_configuration/src/analyzer/linter/rules.rsis excluded by!**/rules.rsand included by**crates/biome_configuration/src/generated/domain_selector.rsis excluded by!**/generated/**,!**/generated/**and included by**crates/biome_configuration/src/generated/linter_options_check.rsis excluded by!**/generated/**,!**/generated/**and included by**crates/biome_diagnostics_categories/src/categories.rsis excluded by!**/categories.rsand included by**crates/biome_js_analyze/tests/specs/nursery/noReactStringRefs/invalid.jsx.snapis excluded by!**/*.snapand included by**crates/biome_js_analyze/tests/specs/nursery/noReactStringRefs/react183/valid.jsx.snapis excluded by!**/*.snapand included by**crates/biome_js_analyze/tests/specs/nursery/noReactStringRefs/valid.jsx.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 (8)
.changeset/fifty-chicken-like.mdcrates/biome_js_analyze/src/lint/nursery/no_react_string_refs.rscrates/biome_js_analyze/tests/specs/nursery/noReactStringRefs/invalid.jsxcrates/biome_js_analyze/tests/specs/nursery/noReactStringRefs/react183/valid.jsxcrates/biome_js_analyze/tests/specs/nursery/noReactStringRefs/react183/valid.package.jsoncrates/biome_js_analyze/tests/specs/nursery/noReactStringRefs/valid.jsxcrates/biome_rule_options/src/lib.rscrates/biome_rule_options/src/no_react_string_refs.rs
✅ Files skipped from review due to trivial changes (7)
- crates/biome_js_analyze/tests/specs/nursery/noReactStringRefs/react183/valid.package.json
- crates/biome_js_analyze/tests/specs/nursery/noReactStringRefs/react183/valid.jsx
- .changeset/fifty-chicken-like.md
- crates/biome_rule_options/src/lib.rs
- crates/biome_js_analyze/tests/specs/nursery/noReactStringRefs/invalid.jsx
- crates/biome_js_analyze/tests/specs/nursery/noReactStringRefs/valid.jsx
- crates/biome_rule_options/src/no_react_string_refs.rs
Summary
This adds
noReactStringRefswhich is a port of https://github.com/jsx-eslint/eslint-plugin-react/blob/master/docs/rules/no-string-refs.mdGenerated by gpt 5.4
Test Plan
snapshots
Docs