From 7a9b6422ea1d496e8d682e946acb745e5d8bb1b0 Mon Sep 17 00:00:00 2001 From: Naoki Ikeguchi Date: Fri, 24 Jan 2025 23:35:54 +0900 Subject: [PATCH 1/3] fix(lint): Support some of wrapped hook calls in useExhaustiveDependencies Signed-off-by: Naoki Ikeguchi --- crates/biome_js_analyze/src/react/hooks.rs | 26 ++++++++++++++++++- .../useExhaustiveDependencies/valid.ts | 19 +++++++++++++- .../useExhaustiveDependencies/valid.ts.snap | 19 ++++++++++++-- 3 files changed, 60 insertions(+), 4 deletions(-) diff --git a/crates/biome_js_analyze/src/react/hooks.rs b/crates/biome_js_analyze/src/react/hooks.rs index 8c3cb9031796..ffb97a56802e 100644 --- a/crates/biome_js_analyze/src/react/hooks.rs +++ b/crates/biome_js_analyze/src/react/hooks.rs @@ -447,7 +447,7 @@ pub fn is_binding_react_stable( let Some(callee) = declarator .initializer() .and_then(|initializer| initializer.expression().ok()) - .and_then(|initializer| initializer.as_js_call_expression()?.callee().ok()) + .and_then(|initializer| unwrap_to_call_expression(initializer)?.callee().ok()) else { return false; }; @@ -474,6 +474,30 @@ pub fn is_binding_react_stable( }) } +/// Unwrap the expression to a call expression without changing the result of the expression, +/// such as type assertions. +fn unwrap_to_call_expression(expression: AnyJsExpression) -> Option { + match expression { + AnyJsExpression::JsCallExpression(expr) => Some(expr), + AnyJsExpression::JsParenthesizedExpression(expr) => { + expr.expression().ok().and_then(unwrap_to_call_expression) + } + AnyJsExpression::JsSequenceExpression(expr) => { + expr.right().ok().and_then(unwrap_to_call_expression) + } + AnyJsExpression::TsAsExpression(expr) => { + expr.expression().ok().and_then(unwrap_to_call_expression) + } + AnyJsExpression::TsSatisfiesExpression(expr) => { + expr.expression().ok().and_then(unwrap_to_call_expression) + } + AnyJsExpression::TsNonNullAssertionExpression(expr) => { + expr.expression().ok().and_then(unwrap_to_call_expression) + } + _ => None, + } +} + #[cfg(test)] mod test { use super::*; diff --git a/crates/biome_js_analyze/tests/specs/correctness/useExhaustiveDependencies/valid.ts b/crates/biome_js_analyze/tests/specs/correctness/useExhaustiveDependencies/valid.ts index 329ce4db8a16..ee52cebd7826 100644 --- a/crates/biome_js_analyze/tests/specs/correctness/useExhaustiveDependencies/valid.ts +++ b/crates/biome_js_analyze/tests/specs/correctness/useExhaustiveDependencies/valid.ts @@ -1,6 +1,6 @@ /* should not generate diagnostics */ -import { useEffect } from "react"; +import { type MutableRefObject, useEffect, useRef } from "react"; // capturing declarations function overloaded(s: string): string; @@ -46,3 +46,20 @@ export function MyComponent3({ outer }: { outer: string[] }) { console.log(a) }, []); } + +// useRef's are still considered as stable even if there's any type assertion +export function MyComponent4() { + const parenthesizedRef = (((useRef()))); + const sequenceRef = (doSomething(), useRef()); + const nonNullAssertedRef = useRef()!; + const castedRef = useRef() as MutableRefObject; + const satisfiedRef = useRef() satisfies MutableRefObject; + + useEffect(() => { + console.log(parenthesizedRef.current); + console.log(sequenceRef.current); + console.log(nonNullAssertedRef.current); + console.log(castedRef.current.innerHTML); + console.log(satisfiedRef.current.innerHTML); + }, []); +} diff --git a/crates/biome_js_analyze/tests/specs/correctness/useExhaustiveDependencies/valid.ts.snap b/crates/biome_js_analyze/tests/specs/correctness/useExhaustiveDependencies/valid.ts.snap index a81f964d7d46..8a09d5a4eb6b 100644 --- a/crates/biome_js_analyze/tests/specs/correctness/useExhaustiveDependencies/valid.ts.snap +++ b/crates/biome_js_analyze/tests/specs/correctness/useExhaustiveDependencies/valid.ts.snap @@ -6,7 +6,7 @@ expression: valid.ts ```ts /* should not generate diagnostics */ -import { useEffect } from "react"; +import { type MutableRefObject, useEffect, useRef } from "react"; // capturing declarations function overloaded(s: string): string; @@ -53,6 +53,21 @@ export function MyComponent3({ outer }: { outer: string[] }) { }, []); } -``` +// useRef's are still considered as stable even if there's any type assertion +export function MyComponent4() { + const parenthesizedRef = (((useRef()))); + const sequenceRef = (doSomething(), useRef()); + const nonNullAssertedRef = useRef()!; + const castedRef = useRef() as MutableRefObject; + const satisfiedRef = useRef() satisfies MutableRefObject; + useEffect(() => { + console.log(parenthesizedRef.current); + console.log(sequenceRef.current); + console.log(nonNullAssertedRef.current); + console.log(castedRef.current.innerHTML); + console.log(satisfiedRef.current.innerHTML); + }, []); +} +``` From 6acd75fefa53df453940589dca8c2139c08ed928 Mon Sep 17 00:00:00 2001 From: Naoki Ikeguchi Date: Sat, 25 Jan 2025 00:49:36 +0900 Subject: [PATCH 2/3] chore: Add changeset Signed-off-by: Naoki Ikeguchi --- ...act_hooks_stable_within_parentheses_or_type_assertions.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/fix_1597_useexhaustivedependencies_now_consider_react_hooks_stable_within_parentheses_or_type_assertions.md diff --git a/.changeset/fix_1597_useexhaustivedependencies_now_consider_react_hooks_stable_within_parentheses_or_type_assertions.md b/.changeset/fix_1597_useexhaustivedependencies_now_consider_react_hooks_stable_within_parentheses_or_type_assertions.md new file mode 100644 index 000000000000..f0ad6fc9841d --- /dev/null +++ b/.changeset/fix_1597_useexhaustivedependencies_now_consider_react_hooks_stable_within_parentheses_or_type_assertions.md @@ -0,0 +1,5 @@ +--- +biome_js_analyze: patch +--- + +# Fix #1597, useExhaustiveDependencies now consider React hooks stable within parentheses or type assertions From 45e96689950cd85bc863dcae9a8c039cee41a3b3 Mon Sep 17 00:00:00 2001 From: Naoki Ikeguchi Date: Sat, 25 Jan 2025 02:08:43 +0900 Subject: [PATCH 3/3] refactor(lint): Turn a recursion into a loop Signed-off-by: Naoki Ikeguchi --- crates/biome_js_analyze/src/react/hooks.rs | 38 ++++++++++++---------- 1 file changed, 20 insertions(+), 18 deletions(-) diff --git a/crates/biome_js_analyze/src/react/hooks.rs b/crates/biome_js_analyze/src/react/hooks.rs index ffb97a56802e..1f9c10869c75 100644 --- a/crates/biome_js_analyze/src/react/hooks.rs +++ b/crates/biome_js_analyze/src/react/hooks.rs @@ -476,25 +476,27 @@ pub fn is_binding_react_stable( /// Unwrap the expression to a call expression without changing the result of the expression, /// such as type assertions. -fn unwrap_to_call_expression(expression: AnyJsExpression) -> Option { - match expression { - AnyJsExpression::JsCallExpression(expr) => Some(expr), - AnyJsExpression::JsParenthesizedExpression(expr) => { - expr.expression().ok().and_then(unwrap_to_call_expression) - } - AnyJsExpression::JsSequenceExpression(expr) => { - expr.right().ok().and_then(unwrap_to_call_expression) - } - AnyJsExpression::TsAsExpression(expr) => { - expr.expression().ok().and_then(unwrap_to_call_expression) - } - AnyJsExpression::TsSatisfiesExpression(expr) => { - expr.expression().ok().and_then(unwrap_to_call_expression) - } - AnyJsExpression::TsNonNullAssertionExpression(expr) => { - expr.expression().ok().and_then(unwrap_to_call_expression) +fn unwrap_to_call_expression(mut expression: AnyJsExpression) -> Option { + loop { + match expression { + AnyJsExpression::JsCallExpression(expr) => return Some(expr), + AnyJsExpression::JsParenthesizedExpression(expr) => { + expression = expr.expression().ok()?; + } + AnyJsExpression::JsSequenceExpression(expr) => { + expression = expr.right().ok()?; + } + AnyJsExpression::TsAsExpression(expr) => { + expression = expr.expression().ok()?; + } + AnyJsExpression::TsSatisfiesExpression(expr) => { + expression = expr.expression().ok()?; + } + AnyJsExpression::TsNonNullAssertionExpression(expr) => { + expression = expr.expression().ok()?; + } + _ => return None, } - _ => None, } }