From 3d61b99934be86f3f60f4798099cbc4e77ccb3cc Mon Sep 17 00:00:00 2001 From: Daiki Nishikawa Date: Wed, 21 Jun 2023 17:56:54 +0900 Subject: [PATCH] fix(rome_js_analyze): useExhaustiveDependencies support function syntax, or React.use* hooks and more. (#4571) Co-authored-by: Emanuele Stoppa --- CHANGELOG.md | 6 + crates/rome_js_analyze/src/react.rs | 17 +- crates/rome_js_analyze/src/react/hooks.rs | 77 ++- .../nursery/use_exhaustive_dependencies.rs | 19 +- .../checkHooksImportedFromReact.js | 20 + .../checkHooksImportedFromReact.js.snap | 55 ++ .../useExhaustiveDependencies/customHook.js | 4 +- .../customHook.js.snap | 51 +- .../extraDependenciesInvalid.js | 4 +- .../extraDependenciesInvalid.js.snap | 132 ++-- .../missingDependenciesInvalid.js | 43 +- .../missingDependenciesInvalid.js.snap | 589 +++++++++++------- .../useExhaustiveDependencies/valid.js | 40 ++ .../useExhaustiveDependencies/valid.js.snap | 40 ++ .../useExhaustiveDependencies/valid.ts | 15 +- .../useExhaustiveDependencies/valid.ts.snap | 14 +- .../lint/rules/useExhaustiveDependencies.md | 104 ++-- website/src/playground/Playground.tsx | 52 +- .../src/playground/components/Resizable.tsx | 2 +- 19 files changed, 889 insertions(+), 395 deletions(-) create mode 100644 crates/rome_js_analyze/tests/specs/nursery/useExhaustiveDependencies/checkHooksImportedFromReact.js create mode 100644 crates/rome_js_analyze/tests/specs/nursery/useExhaustiveDependencies/checkHooksImportedFromReact.js.snap diff --git a/CHANGELOG.md b/CHANGELOG.md index 44c84ed4e13..ab541f29c2a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -57,12 +57,18 @@ parameter decorators: The _Rome_ formatter takes care of removing extra semicolons. Thus, there is no need for this rule. + #### Other changes - [`noRedeclare`](https://docs.rome.tools/lint/rules/noredeclare/): allow redeclare of index signatures are in different type members [#4478](https://github.com/rome/tools/issues/4478) - Fix a crash in the [`NoParameterAssign`](https://docs.rome.tools/lint/rules/noparameterassign/) rule that occurred when there was a bogus binding. [#4323](https://github.com/rome/tools/issues/4323) +- Fix [`useExhaustiveDependencies`](https://docs.rome.tools/lint/rules/useexhaustivedependencies/) rule in the following cases [#4330](https://github.com/rome/tools/issues/4330) + - when the first argument of hooks is a named function + - inside an export default function + - for React.use* hooks + - Improve the diagnostic and the code action of [`useDefaultParameterLast`](https://docs.rome.tools/lint/rules/usedefaultparameterlast/). The diagnostic now reports the last required parameter which should precede optional and default parameters. diff --git a/crates/rome_js_analyze/src/react.rs b/crates/rome_js_analyze/src/react.rs index 2d53124d318..cf4a27b6f39 100644 --- a/crates/rome_js_analyze/src/react.rs +++ b/crates/rome_js_analyze/src/react.rs @@ -142,7 +142,7 @@ impl ReactLibrary { /// List of valid [`React` API] /// /// [`React` API]: https://reactjs.org/docs/react-api.html -const VALID_REACT_API: [&str; 14] = [ +const VALID_REACT_API: [&str; 29] = [ "Component", "PureComponent", "memo", @@ -157,6 +157,21 @@ const VALID_REACT_API: [&str; 14] = [ "Suspense", "startTransition", "Children", + "useEffect", + "useLayoutEffect", + "useInsertionEffect", + "useCallback", + "useMemo", + "useImperativeHandle", + "useState", + "useContext", + "useReducer", + "useRef", + "useDebugValue", + "useDeferredValue", + "useTransition", + "useId", + "useSyncExternalStore", ]; /// Checks if the current [JsCallExpression] is a potential [`React` API]. diff --git a/crates/rome_js_analyze/src/react/hooks.rs b/crates/rome_js_analyze/src/react/hooks.rs index 73d7e1ec053..26ac2b16b22 100644 --- a/crates/rome_js_analyze/src/react/hooks.rs +++ b/crates/rome_js_analyze/src/react/hooks.rs @@ -1,9 +1,11 @@ +use crate::react::{is_react_call_api, ReactLibrary}; use std::collections::{HashMap, HashSet}; -use rome_js_semantic::{Capture, ClosureExtensions, SemanticModel}; +use rome_js_semantic::{Capture, Closure, ClosureExtensions, SemanticModel}; use rome_js_syntax::{ binding_ext::AnyJsIdentifierBinding, AnyJsExpression, JsArrayBindingPattern, - JsArrayBindingPatternElementList, JsCallExpression, JsVariableDeclarator, TextRange, + JsArrayBindingPatternElementList, JsArrowFunctionExpression, JsCallExpression, + JsFunctionExpression, JsVariableDeclarator, TextRange, }; use rome_rowan::AstNode; use serde::{Deserialize, Serialize}; @@ -15,15 +17,45 @@ pub(crate) struct ReactCallWithDependencyResult { pub(crate) dependencies_node: Option, } +pub enum AnyJsFunctionExpression { + JsArrowFunctionExpression(JsArrowFunctionExpression), + JsFunctionExpression(JsFunctionExpression), +} + +impl AnyJsFunctionExpression { + fn closure(&self, model: &SemanticModel) -> Closure { + match self { + Self::JsArrowFunctionExpression(arrow_function) => arrow_function.closure(model), + Self::JsFunctionExpression(function) => function.closure(model), + } + } +} + +impl TryFrom for AnyJsFunctionExpression { + type Error = (); + + fn try_from(expression: AnyJsExpression) -> Result { + match expression { + AnyJsExpression::JsArrowFunctionExpression(arrow_function) => { + Ok(Self::JsArrowFunctionExpression(arrow_function)) + } + AnyJsExpression::JsFunctionExpression(function) => { + Ok(Self::JsFunctionExpression(function)) + } + _ => Err(()), + } + } +} + impl ReactCallWithDependencyResult { /// Returns all [Reference] captured by the closure argument of the React hook. /// See [react_hook_with_dependency]. pub fn all_captures(&self, model: &SemanticModel) -> impl Iterator { self.closure_node .as_ref() - .and_then(|node| node.as_js_arrow_function_expression()) - .map(|arrow_function| { - let closure = arrow_function.closure(model); + .and_then(|node| AnyJsFunctionExpression::try_from(node.clone()).ok()) + .map(|function_expression| { + let closure = function_expression.closure(model); let range = *closure.closure_range(); closure .descendents() @@ -78,6 +110,15 @@ pub(crate) fn react_hook_configuration<'a>( hooks.get(name) } +const HOOKS_WITH_DEPS_API: [&str; 6] = [ + "useEffect", + "useLayoutEffect", + "useInsertionEffect", + "useCallback", + "useMemo", + "useImperativeHandle", +]; + /// Returns the [TextRange] of the hook name; the node of the /// expression of the argument that correspond to the closure of /// the hook; and the node of the dependency list of the hook. @@ -95,18 +136,28 @@ pub(crate) fn react_hook_configuration<'a>( pub(crate) fn react_hook_with_dependency( call: &JsCallExpression, hooks: &HashMap, + model: &SemanticModel, ) -> Option { - let name = call - .callee() - .ok()? - .as_js_identifier_expression()? - .name() - .ok()? - .value_token() - .ok()?; + let expression = call.callee().ok()?; + let name = match &expression { + AnyJsExpression::JsIdentifierExpression(identifier) => { + Some(identifier.name().ok()?.value_token().ok()?) + } + AnyJsExpression::JsStaticMemberExpression(member) => { + Some(member.member().ok()?.as_js_name()?.value_token().ok()?) + } + _ => None, + }?; let function_name_range = name.text_trimmed_range(); let name = name.text_trimmed(); + // check if the hooks api is imported from the react library + if HOOKS_WITH_DEPS_API.contains(&name) + && !is_react_call_api(expression, model, ReactLibrary::React, name) + { + return None; + } + let hook = hooks.get(name)?; let closure_index = hook.closure_index?; let dependencies_index = hook.dependencies_index?; diff --git a/crates/rome_js_analyze/src/semantic_analyzers/nursery/use_exhaustive_dependencies.rs b/crates/rome_js_analyze/src/semantic_analyzers/nursery/use_exhaustive_dependencies.rs index b90a750d451..7143507369b 100644 --- a/crates/rome_js_analyze/src/semantic_analyzers/nursery/use_exhaustive_dependencies.rs +++ b/crates/rome_js_analyze/src/semantic_analyzers/nursery/use_exhaustive_dependencies.rs @@ -27,6 +27,8 @@ declare_rule! { /// ### Invalid /// /// ```js,expect_diagnostic + /// import { useEffect } from "react"; + /// /// function component() { /// let a = 1; /// useEffect(() => { @@ -36,6 +38,8 @@ declare_rule! { /// ``` /// /// ```js,expect_diagnostic + /// import { useEffect } from "react"; + /// /// function component() { /// let b = 1; /// useEffect(() => { @@ -44,6 +48,8 @@ declare_rule! { /// ``` /// /// ```js,expect_diagnostic + /// import { useEffect, useState } from "react"; + /// /// function component() { /// const [name, setName] = useState(); /// useEffect(() => { @@ -54,6 +60,8 @@ declare_rule! { /// ``` /// /// ```js,expect_diagnostic + /// import { useEffect } from "react"; + /// /// function component() { /// let a = 1; /// const b = a + 1; @@ -66,6 +74,8 @@ declare_rule! { /// ## Valid /// /// ```js + /// import { useEffect } from "react"; + /// /// function component() { /// let a = 1; /// useEffect(() => { @@ -75,6 +85,8 @@ declare_rule! { /// ``` /// /// ```js + /// import { useEffect } from "react"; + /// /// function component() { /// const a = 1; /// useEffect(() => { @@ -84,6 +96,8 @@ declare_rule! { /// ``` /// /// ```js + /// import { useEffect } from "react"; + /// /// function component() { /// const [name, setName] = useState(); /// useEffect(() => { @@ -489,6 +503,7 @@ fn function_of_hook_call(call: &JsCallExpression) -> Option { matches!( x.kind(), JsSyntaxKind::JS_FUNCTION_DECLARATION + | JsSyntaxKind::JS_FUNCTION_EXPORT_DEFAULT_DECLARATION | JsSyntaxKind::JS_FUNCTION_EXPRESSION | JsSyntaxKind::JS_ARROW_FUNCTION_EXPRESSION ) @@ -508,9 +523,9 @@ impl Rule for UseExhaustiveDependencies { let mut signals = vec![]; let call = ctx.query(); - if let Some(result) = react_hook_with_dependency(call, &options.hooks_config) { - let model = ctx.model(); + let model = ctx.model(); + if let Some(result) = react_hook_with_dependency(call, &options.hooks_config, model) { let Some(component_function) = function_of_hook_call(call) else { return vec![] }; diff --git a/crates/rome_js_analyze/tests/specs/nursery/useExhaustiveDependencies/checkHooksImportedFromReact.js b/crates/rome_js_analyze/tests/specs/nursery/useExhaustiveDependencies/checkHooksImportedFromReact.js new file mode 100644 index 00000000000..34e548d4ede --- /dev/null +++ b/crates/rome_js_analyze/tests/specs/nursery/useExhaustiveDependencies/checkHooksImportedFromReact.js @@ -0,0 +1,20 @@ +function MyComponent1() { + let a = 1; + React.useEffect(() => { + console.log(a); + }); + + // the rule doesn't show the warnings because the hooks are not imported from react. + useEffect(() => { + console.log(a); + }); +} + +function MyComponent2() { + let a = 1; + const React = { useEffect() {} } + // the rule doesn't show the warnings because `React` is defined by the user. + React.useEffect(() => { + console.log(a); + }); +} diff --git a/crates/rome_js_analyze/tests/specs/nursery/useExhaustiveDependencies/checkHooksImportedFromReact.js.snap b/crates/rome_js_analyze/tests/specs/nursery/useExhaustiveDependencies/checkHooksImportedFromReact.js.snap new file mode 100644 index 00000000000..ff12ddbfcde --- /dev/null +++ b/crates/rome_js_analyze/tests/specs/nursery/useExhaustiveDependencies/checkHooksImportedFromReact.js.snap @@ -0,0 +1,55 @@ +--- +source: crates/rome_js_analyze/tests/spec_tests.rs +expression: checkHooksImportedFromReact.js +--- +# Input +```js +function MyComponent1() { + let a = 1; + React.useEffect(() => { + console.log(a); + }); + + // the rule doesn't show the warnings because the hooks are not imported from react. + useEffect(() => { + console.log(a); + }); +} + +function MyComponent2() { + let a = 1; + const React = { useEffect() {} } + // the rule doesn't show the warnings because `React` is defined by the user. + React.useEffect(() => { + console.log(a); + }); +} + +``` + +# Diagnostics +``` +checkHooksImportedFromReact.js:3:9 lint/nursery/useExhaustiveDependencies ━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! This hook do not specify all of its dependencies. + + 1 │ function MyComponent1() { + 2 │ let a = 1; + > 3 │ React.useEffect(() => { + │ ^^^^^^^^^ + 4 │ console.log(a); + 5 │ }); + + i This dependency is not specified in the hook dependency list. + + 2 │ let a = 1; + 3 │ React.useEffect(() => { + > 4 │ console.log(a); + │ ^ + 5 │ }); + 6 │ + + +``` + + diff --git a/crates/rome_js_analyze/tests/specs/nursery/useExhaustiveDependencies/customHook.js b/crates/rome_js_analyze/tests/specs/nursery/useExhaustiveDependencies/customHook.js index 1dd9c444ea2..4387d80345f 100644 --- a/crates/rome_js_analyze/tests/specs/nursery/useExhaustiveDependencies/customHook.js +++ b/crates/rome_js_analyze/tests/specs/nursery/useExhaustiveDependencies/customHook.js @@ -1,3 +1,5 @@ +import { useEffect } from "react"; + function MyComponent() { let a = 1; useEffect(() => { @@ -6,4 +8,4 @@ function MyComponent() { useMyEffect(() => { console.log(a); }); -} \ No newline at end of file +} diff --git a/crates/rome_js_analyze/tests/specs/nursery/useExhaustiveDependencies/customHook.js.snap b/crates/rome_js_analyze/tests/specs/nursery/useExhaustiveDependencies/customHook.js.snap index 6da45d54712..1e350b302f4 100644 --- a/crates/rome_js_analyze/tests/specs/nursery/useExhaustiveDependencies/customHook.js.snap +++ b/crates/rome_js_analyze/tests/specs/nursery/useExhaustiveDependencies/customHook.js.snap @@ -4,6 +4,8 @@ expression: customHook.js --- # Input ```js +import { useEffect } from "react"; + function MyComponent() { let a = 1; useEffect(() => { @@ -13,53 +15,54 @@ function MyComponent() { console.log(a); }); } + ``` # Diagnostics ``` -customHook.js:3:5 lint/nursery/useExhaustiveDependencies ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ +customHook.js:5:5 lint/nursery/useExhaustiveDependencies ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ ! This hook do not specify all of its dependencies. - 1 │ function MyComponent() { - 2 │ let a = 1; - > 3 │ useEffect(() => { + 3 │ function MyComponent() { + 4 │ let a = 1; + > 5 │ useEffect(() => { │ ^^^^^^^^^ - 4 │ console.log(a); - 5 │ }); + 6 │ console.log(a); + 7 │ }); i This dependency is not specified in the hook dependency list. - 2 │ let a = 1; - 3 │ useEffect(() => { - > 4 │ console.log(a); + 4 │ let a = 1; + 5 │ useEffect(() => { + > 6 │ console.log(a); │ ^ - 5 │ }); - 6 │ useMyEffect(() => { + 7 │ }); + 8 │ useMyEffect(() => { ``` ``` -customHook.js:6:5 lint/nursery/useExhaustiveDependencies ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ +customHook.js:8:5 lint/nursery/useExhaustiveDependencies ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ ! This hook do not specify all of its dependencies. - 4 │ console.log(a); - 5 │ }); - > 6 │ useMyEffect(() => { - │ ^^^^^^^^^^^ - 7 │ console.log(a); - 8 │ }); + 6 │ console.log(a); + 7 │ }); + > 8 │ useMyEffect(() => { + │ ^^^^^^^^^^^ + 9 │ console.log(a); + 10 │ }); i This dependency is not specified in the hook dependency list. - 5 │ }); - 6 │ useMyEffect(() => { - > 7 │ console.log(a); - │ ^ - 8 │ }); - 9 │ } + 7 │ }); + 8 │ useMyEffect(() => { + > 9 │ console.log(a); + │ ^ + 10 │ }); + 11 │ } ``` diff --git a/crates/rome_js_analyze/tests/specs/nursery/useExhaustiveDependencies/extraDependenciesInvalid.js b/crates/rome_js_analyze/tests/specs/nursery/useExhaustiveDependencies/extraDependenciesInvalid.js index 0f9a09185d3..7697dd20b82 100644 --- a/crates/rome_js_analyze/tests/specs/nursery/useExhaustiveDependencies/extraDependenciesInvalid.js +++ b/crates/rome_js_analyze/tests/specs/nursery/useExhaustiveDependencies/extraDependenciesInvalid.js @@ -1,3 +1,5 @@ +import { useEffect } from "react"; + function MyComponent() { let a = 1; useEffect(() => {}, [a]); @@ -26,4 +28,4 @@ function MyComponent1() { useEffect(() => { console.log(someObj) }, [someObj.id]); -} \ No newline at end of file +} diff --git a/crates/rome_js_analyze/tests/specs/nursery/useExhaustiveDependencies/extraDependenciesInvalid.js.snap b/crates/rome_js_analyze/tests/specs/nursery/useExhaustiveDependencies/extraDependenciesInvalid.js.snap index 546c47e6c80..fef188f942a 100644 --- a/crates/rome_js_analyze/tests/specs/nursery/useExhaustiveDependencies/extraDependenciesInvalid.js.snap +++ b/crates/rome_js_analyze/tests/specs/nursery/useExhaustiveDependencies/extraDependenciesInvalid.js.snap @@ -4,6 +4,8 @@ expression: extraDependenciesInvalid.js --- # Input ```js +import { useEffect } from "react"; + function MyComponent() { let a = 1; useEffect(() => {}, [a]); @@ -33,142 +35,144 @@ function MyComponent1() { console.log(someObj) }, [someObj.id]); } + ``` # Diagnostics ``` -extraDependenciesInvalid.js:3:3 lint/nursery/useExhaustiveDependencies ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ +extraDependenciesInvalid.js:5:3 lint/nursery/useExhaustiveDependencies ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ ! This hook specifies more dependencies than necessary. - 1 │ function MyComponent() { - 2 │ let a = 1; - > 3 │ useEffect(() => {}, [a]); + 3 │ function MyComponent() { + 4 │ let a = 1; + > 5 │ useEffect(() => {}, [a]); │ ^^^^^^^^^ - 4 │ } - 5 │ + 6 │ } + 7 │ i This dependency can be removed from the list. - 1 │ function MyComponent() { - 2 │ let a = 1; - > 3 │ useEffect(() => {}, [a]); + 3 │ function MyComponent() { + 4 │ let a = 1; + > 5 │ useEffect(() => {}, [a]); │ ^ - 4 │ } - 5 │ + 6 │ } + 7 │ ``` ``` -extraDependenciesInvalid.js:10:3 lint/nursery/useExhaustiveDependencies ━━━━━━━━━━━━━━━━━━━━━━━━━━━━ +extraDependenciesInvalid.js:12:3 lint/nursery/useExhaustiveDependencies ━━━━━━━━━━━━━━━━━━━━━━━━━━━━ ! This hook specifies more dependencies than necessary. - 8 │ function MyComponent2() { - 9 │ let a = 1, b = 1; - > 10 │ useEffect(() => {}, [a, b]); + 10 │ function MyComponent2() { + 11 │ let a = 1, b = 1; + > 12 │ useEffect(() => {}, [a, b]); │ ^^^^^^^^^ - 11 │ } - 12 │ + 13 │ } + 14 │ i This dependency can be removed from the list. - 8 │ function MyComponent2() { - 9 │ let a = 1, b = 1; - > 10 │ useEffect(() => {}, [a, b]); + 10 │ function MyComponent2() { + 11 │ let a = 1, b = 1; + > 12 │ useEffect(() => {}, [a, b]); │ ^ - 11 │ } - 12 │ + 13 │ } + 14 │ i This dependency can be removed from the list. - 8 │ function MyComponent2() { - 9 │ let a = 1, b = 1; - > 10 │ useEffect(() => {}, [a, b]); + 10 │ function MyComponent2() { + 11 │ let a = 1, b = 1; + > 12 │ useEffect(() => {}, [a, b]); │ ^ - 11 │ } - 12 │ + 13 │ } + 14 │ ``` ``` -extraDependenciesInvalid.js:17:3 lint/nursery/useExhaustiveDependencies ━━━━━━━━━━━━━━━━━━━━━━━━━━━━ +extraDependenciesInvalid.js:19:3 lint/nursery/useExhaustiveDependencies ━━━━━━━━━━━━━━━━━━━━━━━━━━━━ ! This hook specifies more dependencies than necessary. - 15 │ function MyComponent2() { - 16 │ const a = 1; - > 17 │ useEffect(() => {}, [a]); + 17 │ function MyComponent2() { + 18 │ const a = 1; + > 19 │ useEffect(() => {}, [a]); │ ^^^^^^^^^ - 18 │ } - 19 │ + 20 │ } + 21 │ i This dependency can be removed from the list. - 15 │ function MyComponent2() { - 16 │ const a = 1; - > 17 │ useEffect(() => {}, [a]); + 17 │ function MyComponent2() { + 18 │ const a = 1; + > 19 │ useEffect(() => {}, [a]); │ ^ - 18 │ } - 19 │ + 20 │ } + 21 │ ``` ``` -extraDependenciesInvalid.js:26:3 lint/nursery/useExhaustiveDependencies ━━━━━━━━━━━━━━━━━━━━━━━━━━━━ +extraDependenciesInvalid.js:28:3 lint/nursery/useExhaustiveDependencies ━━━━━━━━━━━━━━━━━━━━━━━━━━━━ ! This hook specifies a dependency more specific that its captures - 24 │ function MyComponent1() { - 25 │ let someObj = getObj(); - > 26 │ useEffect(() => { + 26 │ function MyComponent1() { + 27 │ let someObj = getObj(); + > 28 │ useEffect(() => { │ ^^^^^^^^^ - 27 │ console.log(someObj) - 28 │ }, [someObj.id]); + 29 │ console.log(someObj) + 30 │ }, [someObj.id]); i This capture is more generic than... - 25 │ let someObj = getObj(); - 26 │ useEffect(() => { - > 27 │ console.log(someObj) + 27 │ let someObj = getObj(); + 28 │ useEffect(() => { + > 29 │ console.log(someObj) │ ^^^^^^^ - 28 │ }, [someObj.id]); - 29 │ } + 30 │ }, [someObj.id]); + 31 │ } i ...this dependency. - 26 │ useEffect(() => { - 27 │ console.log(someObj) - > 28 │ }, [someObj.id]); + 28 │ useEffect(() => { + 29 │ console.log(someObj) + > 30 │ }, [someObj.id]); │ ^^^^^^^^^^ - 29 │ } + 31 │ } + 32 │ ``` ``` -extraDependenciesInvalid.js:26:3 lint/nursery/useExhaustiveDependencies ━━━━━━━━━━━━━━━━━━━━━━━━━━━━ +extraDependenciesInvalid.js:28:3 lint/nursery/useExhaustiveDependencies ━━━━━━━━━━━━━━━━━━━━━━━━━━━━ ! This hook do not specify all of its dependencies. - 24 │ function MyComponent1() { - 25 │ let someObj = getObj(); - > 26 │ useEffect(() => { + 26 │ function MyComponent1() { + 27 │ let someObj = getObj(); + > 28 │ useEffect(() => { │ ^^^^^^^^^ - 27 │ console.log(someObj) - 28 │ }, [someObj.id]); + 29 │ console.log(someObj) + 30 │ }, [someObj.id]); i This dependency is not specified in the hook dependency list. - 25 │ let someObj = getObj(); - 26 │ useEffect(() => { - > 27 │ console.log(someObj) + 27 │ let someObj = getObj(); + 28 │ useEffect(() => { + > 29 │ console.log(someObj) │ ^^^^^^^ - 28 │ }, [someObj.id]); - 29 │ } + 30 │ }, [someObj.id]); + 31 │ } ``` diff --git a/crates/rome_js_analyze/tests/specs/nursery/useExhaustiveDependencies/missingDependenciesInvalid.js b/crates/rome_js_analyze/tests/specs/nursery/useExhaustiveDependencies/missingDependenciesInvalid.js index 9df4210ba9f..f7b55df875d 100644 --- a/crates/rome_js_analyze/tests/specs/nursery/useExhaustiveDependencies/missingDependenciesInvalid.js +++ b/crates/rome_js_analyze/tests/specs/nursery/useExhaustiveDependencies/missingDependenciesInvalid.js @@ -1,10 +1,13 @@ +import React from "react"; +import { useEffect, useCallback, useMemo, useLayoutEffect, useInsertionEffect, useImperativeHandle } from "react"; + function MyComponent1() { let a = 1; const b = a + 1; useEffect(() => { console.log(a, b); }); - } +} // interaction with other react hooks @@ -82,3 +85,41 @@ const MyComponent8 = React.memo(({ a }) => { console.log(a); }); }); + +// exported functions +export function MyComponent9() { + let a = 1; + useEffect(() => { + console.log(a); + }); +} + +export default function MyComponent10() { + let a = 1; + useEffect(() => { + console.log(a); + }); +} + +// named function +function MyComponent11() { + let a = 1; + useEffect(function inner() { + console.log(a); + }); +} + +function MyComponent12() { + let a = 1; + useEffect(async function inner() { + console.log(a); + }); +} + +// React.useXXX case +function MyComponent13() { + let a = 1; + React.useEffect(() => { + console.log(a); + }); +} diff --git a/crates/rome_js_analyze/tests/specs/nursery/useExhaustiveDependencies/missingDependenciesInvalid.js.snap b/crates/rome_js_analyze/tests/specs/nursery/useExhaustiveDependencies/missingDependenciesInvalid.js.snap index 04249e37e47..c6507708054 100644 --- a/crates/rome_js_analyze/tests/specs/nursery/useExhaustiveDependencies/missingDependenciesInvalid.js.snap +++ b/crates/rome_js_analyze/tests/specs/nursery/useExhaustiveDependencies/missingDependenciesInvalid.js.snap @@ -4,13 +4,16 @@ expression: missingDependenciesInvalid.js --- # Input ```js +import React from "react"; +import { useEffect, useCallback, useMemo, useLayoutEffect, useInsertionEffect, useImperativeHandle } from "react"; + function MyComponent1() { let a = 1; const b = a + 1; useEffect(() => { console.log(a, b); }); - } +} // interaction with other react hooks @@ -89,467 +92,625 @@ const MyComponent8 = React.memo(({ a }) => { }); }); +// exported functions +export function MyComponent9() { + let a = 1; + useEffect(() => { + console.log(a); + }); +} + +export default function MyComponent10() { + let a = 1; + useEffect(() => { + console.log(a); + }); +} + +// named function +function MyComponent11() { + let a = 1; + useEffect(function inner() { + console.log(a); + }); +} + +function MyComponent12() { + let a = 1; + useEffect(async function inner() { + console.log(a); + }); +} + +// React.useXXX case +function MyComponent13() { + let a = 1; + React.useEffect(() => { + console.log(a); + }); +} + ``` # Diagnostics ``` -missingDependenciesInvalid.js:4:5 lint/nursery/useExhaustiveDependencies ━━━━━━━━━━━━━━━━━━━━━━━━━━━ +missingDependenciesInvalid.js:7:5 lint/nursery/useExhaustiveDependencies ━━━━━━━━━━━━━━━━━━━━━━━━━━━ ! This hook do not specify all of its dependencies. - 2 │ let a = 1; - 3 │ const b = a + 1; - > 4 │ useEffect(() => { + 5 │ let a = 1; + 6 │ const b = a + 1; + > 7 │ useEffect(() => { │ ^^^^^^^^^ - 5 │ console.log(a, b); - 6 │ }); + 8 │ console.log(a, b); + 9 │ }); i This dependency is not specified in the hook dependency list. - 3 │ const b = a + 1; - 4 │ useEffect(() => { - > 5 │ console.log(a, b); - │ ^ - 6 │ }); - 7 │ } + 6 │ const b = a + 1; + 7 │ useEffect(() => { + > 8 │ console.log(a, b); + │ ^ + 9 │ }); + 10 │ } ``` ``` -missingDependenciesInvalid.js:4:5 lint/nursery/useExhaustiveDependencies ━━━━━━━━━━━━━━━━━━━━━━━━━━━ +missingDependenciesInvalid.js:7:5 lint/nursery/useExhaustiveDependencies ━━━━━━━━━━━━━━━━━━━━━━━━━━━ ! This hook do not specify all of its dependencies. - 2 │ let a = 1; - 3 │ const b = a + 1; - > 4 │ useEffect(() => { + 5 │ let a = 1; + 6 │ const b = a + 1; + > 7 │ useEffect(() => { │ ^^^^^^^^^ - 5 │ console.log(a, b); - 6 │ }); + 8 │ console.log(a, b); + 9 │ }); i This dependency is not specified in the hook dependency list. - 3 │ const b = a + 1; - 4 │ useEffect(() => { - > 5 │ console.log(a, b); - │ ^ - 6 │ }); - 7 │ } + 6 │ const b = a + 1; + 7 │ useEffect(() => { + > 8 │ console.log(a, b); + │ ^ + 9 │ }); + 10 │ } ``` ``` -missingDependenciesInvalid.js:18:3 lint/nursery/useExhaustiveDependencies ━━━━━━━━━━━━━━━━━━━━━━━━━━ +missingDependenciesInvalid.js:21:3 lint/nursery/useExhaustiveDependencies ━━━━━━━━━━━━━━━━━━━━━━━━━━ ! This hook do not specify all of its dependencies. - 16 │ const deferredValue = useDeferredValue(value); - 17 │ const [isPending, startTransition] = useTransition(); - > 18 │ useEffect(() => { + 19 │ const deferredValue = useDeferredValue(value); + 20 │ const [isPending, startTransition] = useTransition(); + > 21 │ useEffect(() => { │ ^^^^^^^^^ - 19 │ console.log(name); - 20 │ setName(1); + 22 │ console.log(name); + 23 │ setName(1); i This dependency is not specified in the hook dependency list. - 25 │ console.log(memoizedCallback); - 26 │ console.log(memoizedValue); - > 27 │ console.log(deferredValue); + 28 │ console.log(memoizedCallback); + 29 │ console.log(memoizedValue); + > 30 │ console.log(deferredValue); │ ^^^^^^^^^^^^^ - 28 │ - 29 │ console.log(isPending); + 31 │ + 32 │ console.log(isPending); ``` ``` -missingDependenciesInvalid.js:18:3 lint/nursery/useExhaustiveDependencies ━━━━━━━━━━━━━━━━━━━━━━━━━━ +missingDependenciesInvalid.js:21:3 lint/nursery/useExhaustiveDependencies ━━━━━━━━━━━━━━━━━━━━━━━━━━ ! This hook do not specify all of its dependencies. - 16 │ const deferredValue = useDeferredValue(value); - 17 │ const [isPending, startTransition] = useTransition(); - > 18 │ useEffect(() => { + 19 │ const deferredValue = useDeferredValue(value); + 20 │ const [isPending, startTransition] = useTransition(); + > 21 │ useEffect(() => { │ ^^^^^^^^^ - 19 │ console.log(name); - 20 │ setName(1); + 22 │ console.log(name); + 23 │ setName(1); i This dependency is not specified in the hook dependency list. - 23 │ dispatch(1); - 24 │ - > 25 │ console.log(memoizedCallback); + 26 │ dispatch(1); + 27 │ + > 28 │ console.log(memoizedCallback); │ ^^^^^^^^^^^^^^^^ - 26 │ console.log(memoizedValue); - 27 │ console.log(deferredValue); + 29 │ console.log(memoizedValue); + 30 │ console.log(deferredValue); ``` ``` -missingDependenciesInvalid.js:18:3 lint/nursery/useExhaustiveDependencies ━━━━━━━━━━━━━━━━━━━━━━━━━━ +missingDependenciesInvalid.js:21:3 lint/nursery/useExhaustiveDependencies ━━━━━━━━━━━━━━━━━━━━━━━━━━ ! This hook do not specify all of its dependencies. - 16 │ const deferredValue = useDeferredValue(value); - 17 │ const [isPending, startTransition] = useTransition(); - > 18 │ useEffect(() => { + 19 │ const deferredValue = useDeferredValue(value); + 20 │ const [isPending, startTransition] = useTransition(); + > 21 │ useEffect(() => { │ ^^^^^^^^^ - 19 │ console.log(name); - 20 │ setName(1); + 22 │ console.log(name); + 23 │ setName(1); i This dependency is not specified in the hook dependency list. - 20 │ setName(1); - 21 │ - > 22 │ console.log(state); - │ ^^^^^ - 23 │ dispatch(1); + 23 │ setName(1); 24 │ + > 25 │ console.log(state); + │ ^^^^^ + 26 │ dispatch(1); + 27 │ ``` ``` -missingDependenciesInvalid.js:18:3 lint/nursery/useExhaustiveDependencies ━━━━━━━━━━━━━━━━━━━━━━━━━━ +missingDependenciesInvalid.js:21:3 lint/nursery/useExhaustiveDependencies ━━━━━━━━━━━━━━━━━━━━━━━━━━ ! This hook do not specify all of its dependencies. - 16 │ const deferredValue = useDeferredValue(value); - 17 │ const [isPending, startTransition] = useTransition(); - > 18 │ useEffect(() => { + 19 │ const deferredValue = useDeferredValue(value); + 20 │ const [isPending, startTransition] = useTransition(); + > 21 │ useEffect(() => { │ ^^^^^^^^^ - 19 │ console.log(name); - 20 │ setName(1); + 22 │ console.log(name); + 23 │ setName(1); i This dependency is not specified in the hook dependency list. - 17 │ const [isPending, startTransition] = useTransition(); - 18 │ useEffect(() => { - > 19 │ console.log(name); + 20 │ const [isPending, startTransition] = useTransition(); + 21 │ useEffect(() => { + > 22 │ console.log(name); │ ^^^^ - 20 │ setName(1); - 21 │ + 23 │ setName(1); + 24 │ ``` ``` -missingDependenciesInvalid.js:18:3 lint/nursery/useExhaustiveDependencies ━━━━━━━━━━━━━━━━━━━━━━━━━━ +missingDependenciesInvalid.js:21:3 lint/nursery/useExhaustiveDependencies ━━━━━━━━━━━━━━━━━━━━━━━━━━ ! This hook do not specify all of its dependencies. - 16 │ const deferredValue = useDeferredValue(value); - 17 │ const [isPending, startTransition] = useTransition(); - > 18 │ useEffect(() => { + 19 │ const deferredValue = useDeferredValue(value); + 20 │ const [isPending, startTransition] = useTransition(); + > 21 │ useEffect(() => { │ ^^^^^^^^^ - 19 │ console.log(name); - 20 │ setName(1); + 22 │ console.log(name); + 23 │ setName(1); i This dependency is not specified in the hook dependency list. - 27 │ console.log(deferredValue); - 28 │ - > 29 │ console.log(isPending); + 30 │ console.log(deferredValue); + 31 │ + > 32 │ console.log(isPending); │ ^^^^^^^^^ - 30 │ startTransition(); - 31 │ }, []); + 33 │ startTransition(); + 34 │ }, []); ``` ``` -missingDependenciesInvalid.js:18:3 lint/nursery/useExhaustiveDependencies ━━━━━━━━━━━━━━━━━━━━━━━━━━ +missingDependenciesInvalid.js:21:3 lint/nursery/useExhaustiveDependencies ━━━━━━━━━━━━━━━━━━━━━━━━━━ ! This hook do not specify all of its dependencies. - 16 │ const deferredValue = useDeferredValue(value); - 17 │ const [isPending, startTransition] = useTransition(); - > 18 │ useEffect(() => { + 19 │ const deferredValue = useDeferredValue(value); + 20 │ const [isPending, startTransition] = useTransition(); + > 21 │ useEffect(() => { │ ^^^^^^^^^ - 19 │ console.log(name); - 20 │ setName(1); + 22 │ console.log(name); + 23 │ setName(1); i This dependency is not specified in the hook dependency list. - 25 │ console.log(memoizedCallback); - > 26 │ console.log(memoizedValue); + 28 │ console.log(memoizedCallback); + > 29 │ console.log(memoizedValue); │ ^^^^^^^^^^^^^ - 27 │ console.log(deferredValue); - 28 │ + 30 │ console.log(deferredValue); + 31 │ ``` ``` -missingDependenciesInvalid.js:38:3 lint/nursery/useExhaustiveDependencies ━━━━━━━━━━━━━━━━━━━━━━━━━━ +missingDependenciesInvalid.js:41:3 lint/nursery/useExhaustiveDependencies ━━━━━━━━━━━━━━━━━━━━━━━━━━ ! This hook do not specify all of its dependencies. - 36 │ function MyComponent3() { - 37 │ let a = 1; - > 38 │ useEffect(() => console.log(a)); + 39 │ function MyComponent3() { + 40 │ let a = 1; + > 41 │ useEffect(() => console.log(a)); │ ^^^^^^^^^ - 39 │ useCallback(() => console.log(a)); - 40 │ useMemo(() => console.log(a)); + 42 │ useCallback(() => console.log(a)); + 43 │ useMemo(() => console.log(a)); i This dependency is not specified in the hook dependency list. - 36 │ function MyComponent3() { - 37 │ let a = 1; - > 38 │ useEffect(() => console.log(a)); + 39 │ function MyComponent3() { + 40 │ let a = 1; + > 41 │ useEffect(() => console.log(a)); │ ^ - 39 │ useCallback(() => console.log(a)); - 40 │ useMemo(() => console.log(a)); + 42 │ useCallback(() => console.log(a)); + 43 │ useMemo(() => console.log(a)); ``` ``` -missingDependenciesInvalid.js:39:3 lint/nursery/useExhaustiveDependencies ━━━━━━━━━━━━━━━━━━━━━━━━━━ +missingDependenciesInvalid.js:42:3 lint/nursery/useExhaustiveDependencies ━━━━━━━━━━━━━━━━━━━━━━━━━━ ! This hook do not specify all of its dependencies. - 37 │ let a = 1; - 38 │ useEffect(() => console.log(a)); - > 39 │ useCallback(() => console.log(a)); + 40 │ let a = 1; + 41 │ useEffect(() => console.log(a)); + > 42 │ useCallback(() => console.log(a)); │ ^^^^^^^^^^^ - 40 │ useMemo(() => console.log(a)); - 41 │ useImperativeHandle(ref, () => console.log(a)); + 43 │ useMemo(() => console.log(a)); + 44 │ useImperativeHandle(ref, () => console.log(a)); i This dependency is not specified in the hook dependency list. - 37 │ let a = 1; - 38 │ useEffect(() => console.log(a)); - > 39 │ useCallback(() => console.log(a)); + 40 │ let a = 1; + 41 │ useEffect(() => console.log(a)); + > 42 │ useCallback(() => console.log(a)); │ ^ - 40 │ useMemo(() => console.log(a)); - 41 │ useImperativeHandle(ref, () => console.log(a)); + 43 │ useMemo(() => console.log(a)); + 44 │ useImperativeHandle(ref, () => console.log(a)); ``` ``` -missingDependenciesInvalid.js:40:3 lint/nursery/useExhaustiveDependencies ━━━━━━━━━━━━━━━━━━━━━━━━━━ +missingDependenciesInvalid.js:43:3 lint/nursery/useExhaustiveDependencies ━━━━━━━━━━━━━━━━━━━━━━━━━━ ! This hook do not specify all of its dependencies. - 38 │ useEffect(() => console.log(a)); - 39 │ useCallback(() => console.log(a)); - > 40 │ useMemo(() => console.log(a)); + 41 │ useEffect(() => console.log(a)); + 42 │ useCallback(() => console.log(a)); + > 43 │ useMemo(() => console.log(a)); │ ^^^^^^^ - 41 │ useImperativeHandle(ref, () => console.log(a)); - 42 │ useLayoutEffect(() => console.log(a)); + 44 │ useImperativeHandle(ref, () => console.log(a)); + 45 │ useLayoutEffect(() => console.log(a)); i This dependency is not specified in the hook dependency list. - 38 │ useEffect(() => console.log(a)); - 39 │ useCallback(() => console.log(a)); - > 40 │ useMemo(() => console.log(a)); + 41 │ useEffect(() => console.log(a)); + 42 │ useCallback(() => console.log(a)); + > 43 │ useMemo(() => console.log(a)); │ ^ - 41 │ useImperativeHandle(ref, () => console.log(a)); - 42 │ useLayoutEffect(() => console.log(a)); + 44 │ useImperativeHandle(ref, () => console.log(a)); + 45 │ useLayoutEffect(() => console.log(a)); ``` ``` -missingDependenciesInvalid.js:41:3 lint/nursery/useExhaustiveDependencies ━━━━━━━━━━━━━━━━━━━━━━━━━━ +missingDependenciesInvalid.js:44:3 lint/nursery/useExhaustiveDependencies ━━━━━━━━━━━━━━━━━━━━━━━━━━ ! This hook do not specify all of its dependencies. - 39 │ useCallback(() => console.log(a)); - 40 │ useMemo(() => console.log(a)); - > 41 │ useImperativeHandle(ref, () => console.log(a)); + 42 │ useCallback(() => console.log(a)); + 43 │ useMemo(() => console.log(a)); + > 44 │ useImperativeHandle(ref, () => console.log(a)); │ ^^^^^^^^^^^^^^^^^^^ - 42 │ useLayoutEffect(() => console.log(a)); - 43 │ useInsertionEffect(() => console.log(a)); + 45 │ useLayoutEffect(() => console.log(a)); + 46 │ useInsertionEffect(() => console.log(a)); i This dependency is not specified in the hook dependency list. - 39 │ useCallback(() => console.log(a)); - 40 │ useMemo(() => console.log(a)); - > 41 │ useImperativeHandle(ref, () => console.log(a)); + 42 │ useCallback(() => console.log(a)); + 43 │ useMemo(() => console.log(a)); + > 44 │ useImperativeHandle(ref, () => console.log(a)); │ ^ - 42 │ useLayoutEffect(() => console.log(a)); - 43 │ useInsertionEffect(() => console.log(a)); + 45 │ useLayoutEffect(() => console.log(a)); + 46 │ useInsertionEffect(() => console.log(a)); ``` ``` -missingDependenciesInvalid.js:42:3 lint/nursery/useExhaustiveDependencies ━━━━━━━━━━━━━━━━━━━━━━━━━━ +missingDependenciesInvalid.js:45:3 lint/nursery/useExhaustiveDependencies ━━━━━━━━━━━━━━━━━━━━━━━━━━ ! This hook do not specify all of its dependencies. - 40 │ useMemo(() => console.log(a)); - 41 │ useImperativeHandle(ref, () => console.log(a)); - > 42 │ useLayoutEffect(() => console.log(a)); + 43 │ useMemo(() => console.log(a)); + 44 │ useImperativeHandle(ref, () => console.log(a)); + > 45 │ useLayoutEffect(() => console.log(a)); │ ^^^^^^^^^^^^^^^ - 43 │ useInsertionEffect(() => console.log(a)); - 44 │ } + 46 │ useInsertionEffect(() => console.log(a)); + 47 │ } i This dependency is not specified in the hook dependency list. - 40 │ useMemo(() => console.log(a)); - 41 │ useImperativeHandle(ref, () => console.log(a)); - > 42 │ useLayoutEffect(() => console.log(a)); + 43 │ useMemo(() => console.log(a)); + 44 │ useImperativeHandle(ref, () => console.log(a)); + > 45 │ useLayoutEffect(() => console.log(a)); │ ^ - 43 │ useInsertionEffect(() => console.log(a)); - 44 │ } + 46 │ useInsertionEffect(() => console.log(a)); + 47 │ } ``` ``` -missingDependenciesInvalid.js:43:3 lint/nursery/useExhaustiveDependencies ━━━━━━━━━━━━━━━━━━━━━━━━━━ +missingDependenciesInvalid.js:46:3 lint/nursery/useExhaustiveDependencies ━━━━━━━━━━━━━━━━━━━━━━━━━━ ! This hook do not specify all of its dependencies. - 41 │ useImperativeHandle(ref, () => console.log(a)); - 42 │ useLayoutEffect(() => console.log(a)); - > 43 │ useInsertionEffect(() => console.log(a)); + 44 │ useImperativeHandle(ref, () => console.log(a)); + 45 │ useLayoutEffect(() => console.log(a)); + > 46 │ useInsertionEffect(() => console.log(a)); │ ^^^^^^^^^^^^^^^^^^ - 44 │ } - 45 │ + 47 │ } + 48 │ i This dependency is not specified in the hook dependency list. - 41 │ useImperativeHandle(ref, () => console.log(a)); - 42 │ useLayoutEffect(() => console.log(a)); - > 43 │ useInsertionEffect(() => console.log(a)); + 44 │ useImperativeHandle(ref, () => console.log(a)); + 45 │ useLayoutEffect(() => console.log(a)); + > 46 │ useInsertionEffect(() => console.log(a)); │ ^ - 44 │ } - 45 │ + 47 │ } + 48 │ ``` ``` -missingDependenciesInvalid.js:50:3 lint/nursery/useExhaustiveDependencies ━━━━━━━━━━━━━━━━━━━━━━━━━━ +missingDependenciesInvalid.js:53:3 lint/nursery/useExhaustiveDependencies ━━━━━━━━━━━━━━━━━━━━━━━━━━ ! This hook do not specify all of its dependencies. - 48 │ function MyComponent4() { - 49 │ let a = 1; - > 50 │ useEffect(() => { + 51 │ function MyComponent4() { + 52 │ let a = 1; + > 53 │ useEffect(() => { │ ^^^^^^^^^ - 51 │ return () => console.log(a) - 52 │ }, []); + 54 │ return () => console.log(a) + 55 │ }, []); i This dependency is not specified in the hook dependency list. - 49 │ let a = 1; - 50 │ useEffect(() => { - > 51 │ return () => console.log(a) + 52 │ let a = 1; + 53 │ useEffect(() => { + > 54 │ return () => console.log(a) │ ^ - 52 │ }, []); - 53 │ } + 55 │ }, []); + 56 │ } ``` ``` -missingDependenciesInvalid.js:59:3 lint/nursery/useExhaustiveDependencies ━━━━━━━━━━━━━━━━━━━━━━━━━━ +missingDependenciesInvalid.js:62:3 lint/nursery/useExhaustiveDependencies ━━━━━━━━━━━━━━━━━━━━━━━━━━ ! This hook do not specify all of its dependencies. - 57 │ function MyComponent5() { - 58 │ let a = 1; - > 59 │ useEffect(() => { + 60 │ function MyComponent5() { + 61 │ let a = 1; + > 62 │ useEffect(() => { │ ^^^^^^^^^ - 60 │ console.log(a); - 61 │ return () => console.log(a); + 63 │ console.log(a); + 64 │ return () => console.log(a); i This dependency is not specified in the hook dependency list. - 58 │ let a = 1; - 59 │ useEffect(() => { - > 60 │ console.log(a); + 61 │ let a = 1; + 62 │ useEffect(() => { + > 63 │ console.log(a); │ ^ - 61 │ return () => console.log(a); - 62 │ }, []); + 64 │ return () => console.log(a); + 65 │ }, []); i This dependency is not specified in the hook dependency list. - 59 │ useEffect(() => { - 60 │ console.log(a); - > 61 │ return () => console.log(a); + 62 │ useEffect(() => { + 63 │ console.log(a); + > 64 │ return () => console.log(a); │ ^ - 62 │ }, []); - 63 │ } + 65 │ }, []); + 66 │ } ``` ``` -missingDependenciesInvalid.js:69:3 lint/nursery/useExhaustiveDependencies ━━━━━━━━━━━━━━━━━━━━━━━━━━ +missingDependenciesInvalid.js:72:3 lint/nursery/useExhaustiveDependencies ━━━━━━━━━━━━━━━━━━━━━━━━━━ ! This hook do not specify all of its dependencies. - 67 │ function MyComponent6() { - 68 │ let someObj = getObj(); - > 69 │ useEffect(() => { + 70 │ function MyComponent6() { + 71 │ let someObj = getObj(); + > 72 │ useEffect(() => { │ ^^^^^^^^^ - 70 │ console.log(someObj.name) - 71 │ }); + 73 │ console.log(someObj.name) + 74 │ }); i This dependency is not specified in the hook dependency list. - 68 │ let someObj = getObj(); - 69 │ useEffect(() => { - > 70 │ console.log(someObj.name) + 71 │ let someObj = getObj(); + 72 │ useEffect(() => { + > 73 │ console.log(someObj.name) │ ^^^^^^^^^^^^ - 71 │ }); - 72 │ } + 74 │ }); + 75 │ } ``` ``` -missingDependenciesInvalid.js:75:3 lint/nursery/useExhaustiveDependencies ━━━━━━━━━━━━━━━━━━━━━━━━━━ +missingDependenciesInvalid.js:78:3 lint/nursery/useExhaustiveDependencies ━━━━━━━━━━━━━━━━━━━━━━━━━━ ! This hook do not specify all of its dependencies. - 74 │ const MyComponent7 = React.memo(function ({ a }) { - > 75 │ useEffect(() => { + 77 │ const MyComponent7 = React.memo(function ({ a }) { + > 78 │ useEffect(() => { │ ^^^^^^^^^ - 76 │ console.log(a); - 77 │ }); + 79 │ console.log(a); + 80 │ }); i This dependency is not specified in the hook dependency list. - 74 │ const MyComponent7 = React.memo(function ({ a }) { - 75 │ useEffect(() => { - > 76 │ console.log(a); + 77 │ const MyComponent7 = React.memo(function ({ a }) { + 78 │ useEffect(() => { + > 79 │ console.log(a); │ ^ - 77 │ }); - 78 │ }); + 80 │ }); + 81 │ }); ``` ``` -missingDependenciesInvalid.js:81:3 lint/nursery/useExhaustiveDependencies ━━━━━━━━━━━━━━━━━━━━━━━━━━ +missingDependenciesInvalid.js:84:3 lint/nursery/useExhaustiveDependencies ━━━━━━━━━━━━━━━━━━━━━━━━━━ ! This hook do not specify all of its dependencies. - 80 │ const MyComponent8 = React.memo(({ a }) => { - > 81 │ useEffect(() => { + 83 │ const MyComponent8 = React.memo(({ a }) => { + > 84 │ useEffect(() => { │ ^^^^^^^^^ - 82 │ console.log(a); - 83 │ }); + 85 │ console.log(a); + 86 │ }); i This dependency is not specified in the hook dependency list. - 80 │ const MyComponent8 = React.memo(({ a }) => { - 81 │ useEffect(() => { - > 82 │ console.log(a); + 83 │ const MyComponent8 = React.memo(({ a }) => { + 84 │ useEffect(() => { + > 85 │ console.log(a); │ ^ - 83 │ }); - 84 │ }); + 86 │ }); + 87 │ }); + + +``` + +``` +missingDependenciesInvalid.js:92:3 lint/nursery/useExhaustiveDependencies ━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! This hook do not specify all of its dependencies. + + 90 │ export function MyComponent9() { + 91 │ let a = 1; + > 92 │ useEffect(() => { + │ ^^^^^^^^^ + 93 │ console.log(a); + 94 │ }); + + i This dependency is not specified in the hook dependency list. + + 91 │ let a = 1; + 92 │ useEffect(() => { + > 93 │ console.log(a); + │ ^ + 94 │ }); + 95 │ } + + +``` + +``` +missingDependenciesInvalid.js:99:3 lint/nursery/useExhaustiveDependencies ━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! This hook do not specify all of its dependencies. + + 97 │ export default function MyComponent10() { + 98 │ let a = 1; + > 99 │ useEffect(() => { + │ ^^^^^^^^^ + 100 │ console.log(a); + 101 │ }); + + i This dependency is not specified in the hook dependency list. + + 98 │ let a = 1; + 99 │ useEffect(() => { + > 100 │ console.log(a); + │ ^ + 101 │ }); + 102 │ } + + +``` + +``` +missingDependenciesInvalid.js:107:3 lint/nursery/useExhaustiveDependencies ━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! This hook do not specify all of its dependencies. + + 105 │ function MyComponent11() { + 106 │ let a = 1; + > 107 │ useEffect(function inner() { + │ ^^^^^^^^^ + 108 │ console.log(a); + 109 │ }); + + i This dependency is not specified in the hook dependency list. + + 106 │ let a = 1; + 107 │ useEffect(function inner() { + > 108 │ console.log(a); + │ ^ + 109 │ }); + 110 │ } + + +``` + +``` +missingDependenciesInvalid.js:114:3 lint/nursery/useExhaustiveDependencies ━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! This hook do not specify all of its dependencies. + + 112 │ function MyComponent12() { + 113 │ let a = 1; + > 114 │ useEffect(async function inner() { + │ ^^^^^^^^^ + 115 │ console.log(a); + 116 │ }); + + i This dependency is not specified in the hook dependency list. + + 113 │ let a = 1; + 114 │ useEffect(async function inner() { + > 115 │ console.log(a); + │ ^ + 116 │ }); + 117 │ } + + +``` + +``` +missingDependenciesInvalid.js:122:9 lint/nursery/useExhaustiveDependencies ━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! This hook do not specify all of its dependencies. + + 120 │ function MyComponent13() { + 121 │ let a = 1; + > 122 │ React.useEffect(() => { + │ ^^^^^^^^^ + 123 │ console.log(a); + 124 │ }); + + i This dependency is not specified in the hook dependency list. + + 121 │ let a = 1; + 122 │ React.useEffect(() => { + > 123 │ console.log(a); + │ ^ + 124 │ }); + 125 │ } ``` diff --git a/crates/rome_js_analyze/tests/specs/nursery/useExhaustiveDependencies/valid.js b/crates/rome_js_analyze/tests/specs/nursery/useExhaustiveDependencies/valid.js index 60ae2eb6687..5c84dd76315 100644 --- a/crates/rome_js_analyze/tests/specs/nursery/useExhaustiveDependencies/valid.js +++ b/crates/rome_js_analyze/tests/specs/nursery/useExhaustiveDependencies/valid.js @@ -1,5 +1,7 @@ /* should not generate diagnostics */ +import React from "react"; +import { useEffect } from "react"; import doSomething from 'a'; // No captures @@ -127,3 +129,41 @@ const MyComponent11 = React.memo(() => { console.log(outside); }); }); + +// exported functions +export function MyComponent12() { + let a = 1; + useEffect(() => { + console.log(a); + }, [a]); +} + +export default function MyComponent13() { + let a = 1; + useEffect(() => { + console.log(a); + }, [a]); +} + +// named function +function MyComponent14() { + let a = 1; + useEffect(function inner() { + console.log(a); + }, [a]); +} + +function MyComponent15() { + let a = 1; + useEffect(async function inner() { + console.log(a); + }, [a]); +} + +// React.useXXX case +function MyComponent16() { + let a = 1; + React.useEffect(() => { + console.log(a); + }, [a]); +} diff --git a/crates/rome_js_analyze/tests/specs/nursery/useExhaustiveDependencies/valid.js.snap b/crates/rome_js_analyze/tests/specs/nursery/useExhaustiveDependencies/valid.js.snap index 741f155697e..79d246e9ec5 100644 --- a/crates/rome_js_analyze/tests/specs/nursery/useExhaustiveDependencies/valid.js.snap +++ b/crates/rome_js_analyze/tests/specs/nursery/useExhaustiveDependencies/valid.js.snap @@ -6,6 +6,8 @@ expression: valid.js ```js /* should not generate diagnostics */ +import React from "react"; +import { useEffect } from "react"; import doSomething from 'a'; // No captures @@ -134,6 +136,44 @@ const MyComponent11 = React.memo(() => { }); }); +// exported functions +export function MyComponent12() { + let a = 1; + useEffect(() => { + console.log(a); + }, [a]); +} + +export default function MyComponent13() { + let a = 1; + useEffect(() => { + console.log(a); + }, [a]); +} + +// named function +function MyComponent14() { + let a = 1; + useEffect(function inner() { + console.log(a); + }, [a]); +} + +function MyComponent15() { + let a = 1; + useEffect(async function inner() { + console.log(a); + }, [a]); +} + +// React.useXXX case +function MyComponent16() { + let a = 1; + React.useEffect(() => { + console.log(a); + }, [a]); +} + ``` diff --git a/crates/rome_js_analyze/tests/specs/nursery/useExhaustiveDependencies/valid.ts b/crates/rome_js_analyze/tests/specs/nursery/useExhaustiveDependencies/valid.ts index 8d0690c8ef9..d6c0163da19 100644 --- a/crates/rome_js_analyze/tests/specs/nursery/useExhaustiveDependencies/valid.ts +++ b/crates/rome_js_analyze/tests/specs/nursery/useExhaustiveDependencies/valid.ts @@ -1,6 +1,6 @@ /* should not generate diagnostics */ -import useEffect from 'react'; +import { useEffect } from "react"; // capturing declarations function overloaded(s: string): string; @@ -12,7 +12,7 @@ enum A { B = 1 } abstract class C { static D = 1 } class D { constructor() { - + } } @@ -28,4 +28,13 @@ function MyComponent() { console.log(new D()); console.log(M.f()); }, []); -} \ No newline at end of file +} + +// Capturing an object property with optional chaining +export function MyComponent2() { + let selectedArticle: { redirectUrl: string } | undefined; + + useEffect(() => { + if (selectedArticle?.redirectUrl) {} + }, [selectedArticle?.redirectUrl]); +} diff --git a/crates/rome_js_analyze/tests/specs/nursery/useExhaustiveDependencies/valid.ts.snap b/crates/rome_js_analyze/tests/specs/nursery/useExhaustiveDependencies/valid.ts.snap index b5977ca1d31..9c1f071d234 100644 --- a/crates/rome_js_analyze/tests/specs/nursery/useExhaustiveDependencies/valid.ts.snap +++ b/crates/rome_js_analyze/tests/specs/nursery/useExhaustiveDependencies/valid.ts.snap @@ -6,7 +6,7 @@ expression: valid.ts ```js /* should not generate diagnostics */ -import useEffect from 'react'; +import { useEffect } from "react"; // capturing declarations function overloaded(s: string): string; @@ -18,7 +18,7 @@ enum A { B = 1 } abstract class C { static D = 1 } class D { constructor() { - + } } @@ -35,6 +35,16 @@ function MyComponent() { console.log(M.f()); }, []); } + +// Capturing an object property with optional chaining +export function MyComponent2() { + let selectedArticle: { redirectUrl: string } | undefined; + + useEffect(() => { + if (selectedArticle?.redirectUrl) {} + }, [selectedArticle?.redirectUrl]); +} + ``` diff --git a/website/src/pages/lint/rules/useExhaustiveDependencies.md b/website/src/pages/lint/rules/useExhaustiveDependencies.md index 1ac70a86188..323c8aa7f16 100644 --- a/website/src/pages/lint/rules/useExhaustiveDependencies.md +++ b/website/src/pages/lint/rules/useExhaustiveDependencies.md @@ -12,6 +12,8 @@ Enforce all dependencies are correctly specified. ### Invalid ```jsx +import { useEffect } from "react"; + function component() { let a = 1; useEffect(() => { @@ -20,29 +22,31 @@ function component() { } ``` -
nursery/useExhaustiveDependencies.js:3:5 lint/nursery/useExhaustiveDependencies ━━━━━━━━━━━━━━━━━━━━
+
nursery/useExhaustiveDependencies.js:5:5 lint/nursery/useExhaustiveDependencies ━━━━━━━━━━━━━━━━━━━━
 
   ✖ This hook do not specify all of its dependencies.
   
-    1 │ function component() {
-    2 │     let a = 1;
-  > 3 │     useEffect(() => {
+    3 │ function component() {
+    4 │     let a = 1;
+  > 5 │     useEffect(() => {
       │     ^^^^^^^^^
-    4 │         console.log(a);
-    5 │     });
+    6 │         console.log(a);
+    7 │     });
   
   ℹ This dependency is not specified in the hook dependency list.
   
-    2 │     let a = 1;
-    3 │     useEffect(() => {
-  > 4 │         console.log(a);
+    4 │     let a = 1;
+    5 │     useEffect(() => {
+  > 6 │         console.log(a);
       │                     ^
-    5 │     });
-    6 │ }
+    7 │     });
+    8 │ }
   
 

 
 ```jsx
+import { useEffect } from "react";
+
 function component() {
     let b = 1;
     useEffect(() => {
@@ -50,29 +54,31 @@ function component() {
 }
 ```
 
-
nursery/useExhaustiveDependencies.js:3:5 lint/nursery/useExhaustiveDependencies ━━━━━━━━━━━━━━━━━━━━
+
nursery/useExhaustiveDependencies.js:5:5 lint/nursery/useExhaustiveDependencies ━━━━━━━━━━━━━━━━━━━━
 
   ✖ This hook specifies more dependencies than necessary.
   
-    1 │ function component() {
-    2 │     let b = 1;
-  > 3 │     useEffect(() => {
+    3 │ function component() {
+    4 │     let b = 1;
+  > 5 │     useEffect(() => {
       │     ^^^^^^^^^
-    4 │     }, [b]);
-    5 │ }
+    6 │     }, [b]);
+    7 │ }
   
   ℹ This dependency can be removed from the list.
   
-    2 │     let b = 1;
-    3 │     useEffect(() => {
-  > 4 │     }, [b]);
+    4 │     let b = 1;
+    5 │     useEffect(() => {
+  > 6 │     }, [b]);
       │         ^
-    5 │ }
-    6 │ 
+    7 │ }
+    8 │ 
   
 

 
 ```jsx
+import { useEffect, useState } from "react";
+
 function component() {
     const [name, setName] = useState();
     useEffect(() => {
@@ -82,29 +88,31 @@ function component() {
 }
 ```
 
-
nursery/useExhaustiveDependencies.js:3:5 lint/nursery/useExhaustiveDependencies ━━━━━━━━━━━━━━━━━━━━
+
nursery/useExhaustiveDependencies.js:5:5 lint/nursery/useExhaustiveDependencies ━━━━━━━━━━━━━━━━━━━━
 
   ✖ This hook specifies more dependencies than necessary.
   
-    1 │ function component() {
-    2 │     const [name, setName] = useState();
-  > 3 │     useEffect(() => {
+    3 │ function component() {
+    4 │     const [name, setName] = useState();
+  > 5 │     useEffect(() => {
       │     ^^^^^^^^^
-    4 │         console.log(name);
-    5 │         setName("");
+    6 │         console.log(name);
+    7 │         setName("");
   
   ℹ This dependency can be removed from the list.
   
-    4 │         console.log(name);
-    5 │         setName("");
-  > 6 │     }, [name, setName]);
-      │               ^^^^^^^
-    7 │ }
-    8 │ 
+     6 │         console.log(name);
+     7 │         setName("");
+   > 8 │     }, [name, setName]);
+       │               ^^^^^^^
+     9 │ }
+    10 │ 
   
 

 
 ```jsx
+import { useEffect } from "react";
+
 function component() {
     let a = 1;
     const b = a + 1;
@@ -114,31 +122,33 @@ function component() {
 }
 ```
 
-
nursery/useExhaustiveDependencies.js:4:5 lint/nursery/useExhaustiveDependencies ━━━━━━━━━━━━━━━━━━━━
+
nursery/useExhaustiveDependencies.js:6:5 lint/nursery/useExhaustiveDependencies ━━━━━━━━━━━━━━━━━━━━
 
   ✖ This hook do not specify all of its dependencies.
   
-    2 │     let a = 1;
-    3 │     const b = a + 1;
-  > 4 │     useEffect(() => {
+    4 │     let a = 1;
+    5 │     const b = a + 1;
+  > 6 │     useEffect(() => {
       │     ^^^^^^^^^
-    5 │         console.log(b);
-    6 │     });
+    7 │         console.log(b);
+    8 │     });
   
   ℹ This dependency is not specified in the hook dependency list.
   
-    3 │     const b = a + 1;
-    4 │     useEffect(() => {
-  > 5 │         console.log(b);
+    5 │     const b = a + 1;
+    6 │     useEffect(() => {
+  > 7 │         console.log(b);
       │                     ^
-    6 │     });
-    7 │ }
+    8 │     });
+    9 │ }
   
 

 
 ## Valid
 
 ```jsx
+import { useEffect } from "react";
+
 function component() {
     let a = 1;
     useEffect(() => {
@@ -148,6 +158,8 @@ function component() {
 ```
 
 ```jsx
+import { useEffect } from "react";
+
 function component() {
     const a = 1;
     useEffect(() => {
@@ -157,6 +169,8 @@ function component() {
 ```
 
 ```jsx
+import { useEffect } from "react";
+
 function component() {
     const [name, setName] = useState();
     useEffect(() => {
diff --git a/website/src/playground/Playground.tsx b/website/src/playground/Playground.tsx
index 1f65b5d8b28..9dd096c8243 100644
--- a/website/src/playground/Playground.tsx
+++ b/website/src/playground/Playground.tsx
@@ -69,19 +69,22 @@ export default function PlaygroundLoader({
 		if (clipboardStatus !== "normal") {
 			setClipboardStatus("normal");
 		}
-	}, [romeOutput.formatter.ir]);
+	}, [clipboardStatus]);
 
-	const onUpdate = useCallback((viewUpdate: ViewUpdate) => {
-		const cursorPosition = viewUpdate.state.selection.ranges[0]?.from ?? 0;
-		setPlaygroundState((state) =>
-			state.cursorPosition !== cursorPosition
-				? {
-						...state,
-						cursorPosition,
-				  }
-				: state,
-		);
-	}, []);
+	const onUpdate = useCallback(
+		(viewUpdate: ViewUpdate) => {
+			const cursorPosition = viewUpdate.state.selection.ranges[0]?.from ?? 0;
+			setPlaygroundState((state) =>
+				state.cursorPosition !== cursorPosition
+					? {
+							...state,
+							cursorPosition,
+					  }
+					: state,
+			);
+		},
+		[setPlaygroundState],
+	);
 
 	useEffect(() => {
 		scrollAstNodeIntoView(playgroundState.cursorPosition);
@@ -125,18 +128,21 @@ export default function PlaygroundLoader({
 		});
 	}, [romeOutput.syntax.ast]);
 
-	const onChange = useCallback((value: string) => {
-		setPlaygroundState((state) => ({
-			...state,
-			files: {
-				...state.files,
-				[state.currentFile]: {
-					...getFileState(state, state.currentFile),
-					content: value,
+	const onChange = useCallback(
+		(value: string) => {
+			setPlaygroundState((state) => ({
+				...state,
+				files: {
+					...state.files,
+					[state.currentFile]: {
+						...getFileState(state, state.currentFile),
+						content: value,
+					},
 				},
-			},
-		}));
-	}, []);
+			}));
+		},
+		[setPlaygroundState],
+	);
 
 	const { width } = useWindowSize();
 	const hasNarrowViewport = width !== undefined && width <= 1000;
diff --git a/website/src/playground/components/Resizable.tsx b/website/src/playground/components/Resizable.tsx
index 9868015af02..578234176d0 100644
--- a/website/src/playground/components/Resizable.tsx
+++ b/website/src/playground/components/Resizable.tsx
@@ -123,7 +123,7 @@ export default function Resizable({
 			window.removeEventListener("mouseup", onMouseUp);
 			window.removeEventListener("mousemove", onMouseMove);
 		};
-	});
+	}, [ref, isResizing, canResize, handler, sizeStore]);
 
 	useEffect(() => {
 		if (cursor === undefined) {