🐛 useExhaustiveDependencies doesn't support function syntax, or React.use* hooks

[Amnesthesia]

Environment information

yarn run v1.22.17
$ /Users/victor/Development/app/node_modules/.bin/rome rage
CLI:
  Version:                      12.0.0
  Color support:                true

Platform:
  CPU Architecture:             aarch64
  OS:                           macos

Environment:
  ROME_LOG_DIR:                 unset
  NO_COLOR:                     unset
  TERM:                         "xterm-256color"
  JS_RUNTIME_VERSION:           "v18.1.0"
  JS_RUNTIME_NAME:              "node"
  NODE_PACKAGE_MANAGER:         "yarn/1.22.17"

Rome Configuration:
  Status:                       Loaded successfully
  Formatter disabled:           false
  Linter disabled:              false
  Organize imports disabled:    true

Workspace:
  Open Documents:               0

Discovering running Rome servers...

Incompatible Rome Server: ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━

i Rage discovered this running server using an incompatible version of Rome.

Server:
  Version:                      <=10.0.0

Server:
  Status:                       stopped

Incompatible Rome Server: ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━

i Rage discovered this running server using an incompatible version of Rome.

Server:
  Version:                      11.0.0-nightly.763fd98

Done in 0.55s.

What happened?

1. useExhaustiveDependencies doesn't understand where a hook is coming from and only reads by name

lint/nursery/useExhaustiveDependencies will not recognize stable hooks if those hooks are not imported as import { useState } from 'react'

For example, import * as React from 'react' and then React.useState(), will not be treated as a hook with a stable callback in index position 1. I have tried to figure out if I can pass React.useState to options.stables, but this option has been removed in recent versions (and it didn't work when I tried on the versions where it existed)

2. useExhaustiveDependencies doesn't understand named functions

Another issue related to this is that if you declare a useCallback like this:

const cb = useCallback(
  async function NamedFunctionForBetterDebugging() {
    // Anything in here will be ignored and rome will think ALL dependencies can be removed
  },
  [dep1, dep2]
)

Then Rome will advise you to remove all dependencies

3. useExhaustiveDependencies doesn't understand multiple hooks with the same name and different parameters

We have a custom useMemo hook that allows passing in an optional custom isEqual argument as a third argument, and rome will crash trying to parse this because it doesn't look like what it expects a hook named 'useMemo' should look like

✖ processing panicked: index out of bounds: the len is 2 but the index is 2

4. useExhaustiveDependencies is not very good at parsing optional chaining

If a hook does something like this:

useEffect(() => {
   if (selectedArticle?.redirectUrl) {
      redirect(selectedArticle.redirectUrl);
   }
}, [selectedArticle?.redirectUrl])

Then selectedArticle.redirectUrl will receive a complaint with:

 ℹ This dependency is not specified in the hook dependency list.

Because this rule doesn't understand that selectedArticle?.redirectUrl is the same as selectedArticle.redirectUrl, it was just checked for whether or not it was undefined

useEffect is sometimes not checked, or ignored?

I was using a component like this for testing this rule, removing dependencies I know it should complain about and adding extras as well:

export default function Animation(props: IAnimationProps) {
  const { json, size, progress, playing, loop = true, onComplete, onStart } = props;

  const ref = useRef<Lottie>(null);
  const [isPlaying, setPlaying] = useState(playing);
  const options: AnimatedLottieViewProps = useMemo(
    () => ({
      source: json,
      autoplay: true,
      loop
    }),
    [json, loop]
  );

  useEffect(() => {

    if (isPlaying && !playing) {
      setPlaying(false);
    }
  }, [isPlaying]); // Rome should complain about missing dependency: playing, but it doesn't?

Expected result

I would expect Rome to treat any function declaration in a hook as a function regardless of whether its a named or anonymous function and whether or not its an arrow function.

I would also expect rome to understand where a hook is coming from and not just parse the name, e.g useState, but understand that React.useState is the same as useState and import { useState as useWhatever } from 'react' is still useState

I'm not sure if this is possible with how rome parses the AST tree, if this is just a bad linting rule, or if rome is really that opinionated on how to write code?

Code of Conduct

• I agree to follow Rome's Code of Conduct

[ematipico]

cc @rome/core-contributors

[nissy-dev]

I'll take this issue.

[kendallroth]

Adding to this, this rule does not warn when dependencies are defined after the dependency check/array.

Does not trigger warning (dependency defined after useEffect)

useEffect(() => {
  if (!user) return;
  updateUserForm(user);
}, [user]);

const updateUserForm = (updatedUser: any) => {
  userForm.reset({ ... });
};

Triggers warning (dependency defined before useEffect)

const updateUserForm = (updatedUser: any) => {
  userForm.reset({ ... });
};

useEffect(() => {
  if (!user) return;
  updateUserForm(user);
}, [user]);

[nissy-dev]

I think the issues related to the original comment #4330 (comment) were fixed by #4571. Now, I'm looking into #4330 (comment)

Note: I seem that the following lines are related to the issuse.

tools/crates/rome_js_analyze/src/react/hooks.rs

Lines 62 to 63 in 60ee106

	.flat_map(|closure| closure.all_captures())
	.filter(move |capture| capture.declaration_range().intersect(range).is_none())

tools/crates/rome_js_semantic/src/semantic_model/closure.rs

Lines 130 to 164 in 60ee106

	fn next(&mut self) -> Option<Self::Item> {
	'references: loop {
	while let Some(reference) = self.references.pop() {
	let binding = &self.data.bindings[reference.binding_id];
	if self.closure_range.intersect(binding.range).is_none() {
	let reference = &binding.references[reference.reference_id];
	return Some(Capture {
	data: self.data.clone(),
	node: self.data.node_by_range[&reference.range].clone(), // TODO change node to store the range
	ty: CaptureType::ByReference,
	binding_id: binding.id,
	});
	}
	}
	'scopes: while let Some(scope_id) = self.scopes.pop() {
	let scope = &self.data.scopes[scope_id];
	if scope.is_closure {
	continue 'scopes;
	} else {
	self.references.clear();
	self.references
	.extend(scope.read_references.iter().cloned());
	self.references
	.extend(scope.write_references.iter().cloned());
	self.scopes.extend(scope.children.iter());
	continue 'references;
	}
	}
	return None;
	}
	}
	}