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] Bug: warning on useWithoutEffectSuffix #18902

Closed
surgeboris opened this issue May 13, 2020 · 2 comments
Closed

[eslint-plugin-react-hooks] Bug: warning on useWithoutEffectSuffix #18902

surgeboris opened this issue May 13, 2020 · 2 comments

Comments

@surgeboris
Copy link
Contributor

surgeboris commented May 13, 2020

With eslint-plugin-react-hooks@4.0.1 I observe an unexpected warning
on useWithoutEffectSuffix hook call.

Steps To Reproduce

  1. yarn create react-app hooks-bug
  2. cd hooks-bug
  3. npm run eject
  4. npm i eslint-plugin-react-hooks@4.0.1
  5. place code from the examples below into src/App.js
  6. run npx eslint src/App.js
  7. check output for warnings mentioned below

Code example:

function App(props) {
  useWithoutEffectSuffix(props, {});
  return null;
}

function useWithoutEffectSuffix() {}

export default App;

npx eslint src/App.js

./src/App.js
2:3 warning React Hook useWithoutEffectSuffix has a missing dependency: 'props'. Either include it or remove the dependency array react-hooks/exhaustive-deps

✖ 1 problem (0 errors, 1 warning)

The current behavior

react-hooks/exhaustive-deps warning triggered on a custom hook which contains
-Effect- in the middle of it's name (but does NOT contain it as a suffix).

The expected behavior

I would expect react-hooks/exhaustive-deps to succeed without warning on
a custom hook which contains -Effect- in the middle of it's name (but does NOT
contain it as a suffix).

I was under impression that only -Effect suffix should be reserved because of
the following quotes:

I think ideally we'd reserve Effect suffix for things that mirror the dependency API

@gaearon's reply on another issue

New Violations: Check dependencies for all Hooks ending with Effect.

changelog entry

In case I got something wrong and the plan is to reserve Effect word
completely (anywhere in custom hook's name), then we at least need to correct
Changelog accordingly.

@surgeboris surgeboris added the Status: Unconfirmed A potential issue that we haven't yet confirmed as a bug label May 13, 2020
@surgeboris surgeboris changed the title [eslint-plugin-react-hooks] Bug: warning on useWithouEffectSuffix [eslint-plugin-react-hooks] Bug: warning on useWithoutEffectSuffix May 13, 2020
@gaearon
Copy link
Collaborator

gaearon commented May 13, 2020

Would you like to submit a failing test case and/or a fix? You can search for ExhaustiveDeps-test.js in the codebase. That would be very helpful!

@gaearon gaearon added Component: ESLint Rules Type: Bug and removed Status: Unconfirmed A potential issue that we haven't yet confirmed as a bug labels May 13, 2020
surgeboris pushed a commit to surgeboris/react that referenced this issue May 13, 2020
…ebook#18902)

Since we only reserve `-Effect` suffix, react-hooks/exhaustive-deps is
expected to succeed without warning on a custom hook which contains -Effect- in
the middle of it's name (but does NOT contain it as a suffix).
gaearon pushed a commit that referenced this issue May 13, 2020
* [eslint-plugin-react-hooks] reproduce bug with a test and fix it (#18902)

Since we only reserve `-Effect` suffix, react-hooks/exhaustive-deps is
expected to succeed without warning on a custom hook which contains -Effect- in
the middle of it's name (but does NOT contain it as a suffix).

* [eslint-plugin-react-hooks] reproduced bug with a test and fix it

Since we only reserve `-Effect` suffix, react-hooks/exhaustive-deps is expected
to succeed without warning on a render helper which contains -use- in the middle
of it's name (but does NOT contain it as a prefix, since that would violate hook
naming convetion).

Co-authored-by: Boris Sergeyev <boris.sergeyev@quolab.com>
@gaearon
Copy link
Collaborator

gaearon commented May 13, 2020

Appreciate you did the work! I was feeling a bit tired today and this was really nice.

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

2 participants