diff --git a/crates/rome_js_analyze/src/semantic_analyzers/nursery/react_extensive_dependencies.rs b/crates/rome_js_analyze/src/semantic_analyzers/nursery/react_extensive_dependencies.rs index 383ab613b05d..64af9fcdbca9 100644 --- a/crates/rome_js_analyze/src/semantic_analyzers/nursery/react_extensive_dependencies.rs +++ b/crates/rome_js_analyze/src/semantic_analyzers/nursery/react_extensive_dependencies.rs @@ -4,10 +4,10 @@ use rome_console::markup; use rome_js_semantic::{Capture, SemanticModel}; use rome_js_syntax::{ JsAnyCallArgument, JsAnyExpression, JsArrayExpression, JsArrowFunctionExpression, - JsCallExpression, JsSyntaxKind, TextRange, + JsCallExpression, JsSyntaxKind, TextRange, JsIdentifierBinding, }; -use rome_rowan::AstNode; -use std::collections::HashMap; +use rome_rowan::{AstNode, SyntaxNodeCast}; +use std::collections::{HashMap, HashSet}; use crate::utils::react::*; declare_rule! { @@ -26,6 +26,17 @@ impl Rule for ReactExtensiveDependencies { type Signals = Vec; fn run(ctx: &RuleContext) -> Vec { + //TODO: move this to config + let stables = HashSet::from_iter([ + ReactHookStable::new("useState", Some(1)), + ReactHookStable::new("useReducer", Some(1)), + ReactHookStable::new("useTransition", Some(1)), + ReactHookStable::new("useRef", None), + ReactHookStable::new("useContext", None), + ReactHookStable::new("useId", None), + ReactHookStable::new("useSyncExternalStore", None), + ]); + let mut signals = vec![]; let node = ctx.query(); @@ -49,7 +60,13 @@ impl Rule for ReactExtensiveDependencies { | TS_ENUM_DECLARATION | TS_TYPE_ALIAS_DECLARATION | TS_DECLARE_FUNCTION_DECLARATION => None, - _ => Some(capture) + _ => { + let declaration = declaration.syntax() + .clone() + .cast::()?; + let not_stable = !is_stable_binding(&declaration, &stables); + not_stable.then_some(capture) + } } }) }) @@ -66,9 +83,6 @@ impl Rule for ReactExtensiveDependencies { }) .unwrap_or_default(); - dbg!(&captures.iter().map(|x| &x.0).collect::>()); - dbg!(&deps); - let mut add_deps: HashMap> = HashMap::new(); // Search for captures not in the dependency @@ -79,8 +93,9 @@ impl Rule for ReactExtensiveDependencies { } } - // Search for dependencies not captured + //TODO Search for dependencies not captured + // Generate signals for (_, captures) in add_deps { signals.push((range, captures)); } diff --git a/crates/rome_js_analyze/src/utils/react.rs b/crates/rome_js_analyze/src/utils/react.rs index ddd1845c3202..67cc1b9e3291 100644 --- a/crates/rome_js_analyze/src/utils/react.rs +++ b/crates/rome_js_analyze/src/utils/react.rs @@ -1,7 +1,7 @@ use std::collections::HashSet; use rome_js_syntax::{JsIdentifierBinding, JsVariableDeclarator, JsArrayBindingPatternElementList, JsArrayBindingPattern, SourceType}; -use rome_rowan::{SyntaxNodeCast, AstNode}; +use rome_rowan::AstNode; #[derive(PartialEq, Eq, Hash)] pub struct ReactHookStable { @@ -13,7 +13,7 @@ impl ReactHookStable { pub fn new(hook_name: &str, index: Option) -> Self { Self { hook_name: hook_name.into(), index } } } -fn is_react_hook_state(binding: &JsIdentifierBinding, stables: &HashSet) -> bool { +pub fn is_stable_binding(binding: &JsIdentifierBinding, stables: &HashSet) -> bool { fn array_binding_declarator_index(binding: &JsIdentifierBinding) -> Option<(JsVariableDeclarator, Option)> { let index = binding.syntax().index() / 2; let declarator = binding.parent::()? @@ -51,6 +51,7 @@ fn is_react_hook_state(binding: &JsIdentifierBinding, stables: &HashSet { console.log(local); }); - } \ No newline at end of file + } + +// interaction with other react hooks + +function MyComponent4() { + const [name, setName] = useState(0); + const [state, dispatch] = useReducer(); + const memoizedCallback = useCallback(); + const memoizedValue = useMemo(); + const deferredValue = useDeferredValue(value); + const [isPending, startTransition] = useTransition(); + useEffect(() => { + console.log(name); + setName(1); + + console.log(state); + dispatch(1); + + console.log(memoizedCallback); + console.log(memoizedValue); + console.log(deferredValue); + + console.log(isPending); + startTransition(); + }, []); +} \ No newline at end of file diff --git a/crates/rome_js_analyze/tests/specs/nursery/reactExtensiveDependencies/missingCapturesInvalid.js.snap b/crates/rome_js_analyze/tests/specs/nursery/reactExtensiveDependencies/missingCapturesInvalid.js.snap index cb61f6b14e08..040043fd6a1a 100644 --- a/crates/rome_js_analyze/tests/specs/nursery/reactExtensiveDependencies/missingCapturesInvalid.js.snap +++ b/crates/rome_js_analyze/tests/specs/nursery/reactExtensiveDependencies/missingCapturesInvalid.js.snap @@ -10,6 +10,31 @@ function MyComponent() { console.log(local); }); } + +// interaction with other react hooks + +function MyComponent4() { + const [name, setName] = useState(0); + const [state, dispatch] = useReducer(); + const memoizedCallback = useCallback(); + const memoizedValue = useMemo(); + const deferredValue = useDeferredValue(value); + const [isPending, startTransition] = useTransition(); + useEffect(() => { + console.log(name); + setName(1); + + console.log(state); + dispatch(1); + + console.log(memoizedCallback); + console.log(memoizedValue); + console.log(deferredValue); + + console.log(isPending); + startTransition(); + }, []); +} ``` # Diagnostics @@ -37,4 +62,147 @@ missingCapturesInvalid.js:3:5 lint/nursery/reactExtensiveDependencies ━━━ ``` +``` +missingCapturesInvalid.js:17:3 lint/nursery/reactExtensiveDependencies ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! This useEffect has missing dependencies + + 15 │ const deferredValue = useDeferredValue(value); + 16 │ const [isPending, startTransition] = useTransition(); + > 17 │ useEffect(() => { + │ ^^^^^^^^^ + 18 │ console.log(name); + 19 │ setName(1); + + i This capture is not in the dependency list + + 22 │ dispatch(1); + 23 │ + > 24 │ console.log(memoizedCallback); + │ ^^^^^^^^^^^^^^^^ + 25 │ console.log(memoizedValue); + 26 │ console.log(deferredValue); + + +``` + +``` +missingCapturesInvalid.js:17:3 lint/nursery/reactExtensiveDependencies ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! This useEffect has missing dependencies + + 15 │ const deferredValue = useDeferredValue(value); + 16 │ const [isPending, startTransition] = useTransition(); + > 17 │ useEffect(() => { + │ ^^^^^^^^^ + 18 │ console.log(name); + 19 │ setName(1); + + i This capture is not in the dependency list + + 16 │ const [isPending, startTransition] = useTransition(); + 17 │ useEffect(() => { + > 18 │ console.log(name); + │ ^^^^ + 19 │ setName(1); + 20 │ + + +``` + +``` +missingCapturesInvalid.js:17:3 lint/nursery/reactExtensiveDependencies ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! This useEffect has missing dependencies + + 15 │ const deferredValue = useDeferredValue(value); + 16 │ const [isPending, startTransition] = useTransition(); + > 17 │ useEffect(() => { + │ ^^^^^^^^^ + 18 │ console.log(name); + 19 │ setName(1); + + i This capture is not in the dependency list + + 24 │ console.log(memoizedCallback); + 25 │ console.log(memoizedValue); + > 26 │ console.log(deferredValue); + │ ^^^^^^^^^^^^^ + 27 │ + 28 │ console.log(isPending); + + +``` + +``` +missingCapturesInvalid.js:17:3 lint/nursery/reactExtensiveDependencies ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! This useEffect has missing dependencies + + 15 │ const deferredValue = useDeferredValue(value); + 16 │ const [isPending, startTransition] = useTransition(); + > 17 │ useEffect(() => { + │ ^^^^^^^^^ + 18 │ console.log(name); + 19 │ setName(1); + + i This capture is not in the dependency list + + 19 │ setName(1); + 20 │ + > 21 │ console.log(state); + │ ^^^^^ + 22 │ dispatch(1); + 23 │ + + +``` + +``` +missingCapturesInvalid.js:17:3 lint/nursery/reactExtensiveDependencies ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! This useEffect has missing dependencies + + 15 │ const deferredValue = useDeferredValue(value); + 16 │ const [isPending, startTransition] = useTransition(); + > 17 │ useEffect(() => { + │ ^^^^^^^^^ + 18 │ console.log(name); + 19 │ setName(1); + + i This capture is not in the dependency list + + 26 │ console.log(deferredValue); + 27 │ + > 28 │ console.log(isPending); + │ ^^^^^^^^^ + 29 │ startTransition(); + 30 │ }, []); + + +``` + +``` +missingCapturesInvalid.js:17:3 lint/nursery/reactExtensiveDependencies ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! This useEffect has missing dependencies + + 15 │ const deferredValue = useDeferredValue(value); + 16 │ const [isPending, startTransition] = useTransition(); + > 17 │ useEffect(() => { + │ ^^^^^^^^^ + 18 │ console.log(name); + 19 │ setName(1); + + i This capture is not in the dependency list + + 24 │ console.log(memoizedCallback); + > 25 │ console.log(memoizedValue); + │ ^^^^^^^^^^^^^ + 26 │ console.log(deferredValue); + 27 │ + + +``` + diff --git a/crates/rome_js_analyze/tests/specs/nursery/reactExtensiveDependencies/valid.js b/crates/rome_js_analyze/tests/specs/nursery/reactExtensiveDependencies/valid.js index 9facf49a2b62..d4430afa5c6a 100644 --- a/crates/rome_js_analyze/tests/specs/nursery/reactExtensiveDependencies/valid.js +++ b/crates/rome_js_analyze/tests/specs/nursery/reactExtensiveDependencies/valid.js @@ -13,8 +13,6 @@ function MyComponent2() { } // capturing declarations -import { F } from 'something'; - function doSomething() { } class A {} @@ -22,6 +20,40 @@ function MyComponent3() { useEffect(() => { doSomething(); console.log(new A ()); - console.log(F); }, []); } + +// interaction with other react hooks + +function MyComponent4() { + const [name, setName] = useState(0); + const ref = useRef(); + const theme = useContext(); + const [state, dispatch] = useReducer(); + const memoizedCallback = useCallback(); + const memoizedValue = useMemo(); + const deferredValue = useDeferredValue(value); + const [isPending, startTransition] = useTransition(); + const id = useId(); + const externalStore = useSyncExternalStore(); + useEffect(() => { + console.log(name); + setName(1); + + console.log(ref); + + console.log(theme); + + console.log(state); + dispatch(1) + + memoizedCallback(); + + console.log(isPending); + startTransition(); + + console.log(id); + + console.log(externalStore); + }, [name, state, memoizedCallback, memoizedValue, deferredValue, isPending]); +} diff --git a/crates/rome_js_analyze/tests/specs/nursery/reactExtensiveDependencies/valid.js.snap b/crates/rome_js_analyze/tests/specs/nursery/reactExtensiveDependencies/valid.js.snap index 8da1d19986c1..42216f7ee479 100644 --- a/crates/rome_js_analyze/tests/specs/nursery/reactExtensiveDependencies/valid.js.snap +++ b/crates/rome_js_analyze/tests/specs/nursery/reactExtensiveDependencies/valid.js.snap @@ -19,8 +19,6 @@ function MyComponent2() { } // capturing declarations -import { F } from 'something'; - function doSomething() { } class A {} @@ -28,33 +26,43 @@ function MyComponent3() { useEffect(() => { doSomething(); console.log(new A ()); - console.log(F); }, []); } -``` +// interaction with other react hooks -# Diagnostics -``` -valid.js:22:5 lint/nursery/reactExtensiveDependencies ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ - - ! This useEffect has missing dependencies - - 21 │ function MyComponent3() { - > 22 │ useEffect(() => { - │ ^^^^^^^^^ - 23 │ doSomething(); - 24 │ console.log(new A ()); - - i This capture is not in the dependency list - - 23 │ doSomething(); - 24 │ console.log(new A ()); - > 25 │ console.log(F); - │ ^ - 26 │ }, []); - 27 │ } - +function MyComponent4() { + const [name, setName] = useState(0); + const ref = useRef(); + const theme = useContext(); + const [state, dispatch] = useReducer(); + const memoizedCallback = useCallback(); + const memoizedValue = useMemo(); + const deferredValue = useDeferredValue(value); + const [isPending, startTransition] = useTransition(); + const id = useId(); + const externalStore = useSyncExternalStore(); + useEffect(() => { + console.log(name); + setName(1); + + console.log(ref); + + console.log(theme); + + console.log(state); + dispatch(1) + + memoizedCallback(); + + console.log(isPending); + startTransition(); + + console.log(id); + + console.log(externalStore); + }, [name, state, memoizedCallback, memoizedValue, deferredValue, isPending]); +} ```