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

Commit

Permalink
support for stable captures
Browse files Browse the repository at this point in the history
  • Loading branch information
@xunilrj
xunilrj committed Oct 10, 2022
1 parent b79bb09 commit eda2ae6
Show file tree
Hide file tree
Showing 6 changed files with 289 additions and 40 deletions.
31 changes: 23 additions & 8 deletions crates/rome_js_analyze/src/semantic_analyzers/nursery/react_extensive_dependencies.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,10 @@ use rome_console::markup;
use rome_js_semantic::{Capture, SemanticModel};
use rome_js_syntax::{
JsAnyCallArgument, JsAnyExpression, JsArrayExpression, JsArrowFunctionExpression,
JsCallExpression, JsSyntaxKind, TextRange,
JsCallExpression, JsSyntaxKind, TextRange, JsIdentifierBinding,
};
use rome_rowan::AstNode;
use std::collections::HashMap;
use rome_rowan::{AstNode, SyntaxNodeCast};
use std::collections::{HashMap, HashSet};
use crate::utils::react::*;

declare_rule! {
Expand All @@ -26,6 +26,17 @@ impl Rule for ReactExtensiveDependencies {
type Signals = Vec<Self::State>;

fn run(ctx: &RuleContext<Self>) -> Vec<Self::State> {
//TODO: move this to config
let stables = HashSet::from_iter([
ReactHookStable::new("useState", Some(1)),
ReactHookStable::new("useReducer", Some(1)),
ReactHookStable::new("useTransition", Some(1)),
ReactHookStable::new("useRef", None),
ReactHookStable::new("useContext", None),
ReactHookStable::new("useId", None),
ReactHookStable::new("useSyncExternalStore", None),
]);

let mut signals = vec![];

let node = ctx.query();
Expand All @@ -49,7 +60,13 @@ impl Rule for ReactExtensiveDependencies {
| TS_ENUM_DECLARATION
| TS_TYPE_ALIAS_DECLARATION
| TS_DECLARE_FUNCTION_DECLARATION => None,
_ => Some(capture)
_ => {
let declaration = declaration.syntax()
.clone()
.cast::<JsIdentifierBinding>()?;
let not_stable = !is_stable_binding(&declaration, &stables);
not_stable.then_some(capture)
}
}
})
})
Expand All @@ -66,9 +83,6 @@ impl Rule for ReactExtensiveDependencies {
})
.unwrap_or_default();

dbg!(&captures.iter().map(|x| &x.0).collect::<Vec<_>>());
dbg!(&deps);

let mut add_deps: HashMap<String, Vec<Capture>> = HashMap::new();

// Search for captures not in the dependency
Expand All @@ -79,8 +93,9 @@ impl Rule for ReactExtensiveDependencies {
}
}

// Search for dependencies not captured
//TODO Search for dependencies not captured

// Generate signals
for (_, captures) in add_deps {
signals.push((range, captures));
}
Expand Down
7 changes: 4 additions & 3 deletions crates/rome_js_analyze/src/utils/react.rs
View file
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use std::collections::HashSet;

use rome_js_syntax::{JsIdentifierBinding, JsVariableDeclarator, JsArrayBindingPatternElementList, JsArrayBindingPattern, SourceType};
use rome_rowan::{SyntaxNodeCast, AstNode};
use rome_rowan::AstNode;

#[derive(PartialEq, Eq, Hash)]
pub struct ReactHookStable {
Expand All @@ -13,7 +13,7 @@ impl ReactHookStable {
pub fn new(hook_name: &str, index: Option<usize>) -> Self { Self { hook_name: hook_name.into(), index } }
}

fn is_react_hook_state(binding: &JsIdentifierBinding, stables: &HashSet<ReactHookStable>) -> bool {
pub fn is_stable_binding(binding: &JsIdentifierBinding, stables: &HashSet<ReactHookStable>) -> bool {
fn array_binding_declarator_index(binding: &JsIdentifierBinding) -> Option<(JsVariableDeclarator, Option<usize>)> {
let index = binding.syntax().index() / 2;
let declarator = binding.parent::<JsArrayBindingPatternElementList>()?
Expand Down Expand Up @@ -51,6 +51,7 @@ fn is_react_hook_state(binding: &JsIdentifierBinding, stables: &HashSet<ReactHoo
#[cfg(test)]
mod test {
use rome_js_parser::FileId;
use rome_rowan::SyntaxNodeCast;
use super::*;

#[test]
Expand All @@ -69,7 +70,7 @@ mod test {
ReactHookStable::new("useState", Some(1))
]);

assert!(is_react_hook_state(&setName, &stables));
assert!(is_stable_binding(&setName, &stables));
}
}

27 changes: 26 additions & 1 deletion .../rome_js_analyze/tests/specs/nursery/reactExtensiveDependencies/missingCapturesInvalid.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,4 +3,29 @@ function MyComponent() {
useEffect(() => {
console.log(local);
});
}
}

// interaction with other react hooks

function MyComponent4() {
const [name, setName] = useState(0);
const [state, dispatch] = useReducer();
const memoizedCallback = useCallback();
const memoizedValue = useMemo();
const deferredValue = useDeferredValue(value);
const [isPending, startTransition] = useTransition();
useEffect(() => {
console.log(name);
setName(1);

console.log(state);
dispatch(1);

console.log(memoizedCallback);
console.log(memoizedValue);
console.log(deferredValue);

console.log(isPending);
startTransition();
}, []);
}
168 changes: 168 additions & 0 deletions ..._js_analyze/tests/specs/nursery/reactExtensiveDependencies/missingCapturesInvalid.js.snap
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,31 @@ function MyComponent() {
console.log(local);
});
}

// interaction with other react hooks

function MyComponent4() {
const [name, setName] = useState(0);
const [state, dispatch] = useReducer();
const memoizedCallback = useCallback();
const memoizedValue = useMemo();
const deferredValue = useDeferredValue(value);
const [isPending, startTransition] = useTransition();
useEffect(() => {
console.log(name);
setName(1);

console.log(state);
dispatch(1);

console.log(memoizedCallback);
console.log(memoizedValue);
console.log(deferredValue);

console.log(isPending);
startTransition();
}, []);
}
```

# Diagnostics
Expand Down Expand Up @@ -37,4 +62,147 @@ missingCapturesInvalid.js:3:5 lint/nursery/reactExtensiveDependencies ━━━
```

```
missingCapturesInvalid.js:17:3 lint/nursery/reactExtensiveDependencies ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
! This useEffect has missing dependencies
15 │ const deferredValue = useDeferredValue(value);
16 │ const [isPending, startTransition] = useTransition();
> 17 │ useEffect(() => {
│ ^^^^^^^^^
18 │ console.log(name);
19 │ setName(1);
i This capture is not in the dependency list
22 │ dispatch(1);
23 │
> 24 │ console.log(memoizedCallback);
│ ^^^^^^^^^^^^^^^^
25 │ console.log(memoizedValue);
26 │ console.log(deferredValue);
```

```
missingCapturesInvalid.js:17:3 lint/nursery/reactExtensiveDependencies ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
! This useEffect has missing dependencies
15 │ const deferredValue = useDeferredValue(value);
16 │ const [isPending, startTransition] = useTransition();
> 17 │ useEffect(() => {
│ ^^^^^^^^^
18 │ console.log(name);
19 │ setName(1);
i This capture is not in the dependency list
16 │ const [isPending, startTransition] = useTransition();
17 │ useEffect(() => {
> 18 │ console.log(name);
│ ^^^^
19 │ setName(1);
20 │
```

```
missingCapturesInvalid.js:17:3 lint/nursery/reactExtensiveDependencies ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
! This useEffect has missing dependencies
15 │ const deferredValue = useDeferredValue(value);
16 │ const [isPending, startTransition] = useTransition();
> 17 │ useEffect(() => {
│ ^^^^^^^^^
18 │ console.log(name);
19 │ setName(1);
i This capture is not in the dependency list
24 │ console.log(memoizedCallback);
25 │ console.log(memoizedValue);
> 26 │ console.log(deferredValue);
│ ^^^^^^^^^^^^^
27 │
28 │ console.log(isPending);
```

```
missingCapturesInvalid.js:17:3 lint/nursery/reactExtensiveDependencies ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
! This useEffect has missing dependencies
15 │ const deferredValue = useDeferredValue(value);
16 │ const [isPending, startTransition] = useTransition();
> 17 │ useEffect(() => {
│ ^^^^^^^^^
18 │ console.log(name);
19 │ setName(1);
i This capture is not in the dependency list
19 │ setName(1);
20 │
> 21 │ console.log(state);
│ ^^^^^
22 │ dispatch(1);
23 │
```

```
missingCapturesInvalid.js:17:3 lint/nursery/reactExtensiveDependencies ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
! This useEffect has missing dependencies
15 │ const deferredValue = useDeferredValue(value);
16 │ const [isPending, startTransition] = useTransition();
> 17 │ useEffect(() => {
│ ^^^^^^^^^
18 │ console.log(name);
19 │ setName(1);
i This capture is not in the dependency list
26 │ console.log(deferredValue);
27 │
> 28 │ console.log(isPending);
│ ^^^^^^^^^
29 │ startTransition();
30 │ }, []);
```

```
missingCapturesInvalid.js:17:3 lint/nursery/reactExtensiveDependencies ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
! This useEffect has missing dependencies
15 │ const deferredValue = useDeferredValue(value);
16 │ const [isPending, startTransition] = useTransition();
> 17 │ useEffect(() => {
│ ^^^^^^^^^
18 │ console.log(name);
19 │ setName(1);
i This capture is not in the dependency list
24 │ console.log(memoizedCallback);
> 25 │ console.log(memoizedValue);
│ ^^^^^^^^^^^^^
26 │ console.log(deferredValue);
27 │
```


38 changes: 35 additions & 3 deletions crates/rome_js_analyze/tests/specs/nursery/reactExtensiveDependencies/valid.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,15 +13,47 @@ function MyComponent2() {
}

// capturing declarations
import { F } from 'something';

function doSomething() { }
class A {}

function MyComponent3() {
useEffect(() => {
doSomething();
console.log(new A ());
console.log(F);
}, []);
}

// interaction with other react hooks

function MyComponent4() {
const [name, setName] = useState(0);
const ref = useRef();
const theme = useContext();
const [state, dispatch] = useReducer();
const memoizedCallback = useCallback();
const memoizedValue = useMemo();
const deferredValue = useDeferredValue(value);
const [isPending, startTransition] = useTransition();
const id = useId();
const externalStore = useSyncExternalStore();
useEffect(() => {
console.log(name);
setName(1);

console.log(ref);

console.log(theme);

console.log(state);
dispatch(1)

memoizedCallback();

console.log(isPending);
startTransition();

console.log(id);

console.log(externalStore);
}, [name, state, memoizedCallback, memoizedValue, deferredValue, isPending]);
}
Loading

0 comments on commit eda2ae6

Please sign in to comment.