Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

linter: react-hooks/rules-of-hooks false positive #9795

Closed
DreierF opened this issue Mar 15, 2025 · 0 comments · Fixed by #9864
Closed

linter: react-hooks/rules-of-hooks false positive #9795

DreierF opened this issue Mar 15, 2025 · 0 comments · Fixed by #9864
Assignees
Labels
A-linter Area - Linter

Comments

@DreierF
Copy link

DreierF commented Mar 15, 2025

What version of Oxlint are you using?

0.15.15

What command did you run?

pnpm exec oxlint

What does your .oxlintrc.json config file look like?

{
  "$schema": "./node_modules/oxlint/configuration_schema.json",
  "plugins": [
    "typescript",
    "react"
  ],
  "categories": {
    "correctness": "off"
  },
  "rules": {
    "react-hooks/rules-of-hooks": "error"
  }
}

What happened?

function Component() {
	if (!useHasPermission()) {
		return null;
	}
	return <Content />;
}

Code like this causes a FP: React Hook "useHasPermission" is called conditionally. React Hooks must be called in the exact same order in every component render.

The order in which the hook is called is always the same and is also understood by the corresponding ESLint plugin.

@DreierF DreierF added the A-linter Area - Linter label Mar 15, 2025
graphite-app bot pushed a commit that referenced this issue Mar 19, 2025
This PR fixes the construction of the cfg within `oxc_semantic`, by marking the `test` of `IfStatement` as unconditionally visited.

Given the following:
```ts
if (foo) {
  bar();
} else {
  baz();
}
```

Would produce the following graph:

```mermaid
graph TD
    0["bb0"]
    1["bb1 IfStatement"]
    2["bb2 Condition(IdentifierReference(foo))"]
    3["bb3 BlockStatement ExpressionStatement"]
    4["bb4 BlockStatement ExpressionStatement"]
    5["bb5"]

    1 -- "Error(Implicit)" --> 0
    2 -- "Error(Implicit)" --> 0
    3 -- "Error(Implicit)" --> 0
    4 -- "Error(Implicit)" --> 0
    5 -- "Error(Implicit)" --> 0
    1 -- "Normal" --> 2
    1 -- "Normal" --> 4
    3 -- "Normal" --> 5
    2 -- "Jump" --> 3
    4 -- "Normal" --> 5
```

After this change, it produces the following graph:

```mermaid
graph TD
    0["bb0"]
    1["bb1 IfStatement"]
    2["bb2 Condition(IdentifierReference(foo))"]
    3["bb3 BlockStatement ExpressionStatement"]
    4["bb4 BlockStatement ExpressionStatement"]
    5["bb5"]

    1 -- "Error(Implicit)" --> 0
    2 -- "Error(Implicit)" --> 0
    3 -- "Error(Implicit)" --> 0
    4 -- "Error(Implicit)" --> 0
    5 -- "Error(Implicit)" --> 0
    1 -- "Normal" --> 2
    3 -- "Normal" --> 5
    2 -- "Jump" --> 3
    2 -- "Jump" --> 4
    4 -- "Normal" --> 5
```

Rather than jumping from the if statment (`bb1\nIfStatement`) directly to the consequent (`bb4 BlockStatement ExpressionStatement`), it now unconditionally visits the `test` of the if statement. This can be seen in the after graph as the jump to `bb4 BlockStatement ExpressionStatement"` comes **from** `bb2 Condition(IdentifierReference(foo))` rather than from `bb1 IfStatement`.

As a result of this change, `rules_of_hooks`, does not have false positives when a hook is in a position such as:
```ts
if (useMe) {
```
As the cfg would previously think that `useMe` was called conditionally when in fact it was not

fixes #9795
related #9807
@camc314 camc314 closed this as completed Mar 19, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-linter Area - Linter
Projects
None yet
2 participants