Skip to content

[compiler] Enable validateNoVoidUseMemo in eslint & playground #34022

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

Merged
merged 1 commit into from
Jul 28, 2025
Merged

Conversation

poteto
Copy link
Member

@poteto poteto commented Jul 28, 2025

Enables validateNoVoidUseMemo by default only in eslint (it defaults to false otherwise) as well as the playground.

@react-sizebot
Copy link

Comparing: eaee530...b544b15

Critical size changes

Includes critical production bundles, as well as any change greater than 2%:

Name +/- Base Current +/- gzip Base gzip Current gzip
oss-stable/react-dom/cjs/react-dom.production.js = 6.68 kB 6.68 kB +0.16% 1.83 kB 1.83 kB
oss-stable/react-dom/cjs/react-dom-client.production.js = 530.70 kB 530.70 kB = 93.70 kB 93.70 kB
oss-experimental/react-dom/cjs/react-dom.production.js = 6.69 kB 6.69 kB +0.16% 1.83 kB 1.83 kB
oss-experimental/react-dom/cjs/react-dom-client.production.js = 655.25 kB 655.25 kB = 115.40 kB 115.40 kB
facebook-www/ReactDOM-prod.classic.js = 675.13 kB 675.13 kB = 118.75 kB 118.75 kB
facebook-www/ReactDOM-prod.modern.js = 665.56 kB 665.56 kB = 117.11 kB 117.12 kB

Significant size changes

Includes any change greater than 0.2%:

Expand to show
Name +/- Base Current +/- gzip Base gzip Current gzip
oss-stable-semver/eslint-plugin-react-hooks/cjs/eslint-plugin-react-hooks.production.js +0.21% 2,057.88 kB 2,062.13 kB +0.20% 297.41 kB 297.99 kB
oss-stable/eslint-plugin-react-hooks/cjs/eslint-plugin-react-hooks.production.js +0.21% 2,057.88 kB 2,062.13 kB +0.20% 297.41 kB 297.99 kB
oss-experimental/eslint-plugin-react-hooks/cjs/eslint-plugin-react-hooks.production.js +0.21% 2,058.05 kB 2,062.30 kB +0.20% 297.43 kB 298.02 kB
oss-stable-semver/eslint-plugin-react-hooks/cjs/eslint-plugin-react-hooks.development.js +0.21% 2,062.35 kB 2,066.60 kB +0.20% 298.42 kB 299.01 kB
oss-stable/eslint-plugin-react-hooks/cjs/eslint-plugin-react-hooks.development.js +0.21% 2,062.35 kB 2,066.60 kB +0.20% 298.42 kB 299.01 kB
oss-experimental/eslint-plugin-react-hooks/cjs/eslint-plugin-react-hooks.development.js +0.21% 2,062.53 kB 2,066.78 kB +0.20% 298.45 kB 299.03 kB

Generated by 🚫 dangerJS against b544b15

Copy link
Member

@josephsavona josephsavona left a comment

Choose a reason for hiding this comment

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

Can you enable in the playground too?

@poteto poteto changed the title [compiler] Enable validateNoVoidUseMemo in eslint [compiler] Enable validateNoVoidUseMemo in eslint & playground Jul 28, 2025
@poteto
Copy link
Member Author

poteto commented Jul 28, 2025

Can you enable in the playground too?

Enabled!

poteto added a commit that referenced this pull request Jul 28, 2025
…33989)

Adds a new property to ReturnTerminals to disambiguate whether it was
explicit, implicit (arrow function expressions), or void (where it was
omitted). I will use this property in the next PR adding a new
validation pass.

---
[//]: # (BEGIN SAPLING FOOTER)
Stack created with [Sapling](https://sapling-scm.com). Best reviewed
with [ReviewStack](https://reviewstack.dev/facebook/react/pull/33989).
* #34022
* #34002
* #34001
* #33990
* __->__ #33989
poteto added a commit that referenced this pull request Jul 28, 2025
Adds a new validation pass to validate against `useMemo`s that don't
return anything. This usually indicates some kind of "useEffect"-like
code that has side effects that need to be memoized to prevent
overfiring, and is an anti-pattern.

A follow up validation could also look at the return value of `useMemo`s
to see if they are being used.

---
[//]: # (BEGIN SAPLING FOOTER)
Stack created with [Sapling](https://sapling-scm.com). Best reviewed
with [ReviewStack](https://reviewstack.dev/facebook/react/pull/33990).
* #34022
* #34002
* #34001
* __->__ #33990
* #33989
github-actions bot pushed a commit that referenced this pull request Jul 28, 2025
…33989)

Adds a new property to ReturnTerminals to disambiguate whether it was
explicit, implicit (arrow function expressions), or void (where it was
omitted). I will use this property in the next PR adding a new
validation pass.

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

DiffTrain build for [5dd622e](5dd622e)
github-actions bot pushed a commit that referenced this pull request Jul 28, 2025
…33989)

Adds a new property to ReturnTerminals to disambiguate whether it was
explicit, implicit (arrow function expressions), or void (where it was
omitted). I will use this property in the next PR adding a new
validation pass.

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

DiffTrain build for [5dd622e](5dd622e)
poteto added a commit that referenced this pull request Jul 28, 2025
)

Much of the logic in the new validation pass is already implemented in
DropManualMemoization, so let's combine them. I opted to keep the
environment flag so we can more precisely control the rollout.

---
[//]: # (BEGIN SAPLING FOOTER)
Stack created with [Sapling](https://sapling-scm.com). Best reviewed
with [ReviewStack](https://reviewstack.dev/facebook/react/pull/34001).
* #34022
* #34002
* __->__ #34001
github-actions bot pushed a commit that referenced this pull request Jul 28, 2025
Adds a new validation pass to validate against `useMemo`s that don't
return anything. This usually indicates some kind of "useEffect"-like
code that has side effects that need to be memoized to prevent
overfiring, and is an anti-pattern.

A follow up validation could also look at the return value of `useMemo`s
to see if they are being used.

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

DiffTrain build for [c60eebf](c60eebf)
github-actions bot pushed a commit that referenced this pull request Jul 28, 2025
Adds a new validation pass to validate against `useMemo`s that don't
return anything. This usually indicates some kind of "useEffect"-like
code that has side effects that need to be memoized to prevent
overfiring, and is an anti-pattern.

A follow up validation could also look at the return value of `useMemo`s
to see if they are being used.

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

DiffTrain build for [c60eebf](c60eebf)
poteto added a commit that referenced this pull request Jul 28, 2025
…34002)

Noticed this from my previous PR that this pass was throwing on the
first error. This PR is a small refactor to aggregate every violation
and report them all at once.

---
[//]: # (BEGIN SAPLING FOOTER)
Stack created with [Sapling](https://sapling-scm.com). Best reviewed
with [ReviewStack](https://reviewstack.dev/facebook/react/pull/34002).
* #34022
* __->__ #34002
Enables `validateNoVoidUseMemo` by default only in eslint (it defaults to false otherwise) as well as the playground.
Copy link
Member

@josephsavona josephsavona left a comment

Choose a reason for hiding this comment

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

woohoo!

github-actions bot pushed a commit that referenced this pull request Jul 28, 2025
…34002)

Noticed this from my previous PR that this pass was throwing on the
first error. This PR is a small refactor to aggregate every violation
and report them all at once.

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

DiffTrain build for [6b22f31](6b22f31)
@poteto poteto merged commit 7ee7571 into main Jul 28, 2025
252 of 253 checks passed
@poteto poteto deleted the pr34022 branch July 28, 2025 17:42
github-actions bot pushed a commit that referenced this pull request Jul 28, 2025
…34002)

Noticed this from my previous PR that this pass was throwing on the
first error. This PR is a small refactor to aggregate every violation
and report them all at once.

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

DiffTrain build for [6b22f31](6b22f31)
github-actions bot pushed a commit that referenced this pull request Jul 28, 2025
Enables `validateNoVoidUseMemo` by default only in eslint (it defaults
to false otherwise) as well as the playground.

DiffTrain build for [7ee7571](7ee7571)
github-actions bot pushed a commit that referenced this pull request Jul 28, 2025
Enables `validateNoVoidUseMemo` by default only in eslint (it defaults
to false otherwise) as well as the playground.

DiffTrain build for [7ee7571](7ee7571)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed React Core Team Opened by a member of the React Core Team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants