feat(linter): add `react/require-optimization` rule #17370

baevm · 2025-12-26T14:13:57Z

this PR adds react/require-optimization rule, issue #1022

baevm · 2025-12-26T14:14:18Z

crates/oxc_linter/src/rules/react/require_optimization.rs

+    /// ```
+    RequireOptimization,
+    react,
+    perf,


not sure should this be style or perf?

Copilot

Pull request overview

This PR adds the react/require-optimization linter rule that enforces React class components to have a shouldComponentUpdate method or use optimization alternatives like PureComponent or decorators/mixins. The rule helps identify components that may cause unnecessary re-renders and performance issues.

Key Changes

Implements detection of React class components (ES6 and ES5 via createReactClass)
Validates presence of shouldComponentUpdate method
Supports allowDecorators configuration option for custom optimization decorators/mixins
Includes comprehensive test coverage for various component patterns

Reviewed changes

Copilot reviewed 3 out of 4 changed files in this pull request and generated 3 comments.

File	Description
`crates/oxc_linter/src/rules/react/require_optimization.rs`	Main implementation of the rule with component detection, SCU validation, and decorator/mixin support
`crates/oxc_linter/src/snapshots/react_require_optimization.snap`	Snapshot tests showing diagnostic output for various non-optimized component patterns
`crates/oxc_linter/src/rules.rs`	Registers the new rule module in the linter's rule collection
`crates/oxc_linter/src/generated/rule_runner_impls.rs`	Generated code specifying which AST node types trigger the rule (CallExpression and Class)

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2025-12-26T14:16:48Z

crates/oxc_linter/src/rules/react/require_optimization.rs

+/// checks if class is React.Component
+fn is_react_class_component(node: &AstNode) -> bool {
+    let AstKind::Class(class_expr) = node.kind() else {
+        return false;
+    };
+    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()
+        {
+            return ident.name == "React"
+                && member_expr.static_property_name().is_some_and(|name| name == "Component");
+        }
+
+        if let Some(ident_reference) = super_class.get_identifier_reference() {
+            return ident_reference.name == "Component";
+        }
+    }
+
+    false
+}


The function is_react_class_component only checks if a class extends React.Component or Component, but it doesn't exclude classes that extend React.PureComponent or PureComponent. Since PureComponent already implements shouldComponentUpdate with shallow prop comparison, these classes should not be flagged by this rule. The test cases on lines 358-370 expect PureComponent classes to pass, but this implementation would incorrectly flag them.

i dont think thats right, PureComponent automatically implements shouldComponentUpdate and this function will just skip it

Copilot · 2025-12-26T14:16:48Z

crates/oxc_linter/src/rules/react/require_optimization.rs

+#[derive(Debug, Default, Clone, Serialize, Deserialize, JsonSchema)]
+#[schemars(rename_all = "camelCase")]
+pub struct RequireOptimizationConfig {
+    /// Sets the allowed names of decorators. If the variable is present in the chain of decorators, it validates


The documentation comment is incomplete. It says "If the variable is present in the chain of decorators, it validates" but doesn't complete the thought about what it validates or what happens. This should be completed to provide a clear explanation, for example: "If the variable is present in the chain of decorators, the component is considered optimized."

Suggested change

/// Sets the allowed names of decorators. If the variable is present in the chain of decorators, it validates

/// Sets the allowed names of decorators. If the variable is present in the chain of decorators, the component is

/// considered optimized and satisfies this rule.

Copilot · 2025-12-26T14:16:48Z

crates/oxc_linter/src/rules/react/require_optimization.rs

+    OxcDiagnostic::warn("Component is not optimized. Please add a shouldComponentUpdate method.")
+        .with_label(span)


The error message only mentions adding a shouldComponentUpdate method, but doesn't mention the alternative of using PureComponent or React.memo. While the documentation in the "Why is this bad?" section mentions these alternatives, the diagnostic message should also reference them to provide more complete guidance. Consider updating the message to something like: "Component is not optimized. Please add a shouldComponentUpdate method, extend PureComponent, or use React.memo."

Suggested change

OxcDiagnostic::warn("Component is not optimized. Please add a shouldComponentUpdate method.")

.with_label(span)

OxcDiagnostic::warn(

"Component is not optimized. Please add a shouldComponentUpdate method, extend PureComponent, or use React.memo.",

)

.with_label(span)

codspeed-hq · 2025-12-26T14:25:27Z

CodSpeed Performance Report

Merging #17370 will not alter performance

_{Comparing baevm:react/require-optimization (76206c4) with main (d2abc78)}

Summary

✅ 4 untouched
⏩ 41 skipped¹

41 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩

connorshea

A few notes for the docs

crates/oxc_linter/src/rules/react/require_optimization.rs

connorshea · 2025-12-26T16:01:09Z

crates/oxc_linter/src/rules/react/require_optimization.rs

+declare_oxc_lint!(
+    /// ### What it does
+    ///
+    /// Enforce React components to have a shouldComponentUpdate method


Should note that this only applies to class components, since most components in modern react are function-based at this point. Also, based on the react docs I think we should probably have a note in the docs here that this rule isn't encouraged. React is able to optimize this itself in many many cases and having devs implement this themselves is a lot more error-prone. At least, in my opinion.

https://react.dev/reference/react/Component#shouldcomponentupdate

added note about class components

connorshea · 2025-12-26T16:22:07Z

This rule should also have a should_run that checks if it's a jsx file, no need to run on non-jsx/tsx files.

baevm · 2025-12-26T16:35:28Z

This rule should also have a should_run that checks if it's a jsx file, no need to run on non-jsx/tsx files.

fixed

Copilot AI review requested due to automatic review settings December 26, 2025 14:13

baevm requested a review from camc314 as a code owner December 26, 2025 14:13

github-actions bot added A-linter Area - Linter C-enhancement Category - New feature or request labels Dec 26, 2025

baevm commented Dec 26, 2025

View reviewed changes

Copilot started reviewing on behalf of baevm December 26, 2025 14:14 View session

Copilot AI reviewed Dec 26, 2025

View reviewed changes

baevm and others added 4 commits December 26, 2025 19:38

feat(linter): add react/require_optimization rule

8ee8102

fix linter errors

e9f2115

[autofix.ci] apply automated fixes

311af55

fix copilot suggest

c2b7f7f

baevm force-pushed the react/require-optimization branch from beda0e7 to c2b7f7f Compare December 26, 2025 14:38

wrap config in Box

a89adff

connorshea requested changes Dec 26, 2025

View reviewed changes

fix docs

eb485b3

fix json struct mapping

76206c4

camc314 self-assigned this Dec 28, 2025

	/// Sets the allowed names of decorators. If the variable is present in the chain of decorators, it validates
	/// Sets the allowed names of decorators. If the variable is present in the chain of decorators, the component is
	/// considered optimized and satisfies this rule.

		OxcDiagnostic::warn("Component is not optimized. Please add a shouldComponentUpdate method.")
		.with_label(span)

Uh oh!

feat(linter): add react/require-optimization rule #17370

Are you sure you want to change the base?

feat(linter): add react/require-optimization rule #17370

Conversation

baevm commented Dec 26, 2025

Uh oh!

baevm Dec 26, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Key Changes

Reviewed changes

Uh oh!

Copilot AI Dec 26, 2025

Choose a reason for hiding this comment

Uh oh!

baevm Dec 26, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 26, 2025

Choose a reason for hiding this comment

Uh oh!

baevm Dec 26, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 26, 2025

Choose a reason for hiding this comment

Uh oh!

codspeed-hq bot commented Dec 26, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

CodSpeed Performance Report

Merging #17370 will not alter performance

Summary

Footnotes

Uh oh!

connorshea left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

connorshea Dec 26, 2025

Choose a reason for hiding this comment

Uh oh!

baevm Dec 26, 2025

Choose a reason for hiding this comment

Uh oh!

connorshea commented Dec 26, 2025

Uh oh!

baevm commented Dec 26, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

feat(linter): add `react/require-optimization` rule #17370

feat(linter): add `react/require-optimization` rule #17370

codspeed-hq bot commented Dec 26, 2025 •

edited

Loading