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>
<!--
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?
-->
Adds changelog entries for the last two minor releases of
`eslint-plugin-react-hooks`. Fixes#31717.
I chose to not include #31208 (8382581446)
and #32115 (fd2d279984) in the changelog
as they only changed internals that do not affect consumers of the
plugin, and it doesn't seem like the changelog previously included such
changes.
Changes are sorted by importance (rather than by commit date), with the
most important changes first.
## 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.
-->
Docs only, nothing to test.
This change adds more details about prior versions of the plugin's
config, to help people as they migrate from legacy to flat configs
across multiple versions of this plugin. At some point in the 6.0 or 7.0
cycle, it would probably make sense to re-consolidate this into a single
version.
Closes#32494
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>
In preparation for the merging of the compiler plugin into this one
(#32416), this change proactively updates the plugin's `engines`
declaration to require Node versions greater than or equal to 18
BREAKING CHANGE
Co-authored-by: lauren <poteto@users.noreply.github.com>
Update eslint-plugin-react-hooks to be built targetting ES5 instead. For
various reasons our internal infra relies on these files being built
already downleveled.
- [build(eslint-plugin-react-hooks): add
ts-linting](4c0fbe73d9)
This change adds configuration to the eslint config governing
`eslint-plugin-react-hooks` to use the typescript-eslint plugin and
parser. It adds the typescript-recommended config, and configures the
team's preferred `array-type` convention.
- [refactor(eslint-plugin-react-hooks): improve
conditionals](540d0d95bc)
This change addresses several feedback items from
https://github.com/facebook/react/pull/32240
- [ci (eslint-e2e): exclude nested node_modules from
cache](a3279f46a8)
This change removes the nested fixture `node_modules` from being cached,
so that the symbolic link can be made after the build happens.
<!--
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
This change converts the eslint hooks plugin to typescript, which also
allows us to include type declarations in the package, for those using
[typescript eslint
configs](https://eslint.org/blog/2025/01/eslint-v9.18.0-released/#stable-typescript-configuration-file-support).
### Constituent changes that should land before this one
- [x] ~https://github.com/facebook/react/pull/32276~
- [x] https://github.com/facebook/react/pull/32279
- [x] https://github.com/facebook/react/pull/32283
- [x] https://github.com/facebook/react/pull/32393
- [x] https://github.com/facebook/react/pull/32396Closes#30119
---------
Co-authored-by: Lauren Tan <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.
<!--
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 dev dependencies needed to support the migration of the
plugin to typescript.
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
```
This disables symbol renaming in production builds. The original
variable and function names are preserved. All other forms of
compression applied by Closure (dead code elimination, inlining, etc)
are unchanged — the final program is identical to what we were producing
before, just in a more readable form.
The motivation is to make it easier to debug React issues that only
occur in production — the same reason we decided to start shipping
sourcemaps in #28827 and #28827.
However, because most apps run their own minification step on their npm
dependencies, it's not necessary for us to minify the symbols before
publishing — it'll be handled the app, if desired.
This is the same strategy Meta has used to ship React for years. The
React build itself has unminified symbols, but they get minified as part
of Meta's regular build pipeline.
Even if an app does not minify their npm dependencies, gzip covers most
of the cost of symbol renaming anyway.
This saves us from having to ship sourcemaps, which means even apps that
don't have sourcemaps configured will be able to debug the React build
as easily as they would any other npm dependency.
## 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.
- substr is Annex B
- substring silently flips its arguments if they're in the "wrong order", which is confusing
- slice is better than sliced bread (no pun intended) and also it works the same way on Arrays so there's less to remember
---
> I'd be down to just lint and enforce a single form just for the potential compression savings by using a repeated string.
_Originally posted by @sebmarkbage in https://github.com/facebook/react/pull/26663#discussion_r1170455401_
The old version of prettier we were using didn't support the Flow syntax
to access properties in a type using `SomeType['prop']`. This updates
`prettier` and `rollup-plugin-prettier` to the latest versions.
I added the prettier config `arrowParens: "avoid"` to reduce the diff
size as the default has changed in Prettier 2.0. The largest amount of
changes comes from function expressions now having a space. This doesn't
have an option to preserve the old behavior, so we have to update this.
Hermes parser is the preferred parser for Flow code going forward. We
need to upgrade to this parser to support new Flow syntax like function
`this` context type annotations or `ObjectType['prop']` syntax.
Unfortunately, there's quite a few upgrades here to make it work somehow
(dependencies between the changes)
- ~Upgrade `eslint` to `8.*`~ reverted this as the React eslint plugin
tests depend on the older version and there's a [yarn
bug](https://github.com/yarnpkg/yarn/issues/6285) that prevents
`devDependencies` and `peerDependencies` to different versions.
- Remove `eslint-config-fbjs` preset dependency and inline the rules,
imho this makes it a lot clearer what the rules are.
- Remove the turned off `jsx-a11y/*` rules and it's dependency instead
of inlining those from the `fbjs` config.
- Update parser and dependency from `babel-eslint` to `hermes-eslint`.
- `ft-flow/no-unused-expressions` rule replaces `no-unused-expressions`
which now allows standalone type asserts, e.g. `(foo: number);`
- Bunch of globals added to the eslint config
- Disabled `no-redeclare`, seems like the eslint upgrade started making
this more precise and warn against re-defined globals like
`__EXPERIMENTAL__` (in rollup scripts) or `fetch` (when importing fetch
from node-fetch).
- Minor lint fixes like duplicate keys in objects.
We originally had grand plans for using this Event concept for more but
now it's only meant to be used in combination with effects.
It's an Event in the FRP terms, that is triggered from an Effect.
Technically it can also be from another function that itself is
triggered from an existing side-effect but that's kind of an advanced
case.
The canonical case is an effect that triggers an event:
```js
const onHappened = useEffectEvent(() => ...);
useEffect(() => {
onHappened();
}, []);
```
* [useEvent] Lint for presence of useEvent functions in dependency lists
With #25473, the identity of useEvent's return value is no longer stable
across renders. Previously, the ExhaustiveDeps lint rule would only
allow the omission of the useEvent function, but you could still add it
as a dependency.
This PR updates the ExhaustiveDeps rule to explicitly check for the
presence of useEvent functions in dependency lists, and emits a warning
and suggestion/autofixer for removing the dependency.
* Facebook -> Meta in copyright
rg --files | xargs sed -i 's#Copyright (c) Facebook, Inc. and its affiliates.#Copyright (c) Meta Platforms, Inc. and affiliates.#g'
* Manual tweaks
Usage of the new `use` hook needs to conform to the rules of hooks, with
the one exception that it can be called conditionally.
ghstack-source-id: 7ea5beceaf
Pull Request resolved: https://github.com/facebook/react/pull/25370
* [ESLint] Check useEvent references instead
Previously the useEvent check in RulesOfHooks would collect all
definitions of useEvent functions at the top level, record them as
violations, then clear those violations if the useEvent function was
later called or referened inside of an effect or another event.
The flaw with this approach was in the special case where useEvent
functions could be passed by reference inside of effects or events. The
violation would be cleared here (since it was called at least once)
and subsequent usages of the useEvent function would not be properly
checked.
This PR changes it so we check all identifiers that resolve to a
useEvent function, and if they are not in an effect or event must be
called or a lint error is emitted.
Co-authored-by: Dan Abramov <dan.abramov@gmail.com>
* Add comment
Co-authored-by: Dan Abramov <dan.abramov@gmail.com>
This update to the RulesOfHooks rule checks that functions created with
`useEvent` can only be invoked in a `useEffect` callback, in another
event function, or a closure.
They can't be passed down directly as a reference to child components.
This PR also updates the ExhaustiveDeps lint rule to treat useEvent's
return value as stable, so it can be omitted from dependency lists.
Currently this all gated behind an experimental flag.
Co-authored-by: Dan Abramov <dan.abramov@gmail.com>
* Switched RulesOfHooks.js to use BigInt. Added test and updated .eslintrc.js to use es2020.
* Added BigInt as readonly global in eslintrc.cjs.js and eslintrc.cjs2015.js
* Added comment to RulesOfHooks.js that gets rid of BigInt eslint error
* Got rid of changes in .eslintrc.js and yarn.lock
* Move global down
Co-authored-by: stephen cyron <stephen.cyron@fdmgroup.com>
Co-authored-by: dan <dan.abramov@gmail.com>
* Add .browser and .node explicit entry points
This can be useful when the automatic selection doesn't work properly.
* Remove react/index
I'm not sure why I added this in the first place. Perhaps due to how our
builds work somehow.
* Remove build-info.json from files field
* Remove redundant initial of isArray (#21163)
* Reapply prettier
* Type the isArray function with refinement support
This ensures that an argument gets refined just like it does if isArray is
used directly.
I'm not sure how to express with just a direct reference so I added a
function wrapper and confirmed that this does get inlined properly by
closure compiler.
* A few more
* Rename unit test to internal
This is not testing a bundle.
Co-authored-by: Behnam Mohammadi <itten@live.com>
In addition to `TSTypeQuery`, dependency nodes with a `TSTypeReference`
parent need to be ignored as well. Without this fix, generic type
variables will be listed as missing dependencies.
Example:
export function useFoo<T>(): (foo: T) => boolean {
return useCallback((foo: T) => false, []);
}
This will report the following issue:
React Hook useCallback has a missing dependency: 'T'. Either include
it or remove the dependency array
Closes: #19742
* Add babel parser which supports ChainExpression
* Add and fix tests for new babel eslint parser
* extract function to mark node
* refactor for compatibility with eslint v7.7.0+
* Update eslint to v7.7.0
Update hook test since eslint now supports nullish coalescing
The `startTransition` method returned from `useTransition` is a stable
method, like `dispatch` or `setState`. You should not have to specify
it as a hook dependency.
* [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
