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
36 changes: 36 additions & 0 deletions .changeset/huge-cycles-dance.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
---
"@biomejs/biome": minor
---

Implements [#1984](https://github.com/biomejs/biome/issues/1984). Updated [`useHookAtTopLevel`](https://biomejs.dev/linter/rules/use-hook-at-top-level/) to better catch invalid hook usage.

This rule is now capable of finding invalid hook usage in more locations. A diagnostic will now be generated if:
- A hook is used at the module level (top of the file, outside any function).
- A hook is used within a function or method which is not a hook or component, unless it is a function expression (such as arrow functions commonly used in tests).

**Invalid:**

```js
// Invalid: hooks cannot be called at the module level.
useHook();
```

```js
// Invalid: hooks must be called from another hook or component.
function notAHook() {
useHook();
}
```

**Valid:**

```js
// Valid: hooks may be called from function expressions, such as in tests.
test("my hook", () => {
renderHook(() => useHook());

renderHook(function() {
return useHook();
});
});
```
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,12 @@ Invalid
}
```
```js,expect_diagnostic
function notAHook() {
useEffect();
}
```
Valid
```js
Expand All @@ -64,5 +70,11 @@ Valid
}
```
```js
test("the hook", () => {
renderHook(() => useHook());
});
```
```
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ src/hooks/useHook.ts:2:5 lint/correctness/useHookAtTopLevel ━━━━━━

i This means recursive calls are not allowed, because they require a condition in order to terminate.

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


```
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,12 @@ declare_lint_rule! {
/// }
/// ```
///
/// ```js,expect_diagnostic
/// function notAHook() {
/// useEffect();
/// }
/// ```
///
/// ### Valid
///
/// ```js
Expand All @@ -61,6 +67,12 @@ declare_lint_rule! {
/// }
/// ```
///
/// ```js
/// test("the hook", () => {
/// renderHook(() => useHook());
/// });
/// ```
///
pub UseHookAtTopLevel {
version: "1.0.0",
name: "useHookAtTopLevel",
Expand Down Expand Up @@ -88,6 +100,16 @@ impl AnyJsFunctionOrMethod {
false
}

fn is_function_expression(&self) -> bool {
matches!(
self,
Self::AnyJsFunction(
AnyJsFunction::JsArrowFunctionExpression(_)
| AnyJsFunction::JsFunctionExpression(_)
)
)
}

fn name(&self) -> Option<Text> {
match self {
Self::AnyJsFunction(function) => function
Expand Down Expand Up @@ -119,6 +141,8 @@ pub enum SuggestionKind {
EarlyReturn(TextRange),
Nested,
Recursive,
TopLevel,
ComponentOrHook,
}

/// Verifies whether the call expression is at the top level of the component,
Expand Down Expand Up @@ -245,6 +269,13 @@ fn is_nested_function_inside_component_or_hook(function: &AnyJsFunctionOrMethod)
})
}

fn is_top_level_call(call: &JsCallExpression) -> bool {
!call
.syntax()
.ancestors()
.any(|node| AnyJsFunctionOrMethod::can_cast(node.kind()))
}

/// Model for tracking which function calls are preceded by an early return.
///
/// The keys in the model are call sites and each value is the text range of an
Expand Down Expand Up @@ -444,6 +475,14 @@ impl Rule for UseHookAtTopLevel {
return None;
}

if is_top_level_call(call) {
return Some(Suggestion {
hook_name_range: get_hook_name_range()?,
path: vec![call.syntax().text_range_with_trivia()],
kind: SuggestionKind::TopLevel,
});
}

let model = ctx.semantic_model();
let early_returns = ctx.early_returns_model();

Expand Down Expand Up @@ -521,6 +560,16 @@ impl Rule for UseHookAtTopLevel {
}
}

if enclosing_function_if_call_is_at_top_level(call).is_some_and(|function| {
!function.is_react_component_or_hook() && !function.is_function_expression()
}) {
return Some(Suggestion {
hook_name_range: get_hook_name_range()?,
path: vec![call.syntax().text_range_with_trivia()],
kind: SuggestionKind::ComponentOrHook,
});
}

None
}

Expand All @@ -537,6 +586,13 @@ impl Rule for UseHookAtTopLevel {
"unconditionally from the top-level component."
},
SuggestionKind::Recursive => markup! { "This hook is being called recursively." },
SuggestionKind::TopLevel => markup! {
"This hook is being called at the module level, but all hooks must be called from "
"within a hook or component."
},
SuggestionKind::ComponentOrHook => markup! {
"This hook is being called from within a function or method that is not a hook or component."
},
_ if path.len() <= 1 => markup! {
"This hook is being called conditionally, but all hooks must be called in the "
"exact same order in every component render."
Expand All @@ -562,13 +618,18 @@ impl Rule for UseHookAtTopLevel {
diag = diag.detail(
*range,
markup! { "Hooks should not be called after an early return." },
)
);
}

let mut diag = diag.note(markup! {
"For React to preserve state between calls, hooks needs to be called "
"unconditionally and always in the same order."
});
diag = match kind {
SuggestionKind::TopLevel | SuggestionKind::ComponentOrHook => diag.note(markup! {
"Move the hook call into the top level of a hook or component in order to use it."
}),
_ => diag.note(markup! {
"For React to preserve state between calls, hooks needs to be called "
"unconditionally and always in the same order."
}),
};

if matches!(kind, SuggestionKind::Recursive) {
diag = diag.note(markup! {
Expand All @@ -577,8 +638,8 @@ impl Rule for UseHookAtTopLevel {
});
}

let diag = diag.note(markup! {
"See https://reactjs.org/docs/hooks-rules.html#only-call-hooks-at-the-top-level"
diag = diag.note(markup! {
"See https://react.dev/reference/rules/rules-of-hooks"
});
Some(diag)
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
---
source: crates/biome_js_analyze/tests/spec_tests.rs
expression: customHook.js
snapshot_kind: text
---
# Input
```js
Expand Down Expand Up @@ -36,7 +35,7 @@ customHook.js:7:23 lint/correctness/useHookAtTopLevel ━━━━━━━━
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
i See https://react.dev/reference/rules/rules-of-hooks
```
Expand All @@ -55,7 +54,7 @@ customHook.js:12:23 lint/correctness/useHookAtTopLevel ━━━━━━━━
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
i See https://react.dev/reference/rules/rules-of-hooks
```
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
// Invalid as hooks cannot be used at module level.
useHook();

// Invalid as hooks cannot be called in non-component functions.
function notAComponent() {
useHook();
}

// Valid as hook is called in a component (by naming convention).
function AComponent() {
useHook();
}

// Invalid as hooks cannot be called in non-hook functions.
function notUseMyHook() {
useHook();
}
const SomeObject = {
notHook() {
useHook();
},
};
class SomeClass {
notHook() {
useHook();
}
}

// Valid as hook is called in a hook (by naming convention).
function useMyHook() {
useHook();
}

// Valid as hooks can be called within function expressions (for better or worse).
test("the hook", () => {
useHook();
});
test("the hook", function () {
useHook();
});
test("the hook", function named() {
useHook();
});

// Valid as hooks can be called within nested function expressions.
test("more hook", () => {
renderHook(() => useHook());
});
Loading