Skip to content
This repository has been archived by the owner on Aug 31, 2023. It is now read-only.

Commit

Permalink
fix: check wheather hooks api is imported from React or not
Browse files Browse the repository at this point in the history
  • Loading branch information
nissy-dev committed Jun 18, 2023
1 parent 9db1688 commit b644eb6
Show file tree
Hide file tree
Showing 15 changed files with 496 additions and 366 deletions.
17 changes: 16 additions & 1 deletion crates/rome_js_analyze/src/react.rs
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,7 @@ impl ReactLibrary {
/// List of valid [`React` API]
///
/// [`React` API]: https://reactjs.org/docs/react-api.html
const VALID_REACT_API: [&str; 14] = [
const VALID_REACT_API: [&str; 29] = [
"Component",
"PureComponent",
"memo",
Expand All @@ -157,6 +157,21 @@ const VALID_REACT_API: [&str; 14] = [
"Suspense",
"startTransition",
"Children",
"useEffect",
"useLayoutEffect",
"useInsertionEffect",
"useCallback",
"useMemo",
"useImperativeHandle",
"useState",
"useContext",
"useReducer",
"useRef",
"useDebugValue",
"useDeferredValue",
"useTransition",
"useId",
"useSyncExternalStore",
];

/// Checks if the current [JsCallExpression] is a potential [`React` API].
Expand Down
21 changes: 20 additions & 1 deletion crates/rome_js_analyze/src/react/hooks.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
use crate::react::{is_react_call_api, ReactLibrary};
use std::collections::{HashMap, HashSet};

use rome_js_semantic::{Capture, Closure, ClosureExtensions, SemanticModel};
Expand Down Expand Up @@ -109,6 +110,15 @@ pub(crate) fn react_hook_configuration<'a>(
hooks.get(name)
}

const HOOKS_WITH_DEPS_API: [&str; 6] = [
"useEffect",
"useLayoutEffect",
"useInsertionEffect",
"useCallback",
"useMemo",
"useImperativeHandle",
];

/// Returns the [TextRange] of the hook name; the node of the
/// expression of the argument that correspond to the closure of
/// the hook; and the node of the dependency list of the hook.
Expand All @@ -126,8 +136,10 @@ pub(crate) fn react_hook_configuration<'a>(
pub(crate) fn react_hook_with_dependency(
call: &JsCallExpression,
hooks: &HashMap<String, ReactHookConfiguration>,
model: &SemanticModel,
) -> Option<ReactCallWithDependencyResult> {
let name = match call.callee().ok()? {
let expression = call.callee().ok()?;
let name = match &expression {
AnyJsExpression::JsIdentifierExpression(identifier) => {
Some(identifier.name().ok()?.value_token().ok()?)
}
Expand All @@ -139,6 +151,13 @@ pub(crate) fn react_hook_with_dependency(
let function_name_range = name.text_trimmed_range();
let name = name.text_trimmed();

// check if the hooks api is imported from the react library
if HOOKS_WITH_DEPS_API.contains(&name)
&& !is_react_call_api(expression, model, ReactLibrary::React, name)
{
return None;
}

let hook = hooks.get(name)?;
let closure_index = hook.closure_index?;
let dependencies_index = hook.dependencies_index?;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -509,9 +509,9 @@ impl Rule for UseExhaustiveDependencies {
let mut signals = vec![];

let call = ctx.query();
if let Some(result) = react_hook_with_dependency(call, &options.hooks_config) {
let model = ctx.model();
let model = ctx.model();

if let Some(result) = react_hook_with_dependency(call, &options.hooks_config, model) {
let Some(component_function) = function_of_hook_call(call) else {
return vec![]
};
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
function MyComponent1() {
let a = 1;
React.useEffect(() => {
console.log(a);
});

// the rule doesn't show the warnings because the hooks are not imported from react.
useEffect(() => {
console.log(a);
});
}

function MyComponent2() {
let a = 1;
const React = { useEffect() {} }
// the rule doesn't show the warnings because `React` is defined by the user.
React.useEffect(() => {
console.log(a);
});
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
---
source: crates/rome_js_analyze/tests/spec_tests.rs
expression: checkHooksImportedFromReact.js
---
# Input
```js
function MyComponent1() {
let a = 1;
React.useEffect(() => {
console.log(a);
});

// the rule doesn't show the warnings because the hooks are not imported from react.
useEffect(() => {
console.log(a);
});
}

function MyComponent2() {
let a = 1;
const React = { useEffect() {} }
// the rule doesn't show the warnings because `React` is defined by the user.
React.useEffect(() => {
console.log(a);
});
}

```

# Diagnostics
```
checkHooksImportedFromReact.js:3:9 lint/nursery/useExhaustiveDependencies ━━━━━━━━━━━━━━━━━━━━━━━━━━
! This hook do not specify all of its dependencies.
1 │ function MyComponent1() {
2 │ let a = 1;
> 3 │ React.useEffect(() => {
│ ^^^^^^^^^
4 │ console.log(a);
5 │ });
i This dependency is not specified in the hook dependency list.
2 │ let a = 1;
3 │ React.useEffect(() => {
> 4 │ console.log(a);
│ ^
5 │ });
6 │
```


Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import { useEffect } from "react";

function MyComponent() {
let a = 1;
useEffect(() => {
Expand All @@ -6,4 +8,4 @@ function MyComponent() {
useMyEffect(() => {
console.log(a);
});
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@ expression: customHook.js
---
# Input
```js
import { useEffect } from "react";

function MyComponent() {
let a = 1;
useEffect(() => {
Expand All @@ -13,53 +15,54 @@ function MyComponent() {
console.log(a);
});
}

```

# Diagnostics
```
customHook.js:3:5 lint/nursery/useExhaustiveDependencies ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
customHook.js:5:5 lint/nursery/useExhaustiveDependencies ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
! This hook do not specify all of its dependencies.
1 │ function MyComponent() {
2 │ let a = 1;
> 3 │ useEffect(() => {
3 │ function MyComponent() {
4 │ let a = 1;
> 5 │ useEffect(() => {
│ ^^^^^^^^^
4 │ console.log(a);
5 │ });
6 │ console.log(a);
7 │ });
i This dependency is not specified in the hook dependency list.
2 │ let a = 1;
3 │ useEffect(() => {
> 4 │ console.log(a);
4 │ let a = 1;
5 │ useEffect(() => {
> 6 │ console.log(a);
│ ^
5 │ });
6 │ useMyEffect(() => {
7 │ });
8 │ useMyEffect(() => {
```

```
customHook.js:6:5 lint/nursery/useExhaustiveDependencies ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
customHook.js:8:5 lint/nursery/useExhaustiveDependencies ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
! This hook do not specify all of its dependencies.
4 │ console.log(a);
5 │ });
> 6 │ useMyEffect(() => {
│ ^^^^^^^^^^^
7 │ console.log(a);
8 │ });
6 │ console.log(a);
7 │ });
> 8 │ useMyEffect(() => {
│ ^^^^^^^^^^^
9 │ console.log(a);
10 │ });
i This dependency is not specified in the hook dependency list.
5 │ });
6 │ useMyEffect(() => {
> 7 │ console.log(a);
│ ^
8 │ });
9 │ }
7 │ });
8 │ useMyEffect(() => {
> 9 │ console.log(a);
│ ^
10 │ });
11 │ }
```
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import { useEffect } from "react";

function MyComponent() {
let a = 1;
useEffect(() => {}, [a]);
Expand Down Expand Up @@ -26,4 +28,4 @@ function MyComponent1() {
useEffect(() => {
console.log(someObj)
}, [someObj.id]);
}
}
Loading

0 comments on commit b644eb6

Please sign in to comment.