eslint-plugin-query: exhaustive-deps error not triggered when dependency is nested inside then/catch

[DevHusariaSolutions]

Describe the bug

exhaustive-deps triggers only when we use some dependency in non-nested code, but while using then/catch - it cannot detect used dependency.

Your minimal, reproducible example

Steps to reproduce

If we nest it in then/catch callbacks, error is not showing. I know we can use "t" outside whole useQuery hook, it's just for demo. Any other function which would be part of fetchable logic won't trigger error.

Expected behavior

Error should be triggered like in example when we use "t" dependency outside then/catch clauses:

How often does this bug happen?

Every time

Screenshots or Videos

No response

Platform

Windows, VSCode

Tanstack Query adapter

react-query

TanStack Query version

5.14.0

TypeScript version

5.1.3

Additional context

Version of eslint-plugin-query: 5.18.1

[TkDodo]

@Newbie012 can you take a look please?

[IgorAufricht]

The same happens when there is a condition:

  return useSuspenseQuery({
    queryKey: [actionId], // <- eslint doesn't complain about missing baseUrl
    queryFn: () => {
      //const test = baseUrl; // <- if we uncomment this, eslint will start to complain about missing baseUrl in the queryKey
      if (actionId) {
        return baseUrl;
      }
    },
  });

This makes the rule ineffective for many non-trivial cases.

[Newbie012]

Sorry, I totally forgot about this issue.

@TkDodo I can implement that, but it would be a breaking change. Currently, we allow call expressions inside queryFn without explicitly set them in queryKey. For instance, given the following code:

function Component({ api }) {
  const t = useHook();

  useQuery({
    queryKey: ['test'],
    queryFn: () => api.get().then(() => t("foo"))
  })
}

the new behavior will require to pass both t and api.get (or api).

thoughts?

[TkDodo]

I think both of these should error because you can easily run into stale closure issues. However, adding them to the queryKey is not ideal because they likely aren't serializable.

this is again where we either need to support defined exceptions for the rule, or users would have to disable the linter or rewrite their code. It is technically unsafe to do this.

[Newbie012]

So problem still persists. What's the point of passing a value that is not serializable into a query key? you would still get stale closures, since JSON.stringify(fn) would result in undefined if it doesn't have a proper toJSON.

[BenStirrup]

Hello 👋 I would like to inquire if a fix is planned for the non-functioning behavior of the plugin within try/catch/if blocks ?

[MartinDavi]

I think this might the same issue, but unrelated to try / catch, let me know if I should open a separate ticket.

This doesn't warn about the missing searchValue dependency:

useQuery({
    queryKey: ['someQuery'],
    queryFn: searchValue ? () => API.someMethod(searchValue) : skipToken,
  });

This does:

useQuery({
    queryKey: ['someQuery'],
    queryFn: () => API.someMethod(searchValue),
  });