Compare commits

..

2 Commits

Author SHA1 Message Date
Jorge Cabiedes
d879ce9b80 Merge branch 'main' into pr34442 2025-09-09 16:29:58 -07:00
Joseph Savona
acada3035f [compiler] Fix false positive hook return mutation error (#34424)
This was fun. We previously added the MaybeAlias effect in #33984 in
order to describe the semantic that an unknown function call _may_ alias
its return value in its result, but that we don't know this for sure. We
record mutations through MaybeAlias edges when walking backward in the
data flow graph, but downgrade them to conditional mutations. See the
original PR for full context.

That change was sufficient for the original case like

```js
const frozen = useContext();
useEffect(() => {
  frozen.method().property = true;
}, [...]);
```

But it wasn't sufficient for cases where the aliasing occured between
operands:

```js
const dispatch = useDispatch();
<div onClick={(e) => {
  dispatch(...e.target.value)
  e.target.value = ...;
}} />
```

Here we would record a `Capture dispatch <- e.target` effect. Then
during processing of the `event.target.value = ...` assignment we'd
eventually _forward_ from `event` to `dispatch` (along a MaybeAlias
edge). But in #33984 I missed that this forward walk also has to
downgrade to conditional.

In addition to that change, we also have to be a bit more precise about
which set of effects we create for alias/capture/maybe-alias. The new
logic is a bit clearer, I think:

* If the value is frozen, it's an ImmutableCapture edge
* If the values are mutable, it's a Capture
* If it's a context->context, context->mutable, or mutable->context,
count it as MaybeAlias.
2025-09-09 14:07:47 -07:00
25 changed files with 732 additions and 691 deletions

View File

@@ -748,10 +748,14 @@ function applyEffect(
case 'Alias':
case 'Capture': {
CompilerError.invariant(
effect.kind === 'Capture' || initialized.has(effect.into.identifier.id),
effect.kind === 'Capture' ||
effect.kind === 'MaybeAlias' ||
initialized.has(effect.into.identifier.id),
{
reason: `Expected destination value to already be initialized within this instruction for Alias effect`,
description: `Destination ${printPlace(effect.into)} is not initialized in this instruction`,
reason: `Expected destination to already be initialized within this instruction`,
description:
`Destination ${printPlace(effect.into)} is not initialized in this ` +
`instruction for effect ${printAliasingEffect(effect)}`,
details: [
{
kind: 'error',
@@ -767,49 +771,67 @@ function applyEffect(
* copy-on-write semantics, then we can prune the effect
*/
const intoKind = state.kind(effect.into).kind;
let isMutableDesination: boolean;
let destinationType: 'context' | 'mutable' | null = null;
switch (intoKind) {
case ValueKind.Context:
case ValueKind.Mutable:
case ValueKind.MaybeFrozen: {
isMutableDesination = true;
case ValueKind.Context: {
destinationType = 'context';
break;
}
default: {
isMutableDesination = false;
case ValueKind.Mutable:
case ValueKind.MaybeFrozen: {
destinationType = 'mutable';
break;
}
}
const fromKind = state.kind(effect.from).kind;
let isMutableReferenceType: boolean;
let sourceType: 'context' | 'mutable' | 'frozen' | null = null;
switch (fromKind) {
case ValueKind.Context: {
sourceType = 'context';
break;
}
case ValueKind.Global:
case ValueKind.Primitive: {
isMutableReferenceType = false;
break;
}
case ValueKind.Frozen: {
isMutableReferenceType = false;
applyEffect(
context,
state,
{
kind: 'ImmutableCapture',
from: effect.from,
into: effect.into,
},
initialized,
effects,
);
sourceType = 'frozen';
break;
}
default: {
isMutableReferenceType = true;
sourceType = 'mutable';
break;
}
}
if (isMutableDesination && isMutableReferenceType) {
if (sourceType === 'frozen') {
applyEffect(
context,
state,
{
kind: 'ImmutableCapture',
from: effect.from,
into: effect.into,
},
initialized,
effects,
);
} else if (
(sourceType === 'mutable' && destinationType === 'mutable') ||
effect.kind === 'MaybeAlias'
) {
effects.push(effect);
} else if (
(sourceType === 'context' && destinationType != null) ||
(sourceType === 'mutable' && destinationType === 'context')
) {
applyEffect(
context,
state,
{kind: 'MaybeAlias', from: effect.from, into: effect.into},
initialized,
effects,
);
}
break;
}

View File

@@ -779,7 +779,13 @@ class AliasingState {
if (edge.index >= index) {
break;
}
queue.push({place: edge.node, transitive, direction: 'forwards', kind});
queue.push({
place: edge.node,
transitive,
direction: 'forwards',
// Traversing a maybeAlias edge always downgrades to conditional mutation
kind: edge.kind === 'maybeAlias' ? MutationKind.Conditional : kind,
});
}
for (const [alias, when] of node.createdFrom) {
if (when >= index) {
@@ -807,7 +813,12 @@ class AliasingState {
if (when >= index) {
continue;
}
queue.push({place: alias, transitive, direction: 'backwards', kind});
queue.push({
place: alias,
transitive,
direction: 'backwards',
kind,
});
}
/**
* MaybeAlias indicates potential data flow from unknown function calls,

View File

@@ -5,52 +5,91 @@
* LICENSE file in the root directory of this source tree.
*/
import {
CompilerDiagnostic,
CompilerError,
Effect,
ErrorSeverity,
SourceLocation,
} from '..';
import {TypeOf} from 'zod';
import {CompilerError, Effect, ErrorSeverity, SourceLocation} from '..';
import {ErrorCategory} from '../CompilerError';
import {
ArrayExpression,
BasicBlock,
BlockId,
FunctionExpression,
HIRFunction,
IdentifierId,
Instruction,
InstructionValue,
Place,
isSetStateType,
isUseEffectHookType,
isUseStateType,
GeneratedSource,
} from '../HIR';
import {eachInstructionOperand, eachInstructionLValue} from '../HIR/visitors';
import {printInstruction, printPlace} from '../HIR/PrintHIR';
import {
eachInstructionValueOperand,
eachInstructionOperand,
eachTerminalOperand,
eachInstructionLValue,
} from '../HIR/visitors';
import {isMutable} from '../ReactiveScopes/InferReactiveScopeVariables';
import {assertExhaustive} from '../Utils/utils';
type SetStateCall = {
loc: SourceLocation;
derivedDep: DerivationMetadata;
setStateId: IdentifierId;
propsSources: Place[] | undefined; // undefined means state-derived, defined means props-derived
};
type TypeOfValue = 'ignored' | 'fromProps' | 'fromState' | 'fromPropsOrState';
type DerivationMetadata = {
identifierPlace: Place;
sources: Place[];
typeOfValue: TypeOfValue;
place: Place;
sources: Set<Place>;
};
type ErrorMetadata = {
type: TypeOfValue;
description: string | undefined;
loc: SourceLocation;
setStateName: string | undefined | null;
};
function joinValue(
lvalueType: TypeOfValue,
valueType: TypeOfValue,
): TypeOfValue {
if (lvalueType === 'ignored') return valueType;
if (valueType === 'ignored') return lvalueType;
if (lvalueType === valueType) return lvalueType;
return 'fromPropsOrState';
}
function propagateDerivation(
dest: Place,
source: Place | undefined,
derivedFromProps: Map<IdentifierId, Place>,
) {
if (source === undefined) {
return;
}
if (source.identifier.name?.kind === 'promoted') {
derivedFromProps.set(dest.identifier.id, dest);
} else {
derivedFromProps.set(dest.identifier.id, source);
}
}
function updateDerivationMetadata(
target: Place,
sources: DerivationMetadata[],
typeOfValue: TypeOfValue,
derivedTuple: Map<IdentifierId, DerivationMetadata>,
): void {
let newValue: DerivationMetadata = {
identifierPlace: target,
sources: [],
typeOfValue: typeOfValue,
};
for (const source of sources) {
// If the identifier of the source is a promoted identifier, then
// we should set the source as the first named identifier.
if (source.identifierPlace.identifier.name?.kind === 'promoted') {
newValue.sources.push(target);
} else {
newValue.sources.push(...source.sources);
}
}
derivedTuple.set(target.identifier.id, newValue);
}
/**
* Validates that useEffect is not used for derived computations which could/should
@@ -79,22 +118,24 @@ export function validateNoDerivedComputationsInEffects(fn: HIRFunction): void {
const candidateDependencies: Map<IdentifierId, ArrayExpression> = new Map();
const functions: Map<IdentifierId, FunctionExpression> = new Map();
const locals: Map<IdentifierId, IdentifierId> = new Map();
const derivationCache: Map<IdentifierId, DerivationMetadata> = new Map();
const effectSetStates: Map<
string | undefined | null,
Array<Place>
> = new Map();
const setStateCalls: Map<string | undefined | null, Array<Place>> = new Map();
// MY take on this
const valueToType: Map<IdentifierId, TypeOfValue> = new Map();
const valueToSourceProps: Map<IdentifierId, Set<Place>> = new Map();
const valueToSourceStates: Map<IdentifierId, Set<Place>> = new Map();
const valueToSources: Map<IdentifierId, Set<Place>> = new Map();
const errors: Array<ErrorMetadata> = [];
// Sources are still probably not correct
const derivedTuple: Map<IdentifierId, DerivationMetadata> = new Map();
const errors = new CompilerError();
if (fn.fnType === 'Hook') {
for (const param of fn.params) {
if (param.kind === 'Identifier') {
derivationCache.set(param.identifier.id, {
place: param,
sources: new Set([param]),
derivedTuple.set(param.identifier.id, {
identifierPlace: param,
sources: [param],
typeOfValue: 'fromProps',
});
}
@@ -102,22 +143,113 @@ export function validateNoDerivedComputationsInEffects(fn: HIRFunction): void {
} else if (fn.fnType === 'Component') {
const props = fn.params[0];
if (props != null && props.kind === 'Identifier') {
derivationCache.set(props.identifier.id, {
place: props,
sources: new Set([props]),
derivedTuple.set(props.identifier.id, {
identifierPlace: props,
sources: [props],
typeOfValue: 'fromProps',
});
}
}
for (const block of fn.body.blocks.values()) {
parseBlockPhi(block, derivationCache);
for (const phi of block.phis) {
for (const operand of phi.operands.values()) {
const source = derivedTuple.get(operand.identifier.id);
if (source !== undefined && source.typeOfValue === 'fromProps') {
if (
source.identifierPlace.identifier.name === null ||
source.identifierPlace.identifier.name?.kind === 'promoted'
) {
derivedTuple.set(phi.place.identifier.id, {
identifierPlace: phi.place,
sources: [phi.place],
typeOfValue: 'fromProps',
});
} else {
derivedTuple.set(phi.place.identifier.id, {
identifierPlace: phi.place,
sources: source.sources,
typeOfValue: 'fromProps',
});
}
}
}
}
for (const instr of block.instructions) {
const {lvalue, value} = instr;
parseInstr(instr, derivationCache, setStateCalls);
// This needs to be repeated "recursively" on FunctionExpressions
// HERE >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
// DERIVATION LOGIC-----------------------------------------------------
console.log('instr', printInstruction(instr));
console.log('instr', instr);
// console.log('instr lValue', instr.lvalue);
let typeOfValue: TypeOfValue = 'ignored';
// TODO: Add handling for state derived props
let sources: DerivationMetadata[] = [];
for (const operand of eachInstructionValueOperand(value)) {
const opSource = derivedTuple.get(operand.identifier.id);
if (opSource === undefined) {
continue;
}
typeOfValue = joinValue(typeOfValue, opSource.typeOfValue);
sources.push(opSource);
}
// TODO: Add handling for state derived props
if (typeOfValue !== 'ignored') {
for (const lvalue of eachInstructionLValue(instr)) {
updateDerivationMetadata(lvalue, sources, typeOfValue, derivedTuple);
}
for (const operand of eachInstructionValueOperand(value)) {
switch (operand.effect) {
case Effect.Capture:
case Effect.Store:
case Effect.ConditionallyMutate:
case Effect.ConditionallyMutateIterator:
case Effect.Mutate: {
if (isMutable(instr, operand)) {
updateDerivationMetadata(
operand,
sources,
typeOfValue,
derivedTuple,
);
}
break;
}
case Effect.Freeze:
case Effect.Read: {
// no-op
break;
}
case Effect.Unknown: {
CompilerError.invariant(false, {
reason: 'Unexpected unknown effect',
description: null,
loc: operand.loc,
suggestions: null,
});
}
default: {
assertExhaustive(
operand.effect,
`Unexpected effect kind \`${operand.effect}\``,
);
}
}
}
}
console.log('derivedTuple', derivedTuple);
// HERE >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
// console.log('derivedTuple', derivedTuple);
// DERIVATION LOGIC-----------------------------------------------------
if (value.kind === 'LoadLocal') {
locals.set(lvalue.identifier.id, value.place.identifier.id);
} else if (value.kind === 'ArrayExpression') {
@@ -131,6 +263,7 @@ export function validateNoDerivedComputationsInEffects(fn: HIRFunction): void {
const callee =
value.kind === 'CallExpression' ? value.callee : value.property;
// This is a useEffect hook
if (
isUseEffectHookType(callee.identifier) &&
value.args.length === 2 &&
@@ -162,8 +295,7 @@ export function validateNoDerivedComputationsInEffects(fn: HIRFunction): void {
validateEffect(
effectFunction.loweredFunc.func,
dependencies,
derivationCache,
effectSetStates,
derivedTuple,
errors,
);
}
@@ -171,294 +303,64 @@ export function validateNoDerivedComputationsInEffects(fn: HIRFunction): void {
}
}
}
const compilerError = generateCompilerError(
setStateCalls,
effectSetStates,
errors,
);
if (compilerError.hasErrors()) {
throw compilerError;
}
}
function generateCompilerError(
setStateCalls: Map<string | undefined | null, Array<Place>>,
effectSetStates: Map<string | undefined | null, Array<Place>>,
errors: Array<ErrorMetadata>,
): CompilerError {
const throwableErrors = new CompilerError();
for (const error of errors) {
let compilerDiagnostic: CompilerDiagnostic | undefined = undefined;
let detailMessage = '';
switch (error.type) {
case 'fromProps':
detailMessage = 'This state value shadows a value passed as a prop.';
break;
case 'fromPropsOrState':
detailMessage =
'This state value shadows a value passed as a prop or a value from state.';
break;
}
/*
* If we use a setState from an invalid useEffect elsewhere then we probably have to
* hoist state up, else we should calculate in render
*/
if (
setStateCalls.get(error.setStateName)?.length !=
effectSetStates.get(error.setStateName)?.length &&
error.type !== 'fromState'
) {
compilerDiagnostic = CompilerDiagnostic.create({
description: `${error.description} This state value shadows a value passed as a prop. Instead of shadowing the prop with local state, hoist the state to the parent component and update it there.`,
category: `Local state shadows parent state.`,
severity: ErrorSeverity.InvalidReact,
}).withDetail({
kind: 'error',
loc: error.loc,
message: 'this setState synchronizes the state',
});
for (const [key, setStateCallArray] of effectSetStates) {
if (setStateCallArray.length === 0) {
continue;
}
const nonUseEffectSetStateCalls = setStateCalls.get(key);
if (nonUseEffectSetStateCalls) {
for (const place of nonUseEffectSetStateCalls) {
if (!setStateCallArray.includes(place)) {
compilerDiagnostic.withDetail({
kind: 'error',
loc: place.loc,
message:
'this setState updates the shadowed state, but should call an onChange event from the parent',
});
}
}
}
}
} else {
compilerDiagnostic = CompilerDiagnostic.create({
description: `${error.description} Derived values should be computed during render, rather than in effects. Using an effect triggers an additional render which can hurt performance and user experience, potentially briefly showing stale values to the user.`,
category: `Derive values in render, not effects.`,
severity: ErrorSeverity.InvalidReact,
}).withDetail({
kind: 'error',
loc: error.loc,
message: 'This should be computed during render, not in an effect',
});
}
if (compilerDiagnostic) {
throwableErrors.pushDiagnostic(compilerDiagnostic);
}
}
return throwableErrors;
}
function joinValue(
lvalueType: TypeOfValue,
valueType: TypeOfValue,
): TypeOfValue {
if (lvalueType === 'ignored') return valueType;
if (valueType === 'ignored') return lvalueType;
if (lvalueType === valueType) return lvalueType;
return 'fromPropsOrState';
}
function updateDerivationMetadata(
target: Place,
sources: Array<DerivationMetadata> | undefined,
typeOfValue: TypeOfValue | undefined,
derivationCache: Map<IdentifierId, DerivationMetadata>,
): void {
let newValue: DerivationMetadata = {
place: target,
sources: new Set(),
typeOfValue: typeOfValue ?? 'ignored',
};
if (sources !== undefined) {
for (const source of sources) {
/*
* If the identifier of the source is a promoted identifier, then
* we should set the target as the source.
*/
for (const place of source.sources) {
if (
place.identifier.name === null ||
place.identifier.name?.kind === 'promoted'
) {
newValue.sources.add(target);
} else {
newValue.sources.add(place);
}
}
}
}
derivationCache.set(target.identifier.id, newValue);
}
function parseInstr(
instr: Instruction,
derivationCache: Map<IdentifierId, DerivationMetadata>,
setStateCalls: Map<string | undefined | null, Array<Place>>,
): void {
// Recursively parse function expressions
if (instr.value.kind === 'FunctionExpression') {
for (const [, block] of instr.value.loweredFunc.func.body.blocks) {
for (const instr of block.instructions) {
parseInstr(instr, derivationCache, setStateCalls);
}
}
}
let typeOfValue: TypeOfValue = 'ignored';
// Catch any useState hook calls
let sources: Array<DerivationMetadata> = [];
if (
instr.value.kind === 'Destructure' &&
instr.value.lvalue.pattern.kind === 'ArrayPattern' &&
isUseStateType(instr.value.value.identifier)
) {
typeOfValue = 'fromState';
const stateValueSource = instr.value.lvalue.pattern.items[0];
if (stateValueSource.kind === 'Identifier') {
sources.push({
place: stateValueSource,
typeOfValue: typeOfValue,
sources: new Set([stateValueSource]),
});
}
}
if (
instr.value.kind === 'CallExpression' &&
isSetStateType(instr.value.callee.identifier) &&
instr.value.args.length === 1 &&
instr.value.args[0].kind === 'Identifier' &&
instr.value.callee.loc !== GeneratedSource
) {
if (setStateCalls.has(instr.value.callee.loc.identifierName)) {
setStateCalls
.get(instr.value.callee.loc.identifierName)!
.push(instr.value.callee);
} else {
setStateCalls.set(instr.value.callee.loc.identifierName, [
instr.value.callee,
]);
}
}
for (const operand of eachInstructionOperand(instr)) {
const opSource = derivationCache.get(operand.identifier.id);
if (opSource === undefined) {
continue;
}
typeOfValue = joinValue(typeOfValue, opSource.typeOfValue);
sources.push(opSource);
}
if (typeOfValue !== 'ignored') {
for (const lvalue of eachInstructionLValue(instr)) {
updateDerivationMetadata(lvalue, sources, typeOfValue, derivationCache);
}
for (const operand of eachInstructionOperand(instr)) {
switch (operand.effect) {
case Effect.Capture:
case Effect.Store:
case Effect.ConditionallyMutate:
case Effect.ConditionallyMutateIterator:
case Effect.Mutate: {
if (isMutable(instr, operand)) {
updateDerivationMetadata(
operand,
sources,
typeOfValue,
derivationCache,
);
}
break;
}
case Effect.Freeze:
case Effect.Read: {
// no-op
break;
}
case Effect.Unknown: {
CompilerError.invariant(false, {
reason: 'Unexpected unknown effect',
description: null,
loc: operand.loc,
suggestions: null,
});
}
default: {
assertExhaustive(
operand.effect,
`Unexpected effect kind \`${operand.effect}\``,
);
}
}
}
}
}
function parseBlockPhi(
block: BasicBlock,
derivationCache: Map<IdentifierId, DerivationMetadata>,
): void {
for (const phi of block.phis) {
for (const operand of phi.operands.values()) {
const phiSource = derivationCache.get(operand.identifier.id);
if (phiSource !== undefined) {
updateDerivationMetadata(
phi.place,
[phiSource],
phiSource?.typeOfValue,
derivationCache,
);
}
}
if (errors.hasAnyErrors()) {
throw errors;
}
}
function validateEffect(
effectFunction: HIRFunction,
effectDeps: Array<IdentifierId>,
derivationCache: Map<IdentifierId, DerivationMetadata>,
effectSetStates: Map<string | undefined | null, Array<Place>>,
errors: Array<ErrorMetadata>,
derivedTuple: Map<IdentifierId, DerivationMetadata>,
errors: CompilerError,
): void {
let isUsingDerivedDeps = false;
for (const operand of effectFunction.context) {
if (isSetStateType(operand.identifier)) {
continue;
} else if (effectDeps.find(dep => dep === operand.identifier.id) != null) {
continue;
} else if (derivedTuple.has(operand.identifier.id)) {
continue;
} else {
// Captured something other than the effect dep or setState
console.log('early return 1');
return;
}
}
// This might be wrong gotta double check
let hasInvalidDep = false;
for (const dep of effectDeps) {
const depMetadata = derivationCache.get(dep);
const depMetadata = derivedTuple.get(dep);
if (
effectFunction.context.find(operand => operand.identifier.id === dep) !=
null ||
(depMetadata !== undefined && depMetadata.typeOfValue !== 'ignored')
) {
isUsingDerivedDeps = true;
hasInvalidDep = true;
}
}
if (!isUsingDerivedDeps) {
// no prop/state derived deps were used in the body of the effect
if (!hasInvalidDep) {
console.log('early return 2');
// effect dep wasn't actually used in the function
return;
}
const seenBlocks: Set<BlockId> = new Set();
// This variable is suspicious maybe we don't need it?
const values: Map<IdentifierId, Array<IdentifierId>> = new Map();
const effectInvalidlyDerived: Map<IdentifierId, Place[]> = new Map();
const derivedSetStateCall: Array<SetStateCall> = [];
for (const dep of effectDeps) {
values.set(dep, [dep]);
const depMetadata = derivedTuple.get(dep);
if (depMetadata !== undefined) {
effectInvalidlyDerived.set(dep, depMetadata.sources);
}
}
const setStateCalls: Array<SetStateCall> = [];
for (const block of effectFunction.body.blocks.values()) {
for (const pred of block.preds) {
if (!seenBlocks.has(pred)) {
@@ -467,28 +369,33 @@ function validateEffect(
}
}
parseBlockPhi(block, derivationCache);
// TODO: This might need editing
for (const phi of block.phis) {
const aggregateDeps: Set<IdentifierId> = new Set();
let propsSources: Place[] | null = null;
for (const instr of block.instructions) {
if (
instr.value.kind === 'CallExpression' &&
isSetStateType(instr.value.callee.identifier) &&
instr.value.args.length === 1 &&
instr.value.args[0].kind === 'Identifier' &&
instr.value.callee.loc !== GeneratedSource &&
instr.value.callee.loc.identifierName !== undefined &&
instr.value.callee.loc.identifierName !== null
) {
if (effectSetStates.has(instr.value.callee.loc.identifierName)) {
effectSetStates
.get(instr.value.callee.loc.identifierName)!
.push(instr.value.callee);
} else {
effectSetStates.set(instr.value.callee.loc.identifierName, [
instr.value.callee,
]);
for (const operand of phi.operands.values()) {
const deps = values.get(operand.identifier.id);
if (deps != null) {
for (const dep of deps) {
aggregateDeps.add(dep);
}
}
const sources = effectInvalidlyDerived.get(operand.identifier.id);
if (sources != null) {
propsSources = sources;
}
}
if (aggregateDeps.size !== 0) {
values.set(phi.place.identifier.id, Array.from(aggregateDeps));
}
if (propsSources != null) {
effectInvalidlyDerived.set(phi.place.identifier.id, propsSources);
}
}
for (const instr of block.instructions) {
switch (instr.value.kind) {
case 'Primitive':
case 'JSXText':
@@ -496,6 +403,10 @@ function validateEffect(
break;
}
case 'LoadLocal': {
const deps = values.get(instr.value.place.identifier.id);
if (deps != null) {
values.set(instr.lvalue.identifier.id, deps);
}
break;
}
case 'ComputedLoad':
@@ -504,56 +415,95 @@ function validateEffect(
case 'TemplateLiteral':
case 'CallExpression':
case 'MethodCall': {
const aggregateDeps: Set<IdentifierId> = new Set();
for (const operand of eachInstructionOperand(instr)) {
const deps = values.get(operand.identifier.id);
if (deps != null) {
for (const dep of deps) {
aggregateDeps.add(dep);
}
}
}
if (aggregateDeps.size !== 0) {
values.set(instr.lvalue.identifier.id, Array.from(aggregateDeps));
}
if (
instr.value.kind === 'CallExpression' &&
isSetStateType(instr.value.callee.identifier) &&
instr.value.args.length === 1 &&
instr.value.args[0].kind === 'Identifier'
) {
const derivedDep = derivationCache.get(
instr.value.args[0].identifier.id,
);
const deps = values.get(instr.value.args[0].identifier.id);
console.log('deps', deps);
if (deps != null && new Set(deps).size === effectDeps.length) {
// console.log('setState arg', instr.value.args[0].identifier.id);
// console.log('effectInvalidlyDerived', effectInvalidlyDerived);
// console.log('derivedTuple', derivedTuple);
const propSources = derivedTuple.get(
instr.value.args[0].identifier.id,
);
if (derivedDep !== undefined) {
derivedSetStateCall.push({
loc: instr.value.callee.loc,
setStateId: instr.value.callee.identifier.id,
derivedDep: derivedDep,
});
console.log('Final reference', propSources);
if (propSources !== undefined) {
setStateCalls.push({
loc: instr.value.callee.loc,
propsSources: propSources.sources,
});
} else {
setStateCalls.push({
loc: instr.value.callee.loc,
propsSources: undefined,
});
}
} else {
// doesn't depend on all deps
console.log('early return 3');
return;
}
}
break;
}
default: {
console.log('early return 4');
return;
}
}
}
for (const operand of eachTerminalOperand(block.terminal)) {
if (values.has(operand.identifier.id)) {
return;
}
}
seenBlocks.add(block.id);
}
for (const call of derivedSetStateCall) {
const placeNames = Array.from(call.derivedDep.sources)
.map(place => {
return place.identifier.name?.value;
})
.filter(Boolean)
.join(', ');
console.log('setStateCalls', setStateCalls);
for (const call of setStateCalls) {
if (call.propsSources != null) {
const propNames = call.propsSources
.map(place => place.identifier.name?.value)
.join(', ');
const propInfo = propNames != null ? ` (from props '${propNames}')` : '';
let errorDescription = '';
if (call.derivedDep.typeOfValue === 'fromProps') {
errorDescription = `props [${placeNames}].`;
} else if (call.derivedDep.typeOfValue === 'fromState') {
errorDescription = `local state [${placeNames}].`;
errors.push({
reason: `Consider lifting state up to the parent component to make this a controlled component. (https://react.dev/learn/you-might-not-need-an-effect#adjusting-some-state-when-a-prop-changes)`,
description: `You are using props${propInfo} to update local state in an effect.`,
severity: ErrorSeverity.InvalidReact,
loc: call.loc,
suggestions: null,
});
} else {
errorDescription = `both props and local state [${placeNames}].`;
errors.push({
reason:
'You may not need this effect. Values derived from 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:
'This effect updates state based on other state values. ' +
'Consider calculating this value directly during render',
severity: ErrorSeverity.InvalidReact,
loc: call.loc,
suggestions: null,
});
}
errors.push({
type: call.derivedDep.typeOfValue,
description: `This setState() appears to derive a value from ${errorDescription}`,
loc: call.loc,
setStateName:
call.loc !== GeneratedSource ? call.loc.identifierName : undefined,
});
}
}

View File

@@ -24,15 +24,15 @@ function BadExample() {
```
Found 1 error:
Error: Derive values in render, not effects.
Error: You may not need this effect. Values derived from 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)
This setState() appears to derive a value from local state [firstName, lastName]. Derived values should be computed during render, rather than in effects. Using an effect triggers an additional render which can hurt performance and user experience, potentially briefly showing stale values to the user.
This effect updates state based on other state values. Consider calculating this value directly during render.
error.invalid-derived-computation-in-effect.ts:9:4
7 | const [fullName, setFullName] = useState('');
8 | useEffect(() => {
> 9 | setFullName(capitalize(firstName + ' ' + lastName));
| ^^^^^^^^^^^ This should be computed during render, not in an effect
| ^^^^^^^^^^^ You may not need this effect. Values derived from 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)
10 | }, [firstName, lastName]);
11 |
12 | return <div>{fullName}</div>;

View File

@@ -0,0 +1,82 @@
## Input
```javascript
// @compilationMode:"infer"
function Component() {
const dispatch = useDispatch();
// const [state, setState] = useState(0);
return (
<div>
<input
type="file"
onChange={event => {
dispatch(...event.target);
event.target.value = '';
}}
/>
</div>
);
}
function useDispatch() {
'use no memo';
// skip compilation to make it easier to debug the above function
return (...values) => {
console.log(...values);
};
}
export const FIXTURE_ENTRYPOINT = {
fn: Component,
params: [{}],
};
```
## Code
```javascript
import { c as _c } from "react/compiler-runtime"; // @compilationMode:"infer"
function Component() {
const $ = _c(2);
const dispatch = useDispatch();
let t0;
if ($[0] !== dispatch) {
t0 = (
<div>
<input
type="file"
onChange={(event) => {
dispatch(...event.target);
event.target.value = "";
}}
/>
</div>
);
$[0] = dispatch;
$[1] = t0;
} else {
t0 = $[1];
}
return t0;
}
function useDispatch() {
"use no memo";
// skip compilation to make it easier to debug the above function
return (...values) => {
console.log(...values);
};
}
export const FIXTURE_ENTRYPOINT = {
fn: Component,
params: [{}],
};
```
### Eval output
(kind: ok) <div><input type="file"></div>

View File

@@ -0,0 +1,30 @@
// @compilationMode:"infer"
function Component() {
const dispatch = useDispatch();
// const [state, setState] = useState(0);
return (
<div>
<input
type="file"
onChange={event => {
dispatch(...event.target);
event.target.value = '';
}}
/>
</div>
);
}
function useDispatch() {
'use no memo';
// skip compilation to make it easier to debug the above function
return (...values) => {
console.log(...values);
};
}
export const FIXTURE_ENTRYPOINT = {
fn: Component,
params: [{}],
};

View File

@@ -0,0 +1,79 @@
## Input
```javascript
// @validateNoDerivedComputationsInEffects
import {useEffect, useState} from 'react';
function Component({value, enabled}) {
const [localValue, setLocalValue] = useState('');
useEffect(() => {
if (enabled) {
setLocalValue(value);
} else {
setLocalValue('disabled');
}
}, [value, enabled]);
return <div>{localValue}</div>;
}
export const FIXTURE_ENTRYPOINT = {
fn: Component,
params: [{value: 'test', enabled: true}],
};
```
## Code
```javascript
import { c as _c } from "react/compiler-runtime"; // @validateNoDerivedComputationsInEffects
import { useEffect, useState } from "react";
function Component(t0) {
const $ = _c(6);
const { value, enabled } = t0;
const [localValue, setLocalValue] = useState("");
let t1;
let t2;
if ($[0] !== enabled || $[1] !== value) {
t1 = () => {
if (enabled) {
setLocalValue(value);
} else {
setLocalValue("disabled");
}
};
t2 = [value, enabled];
$[0] = enabled;
$[1] = value;
$[2] = t1;
$[3] = t2;
} else {
t1 = $[2];
t2 = $[3];
}
useEffect(t1, t2);
let t3;
if ($[4] !== localValue) {
t3 = <div>{localValue}</div>;
$[4] = localValue;
$[5] = t3;
} else {
t3 = $[5];
}
return t3;
}
export const FIXTURE_ENTRYPOINT = {
fn: Component,
params: [{ value: "test", enabled: true }],
};
```
### Eval output
(kind: ok) <div>test</div>

View File

@@ -0,0 +1,74 @@
## Input
```javascript
// @validateNoDerivedComputationsInEffects
import {useEffect, useState} from 'react';
function Component({value}) {
const [localValue, setLocalValue] = useState('');
useEffect(() => {
console.log('Value changed:', value);
setLocalValue(value);
document.title = `Value: ${value}`;
}, [value]);
return <div>{localValue}</div>;
}
export const FIXTURE_ENTRYPOINT = {
fn: Component,
params: [{value: 'test'}],
};
```
## Code
```javascript
import { c as _c } from "react/compiler-runtime"; // @validateNoDerivedComputationsInEffects
import { useEffect, useState } from "react";
function Component(t0) {
const $ = _c(5);
const { value } = t0;
const [localValue, setLocalValue] = useState("");
let t1;
let t2;
if ($[0] !== value) {
t1 = () => {
console.log("Value changed:", value);
setLocalValue(value);
document.title = `Value: ${value}`;
};
t2 = [value];
$[0] = value;
$[1] = t1;
$[2] = t2;
} else {
t1 = $[1];
t2 = $[2];
}
useEffect(t1, t2);
let t3;
if ($[3] !== localValue) {
t3 = <div>{localValue}</div>;
$[3] = localValue;
$[4] = t3;
} else {
t3 = $[4];
}
return t3;
}
export const FIXTURE_ENTRYPOINT = {
fn: Component,
params: [{ value: "test" }],
};
```
### Eval output
(kind: ok) <div>test</div>
logs: ['Value changed:','test']

View File

@@ -34,15 +34,15 @@ export const FIXTURE_ENTRYPOINT = {
```
Found 1 error:
Error: Derive values in render, not effects.
Error: You may not need this effect. Values derived from 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)
This setState() appears to derive a value from both props and local state [prefix, name]. Derived values should be computed during render, rather than in effects. Using an effect triggers an additional render which can hurt performance and user experience, potentially briefly showing stale values to the user.
This effect updates state based on other state values. Consider calculating this value directly during render.
error.bug-derived-state-from-mixed-deps.ts:9:4
7 |
8 | useEffect(() => {
> 9 | setDisplayName(prefix + name);
| ^^^^^^^^^^^^^^ This should be computed during render, not in an effect
| ^^^^^^^^^^^^^^ You may not need this effect. Values derived from 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)
10 | }, [prefix, name]);
11 |
12 | return (

View File

@@ -1,58 +0,0 @@
## Input
```javascript
// @validateNoDerivedComputationsInEffects
import {useState, useEffect} from 'react';
function Component({props, number}) {
const nothing = 0;
const missDirection = number;
const [displayValue, setDisplayValue] = useState(props.prefix + missDirection + nothing);
useEffect(() => {
setDisplayValue(props.prefix + missDirection + nothing);
}, [props.prefix, missDirection, nothing]);
return (
<div
onClick={() => {
setDisplayValue('clicked');
}}>
{displayValue}
</div>
);
}
```
## Error
```
Found 1 error:
Error: Local state shadows parent state.
This setState() appears to derive a value from props [props, number]. This state value shadows a value passed as a prop. Instead of shadowing the prop with local state, hoist the state to the parent component and update it there.
error.derived-state-from-shadowed-props.ts:10:4
8 |
9 | useEffect(() => {
> 10 | setDisplayValue(props.prefix + missDirection + nothing);
| ^^^^^^^^^^^^^^^ this setState synchronizes the state
11 | }, [props.prefix, missDirection, nothing]);
12 |
13 | return (
error.derived-state-from-shadowed-props.ts:16:8
14 | <div
15 | onClick={() => {
> 16 | setDisplayValue('clicked');
| ^^^^^^^^^^^^^^^ this setState updates the shadowed state, but should call an onChange event from the parent
17 | }}>
18 | {displayValue}
19 | </div>
```

View File

@@ -1,21 +0,0 @@
// @validateNoDerivedComputationsInEffects
import {useState, useEffect} from 'react';
function Component({props, number}) {
const nothing = 0;
const missDirection = number;
const [displayValue, setDisplayValue] = useState(props.prefix + missDirection + nothing);
useEffect(() => {
setDisplayValue(props.prefix + missDirection + nothing);
}, [props.prefix, missDirection, nothing]);
return (
<div
onClick={() => {
setDisplayValue('clicked');
}}>
{displayValue}
</div>
);
}

View File

@@ -1,49 +0,0 @@
## Input
```javascript
// @validateNoDerivedComputationsInEffects
import {useEffect, useState} from 'react';
function Component({value, enabled}) {
const [localValue, setLocalValue] = useState('');
useEffect(() => {
if (enabled) {
setLocalValue(value);
} else {
setLocalValue('disabled');
}
}, [value, enabled]);
return <div>{localValue}</div>;
}
export const FIXTURE_ENTRYPOINT = {
fn: Component,
params: [{value: 'test', enabled: true}],
};
```
## Error
```
Found 1 error:
Error: Derive values in render, not effects.
This setState() appears to derive a value from props [value]. Derived values should be computed during render, rather than in effects. Using an effect triggers an additional render which can hurt performance and user experience, potentially briefly showing stale values to the user.
error.derived-state-with-conditional.ts:9:6
7 | useEffect(() => {
8 | if (enabled) {
> 9 | setLocalValue(value);
| ^^^^^^^^^^^^^ This should be computed during render, not in an effect
10 | } else {
11 | setLocalValue('disabled');
12 | }
```

View File

@@ -1,47 +0,0 @@
## Input
```javascript
// @validateNoDerivedComputationsInEffects
import {useEffect, useState} from 'react';
function Component({value}) {
const [localValue, setLocalValue] = useState('');
useEffect(() => {
console.log('Value changed:', value);
setLocalValue(value);
document.title = `Value: ${value}`;
}, [value]);
return <div>{localValue}</div>;
}
export const FIXTURE_ENTRYPOINT = {
fn: Component,
params: [{value: 'test'}],
};
```
## Error
```
Found 1 error:
Error: Derive values in render, not effects.
This setState() appears to derive a value from props [value]. Derived values should be computed during render, rather than in effects. Using an effect triggers an additional render which can hurt performance and user experience, potentially briefly showing stale values to the user.
error.derived-state-with-side-effects.ts:9:4
7 | useEffect(() => {
8 | console.log('Value changed:', value);
> 9 | setLocalValue(value);
| ^^^^^^^^^^^^^ This should be computed during render, not in an effect
10 | document.title = `Value: ${value}`;
11 | }, [value]);
12 |
```

View File

@@ -1,46 +0,0 @@
## Input
```javascript
// @validateNoDerivedComputationsInEffects
import {useEffect, useState} from 'react';
function Component(props) {
const [displayValue, setDisplayValue] = useState('');
useEffect(() => {
const computed = props.prefix + props.value + props.suffix;
setDisplayValue(computed);
}, [props.prefix, props.value, props.suffix]);
return <div>{displayValue}</div>;
}
export const FIXTURE_ENTRYPOINT = {
fn: Component,
params: [{prefix: '[', value: 'test', suffix: ']'}],
};
```
## Error
```
Found 1 error:
Error: Derive values in render, not effects.
This setState() appears to derive a value from props [props]. Derived values should be computed during render, rather than in effects. Using an effect triggers an additional render which can hurt performance and user experience, potentially briefly showing stale values to the user.
error.invalid-derived-state-from-props-computed.ts:9:4
7 | useEffect(() => {
8 | const computed = props.prefix + props.value + props.suffix;
> 9 | setDisplayValue(computed);
| ^^^^^^^^^^^^^^^ This should be computed during render, not in an effect
10 | }, [props.prefix, props.value, props.suffix]);
11 |
12 | return <div>{displayValue}</div>;
```

View File

@@ -5,19 +5,19 @@
// @validateNoDerivedComputationsInEffects
import {useEffect, useState} from 'react';
function Component({props}) {
const [fullName, setFullName] = useState(props.firstName + ' ' + props.lastName);
function Component({user: {firstName, lastName}}) {
const [fullName, setFullName] = useState('');
useEffect(() => {
setFullName(props.firstName + ' ' + props.lastName);
}, [props.firstName, props.lastName]);
setFullName(firstName + ' ' + lastName);
}, [firstName, lastName]);
return <div>{fullName}</div>;
}
export const FIXTURE_ENTRYPOINT = {
fn: Component,
params: [{firstName: 'John', lastName: 'Doe'}],
params: [{user: {firstName: 'John', lastName: 'Doe'}}],
};
```
@@ -28,16 +28,16 @@ export const FIXTURE_ENTRYPOINT = {
```
Found 1 error:
Error: Derive values in render, not effects.
Error: You may not need this effect. Values derived from 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)
This setState() appears to derive a value from props [props]. Derived values should be computed during render, rather than in effects. Using an effect triggers an additional render which can hurt performance and user experience, potentially briefly showing stale values to the user.
This effect updates state based on other state values. Consider calculating this value directly during render.
error.invalid-derived-state-from-props-destructured.ts:8:4
6 |
7 | useEffect(() => {
> 8 | setFullName(props.firstName + ' ' + props.lastName);
| ^^^^^^^^^^^ This should be computed during render, not in an effect
9 | }, [props.firstName, props.lastName]);
> 8 | setFullName(firstName + ' ' + lastName);
| ^^^^^^^^^^^ You may not need this effect. Values derived from 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 | }, [firstName, lastName]);
10 |
11 | return <div>{fullName}</div>;
```

View File

@@ -1,12 +1,12 @@
// @validateNoDerivedComputationsInEffects
import {useEffect, useState} from 'react';
function Component({props}) {
const [fullName, setFullName] = useState(props.firstName + ' ' + props.lastName);
function Component({firstName, lastName}) {
const [fullName, setFullName] = useState('');
useEffect(() => {
setFullName(props.firstName + ' ' + props.lastName);
}, [props.firstName, props.lastName]);
setFullName(firstName + ' ' + lastName);
}, [firstName, lastName]);
return <div>{fullName}</div>;
}

View File

@@ -28,15 +28,15 @@ export const FIXTURE_ENTRYPOINT = {
```
Found 1 error:
Error: Derive values in render, not effects.
Error: You may not need this effect. Values derived from 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)
This setState() appears to derive a value from props [firstName, lastName]. Derived values should be computed during render, rather than in effects. Using an effect triggers an additional render which can hurt performance and user experience, potentially briefly showing stale values to the user.
This effect updates state based on other state values. Consider calculating this value directly during render.
error.invalid-derived-state-from-props-in-effect.ts:8:4
6 |
7 | useEffect(() => {
> 8 | setFullName(firstName + ' ' + lastName);
| ^^^^^^^^^^^ This should be computed during render, not in an effect
| ^^^^^^^^^^^ You may not need this effect. Values derived from 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 | }, [firstName, lastName]);
10 |
11 | return <div>{fullName}</div>;

View File

@@ -1,43 +0,0 @@
## Input
```javascript
// @validateNoDerivedComputationsInEffects
export default function InProductLobbyGeminiCard(
input = 'empty',
) {
const [currInput, setCurrInput] = useState(input);
useEffect(() => {
setCurrInput(input)
}, [input]);
return (
<div>{currInput}</div>
)
}
```
## Error
```
Found 1 error:
Error: Derive values in render, not effects.
This setState() appears to derive a value from props [input]. Derived values should be computed during render, rather than in effects. Using an effect triggers an additional render which can hurt performance and user experience, potentially briefly showing stale values to the user.
error.invalid-derived-state-from-props-with-default-value.ts:9:4
7 |
8 | useEffect(() => {
> 9 | setCurrInput(input)
| ^^^^^^^^^^^^ This should be computed during render, not in an effect
10 | }, [input]);
11 |
12 | return (
```

View File

@@ -1,15 +0,0 @@
// @validateNoDerivedComputationsInEffects
export default function InProductLobbyGeminiCard(
input = 'empty',
) {
const [currInput, setCurrInput] = useState(input);
useEffect(() => {
setCurrInput(input)
}, [input]);
return (
<div>{currInput}</div>
)
}

View File

@@ -36,15 +36,15 @@ export const FIXTURE_ENTRYPOINT = {
```
Found 1 error:
Error: Derive values in render, not effects.
Error: You may not need this effect. Values derived from 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)
This setState() appears to derive a value from local state [firstName, lastName]. Derived values should be computed during render, rather than in effects. Using an effect triggers an additional render which can hurt performance and user experience, potentially briefly showing stale values to the user.
This effect updates state based on other state values. Consider calculating this value directly during render.
error.invalid-derived-state-from-state-in-effect.ts:10:4
8 |
9 | useEffect(() => {
> 10 | setFullName(firstName + ' ' + lastName);
| ^^^^^^^^^^^ This should be computed during render, not in an effect
| ^^^^^^^^^^^ You may not need this effect. Values derived from 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)
11 | }, [firstName, lastName]);
12 |
13 | return (

View File

@@ -0,0 +1,72 @@
## Input
```javascript
// @validateNoDerivedComputationsInEffects
import {useEffect, useState} from 'react';
function Component(props) {
const [displayValue, setDisplayValue] = useState('');
useEffect(() => {
const computed = props.prefix + props.value + props.suffix;
setDisplayValue(computed);
}, [props.prefix, props.value, props.suffix]);
return <div>{displayValue}</div>;
}
export const FIXTURE_ENTRYPOINT = {
fn: Component,
params: [{prefix: '[', value: 'test', suffix: ']'}],
};
```
## Code
```javascript
import { c as _c } from "react/compiler-runtime"; // @validateNoDerivedComputationsInEffects
import { useEffect, useState } from "react";
function Component(props) {
const $ = _c(7);
const [displayValue, setDisplayValue] = useState("");
let t0;
let t1;
if ($[0] !== props.prefix || $[1] !== props.suffix || $[2] !== props.value) {
t0 = () => {
const computed = props.prefix + props.value + props.suffix;
setDisplayValue(computed);
};
t1 = [props.prefix, props.value, props.suffix];
$[0] = props.prefix;
$[1] = props.suffix;
$[2] = props.value;
$[3] = t0;
$[4] = t1;
} else {
t0 = $[3];
t1 = $[4];
}
useEffect(t0, t1);
let t2;
if ($[5] !== displayValue) {
t2 = <div>{displayValue}</div>;
$[5] = displayValue;
$[6] = t2;
} else {
t2 = $[6];
}
return t2;
}
export const FIXTURE_ENTRYPOINT = {
fn: Component,
params: [{ prefix: "[", value: "test", suffix: "]" }],
};
```
### Eval output
(kind: ok) <div>[test]</div>