* [eslint-plugin-react-cooks] Report constant constructions
The dependency array passed to a React hook can be thought of as a list of cache keys. On each render, if any dependency is not `===` its previous value, the hook will be rerun. Constructing a new object/array/function/etc directly within your render function means that the value will be referentially unique on each render. If you then use that value as a hook dependency, that hook will get a "cache miss" on every render, making the dependency array useless.
This can be especially dangerous since it can cascade. If a hook such as `useMemo` is rerun on each render, not only are we bypassing the option to avoid potentially expensive work, but the value _returned_ by `useMemo` may end up being referentially unique on each render causing other downstream hooks or memoized components to become deoptimized.
* Fix/remove existing tests
* Don't give an autofix of wrapping object declarations
It may not be safe to just wrap the declaration of an object, since the object may get mutated.
Only offer this autofix for functions which are unlikely to get mutated.
Also, update the message to clarify that the entire construction of the value should get wrapped.
* Handle the long tail of nodes that will be referentially unique
* Catch let/var constant constructions on initial assignment
* Trim trailing whitespace
* Address feedback from @gaearon
* Rename "assignment" to "initialization"
* Add test for a constant construction used in multiple dependency arrays
* Rename internal variables
This disambiguates "optional"/"required" because that terminology is taken by optional chaining.
* Handle optional member chains
* Update comment
Co-authored-by: Ricky <rickhanlonii@gmail.com>
Co-authored-by: Ricky <rickhanlonii@gmail.com>
* Revert "Fix ExhaustiveDeps ESLint rule throwing with optional chaining (#19260)"
This reverts commit 0f84b0f02b.
* Re-add a test from #19260
* Remove all code for optional chaining support
* Consistently treat optional chaining as regular chaining
This is not ideal because our suggestions use normal chaining. But it gets rid of all current edge cases.
* Add more tests
* More consistency in treating normal and optional expressions
* Add regression tests for every occurrence of Optional*
Certain code patterns using optional chaining syntax causes
eslint-plugin-react-hooks to throw an error.
We can avoid the throw by adding some guards. I didn't read through the
code to understand how it works, I just added a guard to every place
where it threw, so maybe there is a better fix closer to the root cause
than what I have here.
In my test case, I noticed that the optional chaining that was used in
the code was not included in the suggestions description or output,
but it seems like it should be. This might make a nice future
improvement on top of this fix, so I left a TODO comment to that effect.
Fixes#19243
* [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>
* Add test cases for support exhaustive deps ending in Effect
* Apply the exhaustive deps lint rule to any hook ending with Effect
* Add another test for supporting linting useXEffect hooks
Co-authored-by: Aaron Pettengill <aaron.pettengill@echoman.com>
* Enable prefer-const rule
Stylistically I don't like this but Closure Compiler takes advantage of
this information.
* Auto-fix lints
* Manually fix the remaining callsites
* Refactor: visit CallExpression
Instead of visiting the functions and looking up to see if they're in a Hook call, visit Hook calls and look down to see if there's a callback inside. I will need this refactor so I can visit functions declared outside the call.
* Check deps when callback body is outside the Hook call
* Handle the unknown case
Per discussion at Facebook, we think hooks have reached a tipping point where it is more valuable to lint against potential hooks in classes than to worry about false positives.
Test plan:
```
# run from repo root
yarn test --watch RuleOfHooks
```
* Move remaining things to named exports
The interesting case here is the noop renderers. The wrappers around the
reconciler now changed to use a local export that gets mutated.
ReactNoop and ReactNoopPersistent now have to destructure the object to
list out the names it's going to export. We should probably refactor
ReactNoop away from createReactNoop. Especially since it's also not Flow
typed.
* Switch interactions to star exports
This will have esModule compatibility flag on them. They should ideally
export default instead.
* [eslint-plugin-react-hooks] Fix cyclic caching for loops containing a condition
* [eslint-plugin-react-hooks] prettier write
* [eslint-plugin-react-hooks] Fix set for tests
* Update packages/eslint-plugin-react-hooks/src/RulesOfHooks.js
Co-Authored-By: Luke Kang <kidkkr@icloud.com>
Co-authored-by: Luke Kang <kidkkr@icloud.com>
Dan Abramov
Dan Abramov
CY LimandDan Abramov
Jordan Eldredge
Bianca Del Carretto
Joe Lencioni
Fred Vollmer
YeonJuan
Julien DeniauandDan Abramov
Yann アウネ Eves
Boris SergeevandBoris Sergeyev
Michaël De Boey
Rohith Srivathsav
Kevin Lewis
Benedikt Franke
Chang Yan
Sophie Alpert
Zen
Gabriel McAdamsandDan Abramov
Sebastian Markbåge
Andrew Clark
Ian Obermiller
Simen Bekkhus
Sunil Pai
Moji IzadmehrandLuke Kang