Compare commits

..

1 Commits

Author SHA1 Message Date
Jorge Cabiedes Acosta
1d9ccd1d1b [Compiler] ValidateNoDerivedComputationsInEffects test cases
Summary:
This creates the test cases we expect this first iteration of calculate in render to catch

The goal is to have tests that will be in a good state once we have the first iteration of the calculate in render validation working, which should be pretty limited in what its capturing.

Test Plan:
Test cases
2025-10-20 17:04:25 -07:00
27 changed files with 366 additions and 30 deletions

View File

@@ -103,6 +103,7 @@ import {validateNoFreezingKnownMutableFunctions} from '../Validation/ValidateNoF
import {inferMutationAliasingEffects} from '../Inference/InferMutationAliasingEffects';
import {inferMutationAliasingRanges} from '../Inference/InferMutationAliasingRanges';
import {validateNoDerivedComputationsInEffects} from '../Validation/ValidateNoDerivedComputationsInEffects';
import {validateNoDerivedComputationsInEffects_exp} from '../Validation/ValidateNoDerivedComputationsInEffects_exp';
import {nameAnonymousFunctions} from '../Transform/NameAnonymousFunctions';
export type CompilerPipelineValue =
@@ -275,6 +276,10 @@ function runWithEnvironment(
validateNoDerivedComputationsInEffects(hir);
}
if (env.config.validateNoDerivedComputationsInEffects_exp) {
validateNoDerivedComputationsInEffects_exp(hir);
}
if (env.config.validateNoSetStateInEffects) {
env.logErrors(validateNoSetStateInEffects(hir, env));
}

View File

@@ -334,6 +334,12 @@ export const EnvironmentConfigSchema = z.object({
*/
validateNoDerivedComputationsInEffects: z.boolean().default(false),
/**
* Experimental: Validates that effects are not used to calculate derived data which could instead be computed
* during render. Generates a custom error message for each type of violation.
*/
validateNoDerivedComputationsInEffects_exp: z.boolean().default(false),
/**
* Validates against creating JSX within a try block and recommends using an error boundary
* instead.

View File

@@ -0,0 +1,240 @@
/**
* Copyright (c) Meta Platforms, Inc. and affiliates.
*
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*/
import {CompilerError, SourceLocation} from '..';
import {ErrorCategory} from '../CompilerError';
import {
ArrayExpression,
BlockId,
FunctionExpression,
HIRFunction,
IdentifierId,
isSetStateType,
isUseEffectHookType,
} from '../HIR';
import {
eachInstructionValueOperand,
eachTerminalOperand,
} from '../HIR/visitors';
/**
* Validates that useEffect is not used for derived computations which could/should
* be performed in render.
*
* See https://react.dev/learn/you-might-not-need-an-effect#updating-state-based-on-props-or-state
*
* Example:
*
* ```
* // 🔴 Avoid: redundant state and unnecessary Effect
* const [fullName, setFullName] = useState('');
* useEffect(() => {
* setFullName(firstName + ' ' + lastName);
* }, [firstName, lastName]);
* ```
*
* Instead use:
*
* ```
* // ✅ Good: calculated during rendering
* const fullName = firstName + ' ' + lastName;
* ```
*/
export function validateNoDerivedComputationsInEffects_exp(
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 errors = new CompilerError();
for (const block of fn.body.blocks.values()) {
for (const instr of block.instructions) {
const {lvalue, value} = instr;
if (value.kind === 'LoadLocal') {
locals.set(lvalue.identifier.id, value.place.identifier.id);
} else if (value.kind === 'ArrayExpression') {
candidateDependencies.set(lvalue.identifier.id, value);
} else if (value.kind === 'FunctionExpression') {
functions.set(lvalue.identifier.id, value);
} else if (
value.kind === 'CallExpression' ||
value.kind === 'MethodCall'
) {
const callee =
value.kind === 'CallExpression' ? value.callee : value.property;
if (
isUseEffectHookType(callee.identifier) &&
value.args.length === 2 &&
value.args[0].kind === 'Identifier' &&
value.args[1].kind === 'Identifier'
) {
const effectFunction = functions.get(value.args[0].identifier.id);
const deps = candidateDependencies.get(value.args[1].identifier.id);
if (
effectFunction != null &&
deps != null &&
deps.elements.length !== 0 &&
deps.elements.every(element => element.kind === 'Identifier')
) {
const dependencies: Array<IdentifierId> = deps.elements.map(dep => {
CompilerError.invariant(dep.kind === 'Identifier', {
reason: `Dependency is checked as a place above`,
description: null,
details: [
{
kind: 'error',
loc: value.loc,
message: 'this is checked as a place above',
},
],
});
return locals.get(dep.identifier.id) ?? dep.identifier.id;
});
validateEffect(
effectFunction.loweredFunc.func,
dependencies,
errors,
);
}
}
}
}
}
if (errors.hasAnyErrors()) {
throw errors;
}
}
function validateEffect(
effectFunction: HIRFunction,
effectDeps: Array<IdentifierId>,
errors: CompilerError,
): void {
for (const operand of effectFunction.context) {
if (isSetStateType(operand.identifier)) {
continue;
} else if (effectDeps.find(dep => dep === operand.identifier.id) != null) {
continue;
} else {
// Captured something other than the effect dep or setState
return;
}
}
for (const dep of effectDeps) {
if (
effectFunction.context.find(operand => operand.identifier.id === dep) ==
null
) {
// effect dep wasn't actually used in the function
return;
}
}
const seenBlocks: Set<BlockId> = new Set();
const values: Map<IdentifierId, Array<IdentifierId>> = new Map();
for (const dep of effectDeps) {
values.set(dep, [dep]);
}
const setStateLocations: Array<SourceLocation> = [];
for (const block of effectFunction.body.blocks.values()) {
for (const pred of block.preds) {
if (!seenBlocks.has(pred)) {
// skip if block has a back edge
return;
}
}
for (const phi of block.phis) {
const aggregateDeps: Set<IdentifierId> = new Set();
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);
}
}
}
if (aggregateDeps.size !== 0) {
values.set(phi.place.identifier.id, Array.from(aggregateDeps));
}
}
for (const instr of block.instructions) {
switch (instr.value.kind) {
case 'Primitive':
case 'JSXText':
case 'LoadGlobal': {
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':
case 'PropertyLoad':
case 'BinaryExpression':
case 'TemplateLiteral':
case 'CallExpression':
case 'MethodCall': {
const aggregateDeps: Set<IdentifierId> = new Set();
for (const operand of eachInstructionValueOperand(instr.value)) {
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 deps = values.get(instr.value.args[0].identifier.id);
if (deps != null && new Set(deps).size === effectDeps.length) {
setStateLocations.push(instr.value.callee.loc);
} else {
// doesn't depend on any deps
return;
}
}
break;
}
default: {
return;
}
}
}
for (const operand of eachTerminalOperand(block.terminal)) {
if (values.has(operand.identifier.id)) {
//
return;
}
}
seenBlocks.add(block.id);
}
for (const loc of setStateLocations) {
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,
suggestions: null,
});
}
}

View File

@@ -2,7 +2,7 @@
## Input
```javascript
// @validateNoDerivedComputationsInEffects
// @validateNoDerivedComputationsInEffects_exp
import {useEffect, useState} from 'react';
function Component({value, enabled}) {
@@ -29,7 +29,7 @@ export const FIXTURE_ENTRYPOINT = {
## Code
```javascript
import { c as _c } from "react/compiler-runtime"; // @validateNoDerivedComputationsInEffects
import { c as _c } from "react/compiler-runtime"; // @validateNoDerivedComputationsInEffects_exp
import { useEffect, useState } from "react";
function Component(t0) {

View File

@@ -1,4 +1,4 @@
// @validateNoDerivedComputationsInEffects
// @validateNoDerivedComputationsInEffects_exp
import {useEffect, useState} from 'react';
function Component({value, enabled}) {

View File

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

View File

@@ -1,4 +1,4 @@
// @validateNoDerivedComputationsInEffects
// @validateNoDerivedComputationsInEffects_exp
import {useEffect, useState} from 'react';
export default function Component({input = 'empty'}) {

View File

@@ -0,0 +1,70 @@
## Input
```javascript
// @validateNoDerivedComputationsInEffects_exp
import {useEffect, useState} from 'react';
function Component({shouldChange}) {
const [count, setCount] = useState(0);
useEffect(() => {
if (shouldChange) {
setCount(count + 1);
}
}, [count]);
return <div>{count}</div>;
}
```
## Code
```javascript
import { c as _c } from "react/compiler-runtime"; // @validateNoDerivedComputationsInEffects_exp
import { useEffect, useState } from "react";
function Component(t0) {
const $ = _c(7);
const { shouldChange } = t0;
const [count, setCount] = useState(0);
let t1;
if ($[0] !== count || $[1] !== shouldChange) {
t1 = () => {
if (shouldChange) {
setCount(count + 1);
}
};
$[0] = count;
$[1] = shouldChange;
$[2] = t1;
} else {
t1 = $[2];
}
let t2;
if ($[3] !== count) {
t2 = [count];
$[3] = count;
$[4] = t2;
} else {
t2 = $[4];
}
useEffect(t1, t2);
let t3;
if ($[5] !== count) {
t3 = <div>{count}</div>;
$[5] = count;
$[6] = t3;
} else {
t3 = $[6];
}
return t3;
}
```
### Eval output
(kind: exception) Fixture not implemented

View File

@@ -0,0 +1,15 @@
// @validateNoDerivedComputationsInEffects_exp
import {useEffect, useState} from 'react';
function Component({shouldChange}) {
const [count, setCount] = useState(0);
useEffect(() => {
if (shouldChange) {
setCount(count + 1);
}
}, [count]);
return <div>{count}</div>;
}

View File

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

View File

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

View File

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

View File

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

View File

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

View File

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

View File

@@ -2,7 +2,7 @@
## Input
```javascript
// @validateNoDerivedComputationsInEffects
// @validateNoDerivedComputationsInEffects_exp
import {useEffect, useState} from 'react';
function Component({initialName}) {

View File

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

View File

@@ -2,7 +2,7 @@
## Input
```javascript
// @validateNoDerivedComputationsInEffects
// @validateNoDerivedComputationsInEffects_exp
import {useEffect, useState} from 'react';
function MockComponent({onSet}) {

View File

@@ -1,4 +1,4 @@
// @validateNoDerivedComputationsInEffects
// @validateNoDerivedComputationsInEffects_exp
import {useEffect, useState} from 'react';
function MockComponent({onSet}) {

View File

@@ -2,7 +2,7 @@
## Input
```javascript
// @validateNoDerivedComputationsInEffects
// @validateNoDerivedComputationsInEffects_exp
import {useEffect, useState} from 'react';
function Component({propValue}) {

View File

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

View File

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

View File

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

View File

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

View File

@@ -1,4 +1,4 @@
// @validateNoDerivedComputationsInEffects
// @validateNoDerivedComputationsInEffects_exp
import {useEffect, useState} from 'react';
export default function Component(props) {

View File

@@ -2,7 +2,7 @@
## Input
```javascript
// @validateNoDerivedComputationsInEffects
// @validateNoDerivedComputationsInEffects_exp
import {useEffect, useState} from 'react';
export default function Component({props}) {
@@ -27,7 +27,7 @@ export const FIXTURE_ENTRYPOINT = {
## Code
```javascript
import { c as _c } from "react/compiler-runtime"; // @validateNoDerivedComputationsInEffects
import { c as _c } from "react/compiler-runtime"; // @validateNoDerivedComputationsInEffects_exp
import { useEffect, useState } from "react";
export default function Component(t0) {

View File

@@ -1,4 +1,4 @@
// @validateNoDerivedComputationsInEffects
// @validateNoDerivedComputationsInEffects_exp
import {useEffect, useState} from 'react';
export default function Component({props}) {