OSSForks/react
1
0
Fork 0
mirror of https://github.com/zebrajr/react.git synced 2025-12-06 12:20:20 +01:00
Code Issues Actions 5 Packages Projects Releases Wiki Activity

[compiler] Validate against setState in all effect types (#33753)

Browse Source 
---
[//]: # (BEGIN SAPLING FOOTER)
Stack created with [Sapling](https://sapling-scm.com). Best reviewed
with [ReviewStack](https://reviewstack.dev/facebook/react/pull/33753).
* #33981
* #33777
* #33767
* #33765
* #33760
* #33759
* #33758
* #33751
* #33752
* __->__ #33753
This commit is contained in:
Joseph Savona 2025-07-24 15:36:40 -07:00 committed by GitHub
parent 448f781a52
commit 6f4294af9b
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
12 changed files with 33 additions and 27 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Pipeline.ts
View File

@ -94,7 +94,7 @@ import {validateLocalsNotReassignedAfterRender} from '../Validation/ValidateLoca
import {outlineFunctions} from '../Optimization/OutlineFunctions';
import {propagatePhiTypes} from '../TypeInference/PropagatePhiTypes';
import {lowerContextAccess} from '../Optimization/LowerContextAccess';
import {validateNoSetStateInPassiveEffects} from '../Validation/ValidateNoSetStateInPassiveEffects';
import {validateNoSetStateInEffects} from '../Validation/ValidateNoSetStateInEffects';
import {validateNoJSXInTryStatement} from '../Validation/ValidateNoJSXInTryStatement';
import {propagateScopeDependenciesHIR} from '../HIR/PropagateScopeDependenciesHIR';
import {outlineJSX} from '../Optimization/OutlineJsx';
@ -292,8 +292,8 @@ function runWithEnvironment(
validateNoSetStateInRender(hir).unwrap();
}
if (env.config.validateNoSetStateInPassiveEffects) {
env.logErrors(validateNoSetStateInPassiveEffects(hir));
if (env.config.validateNoSetStateInEffects) {
env.logErrors(validateNoSetStateInEffects(hir));
}
if (env.config.validateNoJSXInTryStatements) {

4
compiler/packages/babel-plugin-react-compiler/src/HIR/Environment.ts
View File

@ -318,10 +318,10 @@ export const EnvironmentConfigSchema = z.object({
validateNoSetStateInRender: z.boolean().default(true),
/**
* Validates that setState is not called directly within a passive effect (useEffect).
* Validates that setState is not called synchronously within an effect (useEffect and friends).
* Scheduling a setState (with an event listener, subscription, etc) is valid.
*/
validateNoSetStateInPassiveEffects: z.boolean().default(false),
validateNoSetStateInEffects: z.boolean().default(false),
/**
* Validates against creating JSX within a try block and recommends using an error boundary

12
compiler/packages/babel-plugin-react-compiler/src/Validation/ValidateNoSetStateInPassiveEffects.ts → compiler/packages/babel-plugin-react-compiler/src/Validation/ValidateNoSetStateInEffects.ts
View File

@ -11,20 +11,22 @@ import {
IdentifierId,
isSetStateType,
isUseEffectHookType,
isUseInsertionEffectHookType,
isUseLayoutEffectHookType,
Place,
} from '../HIR';
import {eachInstructionValueOperand} from '../HIR/visitors';
import {Result} from '../Utils/Result';
/**
* Validates against calling setState in the body of a *passive* effect (useEffect),
* Validates against calling setState in the body of an effect (useEffect and friends),
* while allowing calling setState in callbacks scheduled by the effect.
*
* Calling setState during execution of a useEffect triggers a re-render, which is
* often bad for performance and frequently has more efficient and straightforward
* alternatives. See https://react.dev/learn/you-might-not-need-an-effect for examples.
*/
export function validateNoSetStateInPassiveEffects(
export function validateNoSetStateInEffects(
fn: HIRFunction,
): Result<void, CompilerError> {
const setStateFunctions: Map<IdentifierId, Place> = new Map();
@ -79,7 +81,11 @@ export function validateNoSetStateInPassiveEffects(
instr.value.kind === 'MethodCall'
? instr.value.receiver
: instr.value.callee;
if (isUseEffectHookType(callee.identifier)) {
if (
isUseEffectHookType(callee.identifier) ||
isUseLayoutEffectHookType(callee.identifier) ||
isUseInsertionEffectHookType(callee.identifier)
) {
const arg = instr.value.args[0];
if (arg !== undefined && arg.kind === 'Identifier') {
const setState = setStateFunctions.get(arg.identifier.id);

8
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/invalid-setState-in-useEffect-transitive.expect.md
View File

@ -2,7 +2,7 @@
## Input
```javascript
// @loggerTestOnly @validateNoSetStateInPassiveEffects
// @loggerTestOnly @validateNoSetStateInEffects
import {useEffect, useState} from 'react';
function Component() {
@ -24,7 +24,7 @@ function Component() {
## Code
```javascript
import { c as _c } from "react/compiler-runtime"; // @loggerTestOnly @validateNoSetStateInPassiveEffects
import { c as _c } from "react/compiler-runtime"; // @loggerTestOnly @validateNoSetStateInEffects
import { useEffect, useState } from "react";
function Component() {
@ -65,8 +65,8 @@ function _temp(s) {
## Logs
```
{"kind":"CompileError","detail":{"options":{"reason":"Calling setState directly within a useEffect causes cascading renders and is not recommended. Consider alternatives to useEffect. (https://react.dev/learn/you-might-not-need-an-effect)","description":null,"severity":"InvalidReact","suggestions":null,"loc":{"start":{"line":13,"column":4,"index":272},"end":{"line":13,"column":5,"index":273},"filename":"invalid-setState-in-useEffect-transitive.ts","identifierName":"g"}}},"fnLoc":null}
{"kind":"CompileSuccess","fnLoc":{"start":{"line":4,"column":0,"index":99},"end":{"line":16,"column":1,"index":300},"filename":"invalid-setState-in-useEffect-transitive.ts"},"fnName":"Component","memoSlots":2,"memoBlocks":2,"memoValues":2,"prunedMemoBlocks":0,"prunedMemoValues":0}
{"kind":"CompileError","detail":{"options":{"reason":"Calling setState directly within a useEffect causes cascading renders and is not recommended. Consider alternatives to useEffect. (https://react.dev/learn/you-might-not-need-an-effect)","description":null,"severity":"InvalidReact","suggestions":null,"loc":{"start":{"line":13,"column":4,"index":265},"end":{"line":13,"column":5,"index":266},"filename":"invalid-setState-in-useEffect-transitive.ts","identifierName":"g"}}},"fnLoc":null}
{"kind":"CompileSuccess","fnLoc":{"start":{"line":4,"column":0,"index":92},"end":{"line":16,"column":1,"index":293},"filename":"invalid-setState-in-useEffect-transitive.ts"},"fnName":"Component","memoSlots":2,"memoBlocks":2,"memoValues":2,"prunedMemoBlocks":0,"prunedMemoValues":0}
```
### Eval output

2
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/invalid-setState-in-useEffect-transitive.js
View File

@ -1,4 +1,4 @@
// @loggerTestOnly @validateNoSetStateInPassiveEffects
// @loggerTestOnly @validateNoSetStateInEffects
import {useEffect, useState} from 'react';
function Component() {

8
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/invalid-setState-in-useEffect.expect.md
View File

@ -2,7 +2,7 @@
## Input
```javascript
// @loggerTestOnly @validateNoSetStateInPassiveEffects
// @loggerTestOnly @validateNoSetStateInEffects
import {useEffect, useState} from 'react';
function Component() {
@ -18,7 +18,7 @@ function Component() {
## Code
```javascript
import { c as _c } from "react/compiler-runtime"; // @loggerTestOnly @validateNoSetStateInPassiveEffects
import { c as _c } from "react/compiler-runtime"; // @loggerTestOnly @validateNoSetStateInEffects
import { useEffect, useState } from "react";
function Component() {
@ -45,8 +45,8 @@ function _temp(s) {
## Logs
```
{"kind":"CompileError","detail":{"options":{"reason":"Calling setState directly within a useEffect causes cascading renders and is not recommended. Consider alternatives to useEffect. (https://react.dev/learn/you-might-not-need-an-effect)","description":null,"severity":"InvalidReact","suggestions":null,"loc":{"start":{"line":7,"column":4,"index":187},"end":{"line":7,"column":12,"index":195},"filename":"invalid-setState-in-useEffect.ts","identifierName":"setState"}}},"fnLoc":null}
{"kind":"CompileSuccess","fnLoc":{"start":{"line":4,"column":0,"index":99},"end":{"line":10,"column":1,"index":232},"filename":"invalid-setState-in-useEffect.ts"},"fnName":"Component","memoSlots":1,"memoBlocks":1,"memoValues":1,"prunedMemoBlocks":0,"prunedMemoValues":0}
{"kind":"CompileError","detail":{"options":{"reason":"Calling setState directly within a useEffect causes cascading renders and is not recommended. Consider alternatives to useEffect. (https://react.dev/learn/you-might-not-need-an-effect)","description":null,"severity":"InvalidReact","suggestions":null,"loc":{"start":{"line":7,"column":4,"index":180},"end":{"line":7,"column":12,"index":188},"filename":"invalid-setState-in-useEffect.ts","identifierName":"setState"}}},"fnLoc":null}
{"kind":"CompileSuccess","fnLoc":{"start":{"line":4,"column":0,"index":92},"end":{"line":10,"column":1,"index":225},"filename":"invalid-setState-in-useEffect.ts"},"fnName":"Component","memoSlots":1,"memoBlocks":1,"memoValues":1,"prunedMemoBlocks":0,"prunedMemoValues":0}
```
### Eval output

2
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/invalid-setState-in-useEffect.js
View File

@ -1,4 +1,4 @@
// @loggerTestOnly @validateNoSetStateInPassiveEffects
// @loggerTestOnly @validateNoSetStateInEffects
import {useEffect, useState} from 'react';
function Component() {

4
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/valid-setState-in-useEffect-listener-transitive.expect.md
View File

@ -2,7 +2,7 @@
## Input
```javascript
// @validateNoSetStateInPassiveEffects
// @validateNoSetStateInEffects
import {useEffect, useState} from 'react';
function Component() {
@ -26,7 +26,7 @@ export const FIXTURE_ENTRYPOINT = {
## Code
```javascript
import { c as _c } from "react/compiler-runtime"; // @validateNoSetStateInPassiveEffects
import { c as _c } from "react/compiler-runtime"; // @validateNoSetStateInEffects
import { useEffect, useState } from "react";
function Component() {

2
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/valid-setState-in-useEffect-listener-transitive.js
View File

@ -1,4 +1,4 @@
// @validateNoSetStateInPassiveEffects
// @validateNoSetStateInEffects
import {useEffect, useState} from 'react';
function Component() {

4
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/valid-setState-in-useEffect-listener.expect.md
View File

@ -2,7 +2,7 @@
## Input
```javascript
// @validateNoSetStateInPassiveEffects
// @validateNoSetStateInEffects
import {useEffect, useState} from 'react';
function Component() {
@ -23,7 +23,7 @@ export const FIXTURE_ENTRYPOINT = {
## Code
```javascript
import { c as _c } from "react/compiler-runtime"; // @validateNoSetStateInPassiveEffects
import { c as _c } from "react/compiler-runtime"; // @validateNoSetStateInEffects
import { useEffect, useState } from "react";
function Component() {

2
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/valid-setState-in-useEffect-listener.js
View File

@ -1,4 +1,4 @@
// @validateNoSetStateInPassiveEffects
// @validateNoSetStateInEffects
import {useEffect, useState} from 'react';
function Component() {

6
compiler/packages/babel-plugin-react-compiler/src/__tests__/parseConfigPragma-test.ts
View File

@ -15,11 +15,11 @@ describe('parseConfigPragmaForTests()', () => {
// Validate defaults first to make sure that the parser is getting the value from the pragma,
// and not just missing it and getting the default value
expect(defaultConfig.enableUseTypeAnnotations).toBe(false);
expect(defaultConfig.validateNoSetStateInPassiveEffects).toBe(false);
expect(defaultConfig.validateNoSetStateInEffects).toBe(false);
expect(defaultConfig.validateNoSetStateInRender).toBe(true);
const config = parseConfigPragmaForTests(
'@enableUseTypeAnnotations @validateNoSetStateInPassiveEffects:true @validateNoSetStateInRender:false',
'@enableUseTypeAnnotations @validateNoSetStateInEffects:true @validateNoSetStateInRender:false',
{compilationMode: defaultOptions.compilationMode},
);
expect(config).toEqual({
@ -28,7 +28,7 @@ describe('parseConfigPragmaForTests()', () => {
environment: {
...defaultOptions.environment,
enableUseTypeAnnotations: true,
validateNoSetStateInPassiveEffects: true,
validateNoSetStateInEffects: true,
validateNoSetStateInRender: false,
enableResetCacheOnSourceFileChanges: false,
},