Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 16 additions & 0 deletions .changeset/poor-horses-smash.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
---
"@biomejs/biome": patch
---

Fixed an issue (#6393) where the [useHookAtTopLevel](https://biomejs.dev/linter/rules/use-hook-at-top-level/) rule reported excessive diagnostics for nested hook calls.

The rule now reports only the offending top-level call site, not sub-hooks of composite hooks.

```js
// Before: reported twice (useFoo and useBar).
function useFoo() { return useBar(); }
function Component() {
if (cond) useFoo();
}
// After: reported once at the call to useFoo().
```
Original file line number Diff line number Diff line change
Expand Up @@ -422,6 +422,7 @@ impl Queryable for FunctionCall {
#[derive(Debug)]
pub struct CallPath {
call: JsCallExpression,
is_enclosed_in_component_or_hook: bool,
path: Vec<TextRange>,
}

Expand Down Expand Up @@ -449,10 +450,16 @@ impl Rule for UseHookAtTopLevel {
let root = CallPath {
call: call.clone(),
path: vec![],
is_enclosed_in_component_or_hook: false,
};
let mut calls = vec![root];

while let Some(CallPath { call, mut path }) = calls.pop() {
while let Some(CallPath {
call,
mut path,
is_enclosed_in_component_or_hook,
}) = calls.pop()
{
let range = call.syntax().text_range_with_trivia();

if path.contains(&range) {
Expand Down Expand Up @@ -485,17 +492,27 @@ impl Rule for UseHookAtTopLevel {
});
}

let enclosed = is_enclosed_in_component_or_hook
|| enclosing_function.is_react_component_or_hook();

if let AnyJsFunctionOrMethod::AnyJsFunction(function) = enclosing_function
&& let Some(calls_iter) = function.all_calls(model)
{
for call in calls_iter {
calls.push(CallPath {
call: call.tree(),
path: path.clone(),
is_enclosed_in_component_or_hook: enclosed,
});
}
}
} else {
// Avoid duplicate diagnostics if this path already passed through
// a component/hook. We still keep previously enqueued paths to
// allow recursion detection elsewhere.
if is_enclosed_in_component_or_hook {
continue;
}
return Some(Suggestion {
hook_name_range: get_hook_name_range()?,
path,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -68,13 +68,13 @@ export default function Component5() {
}
};

const Component6 = () => {
const useHook6 = () => {
useEffect();
};

const Component7 = () => {
if (a == 1) {
Component6();
useHook6();
}
};

Expand Down Expand Up @@ -189,4 +189,4 @@ function useRecursiveHookA() {

function useRecursiveHookB() {
useRecursiveHookA();
}
}
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
---
source: crates/biome_js_analyze/tests/spec_tests.rs
assertion_line: 152
expression: invalid.js
---
# Input
Expand Down Expand Up @@ -74,13 +75,13 @@ export default function Component5() {
}
};

const Component6 = () => {
const useHook6 = () => {
useEffect();
};

const Component7 = () => {
if (a == 1) {
Component6();
useHook6();
}
};

Expand Down Expand Up @@ -196,6 +197,7 @@ function useRecursiveHookA() {
function useRecursiveHookB() {
useRecursiveHookA();
}

```

# Diagnostics
Expand Down Expand Up @@ -463,23 +465,14 @@ invalid.js:67:9 lint/correctness/useHookAtTopLevel ━━━━━━━━━
```

```
invalid.js:72:5 lint/correctness/useHookAtTopLevel ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
invalid.js:77:9 lint/correctness/useHookAtTopLevel ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━

× This hook is being called indirectly and conditionally, but all hooks must be called in the exact same order in every component render.

71 │ const Component6 = () => {
> 72 │ useEffect();
│ ^^^^^^^^^
73 │ };
74 │

i This is the call path until the hook.
× This hook is being called conditionally, but all hooks must be called in the exact same order in every component render.

75 │ const Component7 = () => {
> 76 │ if (a == 1) {
> 77 │ Component6();
│ ^^^^^^^^^^^^
76 │ if (a == 1) {
> 77 │ useHook6();
│ ^^^^^^^^
78 │ }
79 │ };

Expand Down Expand Up @@ -881,6 +874,7 @@ invalid.js:187:5 lint/correctness/useHookAtTopLevel ━━━━━━━━━
> 191 │ useRecursiveHookA();
│ ^^^^^^^^^^^^^^^^^^^
192 │ }
193 │

i For React to preserve state between calls, hooks needs to be called unconditionally and always in the same order.

Expand All @@ -900,6 +894,7 @@ invalid.js:191:5 lint/correctness/useHookAtTopLevel ━━━━━━━━━
> 191 │ useRecursiveHookA();
│ ^^^^^^^^^^^^^^^^^
192 │ }
193 │

i This is the call path until the hook.

Expand Down
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
const Component1 = () => {
const useHook = () => {
useEffect() as [];
};

const Component2 = () => {
if (a == 1) {
Component1();
useHook();
}
};
Original file line number Diff line number Diff line change
@@ -1,41 +1,32 @@
---
source: crates/biome_js_analyze/tests/spec_tests.rs
assertion_line: 152
expression: invalid.ts
snapshot_kind: text
---
# Input
```ts
const Component1 = () => {
const useHook = () => {
useEffect() as [];
};

const Component2 = () => {
if (a == 1) {
Component1();
useHook();
}
};

```

# Diagnostics
```
invalid.ts:2:3 lint/correctness/useHookAtTopLevel ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
invalid.ts:7:7 lint/correctness/useHookAtTopLevel ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━

× This hook is being called indirectly and conditionally, but all hooks must be called in the exact same order in every component render.

1 │ const Component1 = () => {
> 2 │ useEffect() as [];
│ ^^^^^^^^^
3 │ };
4 │

i This is the call path until the hook.
× This hook is being called conditionally, but all hooks must be called in the exact same order in every component render.

5 │ const Component2 = () => {
> 6 │ if (a == 1) {
> 7 │ Component1();
│ ^^^^^^^^^^^^
6 │ if (a == 1) {
> 7 │ useHook();
│ ^^^^^^^
8 │ }
9 │ };

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
function useFoo() {
return useBar();
}

function Component() {
if (condition) {
// This call should be reported just once.
// See https://github.com/biomejs/biome/issues/6393
return useFoo();
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
---
source: crates/biome_js_analyze/tests/spec_tests.rs
assertion_line: 152
expression: invalidCompositeHook.js
---
# Input
```js
function useFoo() {
return useBar();
}

function Component() {
if (condition) {
// This call should be reported just once.
// See https://github.com/biomejs/biome/issues/6393
return useFoo();
}
}

```

# Diagnostics
```
invalidCompositeHook.js:9:16 lint/correctness/useHookAtTopLevel ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━

× This hook is being called conditionally, but all hooks must be called in the exact same order in every component render.

7 │ // This call should be reported just once.
8 │ // See https://github.com/biomejs/biome/issues/6393
> 9 │ return useFoo();
│ ^^^^^^
10 │ }
11 │ }

i For React to preserve state between calls, hooks needs to be called unconditionally and always in the same order.

i See https://reactjs.org/docs/hooks-rules.html#only-call-hooks-at-the-top-level


```