Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions .changeset/fifty-chicken-like.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
"@biomejs/biome": patch
---

Added the new nursery rule [`noReactStringRefs`](https://biomejs.dev/linter/rules/no-react-string-refs/), which disallows legacy React string refs such as `ref="hello"` and `this.refs.hello`.

Biome also reports template-literal refs such as ``ref={`hello`}``, so React code can consistently migrate to callback refs, `createRef()`, or `useRef()`.
12 changes: 12 additions & 0 deletions crates/biome_cli/src/execute/migrate/eslint_any_rule_to_biome.rs

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 4 additions & 0 deletions crates/biome_configuration/src/analyzer/linter/rules.rs

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions crates/biome_diagnostics_categories/src/categories.rs

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

199 changes: 199 additions & 0 deletions crates/biome_js_analyze/src/lint/nursery/no_react_string_refs.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,199 @@
use std::sync::Arc;

use camino::Utf8PathBuf;

use crate::react::components::{AnyPotentialReactComponentDeclaration, ReactComponentInfo};
use crate::services::semantic::Semantic;
use biome_analyze::{
Rule, RuleDiagnostic, RuleDomain, RuleSource, context::RuleContext, declare_lint_rule,
};
use biome_console::markup;
use biome_diagnostics::Severity;
use biome_js_syntax::{
AnyJsExpression, AnyJsMemberExpression, AnyJsTemplateElement, AnyJsxAttributeValue,
JsxAttribute,
};
use biome_package::PackageJson;
use biome_rowan::{AstNode, TextRange, declare_node_union};
use biome_rule_options::no_react_string_refs::NoReactStringRefsOptions;

declare_lint_rule! {
/// Disallow string refs in React components.
///
/// String refs are a legacy React feature. Modern React code should use callback refs,
/// `createRef()`, or `useRef()` instead.
///
/// Biome also flags template literal refs, even though upstream only does so through an option.
///
/// ## Examples
///
/// ### Invalid
///
/// ```jsx,expect_diagnostic
/// function Hello() {
/// return <div ref="hello">Hello</div>;
/// }
/// ```
///
/// ```jsx,expect_diagnostic
/// function Hello({ id }) {
/// return <div ref={`hello-${id}`}>Hello</div>;
/// }
/// ```
///
/// ```jsx,expect_diagnostic
/// class Hello extends React.Component {
/// componentDidMount() {
/// this.refs.hello.focus();
/// }
/// }
/// ```
///
/// ### Valid
///
/// ```jsx
/// function Hello() {
/// const helloRef = useRef(null);
/// return <div ref={helloRef}>Hello</div>;
/// }
/// ```
Comment thread
dyc3 marked this conversation as resolved.
///
pub NoReactStringRefs {
version: "next",
name: "noReactStringRefs",
language: "js",
sources: &[RuleSource::EslintReact("no-string-refs").same()],
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 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/lint

Repository: biomejs/biome

Length of output: 1622


🏁 Script executed:

sed -n '75,125p' crates/biome_js_analyze/src/lint/nursery/no_react_string_refs.rs

Repository: biomejs/biome

Length of output: 2049


🏁 Script executed:

sed -n '125,180p' crates/biome_js_analyze/src/lint/nursery/no_react_string_refs.rs

Repository: 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.

Suggested change
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.

domains: &[RuleDomain::React],
recommended: true,
severity: Severity::Warning,
}
}

declare_node_union! {
pub AnyNoReactStringRefsQuery = JsxAttribute | AnyJsMemberExpression
}

pub enum NoReactStringRefsState {
RefAttribute(TextRange),
ThisRefs(TextRange),
}

impl Rule for NoReactStringRefs {
type Query = Semantic<AnyNoReactStringRefsQuery>;
type State = NoReactStringRefsState;
type Signals = Option<Self::State>;
type Options = NoReactStringRefsOptions;

fn run(ctx: &RuleContext<Self>) -> Self::Signals {
match ctx.query() {
AnyNoReactStringRefsQuery::JsxAttribute(attribute) => {
let name = attribute.name().ok()?;
let name = name.as_jsx_name()?;
if name.value_token().ok()?.text_trimmed() != "ref" {
return None;
}

let value = attribute.initializer()?.value().ok()?;
is_string_ref_value(&value)
.then_some(NoReactStringRefsState::RefAttribute(value.range()))
}
AnyNoReactStringRefsQuery::AnyJsMemberExpression(member_expression) => {
if is_react_18_3_or_higher(ctx) {
return None;
}

let refs_range = this_refs_range(member_expression)?;

member_expression
.syntax()
.ancestors()
.filter_map(AnyPotentialReactComponentDeclaration::cast)
.find_map(|declaration| {
ReactComponentInfo::from_declaration(declaration.syntax())
})
.map(|_| NoReactStringRefsState::ThisRefs(refs_range))
}
}
}

fn diagnostic(_ctx: &RuleContext<Self>, state: &Self::State) -> Option<RuleDiagnostic> {
let (range, message, note, help) = match state {
NoReactStringRefsState::RefAttribute(range) => (
*range,
markup! { "String refs are deprecated." },
markup! {
"String refs are a legacy React feature that can make ref usage harder to follow."
},
markup! {
"Use a callback ref or an object ref created with "<Emphasis>"createRef()"</Emphasis>" or "<Emphasis>"useRef()"</Emphasis>" instead."
},
),
NoReactStringRefsState::ThisRefs(range) => (
*range,
markup! { "Using "<Emphasis>"this.refs"</Emphasis>" is deprecated." },
markup! {
"Accessing refs through "<Emphasis>"this.refs"</Emphasis>" relies on React's legacy string ref behavior."
},
markup! {
"Store the ref on an instance field with a callback ref, or switch to "<Emphasis>"createRef()"</Emphasis>" or "<Emphasis>"useRef()"</Emphasis>"."
},
),
};

Some(
RuleDiagnostic::new(rule_category!(), range, message)
.note(note)
.note(help),
)
}
}

/// Check if the attribute value is a string ref, which can be either a string literal or a template literal with only string chunks.
fn is_string_ref_value(value: &AnyJsxAttributeValue) -> bool {
match value {
AnyJsxAttributeValue::AnyJsxTag(_) => false,
AnyJsxAttributeValue::JsxString(_) => true,
AnyJsxAttributeValue::JsxExpressionAttributeValue(expression) => {
match expression.expression().ok() {
Some(AnyJsExpression::AnyJsLiteralExpression(literal)) => {
literal.as_js_string_literal_expression().is_some()
}
Some(AnyJsExpression::JsTemplateExpression(template)) => {
template.elements().into_iter().any(|element| {
matches!(
element,
AnyJsTemplateElement::JsTemplateChunkElement(_)
| AnyJsTemplateElement::JsTemplateElement(_)
)
})
}
_ => false,
}
}
}
}

fn is_react_18_3_or_higher(ctx: &RuleContext<NoReactStringRefs>) -> bool {
ctx.get_service::<Option<(Utf8PathBuf, Arc<PackageJson>)>>()
.and_then(|manifest| {
manifest
.as_ref()
.map(|(_, package_json)| package_json.matches_dependency("react", ">=18.3.0"))
})
== Some(true)
}

/// Returns `Some` if the member expression is `this.refs`, otherwise `None`.
fn this_refs_range(member_expression: &AnyJsMemberExpression) -> Option<TextRange> {
if member_expression
.object()
.ok()?
.as_js_this_expression()
.is_some()
&& member_expression.member_name()?.text() == "refs"
{
return Some(member_expression.range());
}

None
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
/* should generate diagnostics */
import React from "react";

class StringRefs extends React.Component {
componentDidMount() {
this.refs.hello.focus();
this.refs["world"]?.focus();
}

render() {
return (
<section>
<div ref="hello">Hello</div>
<div ref={"world"}>World</div>
<div ref={`template`}>Template</div>
<div ref={`template-${id}`}>Template Expression</div>
</section>
);
}
}
Loading
Loading