This rule was a leftover from a while ago and doesn't actually lint
anything useful. Specifically, you get a lint error if you try to opt
out a component that isn't already bailing out. If there's a bailout the
compiler already safely skips over it, so adding `'use no memo'` there
is unnecessary.
Fixes#31407
---
[//]: # (BEGIN SAPLING FOOTER)
Stack created with [Sapling](https://sapling-scm.com). Best reviewed
with [ReviewStack](https://reviewstack.dev/facebook/react/pull/34703).
* __->__ #34703
* #34700
Previously, the `recommended` config used the legacy ESLint format
(plugins as an array of strings). This causes errors when used with
ESLint v9's `defineConfig()` helper. This was following [eslint's own
docs](https://eslint.org/docs/latest/extend/plugins#backwards-compatibility-for-legacy-configs):
> With this approach, both configuration systems recognize
"recommended". The old config system uses the recommended key while the
current config system uses the flat/recommended key. The defineConfig()
helper first looks at the recommended key, and if that is not in the
correct format, it looks for the flat/recommended key. This allows you
an upgrade path if you’d later like to rename flat/recommended to
recommended when you no longer need to support the old config system.
However,
[`isLegacyConfig()`](https://github.com/eslint/rewrite/blob/main/packages/config-helpers/src/define-config.js#L73-L81)
(also see
[`eslintrcKeys`](https://github.com/eslint/rewrite/blob/main/packages/config-helpers/src/define-config.js#L24-L35))
function doesn't check for the `plugins` key, so our config was
incorrectly treated as flat config despite being in legacy format.
This PR fixes the issue, along with a few other fixes combined:
1. Convert `recommended` to flat config format
2. Separate basic rules (exhaustive-deps, rules-of-hooks) from compiler
rules
3. Add `recommended-latest-legacy` config for non-flat config users who
want all recommended rules (including compiler rules)
4. Adding more types for the exported config
Our shipped presets in 6.x.x will essentially be:
- `recommended-legacy`: legacy (non-flat), with basic rules only
- `recommended-latest-legacy`: legacy (non-flat), all rules (basic +
compiler)
- `flat/recommended`: flat, basic rules only (now the same as
recommended, but to avoid making a breaking change we'll just keep it
around in 6.x.x)
- `recommended-latest`: flat, all rules (basic + compiler)
- `recommended`: flat, basic rules only
In the next breaking release 7.x.x, we will collapse down the presets
into three:
- `recommended-legacy`: all recommended rules
- `recommended`: all recommended rules
- `recommended-experimental`: all recommended rules + new bleeding edge
experimental rules
Closes#34679
---
[//]: # (BEGIN SAPLING FOOTER)
Stack created with [Sapling](https://sapling-scm.com). Best reviewed
with [ReviewStack](https://reviewstack.dev/facebook/react/pull/34700).
* #34703
* __->__ #34700
Follow up to #34649. This adds the compiler rules back so they can be
opted-in 6.1.0, but aren't included in the presets as that would be a
breaking change.
Called Before:
> `logEvent` is a function created with React Hook "useEffectEvent", and
can only be called from the same component.
Called After:
> `logEvent` is a function created with React Hook "useEffectEvent", and
can only be called from Effects and Effect Events in the same component.
Referenced Before:
> `logEvent` is a function created with React Hook "useEffectEvent", and
can only be called from the same component. They cannot be assigned to
variables or passed down.
Referenced After:
> `logEvent` is a function created with React Hook "useEffectEvent", and
can only be called from Effects and Effect Events in the same component.
It cannot be assigned to a variable or passed down.
Like in the diff below, we can read from the shared configuration to
check exhaustive deps.
I allow the classic additionalHooks configuration to override it so that
this change
is backwards compatible.
--
---
[//]: # (BEGIN SAPLING FOOTER)
Stack created with [Sapling](https://sapling-scm.com). Best reviewed
with [ReviewStack](https://reviewstack.dev/facebook/react/pull/34637).
* __->__ #34637
* #34497
We need to be able to specify additional effect hooks for the
RulesOfHooks lint rule
in order to allow useEffectEvent to be called by custom effects.
ExhaustiveDeps
does this with a regex suppplied to the rule, but that regex is not
accessible from
other rules.
This diff introduces a `react-hooks` entry you can put in the eslint
settings that
allows you to specify custom effect hooks and share them across all
rules.
This works like:
```
{
settings: {
'react-hooks': {
additionalEffectHooks: string,
},
},
}
```
The next diff allows useEffect to read from the same configuration.
----
---
[//]: # (BEGIN SAPLING FOOTER)
Stack created with [Sapling](https://sapling-scm.com). Best reviewed
with [ReviewStack](https://reviewstack.dev/facebook/react/pull/34497).
* #34637
* __->__ #34497
With #34176 we now have granular lint rules created for each compiler
ErrorCategory. However, we had remnants of our old error severities
still in use which makes reporting errors quite clunky. Previously you
would need to specify both a category and severity which often ended up
being the same.
This PR moves severity definition into our rules which are generated
from our categories. For now I decided to defer "upgrading" categories
from a simple string to a sum type since we are only using severities to
map errors to eslint severity.
---
[//]: # (BEGIN SAPLING FOOTER)
Stack created with [Sapling](https://sapling-scm.com). Best reviewed
with [ReviewStack](https://reviewstack.dev/facebook/react/pull/34401).
* #34409
* #34404
* #34403
* #34402
* __->__ #34401
NOTE: this is a merged version of @mofeiZ's original PR along with my
edits per offline discussion. The description is updated to reflect the
latest approach.
The key problem we're trying to solve with this PR is to allow
developers more control over the compiler's various validations. The
idea is to have a number of rules targeting a specific category of
issues, such as enforcing immutability of props/state/etc or disallowing
access to refs during render. We don't want to have to run the compiler
again for every single rule, though, so @mofeiZ added an LRU cache that
caches the full compilation output of N most recent files. The first
rule to run on a given file will cause it to get cached, and then
subsequent rules can pull from the cache, with each rule filtering down
to its specific category of errors.
For the categories, I went through and assigned a category roughly 1:1
to existing validations, and then used my judgement on some places that
felt distinct enough to warrant a separate error. Every error in the
compiler now has to supply both a severity (for legacy reasons) and a
category (for ESLint). Each category corresponds 1:1 to a ESLint rule
definition, so that the set of rules is automatically populated based on
the defined categories.
Categories include a flag for whether they should be in the recommended
set or not.
Note that as with the original version of this PR, only
eslint-plugin-react-compiler is changed. We still have to update the
main lint rule.
## Test Plan
* Created a sample project using ESLint v9 and verified that the plugin
can be configured correctly and detects errors
* Edited `fixtures/eslint-v9` and introduced errors, verified that the w
latest config changes in that fixture it correctly detects the errors
* In the sample project, confirmed that the LRU caching is correctly
caching compiler output, ie compiling files just once.
Co-authored-by: Mofei Zhang <feifei0@meta.com>
Work in progress, i'm experimenting with revamping our diagnostic infra.
Starting with a better format for representing errors, with an ability
to point ot multiple locations, along with better printing of errors. Of
course, Babel still controls the printing in the majority case so this
still needs more work.
---
[//]: # (BEGIN SAPLING FOOTER)
Stack created with [Sapling](https://sapling-scm.com). Best reviewed
with [ReviewStack](https://reviewstack.dev/facebook/react/pull/33751).
* #33981
* #33777
* #33767
* #33765
* #33760
* #33759
* #33758
* __->__ #33751
* #33752
* #33753
Summary:
useEffectEvent is meant to be used specifically in combination with
useEffect, and using
the feature in arbitrary closures can lead to surprising reactivity
semantics. In order to
minimize risk in the experimental rollout, we are going to restrict its
usage to being
called directly inside an effect or another useEffectEvent, effectively
enforcing the function
coloring statically. Without an effect system this is the best we can
do.
Summary:
To prepare for automatic effect dependencies, some codebases may want to
codemod
existing useEffect calls with no deps to include an explicit undefined
second argument
in order to preserve the "run on every render" behavior. In sufficiently
large codebases,
this may require a temporary enforcement period where all effects
provide an explicit
dependencies argument.
Outside of migration, relying on a component to render can lead to real
bugs,
especially when working with memoization.
This rule currently has a few false positives, so let's disable it for
now (just in the eslint rule, it's still enabled in the compiler) while
we iterate on it.
This change merges the `react-compiler` rule from
`eslint-plugin-react-compiler` into the `eslint-plugin-react-hooks`
plugin. In order to do the move in a way that keeps commit history with
the moved files, but also no remove them from their origin until a
future cleanup change can be done, I did the `git mv` first, and then
recreated the files that were moved in their original places, as a
separate commit. Unfortunately GH shows the moved files as new instead
of the ones that are truly new. But in the IDE and `git blame`, commit
history is intact with the moved files.
Since this change adds new dependencies, and one of those dependencies
has a higher `engines` declaration for `node` than what the plugin
currently has, this is technically a breaking change and will have to go
out as part of a major release.
### Related Changes
- https://github.com/facebook/react/pull/32458
---------
Co-authored-by: Lauren Tan <poteto@users.noreply.github.com>
This change swaps which config `recommended` is aliasing. In 5.2.0, the
new flat config was introduced as `recommended-latest`, while
`recommended` still pointed at the legacy rc-based config, with a note
that in the next major version `recommended` would be updated to point
at `recommend-latest`. This change makes that swap, and make the default
`recommended` experience the flat config. To continue using the legacy
rc recommended config, please make the following change in your config
```diff
- extends: ['plugin:react-hooks/recommended']
+ extends: ['plugin:react-hooks/recommended-legacy']
```
This change also deprecates `recommended-latest` in favor of
`recommended`. `recommended-latest` will be removed in a future major
version.
The README has been updated to reflect the new usage, and to put the
flat config sections before the legacy config sections.
I also took the opportunity to change the v9 fixture to use a typescript
config, serving as a demonstration for usage as well as a way to
validate the types are correct.
BREAKING CHANGE
---------
Co-authored-by: lauren <poteto@users.noreply.github.com>
Since the compiler plugin is going to be merged into the hooks plugin,
and ultimately decomposed into several more rules, it would be good to
start creating a more traditional folder structure for the plugin. This
change just moves the rules into a `rules` folder.
Co-authored-by: lauren <poteto@users.noreply.github.com>
<!--
Thanks for submitting a pull request!
We appreciate you spending the time to work on these changes. Please
provide enough information so that others can review your pull request.
The three fields below are mandatory.
Before submitting a pull request, please make sure the following is
done:
1. Fork [the repository](https://github.com/facebook/react) and create
your branch from `main`.
2. Run `yarn` in the repository root.
3. If you've fixed a bug or added code that should be tested, add tests!
4. Ensure the test suite passes (`yarn test`). Tip: `yarn test --watch
TestName` is helpful in development.
5. Run `yarn test --prod` to test in the production environment. It
supports the same options as `yarn test`.
6. If you need a debugger, run `yarn test --debug --watch TestName`,
open `chrome://inspect`, and press "Inspect".
7. Format your code with
[prettier](https://github.com/prettier/prettier) (`yarn prettier`).
8. Make sure your code lints (`yarn lint`). Tip: `yarn linc` to only
check changed files.
9. Run the [Flow](https://flowtype.org/) type checks (`yarn flow`).
10. If you haven't already, complete the CLA.
Learn more about contributing:
https://reactjs.org/docs/how-to-contribute.html
-->
## Summary
Contributing to https://github.com/facebook/react/pull/32240, this
change adds the tsconfig, tsup config, and estree type declarations that
will be needed for that plugin's typescript migration.
rollup doesn't inline cjs requires (although it can with an external
plugin), so requiring package.json was causing issues internally at Meta
since that file doesn't exist there.
We could teach our build scripts to do so but given that the eslint meta
field is optional anyways I opted to just hardcode the name and omit the
version.
<!--
Thanks for submitting a pull request!
We appreciate you spending the time to work on these changes. Please
provide enough information so that others can review your pull request.
The three fields below are mandatory.
Before submitting a pull request, please make sure the following is
done:
1. Fork [the repository](https://github.com/facebook/react) and create
your branch from `main`.
2. Run `yarn` in the repository root.
3. If you've fixed a bug or added code that should be tested, add tests!
4. Ensure the test suite passes (`yarn test`). Tip: `yarn test --watch
TestName` is helpful in development.
5. Run `yarn test --prod` to test in the production environment. It
supports the same options as `yarn test`.
6. If you need a debugger, run `yarn test --debug --watch TestName`,
open `chrome://inspect`, and press "Inspect".
7. Format your code with
[prettier](https://github.com/prettier/prettier) (`yarn prettier`).
8. Make sure your code lints (`yarn lint`). Tip: `yarn linc` to only
check changed files.
9. Run the [Flow](https://flowtype.org/) type checks (`yarn flow`).
10. If you haven't already, complete the CLA.
Learn more about contributing:
https://reactjs.org/docs/how-to-contribute.html
-->
## Summary
<!--
Explain the **motivation** for making this change. What existing problem
does the pull request solve?
-->
Currently, `react-hooks/rules-of-hooks` does not support `do/while`
loops - I've also reported this in
https://github.com/facebook/react/issues/28713.
This PR takes a stab at adding support for `do/while` by following the
same logic we already have for detecting `while` loops.
After this PR, any hooks called inside a `do/while` loop will be
considered invalid.
We're also adding some unit tests to confirm that the behavior is
working as expected.
Fixes#28713.
## How did you test this change?
<!--
Demonstrate the code is solid. Example: The exact commands you ran and
their output, screenshots / videos if the pull request changes the user
interface.
How exactly did you verify that your PR solves the issue you wanted to
solve?
If you leave this empty, your PR will very likely be closed.
-->
I've added unit tests that cover the case and verified that they pass by
running:
```
yarn test packages/eslint-plugin-react-hooks/__tests__/ESLintRulesOfHooks-test.js --watch
```
I've also verified that the rest of the tests continue to pass by
running:
```
yarn test
```
and
```
yarn test --prod
```
## 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
We should probably treat `React.use()` the same as `use()` to allow it
within loops and conditionals.
Ideally this would implement a test that `React` is imported or required
from `'react'`, but we don't otherwise implement such a test.
Support Flow `as` expressions in ESLint rules, e.g. `<expr> as <type>`.
This is the same syntax as TypeScript as expressions. I just looked for
any place referencing `TSAsExpression` (the TS node) or
`TypeCastExpression` (the previous Flow syntax) and added a case for
`AsExpression` as well.
## Summary
There is a bug in the `react-hooks/exhaustive-deps` rule that forbids
the dependencies argument from being `undefined`. It triggers the error
that the dependency list is not an array literal. This makes sense in
pre ES5 strict-mode environments as undefined could be redefined, but
should not be a concern in today's JS environments.
**Justification:**
* The deps argument being undefined (for `useEffect` calls etc.) is a
valid use case for hooks that should re-run on every render.
* The deps argument being omitted is considered a valid use case by the
`exhaustive-deps` rule already.
* The TypeScript type definitions support passing `undefined` because
hooks are typed as `useEffect(effect: EffectCallback, deps?:
DependencyList): void;`.
* Since omitting an argument and passing `undefined` are considered
equivalent, this eslint rule should consider them as equivalent too.
Further, I accidentally forgot passing a dependency array to `useEffect`
in code that I shared on Twitter, and people started abusing me about
it. I'd like to create an eslint rule for my projects that requires me
to provide a dep argument in all cases (`undefined`, `[]` or the list of
dependencies) so that I can avoid such problems in the future. This
would also force me to always think about the dependencies instead of
accidentally forgetting them and my hook running on each render. In an
audit of my own codebase I had about 3% of hooks that I want to run on
each render, and adding an explicit `undefined` seems reasonable in
those situations.
It could be argued this could be an option or part of the
`exhaustive-deps` rule, but it's probably better to merge this PR, make
a release and see if my custom eslint rule gains traction in the future.
## How did you test this change?
* Added a test.
* `yarn test ESLintRuleExhaustiveDeps-test`
* Careful code inspection.
Hooks cannot be called in async functions, on either the client or the
server. This mistake sometimes happens when using Server Components,
especially when refactoring a Server Component to a Client Component.
React logs a warning at runtime, but it's even better to catch this with
a lint rule since it will show immediate inline feedback in the editor.
I added this to the existing "Rules of Hooks" ESLint rule.
Arnaud Barré
lauren
Jack Pope
Jordan Brown
Sebastian "Sebbie" Silbermann
Joseph Savona
Benjamin
Jan Kassens
Stephen Zhou
michael faith
Miguel Jiménez Esún
Marin Atanasov
Mark Skelton
Sebastian Silbermann
Ricky
StyleShit
George Zahariev
Christoph Nakazawa
Andrew Clark