Skip to content

[compiler] Add new ValidateNoVoidUseMemo pass #33990

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 2 commits into from
Jul 28, 2025
Merged

[compiler] Add new ValidateNoVoidUseMemo pass #33990

merged 2 commits into from
Jul 28, 2025

Conversation

poteto
Copy link
Member

@poteto poteto commented Jul 24, 2025

Adds a new validation pass to validate against useMemos 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 useMemos to see if they are being used.


Stack created with Sapling. Best reviewed with ReviewStack.

@github-actions github-actions bot added the React Core Team Opened by a member of the React Core Team label Jul 24, 2025
@poteto poteto force-pushed the pr33990 branch 2 times, most recently from 5d968c2 to e8bc72f Compare July 24, 2025 23:49
for (let item of items) {
if (item.match) return item;
}
return null;
Copy link
Member

Choose a reason for hiding this comment

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

Leaving this out seems kinda fine and more stylistic. Should we lint for at least one non-void return? Consistent return I think already exists as an independent lint outside React's linters.

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.

discussed offline but this makes sense. alternative is to move this into DropManualMemo, which already does the temporary mapping for you

poteto added 2 commits July 28, 2025 12:37
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.
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.
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 poteto merged commit c60eebf into main Jul 28, 2025
21 checks passed
@poteto poteto deleted the pr33990 branch July 28, 2025 16:46
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)
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)
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