Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bug: react-hooks/exhaustive-deps does not accept readonly arrays as deps #25844

Closed
milichev opened this issue Dec 7, 2022 · 0 comments · Fixed by #28189
Closed

Bug: react-hooks/exhaustive-deps does not accept readonly arrays as deps #25844

milichev opened this issue Dec 7, 2022 · 0 comments · Fixed by #28189
Labels
Component: ESLint Rules Status: Unconfirmed A potential issue that we haven't yet confirmed as a bug

Comments

@milichev
Copy link

milichev commented Dec 7, 2022

React version:

react: 16.8.6
eslint-plugin-react-hooks: 4.2.0

Steps To Reproduce

  1. Properly configure the react-hooks/exhaustive-deps ESLint rule
  2. In your code, pass the deps argument as read-only array literal:
const useExample = ({ value, callback }: { value: number; callback: (value: number) => void }) => {
    useEffect(
        () => {
            callback(value);
        },
        // In the next line, the const assertion makes the react-hooks/exhaustive-deps rule to emit an error
        [value, callback] as const
    );
};

The current behavior

ESLint emits an error: React Hook useEffect was passed a dependency list that is not an array literal. This means we can't statically verify whether you've passed the correct dependencies. (react-hooks/exhaustive-deps)

At the same time, useEffect hook itself accepts read-only arrays:

// node_modules/@types/react/index.d.ts
type DependencyList = ReadonlyArray<any>;
function useEffect(effect: EffectCallback, deps?: DependencyList): void;

The tsc with rather strict compilation options success too.

The expected behavior

The react-hooks/exhaustive-deps rule should accept readonly array literals as well.

Rationale

A strongly typed deps parameter is used in a custom hook for type inference.

image

I'm going to take a look myself once I have a slack time (it should not be complex: something like checking the isReadonly flag of the array expression node), but would appreciate any input or contribution.

@milichev milichev added the Status: Unconfirmed A potential issue that we haven't yet confirmed as a bug label Dec 7, 2022
eps1lon pushed a commit that referenced this issue Feb 1, 2024
…ray (#28189)

## Summary

This PR closes #25844
The original issue talks about `as const`, but seems like it fails for
any `as X` expressions since it adds another nesting level to the AST.

EDIT: Also closes #20162

## How did you test this change?

Added unit tests
github-actions bot pushed a commit that referenced this issue Feb 1, 2024
…ray (#28189)

## Summary

This PR closes #25844
The original issue talks about `as const`, but seems like it fails for
any `as X` expressions since it adds another nesting level to the AST.

EDIT: Also closes #20162

## How did you test this change?

Added unit tests

DiffTrain build for [a1433ca](a1433ca)
gaearon pushed a commit that referenced this issue Feb 3, 2024
…ray (#28189)

## Summary

This PR closes #25844
The original issue talks about `as const`, but seems like it fails for
any `as X` expressions since it adds another nesting level to the AST.

EDIT: Also closes #20162

## How did you test this change?

Added unit tests
EdisonVan pushed a commit to EdisonVan/react that referenced this issue Apr 15, 2024
…ray (facebook#28189)

## Summary

This PR closes facebook#25844
The original issue talks about `as const`, but seems like it fails for
any `as X` expressions since it adds another nesting level to the AST.

EDIT: Also closes facebook#20162

## How did you test this change?

Added unit tests
bigfootjon pushed a commit that referenced this issue Apr 18, 2024
…ray (#28189)

## Summary

This PR closes #25844
The original issue talks about `as const`, but seems like it fails for
any `as X` expressions since it adds another nesting level to the AST.

EDIT: Also closes #20162

## How did you test this change?

Added unit tests

DiffTrain build for commit a1433ca.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Component: ESLint Rules Status: Unconfirmed A potential issue that we haven't yet confirmed as a bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants