diff --git a/apps/oxlint/src/snapshots/fixtures__issue_11644_-c .oxlintrc.json@oxlint.snap b/apps/oxlint/src/snapshots/fixtures__issue_11644_-c .oxlintrc.json@oxlint.snap index 3e159b0cf2adf..ed83557cb465c 100644 --- a/apps/oxlint/src/snapshots/fixtures__issue_11644_-c .oxlintrc.json@oxlint.snap +++ b/apps/oxlint/src/snapshots/fixtures__issue_11644_-c .oxlintrc.json@oxlint.snap @@ -6,7 +6,7 @@ arguments: -c .oxlintrc.json working directory: fixtures/issue_11644 ---------- Found 0 warnings and 0 errors. -Finished in ms on 1 file with 163 rules using 1 threads. +Finished in ms on 1 file with 164 rules using 1 threads. ---------- CLI result: LintSucceeded ---------- diff --git a/crates/oxc_linter/src/generated/rule_runner_impls.rs b/crates/oxc_linter/src/generated/rule_runner_impls.rs index 3585572e61e55..932a48f1ac779 100644 --- a/crates/oxc_linter/src/generated/rule_runner_impls.rs +++ b/crates/oxc_linter/src/generated/rule_runner_impls.rs @@ -2408,6 +2408,12 @@ impl RuleRunner for crate::rules::react::no_danger_with_children::NoDangerWithCh const RUN_FUNCTIONS: RuleRunFunctionsImplemented = RuleRunFunctionsImplemented::Run; } +impl RuleRunner for crate::rules::react::no_did_mount_set_state::NoDidMountSetState { + const NODE_TYPES: Option<&AstTypesBitset> = + Some(&AstTypesBitset::from_types(&[AstType::CallExpression])); + const RUN_FUNCTIONS: RuleRunFunctionsImplemented = RuleRunFunctionsImplemented::Run; +} + impl RuleRunner for crate::rules::react::no_direct_mutation_state::NoDirectMutationState { const NODE_TYPES: Option<&AstTypesBitset> = Some(&AstTypesBitset::from_types(&[ AstType::AssignmentExpression, diff --git a/crates/oxc_linter/src/rules.rs b/crates/oxc_linter/src/rules.rs index 79f486f467a20..9217d61935025 100644 --- a/crates/oxc_linter/src/rules.rs +++ b/crates/oxc_linter/src/rules.rs @@ -381,6 +381,7 @@ pub(crate) mod react { pub mod no_children_prop; pub mod no_danger; pub mod no_danger_with_children; + pub mod no_did_mount_set_state; pub mod no_direct_mutation_state; pub mod no_find_dom_node; pub mod no_is_mounted; @@ -1082,6 +1083,7 @@ oxc_macros::declare_all_lint_rules! { react::jsx_no_useless_fragment, react::jsx_props_no_spread_multi, react::jsx_props_no_spreading, + react::no_did_mount_set_state, react::no_namespace, react::no_array_index_key, react::no_children_prop, diff --git a/crates/oxc_linter/src/rules/react/no_did_mount_set_state.rs b/crates/oxc_linter/src/rules/react/no_did_mount_set_state.rs new file mode 100644 index 0000000000000..bcda0401f0ac1 --- /dev/null +++ b/crates/oxc_linter/src/rules/react/no_did_mount_set_state.rs @@ -0,0 +1,426 @@ +use oxc_ast::{AstKind, ast::Expression}; +use oxc_diagnostics::OxcDiagnostic; +use oxc_macros::declare_oxc_lint; +use oxc_span::{GetSpan, Span}; +use schemars::JsonSchema; +use serde::{Deserialize, Serialize}; + +use crate::{ + AstNode, + context::LintContext, + rule::{DefaultRuleConfig, Rule}, + utils::{is_es5_component, is_es6_component}, +}; + +fn no_did_mount_set_state_diagnostic(span: Span) -> OxcDiagnostic { + OxcDiagnostic::warn("Do not use `setState` in `componentDidMount`.") + .with_help("Updating state after a component mount triggers a second render() call and can lead to property/layout thrashing.") + .with_label(span) +} + +#[derive(Debug, Default, Clone, Serialize, Deserialize, JsonSchema)] +#[serde(rename_all = "kebab-case")] +pub enum NoDidMountSetStateConfig { + #[default] + #[serde(skip)] + Allowed, + DisallowInFunc, +} + +#[derive(Debug, Default, Clone, Deserialize)] +pub struct NoDidMountSetState(NoDidMountSetStateConfig); + +declare_oxc_lint!( + /// ### What it does + /// + /// Disallows using `setState` in the `componentDidMount` lifecycle method. + /// + /// ### Why is this bad? + /// + /// Updating the state after a component mount will trigger a second `render()` call and can lead to property/layout thrashing. + /// This can cause performance issues and unexpected behavior. + /// + /// ### Examples + /// + /// Examples of **incorrect** code for this rule: + /// ```jsx + /// var Hello = createReactClass({ + /// componentDidMount: function() { + /// this.setState({ + /// name: this.props.name.toUpperCase() + /// }); + /// }, + /// render: function() { + /// return
Hello {this.state.name}
; + /// } + /// }); + /// ``` + /// + /// Examples of **correct** code for this rule: + /// ```jsx + /// var Hello = createReactClass({ + /// componentDidMount: function() { + /// this.onMount(function callback(newName) { + /// this.setState({ + /// name: newName + /// }); + /// }); + /// }, + /// render: function() { + /// return
Hello {this.state.name}
; + /// } + /// }); + /// ``` + /// + /// ### Options + /// + /// The rule accepts a string value `"disallow-in-func"`: + /// + /// ```json + /// { + /// "react/no-did-mount-set-state": ["error", "disallow-in-func"] + /// } + /// ``` + /// + /// When set, also disallows `setState` calls in nested functions within `componentDidMount`. + NoDidMountSetState, + react, + correctness, + config = NoDidMountSetStateConfig, +); + +impl Rule for NoDidMountSetState { + fn from_configuration(value: serde_json::Value) -> Self { + serde_json::from_value::>(value) + .unwrap_or_default() + .into_inner() + } + + fn run<'a>(&self, node: &AstNode<'a>, ctx: &LintContext<'a>) { + let AstKind::CallExpression(call_expr) = node.kind() else { return }; + + let Some(member_expr) = call_expr.callee.as_member_expression() else { return }; + + if !matches!(member_expr.object(), Expression::ThisExpression(_)) + || member_expr.static_property_name().is_none_or(|name| name != "setState") + { + return; + } + + let ancestors: Vec<_> = ctx.nodes().ancestors(node.id()).skip(1).collect(); + + let component_did_mount_index = + ancestors.iter().position(|ancestor| match ancestor.kind() { + AstKind::ObjectProperty(prop) + if prop.key.static_name().is_some_and(|key| key == "componentDidMount") => + { + true + } + AstKind::MethodDefinition(method) + if method.key.static_name().is_some_and(|name| name == "componentDidMount") => + { + true + } + AstKind::PropertyDefinition(prop) + if prop.key.static_name().is_some_and(|name| name == "componentDidMount") => + { + true + } + _ => false, + }); + + let Some(component_did_mount_idx) = component_did_mount_index else { + return; + }; + + let in_component_did_mount = ancestors[component_did_mount_idx..] + .iter() + .any(|ancestor| is_es5_component(ancestor) || is_es6_component(ancestor)); + + if !in_component_did_mount { + return; + } + + let function_count_before_component_did_mount = ancestors[..component_did_mount_idx] + .iter() + .filter(|ancestor| { + matches!( + ancestor.kind(), + AstKind::Function(_) | AstKind::ArrowFunctionExpression(_) + ) + }) + .count(); + + let in_nested_function = function_count_before_component_did_mount > 1; + + if in_nested_function && !matches!(self.0, NoDidMountSetStateConfig::DisallowInFunc) { + return; + } + + ctx.diagnostic(no_did_mount_set_state_diagnostic(call_expr.callee.span())); + } +} + +#[test] +fn test() { + use crate::tester::Tester; + + let pass = vec![ + " + var Hello = createReactClass({ + render: function() { + return
Hello {this.props.name}
; + } + }); + ", + " + var Hello = createReactClass({ + componentDidMount: function() {} + }); + ", + " + var Hello = createReactClass({ + componentDidMount: function() { + someNonMemberFunction(arg); + this.someHandler = this.setState; + } + }); + ", + " + var Hello = createReactClass({ + componentDidMount: function() { + someClass.onSomeEvent(function(data) { + this.setState({ + data: data + }); + }) + } + }); + ", + " + var Hello = createReactClass({ + componentDidMount: function() { + function handleEvent(data) { + this.setState({ + data: data + }); + } + someClass.onSomeEvent(handleEvent) + } + }); + ", + " + class Hello extends React.Component { + componentDidMount() { + this.handleEvent(() => { + this.setState({ data: 123 }); + }); + } + } + ", + " + class Hello extends React.Component { + componentDidUpdate() { + this.setState({ data: 123 }); + } + } + ", + " + class Hello extends React.Component { + componentWillMount() { + this.setState({ data: 123 }); + } + } + ", + " + var Hello = createReactClass({ + componentDidUpdate: function() { + this.setState({ data: 123 }); + } + }); + ", + " + function Hello() { + this.setState({ data: 123 }); + } + ", + " + var Hello = createReactClass({ + componentDidMount: function() { + setTimeout(() => { + this.setState({ data: 123 }); + }, 100); + } + }); + ", + " + class Hello extends React.Component { + componentDidMount() { + Promise.resolve().then(() => { + this.setState({ data: 123 }); + }); + } + } + ", + ]; + + let fail = vec![ + " + var Hello = createReactClass({ + componentDidMount: function() { + this.setState({ + name: this.props.name.toUpperCase() + }); + }, + render: function() { + return
Hello {this.state.name}
; + } + }); + ", + " + var Hello = createReactClass({ + componentDidMount: function componentDidMount() { + this.setState({ + name: this.props.name.toUpperCase() + }); + } + }); + ", + " + class Hello extends React.Component { + componentDidMount() { + this.setState({ + name: this.props.name.toUpperCase() + }); + } + } + ", + " + class Hello extends React.Component { + componentDidMount = () => { + this.setState({ + name: this.props.name.toUpperCase() + }); + } + } + ", + " + var Hello = createReactClass({ + componentDidMount: function() { + this.setState({ data: 1 }); + someClass.onSomeEvent(function(data) { + this.setState({ data: 2 }); + }) + } + }); + ", + " + class Hello extends React.Component { + componentDidMount() { + if (true) { + this.setState({ data: 123 }); + } + } + } + ", + " + class Hello extends React.Component { + componentDidMount() { + const x = true ? this.setState({ data: 123 }) : null; + } + } + ", + ]; + + Tester::new(NoDidMountSetState::NAME, NoDidMountSetState::PLUGIN, pass, fail) + .test_and_snapshot(); +} + +#[test] +fn test_disallow_in_func() { + use crate::tester::Tester; + + let pass = vec![ + ( + " + var Hello = createReactClass({ + componentDidMount: function() {} + }); + ", + Some(serde_json::json!(["disallow-in-func"])), + None, + ), + ( + " + var Hello = createReactClass({ + render: function() { + return
Hello {this.props.name}
; + } + }); + ", + Some(serde_json::json!(["disallow-in-func"])), + None, + ), + ]; + + let fail = vec![ + ( + " + var Hello = createReactClass({ + componentDidMount: function() { + this.setState({ + name: this.props.name.toUpperCase() + }); + } + }); + ", + Some(serde_json::json!(["disallow-in-func"])), + None, + ), + ( + " + var Hello = createReactClass({ + componentDidMount: function() { + someClass.onSomeEvent(function(data) { + this.setState({ + data: data + }); + }) + } + }); + ", + Some(serde_json::json!(["disallow-in-func"])), + None, + ), + ( + " + var Hello = createReactClass({ + componentDidMount: function() { + setTimeout(() => { + this.setState({ data: 123 }); + }, 100); + } + }); + ", + Some(serde_json::json!(["disallow-in-func"])), + None, + ), + ( + " + class Hello extends React.Component { + componentDidMount() { + Promise.resolve().then(() => { + this.setState({ data: 123 }); + }); + } + } + ", + Some(serde_json::json!(["disallow-in-func"])), + None, + ), + ]; + + Tester::new(NoDidMountSetState::NAME, NoDidMountSetState::PLUGIN, pass, fail) + .with_snapshot_suffix("disallow_in_func") + .test_and_snapshot(); +} diff --git a/crates/oxc_linter/src/snapshots/react_no_did_mount_set_state.snap b/crates/oxc_linter/src/snapshots/react_no_did_mount_set_state.snap new file mode 100644 index 0000000000000..997ed5ca8fd7c --- /dev/null +++ b/crates/oxc_linter/src/snapshots/react_no_did_mount_set_state.snap @@ -0,0 +1,65 @@ +--- +source: crates/oxc_linter/src/tester.rs +--- + ⚠ eslint-plugin-react(no-did-mount-set-state): Do not use `setState` in `componentDidMount`. + ╭─[no_did_mount_set_state.tsx:4:13] + 3 │ componentDidMount: function() { + 4 │ this.setState({ + · ───────────── + 5 │ name: this.props.name.toUpperCase() + ╰──── + help: Updating state after a component mount triggers a second render() call and can lead to property/layout thrashing. + + ⚠ eslint-plugin-react(no-did-mount-set-state): Do not use `setState` in `componentDidMount`. + ╭─[no_did_mount_set_state.tsx:4:13] + 3 │ componentDidMount: function componentDidMount() { + 4 │ this.setState({ + · ───────────── + 5 │ name: this.props.name.toUpperCase() + ╰──── + help: Updating state after a component mount triggers a second render() call and can lead to property/layout thrashing. + + ⚠ eslint-plugin-react(no-did-mount-set-state): Do not use `setState` in `componentDidMount`. + ╭─[no_did_mount_set_state.tsx:4:13] + 3 │ componentDidMount() { + 4 │ this.setState({ + · ───────────── + 5 │ name: this.props.name.toUpperCase() + ╰──── + help: Updating state after a component mount triggers a second render() call and can lead to property/layout thrashing. + + ⚠ eslint-plugin-react(no-did-mount-set-state): Do not use `setState` in `componentDidMount`. + ╭─[no_did_mount_set_state.tsx:4:13] + 3 │ componentDidMount = () => { + 4 │ this.setState({ + · ───────────── + 5 │ name: this.props.name.toUpperCase() + ╰──── + help: Updating state after a component mount triggers a second render() call and can lead to property/layout thrashing. + + ⚠ eslint-plugin-react(no-did-mount-set-state): Do not use `setState` in `componentDidMount`. + ╭─[no_did_mount_set_state.tsx:4:13] + 3 │ componentDidMount: function() { + 4 │ this.setState({ data: 1 }); + · ───────────── + 5 │ someClass.onSomeEvent(function(data) { + ╰──── + help: Updating state after a component mount triggers a second render() call and can lead to property/layout thrashing. + + ⚠ eslint-plugin-react(no-did-mount-set-state): Do not use `setState` in `componentDidMount`. + ╭─[no_did_mount_set_state.tsx:5:15] + 4 │ if (true) { + 5 │ this.setState({ data: 123 }); + · ───────────── + 6 │ } + ╰──── + help: Updating state after a component mount triggers a second render() call and can lead to property/layout thrashing. + + ⚠ eslint-plugin-react(no-did-mount-set-state): Do not use `setState` in `componentDidMount`. + ╭─[no_did_mount_set_state.tsx:4:30] + 3 │ componentDidMount() { + 4 │ const x = true ? this.setState({ data: 123 }) : null; + · ───────────── + 5 │ } + ╰──── + help: Updating state after a component mount triggers a second render() call and can lead to property/layout thrashing. diff --git a/crates/oxc_linter/src/snapshots/react_no_did_mount_set_state@disallow_in_func.snap b/crates/oxc_linter/src/snapshots/react_no_did_mount_set_state@disallow_in_func.snap new file mode 100644 index 0000000000000..2e24ab6a34ec4 --- /dev/null +++ b/crates/oxc_linter/src/snapshots/react_no_did_mount_set_state@disallow_in_func.snap @@ -0,0 +1,38 @@ +--- +source: crates/oxc_linter/src/tester.rs +--- + ⚠ eslint-plugin-react(no-did-mount-set-state): Do not use `setState` in `componentDidMount`. + ╭─[no_did_mount_set_state.tsx:4:17] + 3 │ componentDidMount: function() { + 4 │ this.setState({ + · ───────────── + 5 │ name: this.props.name.toUpperCase() + ╰──── + help: Updating state after a component mount triggers a second render() call and can lead to property/layout thrashing. + + ⚠ eslint-plugin-react(no-did-mount-set-state): Do not use `setState` in `componentDidMount`. + ╭─[no_did_mount_set_state.tsx:5:19] + 4 │ someClass.onSomeEvent(function(data) { + 5 │ this.setState({ + · ───────────── + 6 │ data: data + ╰──── + help: Updating state after a component mount triggers a second render() call and can lead to property/layout thrashing. + + ⚠ eslint-plugin-react(no-did-mount-set-state): Do not use `setState` in `componentDidMount`. + ╭─[no_did_mount_set_state.tsx:5:19] + 4 │ setTimeout(() => { + 5 │ this.setState({ data: 123 }); + · ───────────── + 6 │ }, 100); + ╰──── + help: Updating state after a component mount triggers a second render() call and can lead to property/layout thrashing. + + ⚠ eslint-plugin-react(no-did-mount-set-state): Do not use `setState` in `componentDidMount`. + ╭─[no_did_mount_set_state.tsx:5:19] + 4 │ Promise.resolve().then(() => { + 5 │ this.setState({ data: 123 }); + · ───────────── + 6 │ }); + ╰──── + help: Updating state after a component mount triggers a second render() call and can lead to property/layout thrashing.