From 1aac07904dc258a3dcdde4138a845a780fb52158 Mon Sep 17 00:00:00 2001 From: Dunqing <29533304+Dunqing@users.noreply.github.com> Date: Thu, 22 Jan 2026 06:32:53 +0000 Subject: [PATCH] perf(linter/exhaustive-deps): simplify the logic of checking if the identifier it is a dependency of hook (#18350) From what I understand from the logic, only variables declared at the component level should be dependencies; otherwise, they are normal variables outside of the component or inside the child scope. import { useEffect } from "react"; 1. Variable was declared outside the component scope ```tsx const id = crypto.randomUUID(); function MyComponent() { useEffect(() => { console.log(id); }, []); return
; } ``` 2. Variable was declared inside a child scope ```tsx function MyComponent() { useEffect(() => { const id = crypto.randomUUID(); console.log(id); }, []); return
; } ``` 3. Variable was redeclared in the same scope (should add `id` to dependencies) ```tsx function MyComponent() { const id = crypto.randomUUID(); useEffect(() => { const id = crypto.randomUUID(); console.log(id); }, []); return
; } ``` Only case 3) should add `id` into the dependencies. So simplifying the logic to check `declaration.scope_id() != component_scope_id` is sufficient. ### Performance This should have a significant performance improvement as it avoids the unnecessary two loops. By checking the initial PR of this rule https://github.com/oxc-project/oxc/pull/7151, there are also no performance changes, so I guess there are no such cases in the benchmark files. --- .../src/rules/react/exhaustive_deps.rs | 75 +++++++------------ 1 file changed, 26 insertions(+), 49 deletions(-) diff --git a/crates/oxc_linter/src/rules/react/exhaustive_deps.rs b/crates/oxc_linter/src/rules/react/exhaustive_deps.rs index 15c1eaf8988c2..677b8f90bf688 100644 --- a/crates/oxc_linter/src/rules/react/exhaustive_deps.rs +++ b/crates/oxc_linter/src/rules/react/exhaustive_deps.rs @@ -922,51 +922,39 @@ fn is_identifier_a_dependency_impl<'a>( component_scope_id: ScopeId, visited: &mut FxHashSet, ) -> bool { - // if it is a global e.g. `console` or `window`, then it's not a dependency - if ctx.scoping().root_unresolved_references().contains_key(ident_name.as_str()) { - return false; - } - let Some(declaration) = get_declaration_from_reference_id(ident_reference_id, ctx) else { + // No declaration means it's a global variable, e.g. `console` or `window`, + // which are not dependencies return false; }; - let semantic = ctx.semantic(); - let scopes = semantic.scoping(); - - // if the variable was declared in the root scope, then it's not a dependency - if declaration.scope_id() == scopes.root_scope_id() { - return false; - } - - // Variable was declared outside the component scope - // ```tsx - // const id = crypto.randomUUID(); - // function MyComponent() { - // useEffect(() => { - // console.log(id); - // }, []); - // return
; - // } - // ``` - if scopes - .scope_ancestors(component_scope_id) - .skip(1) - .any(|parent| parent == declaration.scope_id()) - { + // As long as the variable is not declared inside the component, it is not a dependency. + if declaration.scope_id() != component_scope_id { + // 1. Variable was declared outside the component scope + // ```tsx + // const id = crypto.randomUUID(); + // function MyComponent() { + // useEffect(() => { + // console.log(id); + // }, []); + // return
; + // } + // ``` + // + // 2. Variable was declared inside a child scope + // ```tsx + // function MyComponent() { + // useEffect(() => { + // const id = crypto.randomUUID(); + // console.log(id); + // }, []); + // return
; + // } + // ``` return false; } - // Variable was declared inside a child scope - // ```tsx - // function MyComponent() { - // useEffect(() => { - // const id = crypto.randomUUID(); - // console.log(id); - // }, []); - // return
; - // } - if scopes.iter_all_scope_child_ids(component_scope_id).any(|id| id == declaration.scope_id()) { + if declaration.span().contains_inclusive(ident_span) { return false; } @@ -981,17 +969,6 @@ fn is_identifier_a_dependency_impl<'a>( return false; } - // Using a declaration recursively is ok - // ```tsx - // function MyComponent() { - // const recursive = useCallback((n: number): number => (n <= 0 ? 0 : n + recursive(n - 1)), []); - // return recursive - // } - // ``` - if declaration.span().contains_inclusive(ident_span) { - return false; - } - true }