Skip to content

Conversation

poteto
Copy link
Member

@poteto poteto commented Jul 25, 2025

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.


Stack created with Sapling. Best reviewed with ReviewStack.

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.

awesomesauce

@poteto poteto force-pushed the pr34002 branch 2 times, most recently from 1b11651 to 7872b2e Compare July 28, 2025 16:15
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
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.
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 poteto merged commit 6b22f31 into main Jul 28, 2025
21 checks passed
@poteto poteto deleted the pr34002 branch July 28, 2025 17:25
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
…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)
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.

3 participants