-
-
Notifications
You must be signed in to change notification settings - Fork 988
feat(lint/js): add noReactStringRefs
#9922
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| 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()`. |
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.
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.
| 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>; | ||||||
| /// } | ||||||
| /// ``` | ||||||
| /// | ||||||
| pub NoReactStringRefs { | ||||||
| version: "next", | ||||||
| name: "noReactStringRefs", | ||||||
| language: "js", | ||||||
| sources: &[RuleSource::EslintReact("no-string-refs").same()], | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧩 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 Line 65 marks this as behaviour-identical to ESLint, but the rule unconditionally flags template literal refs (lines 156–165), whereas ESLint's Suggested fix- sources: &[RuleSource::EslintReact("no-string-refs").same()],
+ sources: &[RuleSource::EslintReact("no-string-refs").inspired()],📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||
| 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> | ||
| ); | ||
| } | ||
| } |
Uh oh!
There was an error while loading. Please reload this page.