Skip to content

[Compiler] Don't count a setState in the dependency array of the effect it is called on as a usage#35134

Merged
jorge-cab merged 1 commit intomainfrom
pr35134
Nov 14, 2025
Merged

[Compiler] Don't count a setState in the dependency array of the effect it is called on as a usage#35134
jorge-cab merged 1 commit intomainfrom
pr35134

Conversation

@jorge-cab
Copy link
Copy Markdown
Contributor

@jorge-cab jorge-cab commented Nov 13, 2025

Summary:
The validation only allows setState declaration as a usage outside of the effect.

Another edge case is that if you add the setState being validated in the dependency array you also make the validation opt out since it counts as a usage outside of the effect.

Added a bit of logic to consider the effect's deps when creating the cache for setState usages within the effect

Test Plan:
Added a fixture


Stack created with Sapling. Best reviewed with ReviewStack.

…ct it is called on as a usage

Summary:
The validation only allows setState declaration as a usage outside of the effect.

Another edge case is that if you add the setState being validated in the dependency array you also make the validation opt out since it counts as a usage outside of the effect.

Added a bit of logic to consider the effect's deps when creating the cache for setState usages within the effect

Test Plan:
Added a fixture
Comment on lines +3 to +7
function Component({prop}) {
const [s, setS] = useState(0);
useEffect(() => {
setS(prop);
}, [prop, setS]);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: maybe pass the setState down in props that way it's reactive and has to be included as a dep?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That would disallow the lint as per the next diff 😅 might have to keep it like this in the short term

@jorge-cab jorge-cab merged commit de97ef9 into main Nov 14, 2025
18 of 27 checks passed
jorge-cab added a commit that referenced this pull request Nov 14, 2025
…ns lint (#35135)

Summary:
This only matters when enableTreatSetIdentifiersAsStateSetters=true

This pattern is still bad. But Right now the validation can only
recommend to move stuff to "calculate in render"

A global setState should not be moved to render, not even conditionally
and you can't remove state without crossing Component boundaries, which
makes this a different kind of fix.

So while we are only suggesting "calculate in render" as a fix we should
disallow the lint from throwing in this case IMO

Test Plan:
Added a fixture

---
[//]: # (BEGIN SAPLING FOOTER)
Stack created with [Sapling](https://sapling-scm.com). Best reviewed
with [ReviewStack](https://reviewstack.dev/facebook/react/pull/35135).
* __->__ #35135
* #35134
github-actions Bot pushed a commit that referenced this pull request Nov 14, 2025
…ct it is called on as a usage (#35134)

Summary:
The validation only allows setState declaration as a usage outside of
the effect.

Another edge case is that if you add the setState being validated in the
dependency array you also make the validation opt out since it counts as
a usage outside of the effect.

Added a bit of logic to consider the effect's deps when creating the
cache for setState usages within the effect

Test Plan:
Added a fixture

---
[//]: # (BEGIN SAPLING FOOTER)
Stack created with [Sapling](https://sapling-scm.com). Best reviewed
with [ReviewStack](https://reviewstack.dev/facebook/react/pull/35134).
* #35135
* __->__ #35134

DiffTrain build for [de97ef9](de97ef9)
github-actions Bot pushed a commit that referenced this pull request Nov 14, 2025
…ct it is called on as a usage (#35134)

Summary:
The validation only allows setState declaration as a usage outside of
the effect.

Another edge case is that if you add the setState being validated in the
dependency array you also make the validation opt out since it counts as
a usage outside of the effect.

Added a bit of logic to consider the effect's deps when creating the
cache for setState usages within the effect

Test Plan:
Added a fixture

---
[//]: # (BEGIN SAPLING FOOTER)
Stack created with [Sapling](https://sapling-scm.com). Best reviewed
with [ReviewStack](https://reviewstack.dev/facebook/react/pull/35134).
* #35135
* __->__ #35134

DiffTrain build for [de97ef9](de97ef9)
github-actions Bot pushed a commit that referenced this pull request Nov 14, 2025
…ns lint (#35135)

Summary:
This only matters when enableTreatSetIdentifiersAsStateSetters=true

This pattern is still bad. But Right now the validation can only
recommend to move stuff to "calculate in render"

A global setState should not be moved to render, not even conditionally
and you can't remove state without crossing Component boundaries, which
makes this a different kind of fix.

So while we are only suggesting "calculate in render" as a fix we should
disallow the lint from throwing in this case IMO

Test Plan:
Added a fixture

---
[//]: # (BEGIN SAPLING FOOTER)
Stack created with [Sapling](https://sapling-scm.com). Best reviewed
with [ReviewStack](https://reviewstack.dev/facebook/react/pull/35135).
* __->__ #35135
* #35134

DiffTrain build for [257b033](257b033)
github-actions Bot pushed a commit that referenced this pull request Nov 14, 2025
…ns lint (#35135)

Summary:
This only matters when enableTreatSetIdentifiersAsStateSetters=true

This pattern is still bad. But Right now the validation can only
recommend to move stuff to "calculate in render"

A global setState should not be moved to render, not even conditionally
and you can't remove state without crossing Component boundaries, which
makes this a different kind of fix.

So while we are only suggesting "calculate in render" as a fix we should
disallow the lint from throwing in this case IMO

Test Plan:
Added a fixture

---
[//]: # (BEGIN SAPLING FOOTER)
Stack created with [Sapling](https://sapling-scm.com). Best reviewed
with [ReviewStack](https://reviewstack.dev/facebook/react/pull/35135).
* __->__ #35135
* #35134

DiffTrain build for [257b033](257b033)
manNomi pushed a commit to manNomi/react that referenced this pull request Nov 15, 2025
…ct it is called on as a usage (facebook#35134)

Summary:
The validation only allows setState declaration as a usage outside of
the effect.

Another edge case is that if you add the setState being validated in the
dependency array you also make the validation opt out since it counts as
a usage outside of the effect.

Added a bit of logic to consider the effect's deps when creating the
cache for setState usages within the effect

Test Plan:
Added a fixture

---
[//]: # (BEGIN SAPLING FOOTER)
Stack created with [Sapling](https://sapling-scm.com). Best reviewed
with [ReviewStack](https://reviewstack.dev/facebook/react/pull/35134).
* facebook#35135
* __->__ facebook#35134
manNomi pushed a commit to manNomi/react that referenced this pull request Nov 15, 2025
…ns lint (facebook#35135)

Summary:
This only matters when enableTreatSetIdentifiersAsStateSetters=true

This pattern is still bad. But Right now the validation can only
recommend to move stuff to "calculate in render"

A global setState should not be moved to render, not even conditionally
and you can't remove state without crossing Component boundaries, which
makes this a different kind of fix.

So while we are only suggesting "calculate in render" as a fix we should
disallow the lint from throwing in this case IMO

Test Plan:
Added a fixture

---
[//]: # (BEGIN SAPLING FOOTER)
Stack created with [Sapling](https://sapling-scm.com). Best reviewed
with [ReviewStack](https://reviewstack.dev/facebook/react/pull/35135).
* __->__ facebook#35135
* facebook#35134
github-actions Bot pushed a commit to code/lib-react that referenced this pull request Nov 16, 2025
…ns lint (facebook#35135)

Summary:
This only matters when enableTreatSetIdentifiersAsStateSetters=true

This pattern is still bad. But Right now the validation can only
recommend to move stuff to "calculate in render"

A global setState should not be moved to render, not even conditionally
and you can't remove state without crossing Component boundaries, which
makes this a different kind of fix.

So while we are only suggesting "calculate in render" as a fix we should
disallow the lint from throwing in this case IMO

Test Plan:
Added a fixture

---
[//]: # (BEGIN SAPLING FOOTER)
Stack created with [Sapling](https://sapling-scm.com). Best reviewed
with [ReviewStack](https://reviewstack.dev/facebook/react/pull/35135).
* __->__ facebook#35135
* facebook#35134

DiffTrain build for [257b033](facebook@257b033)
github-actions Bot pushed a commit to code/lib-react that referenced this pull request Nov 16, 2025
…ns lint (facebook#35135)

Summary:
This only matters when enableTreatSetIdentifiersAsStateSetters=true

This pattern is still bad. But Right now the validation can only
recommend to move stuff to "calculate in render"

A global setState should not be moved to render, not even conditionally
and you can't remove state without crossing Component boundaries, which
makes this a different kind of fix.

So while we are only suggesting "calculate in render" as a fix we should
disallow the lint from throwing in this case IMO

Test Plan:
Added a fixture

---
[//]: # (BEGIN SAPLING FOOTER)
Stack created with [Sapling](https://sapling-scm.com). Best reviewed
with [ReviewStack](https://reviewstack.dev/facebook/react/pull/35135).
* __->__ facebook#35135
* facebook#35134

DiffTrain build for [257b033](facebook@257b033)
736-c41-2c1-e464fc974 pushed a commit to Swiss-Armed-Forces/Loom that referenced this pull request Apr 27, 2026
This MR contains the following updates:

| Package | Type | Update | Change | OpenSSF |
|---|---|---|---|---|
| [@typescript-eslint/eslint-plugin](https://typescript-eslint.io/packages/eslint-plugin) ([source](https://github.com/typescript-eslint/typescript-eslint/tree/HEAD/packages/eslint-plugin)) | devDependencies | minor | [`8.58.2` → `8.59.0`](https://renovatebot.com/diffs/npm/@typescript-eslint%2feslint-plugin/8.58.2/8.59.0) | [![OpenSSF Scorecard](https://api.securityscorecards.dev/projects/github.com/typescript-eslint/typescript-eslint/badge)](https://securityscorecards.dev/viewer/?uri=github.com/typescript-eslint/typescript-eslint) |
| [@typescript-eslint/parser](https://typescript-eslint.io/packages/parser) ([source](https://github.com/typescript-eslint/typescript-eslint/tree/HEAD/packages/parser)) | devDependencies | minor | [`8.58.2` → `8.59.0`](https://renovatebot.com/diffs/npm/@typescript-eslint%2fparser/8.58.2/8.59.0) | [![OpenSSF Scorecard](https://api.securityscorecards.dev/projects/github.com/typescript-eslint/typescript-eslint/badge)](https://securityscorecards.dev/viewer/?uri=github.com/typescript-eslint/typescript-eslint) |
| [ajv](https://ajv.js.org) ([source](https://github.com/ajv-validator/ajv)) | dependencies | minor | [`8.18.0` → `8.20.0`](https://renovatebot.com/diffs/npm/ajv/8.18.0/8.20.0) | [![OpenSSF Scorecard](https://api.securityscorecards.dev/projects/github.com/ajv-validator/ajv/badge)](https://securityscorecards.dev/viewer/?uri=github.com/ajv-validator/ajv) |
| [eslint-plugin-react-hooks](https://react.dev/) ([source](https://github.com/facebook/react/tree/HEAD/packages/eslint-plugin-react-hooks)) | devDependencies | minor | [`7.0.1` → `7.1.1`](https://renovatebot.com/diffs/npm/eslint-plugin-react-hooks/7.0.1/7.1.1) | [![OpenSSF Scorecard](https://api.securityscorecards.dev/projects/github.com/facebook/react/badge)](https://securityscorecards.dev/viewer/?uri=github.com/facebook/react) |
| [react-toastify](https://github.com/fkhadra/react-toastify) | dependencies | minor | [`11.0.5` → `11.1.0`](https://renovatebot.com/diffs/npm/react-toastify/11.0.5/11.1.0) | [![OpenSSF Scorecard](https://api.securityscorecards.dev/projects/github.com/fkhadra/react-toastify/badge)](https://securityscorecards.dev/viewer/?uri=github.com/fkhadra/react-toastify) |
| [typescript-eslint](https://typescript-eslint.io/packages/typescript-eslint) ([source](https://github.com/typescript-eslint/typescript-eslint/tree/HEAD/packages/typescript-eslint)) | devDependencies | minor | [`8.58.2` → `8.59.0`](https://renovatebot.com/diffs/npm/typescript-eslint/8.58.2/8.59.0) | [![OpenSSF Scorecard](https://api.securityscorecards.dev/projects/github.com/typescript-eslint/typescript-eslint/badge)](https://securityscorecards.dev/viewer/?uri=github.com/typescript-eslint/typescript-eslint) |
| [vite-plugin-static-copy](https://github.com/sapphi-red/vite-plugin-static-copy) | devDependencies | minor | [`4.0.1` → `4.1.0`](https://renovatebot.com/diffs/npm/vite-plugin-static-copy/4.0.1/4.1.0) | [![OpenSSF Scorecard](https://api.securityscorecards.dev/projects/github.com/sapphi-red/vite-plugin-static-copy/badge)](https://securityscorecards.dev/viewer/?uri=github.com/sapphi-red/vite-plugin-static-copy) |

---

### Release Notes

<details>
<summary>typescript-eslint/typescript-eslint (@&#8203;typescript-eslint/eslint-plugin)</summary>

### [`v8.59.0`](https://github.com/typescript-eslint/typescript-eslint/blob/HEAD/packages/eslint-plugin/CHANGELOG.md#8590-2026-04-20)

[Compare Source](typescript-eslint/typescript-eslint@v8.58.2...v8.59.0)

##### 🚀 Features

- **eslint-plugin:** \[no-unnecessary-type-assertion] report more cases based on assignability ([#&#8203;11789](typescript-eslint/typescript-eslint#11789))

##### ❤️ Thank You

- Ulrich Stark

See [GitHub Releases](https://github.com/typescript-eslint/typescript-eslint/releases/tag/v8.59.0) for more information.

You can read about our [versioning strategy](https://typescript-eslint.io/users/versioning) and [releases](https://typescript-eslint.io/users/releases) on our website.

</details>

<details>
<summary>typescript-eslint/typescript-eslint (@&#8203;typescript-eslint/parser)</summary>

### [`v8.59.0`](https://github.com/typescript-eslint/typescript-eslint/blob/HEAD/packages/parser/CHANGELOG.md#8590-2026-04-20)

[Compare Source](typescript-eslint/typescript-eslint@v8.58.2...v8.59.0)

This was a version bump only for parser to align it with other projects, there were no code changes.

See [GitHub Releases](https://github.com/typescript-eslint/typescript-eslint/releases/tag/v8.59.0) for more information.

You can read about our [versioning strategy](https://typescript-eslint.io/users/versioning) and [releases](https://typescript-eslint.io/users/releases) on our website.

</details>

<details>
<summary>ajv-validator/ajv (ajv)</summary>

### [`v8.20.0`](https://github.com/ajv-validator/ajv/releases/tag/v8.20.0)

[Compare Source](ajv-validator/ajv@v8.18.0...v8.20.0)

#### What's Changed

- fix: add support for node 22/24, drop node 16/21 by [@&#8203;jasoniangreen](https://github.com/jasoniangreen) in [#&#8203;2580](ajv-validator/ajv#2580)
- fix: add ES2022.RegExp for RegExpIndicesArray by [@&#8203;SignpostMarv](https://github.com/SignpostMarv) in [#&#8203;2604](ajv-validator/ajv#2604)

**Full Changelog**: <ajv-validator/ajv@v8.19.0...v8.20.0>

</details>

<details>
<summary>facebook/react (eslint-plugin-react-hooks)</summary>

### [`v7.1.1`](https://github.com/facebook/react/blob/HEAD/packages/eslint-plugin-react-hooks/CHANGELOG.md#711)

[Compare Source](https://github.com/facebook/react/compare/eslint-plugin-react-hooks@7.1.0...eslint-plugin-react-hooks@7.1.1)

**Note:** 7.1.0 accidentally removed the `component-hook-factories` rule, causing errors for users who referenced it in their ESLint config. This is now fixed.

- Add deprecated no-op `component-hook-factories` rule for backwards compatibility. ([@&#8203;mofeiZ](https://github.com/mofeiZ) in [#&#8203;36307](facebook/react#36307))

### [`v7.1.0`](https://github.com/facebook/react/blob/HEAD/packages/eslint-plugin-react-hooks/CHANGELOG.md#710)

[Compare Source](https://github.com/facebook/react/compare/408b38ef7304faf022d2a37110c57efce12c6bad...eslint-plugin-react-hooks@7.1.0)

This release adds ESLint v10 support, improves performance by skipping compilation for non-React files, and includes compiler lint improvements including better `set-state-in-effect` detection, improved ref validation, and more helpful error reporting.

- Add ESLint v10 support. ([@&#8203;azat-io](https://github.com/azat-io) in [#&#8203;35720](facebook/react#35720))
- Skip compilation for non-React files to improve performance. ([@&#8203;josephsavona](https://github.com/josephsavona) in [#&#8203;35589](facebook/react#35589))
- Fix exhaustive deps bug with Flow type casting. ([@&#8203;jorge-cab](https://github.com/jorge-cab) in [#&#8203;35691](facebook/react#35691))
- Fix `useEffectEvent` checks in component syntax. ([@&#8203;jbrown215](https://github.com/jbrown215) in [#&#8203;35041](facebook/react#35041))
- Improved `set-state-in-effect` validation with fewer false negatives. ([@&#8203;jorge-cab](https://github.com/jorge-cab) in [#&#8203;35134](facebook/react#35134), [@&#8203;josephsavona](https://github.com/josephsavona) in [#&#8203;35147](facebook/react#35147), [@&#8203;jackpope](https://github.com/jackpope) in [#&#8203;35214](facebook/react#35214), [@&#8203;chesnokov-tony](https://github.com/chesnokov-tony) in [#&#8203;35419](facebook/react#35419), [@&#8203;jsleitor](https://github.com/jsleitor) in [#&#8203;36107](facebook/react#36107))
- Improved ref validation for non-mutating functions and event handler props. ([@&#8203;josephsavona](https://github.com/josephsavona) in [#&#8203;35893](facebook/react#35893), [@&#8203;kolvian](https://github.com/kolvian) in [#&#8203;35062](facebook/react#35062))
- Compiler now reports all errors instead of stopping at the first. ([@&#8203;josephsavona](https://github.com/josephsavona) in [#&#8203;35873](https://github.com/facebook/react/pull/35873)–[#&#8203;35884](https://github.com/facebook/react/pull/35884))
- Improved source locations and error display in compiler diagnostics. ([@&#8203;nathanmarks](https://github.com/nathanmarks) in [#&#8203;35348](facebook/react#35348), [@&#8203;josephsavona](https://github.com/josephsavona) in [#&#8203;34963](facebook/react#34963))

</details>

<details>
<summary>fkhadra/react-toastify (react-toastify)</summary>

### [`v11.1.0`](https://github.com/fkhadra/react-toastify/releases/tag/v11.1.0)

[Compare Source](fkhadra/react-toastify@v11.0.5...v11.1.0)

### Release Notes

#### Features

- **CSP nonce support.** `<ToastContainer nonce={...}>` applies the nonce to the
  injected `<style>` tag. Closes [#&#8203;1209](fkhadra/react-toastify#1209).

#### Fixes

- `onChange` fires `status: 'removed'` synchronously on `toast.dismiss()` instead
  of after the exit animation — observers (incl. `useNotificationCenter`) now
  see correctly ordered events. Also guards against double-`onClose`. Closes [#&#8203;1275](fkhadra/react-toastify#1275).
- Touch drag no longer re-pauses the toast on release — the old check compared a
  PointerEvent against `'touchend'`, which never matched. Closes [#&#8203;1217](fkhadra/react-toastify#1217).
- Vertical drag now visually moves the toast (`--y` gets a unit). Thanks
  [@&#8203;janpaepke](https://github.com/janpaepke), [#&#8203;1277](fkhadra/react-toastify#1277).
- Stacked scale is clamped at 0.5, preventing zero/negative scale in deep
  stacks. Closes [#&#8203;1171](fkhadra/react-toastify#1171), [#&#8203;1174](fkhadra/react-toastify#1174).
- Stacked container respects mobile `100vw` again. Closes [#&#8203;1234](fkhadra/react-toastify#1234).

#### Accessibility

- `role="progressbar"` now includes `aria-valuenow`, `aria-valuemin`, `aria-valuemax`.
  Thanks [@&#8203;singhankit001](https://github.com/singhankit001), [#&#8203;1283](fkhadra/react-toastify#1283). Closes [#&#8203;1259](fkhadra/react-toastify#1259).

#### Internal

- Migrated to a pnpm workspace (`pnpm link .` no longer required for contributors).
  Publish layout unchanged — addon still ships inside the main package.
- CSS now injected at mount via `useStyleSheet` (prerequisite for `nonce`).
- Dep bumps: TypeScript 6, Vite 8, Cypress 15, React 19.2, plus the rest.
- CI: `upload-artifact` v3 → v4.

Thanks to [@&#8203;janpaepke](https://github.com/janpaepke), [@&#8203;singhankit001](https://github.com/singhankit001), and reporters of the fixed issues.

</details>

<details>
<summary>typescript-eslint/typescript-eslint (typescript-eslint)</summary>

### [`v8.59.0`](https://github.com/typescript-eslint/typescript-eslint/blob/HEAD/packages/typescript-eslint/CHANGELOG.md#8590-2026-04-20)

[Compare Source](typescript-eslint/typescript-eslint@v8.58.2...v8.59.0)

This was a version bump only for typescript-eslint to align it with other projects, there were no code changes.

See [GitHub Releases](https://github.com/typescript-eslint/typescript-eslint/releases/tag/v8.59.0) for more information.

You can read about our [versioning strategy](https://typescript-eslint.io/users/versioning) and [releases](https://typescript-eslint.io/users/releases) on our website.

</details>

<details>
<summary>sapphi-red/vite-plugin-static-copy (vite-plugin-static-copy)</summary>

### [`v4.1.0`](https://github.com/sapphi-red/vite-plugin-static-copy/blob/HEAD/CHANGELOG.md#410)

[Compare Source](https://github.com/sapphi-red/vite-plugin-static-copy/compare/vite-plugin-static-copy@4.0.1...vite-plugin-static-copy@4.1.0)

##### Minor Changes

- [#&#8203;251](sapphi-red/vite-plugin-static-copy#251) [`7672842`](sapphi-red/vite-plugin-static-copy@7672842) Thanks [@&#8203;sapphi-red](https://github.com/sapphi-red)! - Add `name` property to the `rename` object form and allow rename functions to return a `RenameObject`. The `name` property replaces the file's basename (filename + extension), and can be combined with `stripBase` to both flatten directory structure and rename the file in one step. Rename functions can now return `{ name, stripBase }` objects instead of only strings, making it easier to declaratively control output paths from dynamic rename logic.

  ```js
  // node_modules/lib/dist/index.js → vendor/lib.js
  { src: 'node_modules/lib/dist/index.js', dest: 'vendor', rename: { name: 'lib.js', stripBase: true } }

  // src/pages/events/test.html → dist/events/index.html
  { src: 'src/pages/**/*.html', dest: 'dist/', rename: { stripBase: 2, name: 'index.html' } }
  ```

</details>

---

 - [ ] <!-- rebase-check -->If you want to rebase/retry this MR, check this box

---

This MR has been generated by [Mend Renovate](https://github.com/renovatebot/renovate).
<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiI0My4xMjkuMCIsInVwZGF0ZWRJblZlciI6IjQzLjE0MS4zIiwidGFyZ2V0QnJhbmNoIjoibWFpbiIsImxhYmVscyI6WyJkZXBlbmRlbmNpZXMiLCJyZW5vdmF0ZSJdfQ==-->

See merge request swiss-armed-forces/cyber-command/cea/loom!480

Co-authored-by: Loom MR Pipeline Trigger <group_103951964_bot_9504bb8dead6d4e406ad817a607f24be@noreply.gitlab.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants