From 216b39c04b5e064494b805e58b436ffcf8ee18b1 Mon Sep 17 00:00:00 2001 From: msdlisper <1170167213@qq.com> Date: Sat, 28 Oct 2023 21:40:16 +0800 Subject: [PATCH 1/2] fix: bug with useContext/useSyncExternalStore (#609) --- .../use_exhaustive_dependencies.rs | 3 -- .../useExhaustiveDependencies/valid.js | 13 +++++-- .../useExhaustiveDependencies/valid.js.snap | 14 ++++++-- .../rules/use-exhaustive-dependencies.md | 35 +++++++++---------- 4 files changed, 39 insertions(+), 26 deletions(-) diff --git a/crates/biome_js_analyze/src/semantic_analyzers/correctness/use_exhaustive_dependencies.rs b/crates/biome_js_analyze/src/semantic_analyzers/correctness/use_exhaustive_dependencies.rs index bf6f368ffb96..f7de9dc08021 100644 --- a/crates/biome_js_analyze/src/semantic_analyzers/correctness/use_exhaustive_dependencies.rs +++ b/crates/biome_js_analyze/src/semantic_analyzers/correctness/use_exhaustive_dependencies.rs @@ -207,9 +207,6 @@ impl Default for ReactExtensiveDependenciesOptions { StableReactHookConfiguration::new("useReducer", Some(1)), StableReactHookConfiguration::new("useTransition", Some(1)), StableReactHookConfiguration::new("useRef", None), - StableReactHookConfiguration::new("useContext", None), - StableReactHookConfiguration::new("useId", None), - StableReactHookConfiguration::new("useSyncExternalStore", None), ]); Self { diff --git a/crates/biome_js_analyze/tests/specs/correctness/useExhaustiveDependencies/valid.js b/crates/biome_js_analyze/tests/specs/correctness/useExhaustiveDependencies/valid.js index 5c84dd763152..9574877a4de8 100644 --- a/crates/biome_js_analyze/tests/specs/correctness/useExhaustiveDependencies/valid.js +++ b/crates/biome_js_analyze/tests/specs/correctness/useExhaustiveDependencies/valid.js @@ -1,7 +1,7 @@ /* should not generate diagnostics */ import React from "react"; -import { useEffect } from "react"; +import { useEffect, useSyncExternalStore } from "react"; import doSomething from 'a'; // No captures @@ -62,7 +62,7 @@ function MyComponent4() { console.log(id); console.log(externalStore); - }, [name, state, memoizedCallback, memoizedValue, isPending]); + }, [name, state, memoizedCallback, memoizedValue, isPending, externalStore, id, theme]); } // all hooks with dependencies @@ -167,3 +167,12 @@ function MyComponent16() { console.log(a); }, [a]); } + +// https://github.com/biomejs/biome/issues/609 +function MyComponent17() { + const data = useSyncExternalStore(subscribe, getSnapshot); + + useEffect(() => { + console.log(data); + }, [data]); +} diff --git a/crates/biome_js_analyze/tests/specs/correctness/useExhaustiveDependencies/valid.js.snap b/crates/biome_js_analyze/tests/specs/correctness/useExhaustiveDependencies/valid.js.snap index 5171784a7ea9..3a31d3b0c001 100644 --- a/crates/biome_js_analyze/tests/specs/correctness/useExhaustiveDependencies/valid.js.snap +++ b/crates/biome_js_analyze/tests/specs/correctness/useExhaustiveDependencies/valid.js.snap @@ -1,5 +1,6 @@ --- source: crates/biome_js_analyze/tests/spec_tests.rs +assertion_line: 83 expression: valid.js --- # Input @@ -7,7 +8,7 @@ expression: valid.js /* should not generate diagnostics */ import React from "react"; -import { useEffect } from "react"; +import { useEffect, useSyncExternalStore } from "react"; import doSomething from 'a'; // No captures @@ -68,7 +69,7 @@ function MyComponent4() { console.log(id); console.log(externalStore); - }, [name, state, memoizedCallback, memoizedValue, isPending]); + }, [name, state, memoizedCallback, memoizedValue, isPending, externalStore, id, theme]); } // all hooks with dependencies @@ -174,6 +175,15 @@ function MyComponent16() { }, [a]); } +// https://github.com/biomejs/biome/issues/609 +function MyComponent17() { + const data = useSyncExternalStore(subscribe, getSnapshot); + + useEffect(() => { + console.log(data); + }, [data]); +} + ``` diff --git a/website/src/content/docs/linter/rules/use-exhaustive-dependencies.md b/website/src/content/docs/linter/rules/use-exhaustive-dependencies.md index 0e30387ccba4..7d85f57acf40 100644 --- a/website/src/content/docs/linter/rules/use-exhaustive-dependencies.md +++ b/website/src/content/docs/linter/rules/use-exhaustive-dependencies.md @@ -23,14 +23,11 @@ The rule will inspect the following **known** hooks: - `useMemo` - `useImperativeHandle` - `useState` -- `useContext` - `useReducer` - `useRef` - `useDebugValue` - `useDeferredValue` - `useTransition` -- `useId` -- `useSyncExternalStore` If you want to add more hooks to the rule, check the [#options](options). @@ -52,23 +49,23 @@ function component() { 
correctness/useExhaustiveDependencies.js:5:5 lint/correctness/useExhaustiveDependencies ━━━━━━━━━━━━
 
   ✖ This hook does not specify all of its dependencies.
-  
+
     3 │ function component() {
     4 │     let a = 1;
   > 5 │     useEffect(() => {
       │     ^^^^^^^^^
     6 │         console.log(a);
     7 │     });
-  
+
   ℹ This dependency is not specified in the hook dependency list.
-  
+
     4 │     let a = 1;
     5 │     useEffect(() => {
   > 6 │         console.log(a);
       │                     ^
     7 │     });
     8 │ }
-  
+
```jsx @@ -84,23 +81,23 @@ function component() { 
correctness/useExhaustiveDependencies.js:5:5 lint/correctness/useExhaustiveDependencies ━━━━━━━━━━━━
 
   ✖ This hook specifies more dependencies than necessary.
-  
+
     3 │ function component() {
     4 │     let b = 1;
   > 5 │     useEffect(() => {
       │     ^^^^^^^^^
     6 │     }, [b]);
     7 │ }
-  
+
   ℹ This dependency can be removed from the list.
-  
+
     4 │     let b = 1;
     5 │     useEffect(() => {
   > 6 │     }, [b]);
       │         ^
     7 │ }
     8 │ 
-  
+
```jsx @@ -118,23 +115,23 @@ function component() { 
correctness/useExhaustiveDependencies.js:5:5 lint/correctness/useExhaustiveDependencies ━━━━━━━━━━━━
 
   ✖ This hook specifies more dependencies than necessary.
-  
+
     3 │ function component() {
     4 │     const [name, setName] = useState();
   > 5 │     useEffect(() => {
       │     ^^^^^^^^^
     6 │         console.log(name);
     7 │         setName("");
-  
+
   ℹ This dependency can be removed from the list.
-  
+
      6 │         console.log(name);
      7 │         setName("");
    > 8 │     }, [name, setName]);
        │               ^^^^^^^
      9 │ }
     10 │ 
-  
+
```jsx @@ -152,23 +149,23 @@ function component() { 
correctness/useExhaustiveDependencies.js:6:5 lint/correctness/useExhaustiveDependencies ━━━━━━━━━━━━
 
   ✖ This hook does not specify all of its dependencies.
-  
+
     4 │     let a = 1;
     5 │     const b = a + 1;
   > 6 │     useEffect(() => {
       │     ^^^^^^^^^
     7 │         console.log(b);
     8 │     });
-  
+
   ℹ This dependency is not specified in the hook dependency list.
-  
+
     5 │     const b = a + 1;
     6 │     useEffect(() => {
   > 7 │         console.log(b);
       │                     ^
     8 │     });
     9 │ }
-  
+
## Valid From 106c23f3bf2e703444cb5294bf7a2b7c3f3a676b Mon Sep 17 00:00:00 2001 From: msdlisper <1170167213@qq.com> Date: Sat, 28 Oct 2023 22:21:35 +0800 Subject: [PATCH 2/2] docs: update docs (#609) --- .../use_exhaustive_dependencies.rs | 9 ------ .../rules/use-exhaustive-dependencies.md | 32 +++++++++---------- 2 files changed, 16 insertions(+), 25 deletions(-) diff --git a/crates/biome_js_analyze/src/semantic_analyzers/correctness/use_exhaustive_dependencies.rs b/crates/biome_js_analyze/src/semantic_analyzers/correctness/use_exhaustive_dependencies.rs index f7de9dc08021..adb8c4b85d6a 100644 --- a/crates/biome_js_analyze/src/semantic_analyzers/correctness/use_exhaustive_dependencies.rs +++ b/crates/biome_js_analyze/src/semantic_analyzers/correctness/use_exhaustive_dependencies.rs @@ -36,14 +36,11 @@ declare_rule! { /// - `useMemo` /// - `useImperativeHandle` /// - `useState` - /// - `useContext` /// - `useReducer` /// - `useRef` /// - `useDebugValue` /// - `useDeferredValue` /// - `useTransition` - /// - `useId` - /// - `useSyncExternalStore` /// /// If you want to add more hooks to the rule, check the [#options](options). /// @@ -180,7 +177,6 @@ impl Default for ReactExtensiveDependenciesOptions { ("useMemo".to_string(), (0, 1).into()), ("useImperativeHandle".to_string(), (1, 2).into()), ("useState".to_string(), ReactHookConfiguration::default()), - ("useContext".to_string(), ReactHookConfiguration::default()), ("useReducer".to_string(), ReactHookConfiguration::default()), ("useRef".to_string(), ReactHookConfiguration::default()), ( @@ -195,11 +191,6 @@ impl Default for ReactExtensiveDependenciesOptions { "useTransition".to_string(), ReactHookConfiguration::default(), ), - ("useId".to_string(), ReactHookConfiguration::default()), - ( - "useSyncExternalStore".to_string(), - ReactHookConfiguration::default(), - ), ]); let stable_config = FxHashSet::from_iter([ diff --git a/website/src/content/docs/linter/rules/use-exhaustive-dependencies.md b/website/src/content/docs/linter/rules/use-exhaustive-dependencies.md index 7d85f57acf40..8df5e9fb8f87 100644 --- a/website/src/content/docs/linter/rules/use-exhaustive-dependencies.md +++ b/website/src/content/docs/linter/rules/use-exhaustive-dependencies.md @@ -49,23 +49,23 @@ function component() { 
correctness/useExhaustiveDependencies.js:5:5 lint/correctness/useExhaustiveDependencies ━━━━━━━━━━━━
 
   ✖ This hook does not specify all of its dependencies.
-
+  
     3 │ function component() {
     4 │     let a = 1;
   > 5 │     useEffect(() => {
       │     ^^^^^^^^^
     6 │         console.log(a);
     7 │     });
-
+  
   ℹ This dependency is not specified in the hook dependency list.
-
+  
     4 │     let a = 1;
     5 │     useEffect(() => {
   > 6 │         console.log(a);
       │                     ^
     7 │     });
     8 │ }
-
+
```jsx @@ -81,23 +81,23 @@ function component() { 
correctness/useExhaustiveDependencies.js:5:5 lint/correctness/useExhaustiveDependencies ━━━━━━━━━━━━
 
   ✖ This hook specifies more dependencies than necessary.
-
+  
     3 │ function component() {
     4 │     let b = 1;
   > 5 │     useEffect(() => {
       │     ^^^^^^^^^
     6 │     }, [b]);
     7 │ }
-
+  
   ℹ This dependency can be removed from the list.
-
+  
     4 │     let b = 1;
     5 │     useEffect(() => {
   > 6 │     }, [b]);
       │         ^
     7 │ }
     8 │ 
-
+
```jsx @@ -115,23 +115,23 @@ function component() { 
correctness/useExhaustiveDependencies.js:5:5 lint/correctness/useExhaustiveDependencies ━━━━━━━━━━━━
 
   ✖ This hook specifies more dependencies than necessary.
-
+  
     3 │ function component() {
     4 │     const [name, setName] = useState();
   > 5 │     useEffect(() => {
       │     ^^^^^^^^^
     6 │         console.log(name);
     7 │         setName("");
-
+  
   ℹ This dependency can be removed from the list.
-
+  
      6 │         console.log(name);
      7 │         setName("");
    > 8 │     }, [name, setName]);
        │               ^^^^^^^
      9 │ }
     10 │ 
-
+
```jsx @@ -149,23 +149,23 @@ function component() { 
correctness/useExhaustiveDependencies.js:6:5 lint/correctness/useExhaustiveDependencies ━━━━━━━━━━━━
 
   ✖ This hook does not specify all of its dependencies.
-
+  
     4 │     let a = 1;
     5 │     const b = a + 1;
   > 6 │     useEffect(() => {
       │     ^^^^^^^^^
     7 │         console.log(b);
     8 │     });
-
+  
   ℹ This dependency is not specified in the hook dependency list.
-
+  
     5 │     const b = a + 1;
     6 │     useEffect(() => {
   > 7 │         console.log(b);
       │                     ^
     8 │     });
     9 │ }
-
+
## Valid