Facebook/react
1
0
Fork 0
mirror of https://github.com/facebook/react.git synced 2025-11-01 09:12:30 +00:00
Code Issues Packages Projects Releases Wiki Activity

[Compiler] Don't throw calculate in render if the blamed setter is used outside of the effect

Browse Source 
Summary:
If the setter is used both inside and outside the effect then usually the solution is more complex and requires hoisting state up to a parent component since we can't just remove the local state.

To do this, we now have 2 caches that track setState usages (not just calls) since if the effect is passed as an argument or called outside the effect the solution gets more complex which we are trying to avoid for now
This commit is contained in:
Jorge Cabiedes Acosta
2025-10-21 15:01:00 -07:00
parent 7e67b0e03e
commit eb6869fe4c
7 changed files with 237 additions and 103 deletions
Download Patch File Download Diff File Expand all files Collapse all files
compiler/packages/babel-plugin-react-compiler/src/Validation/ValidateNoDerivedComputationsInEffects_exp.ts
+68 -10
View File
@@ -20,6 +20,8 @@ import {
isUseStateType,
BasicBlock,
isUseRefType,
GeneratedSource,
SourceLocation,
} from '../HIR';
import {eachInstructionLValue, eachInstructionOperand} from '../HIR/visitors';
import {isMutable} from '../ReactiveScopes/InferReactiveScopeVariables';
@@ -38,6 +40,8 @@ type ValidationContext = {
readonly errors: CompilerError;
readonly derivationCache: DerivationCache;
readonly effects: Set<HIRFunction>;
readonly setStateCache: Map<string | undefined | null, Array<Place>>;
readonly effectSetStateCache: Map<string | undefined | null, Array<Place>>;
};
class DerivationCache {
@@ -148,11 +152,19 @@ export function validateNoDerivedComputationsInEffects_exp(
const errors = new CompilerError();
const effects: Set<HIRFunction> = new Set();
const setStateCache: Map<string | undefined | null, Array<Place>> = new Map();
const effectSetStateCache: Map<
string | undefined | null,
Array<Place>
> = new Map();
const context: ValidationContext = {
functions,
errors,
derivationCache,
effects,
setStateCache,
effectSetStateCache,
};
if (fn.fnType === 'Hook') {
@@ -178,13 +190,16 @@ export function validateNoDerivedComputationsInEffects_exp(
}
}
let isFirstPass = true;
do {
for (const block of fn.body.blocks.values()) {
recordPhiDerivations(block, context);
for (const instr of block.instructions) {
recordInstructionDerivations(instr, context);
recordInstructionDerivations(instr, context, isFirstPass);
}
}
isFirstPass = false;
} while (context.derivationCache.snapshot());
for (const effect of effects) {
@@ -239,6 +254,7 @@ function joinValue(
function recordInstructionDerivations(
instr: Instruction,
context: ValidationContext,
isFirstPass: boolean,
): void {
let typeOfValue: TypeOfValue = 'ignored';
const sources: Set<IdentifierId> = new Set();
@@ -247,7 +263,7 @@ function recordInstructionDerivations(
context.functions.set(lvalue.identifier.id, value);
for (const [, block] of value.loweredFunc.func.body.blocks) {
for (const instr of block.instructions) {
recordInstructionDerivations(instr, context);
recordInstructionDerivations(instr, context, isFirstPass);
}
}
} else if (value.kind === 'CallExpression' || value.kind === 'MethodCall') {
@@ -273,6 +289,18 @@ function recordInstructionDerivations(
}
for (const operand of eachInstructionOperand(instr)) {
if (
isSetStateType(operand.identifier) &&
operand.loc !== GeneratedSource &&
isFirstPass
) {
if (context.setStateCache.has(operand.loc.identifierName)) {
context.setStateCache.get(operand.loc.identifierName)!.push(operand);
} else {
context.setStateCache.set(operand.loc.identifierName, [operand]);
}
}
const operandMetadata = context.derivationCache.cache.get(
operand.identifier.id,
);
@@ -347,6 +375,7 @@ function validateEffect(
const effectDerivedSetStateCalls: Array<{
value: CallExpression;
loc: SourceLocation;
sourceIds: Set<IdentifierId>;
}> = [];
@@ -365,6 +394,23 @@ function validateEffect(
return;
}
for (const operand of eachInstructionOperand(instr)) {
if (
isSetStateType(operand.identifier) &&
operand.loc !== GeneratedSource
) {
if (context.effectSetStateCache.has(operand.loc.identifierName)) {
context.effectSetStateCache
.get(operand.loc.identifierName)!
.push(operand);
} else {
context.effectSetStateCache.set(operand.loc.identifierName, [
operand,
]);
}
}
}
if (
instr.value.kind === 'CallExpression' &&
isSetStateType(instr.value.callee.identifier) &&
@@ -378,6 +424,7 @@ function validateEffect(
if (argMetadata !== undefined) {
effectDerivedSetStateCalls.push({
value: instr.value,
loc: instr.value.callee.loc,
sourceIds: argMetadata.sourcesIds,
});
}
@@ -410,13 +457,24 @@ function validateEffect(
}
for (const derivedSetStateCall of effectDerivedSetStateCalls) {
context.errors.push({
category: ErrorCategory.EffectDerivationsOfState,
reason:
'Values derived from props and state should be calculated during render, not in an effect. (https://react.dev/learn/you-might-not-need-an-effect#updating-state-based-on-props-or-state)',
description: null,
loc: derivedSetStateCall.value.callee.loc,
suggestions: null,
});
if (
derivedSetStateCall.loc !== GeneratedSource &&
context.effectSetStateCache.has(derivedSetStateCall.loc.identifierName) &&
context.setStateCache.has(derivedSetStateCall.loc.identifierName) &&
context.effectSetStateCache.get(derivedSetStateCall.loc.identifierName)!
.length ===
context.setStateCache.get(derivedSetStateCall.loc.identifierName)!
.length -
1
) {
context.errors.push({
category: ErrorCategory.EffectDerivationsOfState,
reason:
'Values derived from props and state should be calculated during render, not in an effect. (https://react.dev/learn/you-might-not-need-an-effect#updating-state-based-on-props-or-state)',
description: null,
loc: derivedSetStateCall.value.callee.loc,
suggestions: null,
});
}
}
}
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/derived-state-from-prop-setter-call-outside-effect-no-error.expect.md
+84
View File
@@ -0,0 +1,84 @@
## Input
```javascript
// @validateNoDerivedComputationsInEffects_exp
import {useEffect, useState} from 'react';
function Component({initialName}) {
const [name, setName] = useState('');
useEffect(() => {
setName(initialName);
}, [initialName]);
return (
<div>
<input value={name} onChange={e => setName(e.target.value)} />
</div>
);
}
export const FIXTURE_ENTRYPOINT = {
fn: Component,
params: [{initialName: 'John'}],
};
```
## Code
```javascript
import { c as _c } from "react/compiler-runtime"; // @validateNoDerivedComputationsInEffects_exp
import { useEffect, useState } from "react";
function Component(t0) {
const $ = _c(6);
const { initialName } = t0;
const [name, setName] = useState("");
let t1;
let t2;
if ($[0] !== initialName) {
t1 = () => {
setName(initialName);
};
t2 = [initialName];
$[0] = initialName;
$[1] = t1;
$[2] = t2;
} else {
t1 = $[1];
t2 = $[2];
}
useEffect(t1, t2);
let t3;
if ($[3] === Symbol.for("react.memo_cache_sentinel")) {
t3 = (e) => setName(e.target.value);
$[3] = t3;
} else {
t3 = $[3];
}
let t4;
if ($[4] !== name) {
t4 = (
<div>
<input value={name} onChange={t3} />
</div>
);
$[4] = name;
$[5] = t4;
} else {
t4 = $[5];
}
return t4;
}
export const FIXTURE_ENTRYPOINT = {
fn: Component,
params: [{ initialName: "John" }],
};
```
### Eval output
(kind: ok) <div><input value="John"></div>
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/error.derived-state-from-prop-setter-call-outside-effect-no-error.js → compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/derived-state-from-prop-setter-call-outside-effect-no-error.js
View File
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/derived-state-from-prop-setter-used-outside-effect-no-error.expect.md
+85
View File
@@ -0,0 +1,85 @@
## Input
```javascript
// @validateNoDerivedComputationsInEffects_exp
import {useEffect, useState} from 'react';
function MockComponent({onSet}) {
return <div onClick={() => onSet('clicked')}>Mock Component</div>;
}
function Component({propValue}) {
const [value, setValue] = useState(null);
useEffect(() => {
setValue(propValue);
}, [propValue]);
return <MockComponent onSet={setValue} />;
}
export const FIXTURE_ENTRYPOINT = {
fn: Component,
params: [{propValue: 'test'}],
};
```
## Code
```javascript
import { c as _c } from "react/compiler-runtime"; // @validateNoDerivedComputationsInEffects_exp
import { useEffect, useState } from "react";
function MockComponent(t0) {
const $ = _c(2);
const { onSet } = t0;
let t1;
if ($[0] !== onSet) {
t1 = <div onClick={() => onSet("clicked")}>Mock Component</div>;
$[0] = onSet;
$[1] = t1;
} else {
t1 = $[1];
}
return t1;
}
function Component(t0) {
const $ = _c(4);
const { propValue } = t0;
const [, setValue] = useState(null);
let t1;
let t2;
if ($[0] !== propValue) {
t1 = () => {
setValue(propValue);
};
t2 = [propValue];
$[0] = propValue;
$[1] = t1;
$[2] = t2;
} else {
t1 = $[1];
t2 = $[2];
}
useEffect(t1, t2);
let t3;
if ($[3] === Symbol.for("react.memo_cache_sentinel")) {
t3 = <MockComponent onSet={setValue} />;
$[3] = t3;
} else {
t3 = $[3];
}
return t3;
}
export const FIXTURE_ENTRYPOINT = {
fn: Component,
params: [{ propValue: "test" }],
};
```
### Eval output
(kind: ok) <div>Mock Component</div>
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/error.derived-state-from-prop-setter-used-outside-effect-no-error.js → compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/derived-state-from-prop-setter-used-outside-effect-no-error.js
View File
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/error.derived-state-from-prop-setter-call-outside-effect-no-error.expect.md
-47
View File
@@ -1,47 +0,0 @@
## Input
```javascript
// @validateNoDerivedComputationsInEffects_exp
import {useEffect, useState} from 'react';
function Component({initialName}) {
const [name, setName] = useState('');
useEffect(() => {
setName(initialName);
}, [initialName]);
return (
<div>
<input value={name} onChange={e => setName(e.target.value)} />
</div>
);
}
export const FIXTURE_ENTRYPOINT = {
fn: Component,
params: [{initialName: 'John'}],
};
```
## Error
```
Found 1 error:
Error: Values derived from props and state should be calculated during render, not in an effect. (https://react.dev/learn/you-might-not-need-an-effect#updating-state-based-on-props-or-state)
error.derived-state-from-prop-setter-call-outside-effect-no-error.ts:8:4
6 |
7 | useEffect(() => {
> 8 | setName(initialName);
| ^^^^^^^ Values derived from props and state should be calculated during render, not in an effect. (https://react.dev/learn/you-might-not-need-an-effect#updating-state-based-on-props-or-state)
9 | }, [initialName]);
10 |
11 | return (
```
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/effect-derived-computations/error.derived-state-from-prop-setter-used-outside-effect-no-error.expect.md
-46
View File
@@ -1,46 +0,0 @@
## Input
```javascript
// @validateNoDerivedComputationsInEffects_exp
import {useEffect, useState} from 'react';
function MockComponent({onSet}) {
return <div onClick={() => onSet('clicked')}>Mock Component</div>;
}
function Component({propValue}) {
const [value, setValue] = useState(null);
useEffect(() => {
setValue(propValue);
}, [propValue]);
return <MockComponent onSet={setValue} />;
}
export const FIXTURE_ENTRYPOINT = {
fn: Component,
params: [{propValue: 'test'}],
};
```
## Error
```
Found 1 error:
Error: Values derived from props and state should be calculated during render, not in an effect. (https://react.dev/learn/you-might-not-need-an-effect#updating-state-based-on-props-or-state)
error.derived-state-from-prop-setter-used-outside-effect-no-error.ts:11:4
9 | const [value, setValue] = useState(null);
10 | useEffect(() => {
> 11 | setValue(propValue);
| ^^^^^^^^ Values derived from props and state should be calculated during render, not in an effect. (https://react.dev/learn/you-might-not-need-an-effect#updating-state-based-on-props-or-state)
12 | }, [propValue]);
13 |
14 | return <MockComponent onSet={setValue} />;
```