Skip to content

feat(linter): add react/no-redundant-should-component-update rule #16147

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.

Sign up for GitHub

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

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
+320 −0
Open

feat(linter): add react/no-redundant-should-component-update rule #16147

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
3db27dd
feat(linter): add react/no-redundant-should-component-update
baevm Nov 25, 2025
184b5fe
fix clippy
baevm Nov 25, 2025
dba5c3a
[autofix.ci] apply automated fixes
autofix-ci[bot] Nov 26, 2025
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 crates/oxc_linter/src/generated/rule_runner_impls.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2328,6 +2328,13 @@ impl RuleRunner for crate::rules::react::no_namespace::NoNamespace {
const RUN_FUNCTIONS: RuleRunFunctionsImplemented = RuleRunFunctionsImplemented::Run;
}

impl RuleRunner
for crate::rules::react::no_redundant_should_component_update::NoRedundantShouldComponentUpdate
{
const NODE_TYPES: Option<&AstTypesBitset> = None;
const RUN_FUNCTIONS: RuleRunFunctionsImplemented = RuleRunFunctionsImplemented::Run;
}

impl RuleRunner for crate::rules::react::no_render_return_value::NoRenderReturnValue {
const NODE_TYPES: Option<&AstTypesBitset> =
Some(&AstTypesBitset::from_types(&[AstType::CallExpression]));
Expand Down
2 changes: 2 additions & 0 deletions crates/oxc_linter/src/rules.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -370,6 +370,7 @@ pub(crate) mod react {
pub mod no_find_dom_node;
pub mod no_is_mounted;
pub mod no_namespace;
pub mod no_redundant_should_component_update;
pub mod no_render_return_value;
pub mod no_set_state;
pub mod no_string_refs;
Expand Down Expand Up @@ -1047,6 +1048,7 @@ oxc_macros::declare_all_lint_rules! {
react::no_direct_mutation_state,
react::no_find_dom_node,
react::no_is_mounted,
react::no_redundant_should_component_update,
react::no_render_return_value,
react::no_set_state,
react::no_string_refs,
Expand Down
243 changes: 243 additions & 0 deletions crates/oxc_linter/src/rules/react/no_redundant_should_component_update.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,243 @@
use oxc_ast::{
AstKind,
ast::{Class, Expression},
};
use oxc_diagnostics::OxcDiagnostic;
use oxc_macros::declare_oxc_lint;
use oxc_span::Span;

use crate::{AstNode, context::LintContext, rule::Rule};
Copy link

Copilot AI Nov 26, 2025

Choose a reason for hiding this comment

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

The import is missing ContextHost which is needed for the should_run method implementation. Update the import to:\n\nrust\nuse crate::{AstNode, context::{ContextHost, LintContext}, rule::Rule};\n

Suggested change
use crate::{AstNode, context::LintContext, rule::Rule};
use crate::{AstNode, context::{ContextHost, LintContext}, rule::Rule};

Copilot uses AI. Check for mistakes.

fn no_redundant_should_component_update_diagnostic(
span: Span,
component_name: &str,
) -> OxcDiagnostic {
let msg = format!(
"{component_name} does not need shouldComponentUpdate when extending React.PureComponent."
);
OxcDiagnostic::warn(msg).with_label(span)
}

#[derive(Debug, Default, Clone)]
pub struct NoRedundantShouldComponentUpdate;

declare_oxc_lint!(
/// ### What it does
///
/// Disallow usage of shouldComponentUpdate when extending React.PureComponent
///
/// ### Why is this bad?
///
/// React.PureComponent automatically implements shouldComponentUpdate with a shallow prop and state comparison.
/// Defining shouldComponentUpdate in a class that extends PureComponent is redundant and defeats the purpose
/// of using PureComponent. If you need custom comparison logic, extend React.Component instead.
///
/// ### Examples
///
/// Examples of **incorrect** code for this rule:
/// ```jsx
/// class Foo extends React.PureComponent {
/// shouldComponentUpdate() {
/// // do check
/// }
///
/// render() {
/// return <div>Radical!</div>
/// }
///}
///
///function Bar() {
/// return class Baz extends React.PureComponent {
/// shouldComponentUpdate() {
/// // do check
/// }
///
/// render() {
/// return <div>Groovy!</div>
/// }
/// }
///}
/// ```
///
/// Examples of **correct** code for this rule:
/// ```jsx
/// class Foo extends React.Component {
/// shouldComponentUpdate() {
/// // do check
/// }
///
/// render() {
/// return <div>Radical!</div>
/// }
/// }
///
/// function Bar() {
/// return class Baz extends React.Component {
/// shouldComponentUpdate() {
/// // do check
/// }
///
/// render() {
/// return <div>Groovy!</div>
/// }
/// }
/// }
///
/// class Qux extends React.PureComponent {
/// render() {
/// return <div>Tubular!</div>
/// }
/// }
/// ```
NoRedundantShouldComponentUpdate,
react,
style,
);

impl Rule for NoRedundantShouldComponentUpdate {
fn run<'a>(&self, node: &AstNode<'a>, ctx: &LintContext<'a>) {
Comment on lines +97 to +98
Copy link

Copilot AI Nov 26, 2025

Choose a reason for hiding this comment

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

The rule is missing the should_run method that checks if the source file is JSX. All React rules in this codebase implement should_run to skip non-JSX files for performance. Add this method after the run method:\n\nrust\nfn should_run(&self, ctx: &ContextHost) -> bool {\n ctx.source_type().is_jsx()\n}\n\n\nYou'll also need to import ContextHost in the imports section.

Copilot uses AI. Check for mistakes.
let Some(class_node) = is_react_pure_component(node) else {
return;
};

if has_should_component_update(class_node) {
let component_name = class_node
.name()
.or_else(|| {
// e.g. var Foo = class extends PureComponent
let parent = ctx.nodes().parent_node(node.id());
if let AstKind::VariableDeclarator(declarator) = parent.kind() {
declarator.id.get_identifier_name()
} else {
None
}
})
.map_or("", |name| name.as_str());

ctx.diagnostic(no_redundant_should_component_update_diagnostic(
class_node.span,
component_name,
));
}
}
}

fn has_should_component_update(class: &Class<'_>) -> bool {
class
.body
.body
.iter()
.any(|prop| prop.static_name().is_some_and(|name| name == "shouldComponentUpdate"))
}

/// Checks if class is React.PureComponent and returns this class if true
fn is_react_pure_component<'a>(node: &'a AstNode) -> Option<&'a Class<'a>> {
let AstKind::Class(class_expr) = node.kind() else {
return None;
};
if let Some(super_class) = &class_expr.super_class {
if let Some(member_expr) = super_class.as_member_expression()
&& let Expression::Identifier(ident) = member_expr.object()
{
let is_pure = ident.name == "React"
&& member_expr.static_property_name().is_some_and(|name| name == "PureComponent");

return if is_pure { Some(class_expr) } else { None };
}

if let Some(ident_reference) = super_class.get_identifier_reference() {
let is_pure = ident_reference.name == "PureComponent";
return if is_pure { Some(class_expr) } else { None };
}
}

None
}

#[test]
fn test() {
use crate::tester::Tester;

let pass = vec![
"
class Foo extends React.Component {
shouldComponentUpdate() {
return true;
}
}
",
"
class Foo extends React.Component {
shouldComponentUpdate = () => {
return true;
}
}
",
"
function Foo() {
return class Bar extends React.Component {
shouldComponentUpdate() {
return true;
}
};
}
",
];

let fail = vec![
"
class Foo extends React.PureComponent {
shouldComponentUpdate() {
return true;
}
}
",
"
class Foo extends PureComponent {
shouldComponentUpdate() {
return true;
}
}
",
"
class Foo extends React.PureComponent {
shouldComponentUpdate = () => {
return true;
}
}
",
"
function Foo() {
return class Bar extends React.PureComponent {
shouldComponentUpdate() {
return true;
}
};
}
",
"
function Foo() {
return class Bar extends PureComponent {
shouldComponentUpdate() {
return true;
}
};
}
",
"
var Foo = class extends PureComponent {
shouldComponentUpdate() {
return true;
}
}
",
];

Tester::new(
NoRedundantShouldComponentUpdate::NAME,
NoRedundantShouldComponentUpdate::PLUGIN,
pass,
fail,
)
.test_and_snapshot();
}
68 changes: 68 additions & 0 deletions crates/oxc_linter/src/snapshots/react_no_redundant_should_component_update.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
---
source: crates/oxc_linter/src/tester.rs
---
⚠ eslint-plugin-react(no-redundant-should-component-update): Foo does not need shouldComponentUpdate when extending React.PureComponent.
╭─[no_redundant_should_component_update.tsx:2:12]
1 │
2 │ ╭─▶ class Foo extends React.PureComponent {
3 │ │ shouldComponentUpdate() {
4 │ │ return true;
5 │ │ }
6 │ ╰─▶ }
7 │
╰────

⚠ eslint-plugin-react(no-redundant-should-component-update): Foo does not need shouldComponentUpdate when extending React.PureComponent.
╭─[no_redundant_should_component_update.tsx:2:12]
1 │
2 │ ╭─▶ class Foo extends PureComponent {
3 │ │ shouldComponentUpdate() {
4 │ │ return true;
5 │ │ }
6 │ ╰─▶ }
7 │
╰────

⚠ eslint-plugin-react(no-redundant-should-component-update): Foo does not need shouldComponentUpdate when extending React.PureComponent.
╭─[no_redundant_should_component_update.tsx:2:12]
1 │
2 │ ╭─▶ class Foo extends React.PureComponent {
3 │ │ shouldComponentUpdate = () => {
4 │ │ return true;
5 │ │ }
6 │ ╰─▶ }
7 │
╰────

⚠ eslint-plugin-react(no-redundant-should-component-update): Bar does not need shouldComponentUpdate when extending React.PureComponent.
╭─[no_redundant_should_component_update.tsx:3:21]
2 │ function Foo() {
3 │ ╭─▶ return class Bar extends React.PureComponent {
4 │ │ shouldComponentUpdate() {
5 │ │ return true;
6 │ │ }
7 │ ╰─▶ };
8 │ }
╰────

⚠ eslint-plugin-react(no-redundant-should-component-update): Bar does not need shouldComponentUpdate when extending React.PureComponent.
╭─[no_redundant_should_component_update.tsx:3:21]
2 │ function Foo() {
3 │ ╭─▶ return class Bar extends PureComponent {
4 │ │ shouldComponentUpdate() {
5 │ │ return true;
6 │ │ }
7 │ ╰─▶ };
8 │ }
╰────

⚠ eslint-plugin-react(no-redundant-should-component-update): Foo does not need shouldComponentUpdate when extending React.PureComponent.
╭─[no_redundant_should_component_update.tsx:2:22]
1 │
2 │ ╭─▶ var Foo = class extends PureComponent {
3 │ │ shouldComponentUpdate() {
4 │ │ return true;
5 │ │ }
6 │ ╰─▶ }
7 │
╰────
Loading