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

eslint-plugin-react-hooks should report errors inside unnamed functions #14404

Open
paboulos opened this issue Dec 8, 2018 · 18 comments
Open

Comments

@paboulos
Copy link

paboulos commented Dec 8, 2018

I want to report a bug for the hooks plugin.

What is the current behavior?
There was no error report after running eslint, but the component failed when running in the browser.
From the chrome dev console it reported "Uncaught Error: Rendered fewer hooks than expected. This may be caused by an accidental early return statement."

**If the current behavior is a bug, please provide the steps to reproduce and if possible a minimal demo of the problem. Your bug will get fixed much faster if we can run your code and it doesn't have dependencies other than React.
Here is a link to the github repo:
https://github.com/paboulos/react-hooks-eslint

What is the expected behavior?
Followed The Hooks API guide which says React hooks provides a linter plugin to enforce these rules automatically.Therefore it should have reported a usage violation when the eslint hooks plugin is specified.
Which versions of React, and which browser / OS are affected by this issue? Did this work in previous versions of React?
Using window 10 OS and Chrome browser.

  1. First ran npx create-react-app Hooks
  2. Installed react 16.7.0-alpha.2 and react-dom 16.7.0-alpha.2
  3. Installed eslint dev dependencies:
    "babel-eslint": "9.0.0",
    "babel-loader": "8.0.4",
    "eslint": "5.9.0",
    "eslint-config-airbnb": "17.1.0",
    "eslint-loader": "2.1.1",
    "eslint-plugin-import": "2.14.0",
    "eslint-plugin-jsx-a11y": "6.1.2",
    "eslint-plugin-react": "7.11.1",
    "eslint-plugin-react-hooks": "0.0.0"
  4. Created the .eslintrc.json following the instructions from the Hooks API Doc
    Then ran package script lint as follows: "npm run lint"
    no errors reported.
    Then ran package script start as follows: "npm start"
    The React component CountHooks calls useState incorrectly and reports error in the browser dev console.
@threepointone
Copy link
Contributor

I wasn't able to reproduce this in codesandbox (ie - it shows as a lint error correctly - https://codesandbox.io/s/n4pjo8xvjj).

It also appears you've set up eslint with this rule

 "react-hooks/rules-of-hooks": ["error","never"],

I'm not super familiar with eslint config options, but doesn't "never" mean the rule is disabled?

@oskarv
Copy link

oskarv commented Dec 8, 2018

Yeah, you should try it with config from documentation:
https://reactjs.org/docs/hooks-rules.html#eslint-plugin
// ESLint configuration "rules": { "react-hooks/rules-of-hooks": "error" }
It is slightly different from what you have in your project.

@paboulos
Copy link
Author

paboulos commented Dec 8, 2018

See below, and changed it to "error" as per the npm package doc for hooks, still doesn't work on windows 10 OS.
Are you saying there is no support on windows?
https://www.npmjs.com/package/eslint-plugin-react-hooks says this is what it should be.
{
"plugins": [
// ...
"react-hooks"
],
"rules": {
// ...
"react-hooks/rules-of-hooks": "error"
}
}

And this is what is in my lint config:
{
"plugins": [
"react",
"react-hooks"
],
"extends": [
"react-app",
"airbnb"
],
"rules": {
"react-hooks/rules-of-hooks": "error",
"react/jsx-filename-extension": [1, { "extensions": [".js", ".jsx"] }],
"semi": ["error", "always"],
"quotes": ["error", "single"],
"jsx-quotes": ["error", "prefer-double"],
"react/jsx-one-expression-per-line": "off"
},
"settings": {
"import/ignore": [".css$","node_modules/*"]
}
}

@oskarv
Copy link

oskarv commented Dec 8, 2018

Dumb question: Did you run npm install?

@paboulos
Copy link
Author

paboulos commented Dec 8, 2018

Yes and did it now to be sure: "npm install" and "npm run lint".
Still no problem from eslint reported.

@paboulos
Copy link
Author

paboulos commented Dec 8, 2018

I just cloned the repo to Ubuntu and ran "npm install" and "npm run lint". Also shows no error from eslint.
To confirm eslint detection I then deleted a semicolon. It reports missing semicolon error. So the lint problem is specific to the hooks error only.

@gaearon
Copy link
Collaborator

gaearon commented Jan 18, 2019

We'll need a full reproducing project with exact instructions to reproduce. Thanks.

Sorry, you provided one! I missed it.

@gaearon
Copy link
Collaborator

gaearon commented Jan 18, 2019

Seems like it doesn't work with a definition like this:

export default () => {
  if (condition) {
    useState()
  }
}

or like this:

export default function() {
  if (condition) {
    useState()
  }
}

but works with a named function:

export default function App() {
  if (condition) {
    useState()
  }
}

We strongly encourage you to give name to your components (so that they show up in DevTools and warnings properly). But we should probably fix this.

@oskarv
Copy link

oskarv commented Jan 19, 2019

Great catch!
I was only looking at the config not the actual code.

@Yurickh
Copy link
Contributor

Yurickh commented Jan 23, 2019

The linter as of today is looking at the name of the function to check if it a component or a custom hook.
I'm not really sure if it is ok for it to consider every anonymously exported function a component/custom hook.
I don't think it makes sense to check for hook usage outside of these contexts, either.

@nikek
Copy link

nikek commented Feb 7, 2019

I just stumbled upon this too. And changing the anonymous function to a named one indeed made the linter find the hooks issue i was expecting it to find. TIL: Name components and custom hooks that I export default.

@paboulos
Copy link
Author

paboulos commented Feb 7, 2019

Yes it is a common practice which is why a naming components rule belongs in the linter.

@gaearon gaearon changed the title eslint-plugin-react-hooks doesn't report error when calling Hooks inside conditions eslint-plugin-react-hooks doesn't report error inside unnamed functions Feb 8, 2019
@gaearon gaearon changed the title eslint-plugin-react-hooks doesn't report error inside unnamed functions eslint-plugin-react-hooks should report errors inside unnamed functions Feb 8, 2019
@wouterh
Copy link

wouterh commented May 7, 2019

The same problem arises when you wrap a component in observer from https://github.com/mobxjs/mobx-react-lite.

This is caught as an error:

export const App = () => {
  if(condition) {
    useState()
  }
}

but this isn't:

export const App = observer(() => {
  if(condition) {
    useState()
  }
})

The workaround is to declare it as follows:

const AppBase = () => {
  if(condition) {
    useState()
  }
}

export const App = observer(AppBase)

@thisissami
Copy link

Ah, ok... I'm happy to have found this thread. I was confused! https://stackoverflow.com/questions/56605578/how-can-i-get-eslint-plugin-react-hooks-to-lint-functional-components-that-are

I'll just add names to my default exports then. :)

@stale
Copy link

stale bot commented Jan 10, 2020

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contribution.

@stale stale bot added the Resolution: Stale Automatically closed due to inactivity label Jan 10, 2020
@stale
Copy link

stale bot commented Jan 17, 2020

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!

@rickhanlonii
Copy link
Member

Here's a PR with tests for these cases and others #28147

Related issues:

@upsuper
Copy link

upsuper commented Mar 18, 2024

Two more cases I found:

Component returned from a factory:

function createView() {
  return () => {
    if (Math.random() > 0.5) {
      return null;
    }
    React.useEffect(() => {}, []); // should error but doesn't
    return <div/>;
}

Functional component as a field of class:

class Foo {
  private readonly View = () => {
    if (Math.random() > 0.5) {
      return null;
    }
    React.useEffect(() => {}, []); // should error but doesn't
    return <div />;
  };
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

10 participants