let react-hooks/exhaustive-deps additionalHooks have a callback in a different position from 0

[joeflateau]

Summary

This pull request allows Hooks specified in react-hooks/exhaustive-deps additionalHooks have a callback parameter in a position other than 0.

In my case, I have a hook where the callback is in position 1 rather than 0:

function useDerrivedState<T>(initialState:T,  asyncEffect: () =>  Promise<T>, deps: DependencyList) {
  const [state, setState] = useState(initialState);
  useEffect(() => asyncEffect().then(result => setState(result)), deps);
  return [state];
}

(simplified version, real version handles aborts/errors/etc but not relevant to this change)

I could refactor the hook and put initialState after the callback/deps (at pos 2 rather than 0) but I feel like it reads out of order/more difficult to reason about.

How did you test this change?

Added valid/invalid tests. Ran tests with yarn test --watch ESLintRuleExhaustiveDeps. Existing and new tests pass.

[sizebot]

Comparing: df5d32f...2092c58

Critical size changes

Includes critical production bundles, as well as any change greater than 2%:

Name	+/-	Base	Current	+/- gzip	Base gzip	Current gzip
oss-stable/react-dom/cjs/react-dom.production.min.js	=	131.21 kB	131.21 kB	=	41.98 kB	41.98 kB
oss-experimental/react-dom/cjs/react-dom.production.min.js	=	136.47 kB	136.47 kB	=	43.57 kB	43.57 kB
facebook-www/ReactDOM-prod.classic.js	=	434.63 kB	434.63 kB	=	79.87 kB	79.86 kB
facebook-www/ReactDOM-prod.modern.js	=	419.63 kB	419.63 kB	=	77.50 kB	77.50 kB
facebook-www/ReactDOMForked-prod.classic.js	=	434.63 kB	434.63 kB	=	79.87 kB	79.87 kB

Significant size changes

Includes any change greater than 0.2%:

Expand to show

Name	+/-	Base	Current	+/- gzip	Base gzip	Current gzip
oss-experimental/eslint-plugin-react-hooks/cjs/eslint-plugin-react-hooks.production.min.js	+1.28%	25.63 kB	25.96 kB	+0.35%	8.78 kB	8.81 kB
oss-stable-semver/eslint-plugin-react-hooks/cjs/eslint-plugin-react-hooks.production.min.js	+1.28%	25.63 kB	25.96 kB	+0.35%	8.78 kB	8.81 kB
oss-stable/eslint-plugin-react-hooks/cjs/eslint-plugin-react-hooks.production.min.js	+1.28%	25.63 kB	25.96 kB	+0.35%	8.78 kB	8.81 kB
oss-experimental/eslint-plugin-react-hooks/cjs/eslint-plugin-react-hooks.development.js	+1.14%	87.43 kB	88.43 kB	+1.04%	20.77 kB	20.98 kB
oss-stable-semver/eslint-plugin-react-hooks/cjs/eslint-plugin-react-hooks.development.js	+1.14%	87.43 kB	88.43 kB	+1.04%	20.77 kB	20.98 kB
oss-stable/eslint-plugin-react-hooks/cjs/eslint-plugin-react-hooks.development.js	+1.14%	87.43 kB	88.43 kB	+1.04%	20.77 kB	20.98 kB

Generated by 🚫 dangerJS against 2092c58

[github-actions]

This pull request has been automatically marked as stale. If this pull request is still relevant, please leave any comment (for example, "bump"), and we'll keep it open. We are sorry that we haven't been able to prioritize reviewing it yet. Your contribution is very much appreciated.

[ahoisl]

Would be great to have this merged...

[bjarkebech]

+1

[DetweilerRyan]

+1

[github-actions]

This pull request has been automatically marked as stale. If this pull request is still relevant, please leave any comment (for example, "bump"), and we'll keep it open. We are sorry that we haven't been able to prioritize reviewing it yet. Your contribution is very much appreciated.

[ahoisl]

bump

[cyrilchapon]

I have what I believe to be a very legit use case.
I'm using ts-rest query client which is a typed wrapper around react-query (saying that because it's important for the following).

My custom hook produces a queryKey and queryData with a "factory" similar to useMemo

export const useSkippableQueryData = <TAppRoute extends AppRoute>(
  _route: TAppRoute,
  mainObjectName: ApiModelType,
  queryDataFactory: () => SkippableQueryData<TAppRoute>,
  dependencies: DependencyList,
): [QueryKey, SkippableQueryData<TAppRoute>] => {
  const [queryKey, queryData] = useMemo<
    [QueryKey, SkippableQueryData<TAppRoute>]
  >(() => {
    const queryData = queryDataFactory();

    const queryKey: QueryKey = [
      mainObjectName,
      _route.method,
      _route.path,
      queryData,
    ];

    return [queryKey, queryData];
    // eslint-disable-next-line react-hooks/exhaustive-deps
  }, [...dependencies, _route, mainObjectName, queryDataFactory]);

  return [queryKey, queryData];
};

The important thing is I basically can't refactor this to put the callback in first position, due to how this is used :

const [queryKey, queryData] = useSkippableQueryData(
  contract.listCompanyExpense,
  ApiModel.Expense,
  () => ({
    // This is typed, thanks to the contract stuff above
    params: { companyId: myCompanyId }
  }),
  [myCompanyId]
)

I deadly want my devs not to forget any dependency as it's critical in there; but in such a case it would require to declare useSkippableQueryData and also say the callback is in third position.

[trikadin]

please merge this pr 🙏

[kttmv]

+1 please merge this

[github-actions]

This pull request has been automatically marked as stale. If this pull request is still relevant, please leave any comment (for example, "bump"), and we'll keep it open. We are sorry that we haven't been able to prioritize reviewing it yet. Your contribution is very much appreciated.

[ahoisl]

Bump, still relevant in my opinion

[danielrentz]

May I suggest a much simpler syntax for this option? (I always wondered why the original option is a regex instead of a string array...)

additionalHooks: {
  useMyHook: 0,
  useMyOtherHook: 1,
}

This is how the (now unmaintained) plugin https://github.com/stoikio/eslint-plugin-react-hooks-static-deps does it

[github-actions]

This pull request has been automatically marked as stale. If this pull request is still relevant, please leave any comment (for example, "bump"), and we'll keep it open. We are sorry that we haven't been able to prioritize reviewing it yet. Your contribution is very much appreciated.

[github-actions]

Closing this pull request after a prolonged period of inactivity. If this issue is still present in the latest release, please ask for this pull request to be reopened. Thank you!

[trikadin]

this issue is still present AND relevant. Please fix it 🙏

[cyrilchapon]

Go biome. The project is maintained and the rule is still greatly configurable.
https://biomejs.dev/linter/rules/use-exhaustive-dependencies/