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: rules-of-hooks violation not caught by eslint plugin #22520

Closed
pke opened this issue Oct 6, 2021 · 11 comments
Closed

Bug: rules-of-hooks violation not caught by eslint plugin #22520

pke opened this issue Oct 6, 2021 · 11 comments
Labels
Component: ESLint Rules Resolution: Stale Automatically closed due to inactivity Status: Unconfirmed A potential issue that we haven't yet confirmed as a bug

Comments

@pke
Copy link

pke commented Oct 6, 2021

Given the following code:

const createHandler = () => (event) => {
  const { t } = useTranslation()

  Alert.alert(t("title"), t("message"))
}

function MyComponent() {
  return <Button onPress={createHandler()}/>
}

the rules-of-hooks linter rule is not triggered for the createHandler as it should.
The linter rule should detect its not a react component, since it does not return any ReactElement or null.

@pke pke added the Status: Unconfirmed A potential issue that we haven't yet confirmed as a bug label Oct 6, 2021
@stale
Copy link

stale bot commented Jan 8, 2022

This issue has been automatically marked as stale. If this issue is still affecting you, please leave any comment (for example, "bump"), and we'll keep it open. We are sorry that we haven't been able to prioritize it yet. If you have any new additional information, please include it with your comment!

@stale stale bot added the Resolution: Stale Automatically closed due to inactivity label Jan 8, 2022
@dcheng1290
Copy link

@pke did you resolve this issue by any chance?

@stale stale bot removed the Resolution: Stale Automatically closed due to inactivity label Feb 22, 2022
@pke
Copy link
Author

pke commented Feb 22, 2022

No

@eps1lon
Copy link
Collaborator

eps1lon commented Feb 23, 2022

This may have been an intentional case that's missed so existing code isn't falsely flagged. Assuming that any use* function is a hook, the plugin would trigger some false-positives in the ecosystem. So the plugin is probably assuming here that useTranslate is not a hook because it's neither called in another hook nor directly during render. Tracking createHandler usage would be really hard to accomplish.

Consider e.g. useFakeTimers in sinon. If this would be considered a hook by the plugin, people would probably disable it due to the noise it creates.

Maybe @gaearon can confirm this is intentional.

Does the plugin flag usage of built-in hooks inside createHandler?

@dcheng1290
Copy link

Makes sense regarding existing code getting falsely flagged. I was wondering if the plugin currently has a rule for us to enable in order to capture this case. Thanks!

@eduardodallmann
Copy link

I simply import like this: import { useTranslation as translation } from 'react-i18next';

@pke
Copy link
Author

pke commented Jan 19, 2023

How is that helping solving the bug shown in this issue?

@eduardodallmann
Copy link

Sorry, I understood the problem to be an eslint warning in the "react-hooks/rules-of-hooks" rule for using a use* hook outside of a react component. Then I tricked eslint by renaming the import. My mistake.

rickhanlonii added a commit that referenced this issue Jan 29, 2024
These though fail.

Anon default export: #21181
Promise callbacks: #26186
Returning anon functions: #22520
gaearon pushed a commit that referenced this issue Feb 3, 2024
These though fail.

Anon default export: #21181
Promise callbacks: #26186
Returning anon functions: #22520
Copy link

This issue has been automatically marked as stale. If this issue is still affecting you, please leave any comment (for example, "bump"), and we'll keep it open. We are sorry that we haven't been able to prioritize it yet. If you have any new additional information, please include it with your comment!

@github-actions github-actions bot added the Resolution: Stale Automatically closed due to inactivity label Apr 10, 2024
EdisonVan pushed a commit to EdisonVan/react that referenced this issue Apr 15, 2024
These though fail.

Anon default export: facebook#21181
Promise callbacks: facebook#26186
Returning anon functions: facebook#22520
Copy link

Closing this issue after a prolonged period of inactivity. If this issue is still present in the latest release, please create a new issue with up-to-date information. Thank you!

@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale Apr 17, 2024
@scamden
Copy link

scamden commented Apr 26, 2024

this was closed but this is still definitely an issue:

for example when using jotai it's really easy for a dev to accidentally think they can use a hook and it's not caught:

export const atomBiSavedViewExamples = atom(
  async (get) => {
   // no error here..
    const showWatershedSavedViewTemplates = useFeatureFlag(
      'myFlag'
    );
   //...etc
});

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

No branches or pull requests

5 participants