Compare commits
1 Commits
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
cd1d0dc2dc |
@@ -22,6 +22,7 @@ import {
|
||||
BasicBlock,
|
||||
isUseRefType,
|
||||
SourceLocation,
|
||||
ArrayExpression,
|
||||
} from '../HIR';
|
||||
import {eachInstructionLValue, eachInstructionOperand} from '../HIR/visitors';
|
||||
import {isMutable} from '../ReactiveScopes/InferReactiveScopeVariables';
|
||||
@@ -36,11 +37,17 @@ type DerivationMetadata = {
|
||||
isStateSource: boolean;
|
||||
};
|
||||
|
||||
type EffectMetadata = {
|
||||
effect: HIRFunction;
|
||||
dependencies: ArrayExpression;
|
||||
};
|
||||
|
||||
type ValidationContext = {
|
||||
readonly functions: Map<IdentifierId, FunctionExpression>;
|
||||
readonly candidateDependencies: Map<IdentifierId, ArrayExpression>;
|
||||
readonly errors: CompilerError;
|
||||
readonly derivationCache: DerivationCache;
|
||||
readonly effects: Set<HIRFunction>;
|
||||
readonly effectsCache: Map<IdentifierId, EffectMetadata>;
|
||||
readonly setStateLoads: Map<IdentifierId, IdentifierId | null>;
|
||||
readonly setStateUsages: Map<IdentifierId, Set<SourceLocation>>;
|
||||
};
|
||||
@@ -175,18 +182,20 @@ export function validateNoDerivedComputationsInEffects_exp(
|
||||
fn: HIRFunction,
|
||||
): Result<void, CompilerError> {
|
||||
const functions: Map<IdentifierId, FunctionExpression> = new Map();
|
||||
const candidateDependencies: Map<IdentifierId, ArrayExpression> = new Map();
|
||||
const derivationCache = new DerivationCache();
|
||||
const errors = new CompilerError();
|
||||
const effects: Set<HIRFunction> = new Set();
|
||||
const effectsCache: Map<IdentifierId, EffectMetadata> = new Map();
|
||||
|
||||
const setStateLoads: Map<IdentifierId, IdentifierId> = new Map();
|
||||
const setStateUsages: Map<IdentifierId, Set<SourceLocation>> = new Map();
|
||||
|
||||
const context: ValidationContext = {
|
||||
functions,
|
||||
candidateDependencies,
|
||||
errors,
|
||||
derivationCache,
|
||||
effects,
|
||||
effectsCache,
|
||||
setStateLoads,
|
||||
setStateUsages,
|
||||
};
|
||||
@@ -229,8 +238,8 @@ export function validateNoDerivedComputationsInEffects_exp(
|
||||
isFirstPass = false;
|
||||
} while (context.derivationCache.snapshot());
|
||||
|
||||
for (const effect of effects) {
|
||||
validateEffect(effect, context);
|
||||
for (const [, effect] of effectsCache) {
|
||||
validateEffect(effect.effect, effect.dependencies, context);
|
||||
}
|
||||
|
||||
return errors.asResult();
|
||||
@@ -354,8 +363,14 @@ function recordInstructionDerivations(
|
||||
value.args[1].kind === 'Identifier'
|
||||
) {
|
||||
const effectFunction = context.functions.get(value.args[0].identifier.id);
|
||||
if (effectFunction != null) {
|
||||
context.effects.add(effectFunction.loweredFunc.func);
|
||||
const deps = context.candidateDependencies.get(
|
||||
value.args[1].identifier.id,
|
||||
);
|
||||
if (effectFunction != null && deps != null) {
|
||||
context.effectsCache.set(value.args[0].identifier.id, {
|
||||
effect: effectFunction.loweredFunc.func,
|
||||
dependencies: deps,
|
||||
});
|
||||
}
|
||||
} else if (isUseStateType(lvalue.identifier) && value.args.length > 0) {
|
||||
typeOfValue = 'fromState';
|
||||
@@ -367,6 +382,8 @@ function recordInstructionDerivations(
|
||||
);
|
||||
return;
|
||||
}
|
||||
} else if (value.kind === 'ArrayExpression') {
|
||||
context.candidateDependencies.set(lvalue.identifier.id, value);
|
||||
}
|
||||
|
||||
for (const operand of eachInstructionOperand(instr)) {
|
||||
@@ -596,6 +613,7 @@ function getFnLocalDeps(
|
||||
|
||||
function validateEffect(
|
||||
effectFunction: HIRFunction,
|
||||
dependencies: ArrayExpression,
|
||||
context: ValidationContext,
|
||||
): void {
|
||||
const seenBlocks: Set<BlockId> = new Set();
|
||||
@@ -612,6 +630,16 @@ function validateEffect(
|
||||
Set<SourceLocation>
|
||||
> = new Map();
|
||||
|
||||
// Consider setStates in the effect's dependency array as being part of effectSetStateUsages
|
||||
for (const dep of dependencies.elements) {
|
||||
if (dep.kind === 'Identifier') {
|
||||
const root = getRootSetState(dep.identifier.id, context.setStateLoads);
|
||||
if (root !== null) {
|
||||
effectSetStateUsages.set(root, new Set([dep.loc]));
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
let cleanUpFunctionDeps: Set<IdentifierId> | undefined;
|
||||
|
||||
const globals: Set<IdentifierId> = new Set();
|
||||
|
||||
@@ -0,0 +1,63 @@
|
||||
|
||||
## Input
|
||||
|
||||
```javascript
|
||||
// @validateNoDerivedComputationsInEffects_exp @loggerTestOnly
|
||||
|
||||
function Component({prop}) {
|
||||
const [s, setS] = useState(0);
|
||||
useEffect(() => {
|
||||
setS(prop);
|
||||
}, [prop, setS]);
|
||||
|
||||
return <div>{prop}</div>;
|
||||
}
|
||||
|
||||
```
|
||||
|
||||
## Code
|
||||
|
||||
```javascript
|
||||
import { c as _c } from "react/compiler-runtime"; // @validateNoDerivedComputationsInEffects_exp @loggerTestOnly
|
||||
|
||||
function Component(t0) {
|
||||
const $ = _c(5);
|
||||
const { prop } = t0;
|
||||
const [, setS] = useState(0);
|
||||
let t1;
|
||||
let t2;
|
||||
if ($[0] !== prop) {
|
||||
t1 = () => {
|
||||
setS(prop);
|
||||
};
|
||||
t2 = [prop, setS];
|
||||
$[0] = prop;
|
||||
$[1] = t1;
|
||||
$[2] = t2;
|
||||
} else {
|
||||
t1 = $[1];
|
||||
t2 = $[2];
|
||||
}
|
||||
useEffect(t1, t2);
|
||||
let t3;
|
||||
if ($[3] !== prop) {
|
||||
t3 = <div>{prop}</div>;
|
||||
$[3] = prop;
|
||||
$[4] = t3;
|
||||
} else {
|
||||
t3 = $[4];
|
||||
}
|
||||
return t3;
|
||||
}
|
||||
|
||||
```
|
||||
|
||||
## Logs
|
||||
|
||||
```
|
||||
{"kind":"CompileError","detail":{"options":{"description":"Using an effect triggers an additional render which can hurt performance and user experience, potentially briefly showing stale values to the user\n\nThis setState call is setting a derived value that depends on the following reactive sources:\n\nProps: [prop]\n\nData Flow Tree:\n└── prop (Prop)\n\nSee: https://react.dev/learn/you-might-not-need-an-effect#updating-state-based-on-props-or-state","category":"EffectDerivationsOfState","reason":"You might not need an effect. Derive values in render, not effects.","details":[{"kind":"error","loc":{"start":{"line":6,"column":4,"index":150},"end":{"line":6,"column":8,"index":154},"filename":"effect-used-in-dep-array-still-errors.ts","identifierName":"setS"},"message":"This should be computed during render, not in an effect"}]}},"fnLoc":null}
|
||||
{"kind":"CompileSuccess","fnLoc":{"start":{"line":3,"column":0,"index":64},"end":{"line":10,"column":1,"index":212},"filename":"effect-used-in-dep-array-still-errors.ts"},"fnName":"Component","memoSlots":5,"memoBlocks":2,"memoValues":3,"prunedMemoBlocks":0,"prunedMemoValues":0}
|
||||
```
|
||||
|
||||
### Eval output
|
||||
(kind: exception) Fixture not implemented
|
||||
@@ -0,0 +1,10 @@
|
||||
// @validateNoDerivedComputationsInEffects_exp @loggerTestOnly
|
||||
|
||||
function Component({prop}) {
|
||||
const [s, setS] = useState(0);
|
||||
useEffect(() => {
|
||||
setS(prop);
|
||||
}, [prop, setS]);
|
||||
|
||||
return <div>{prop}</div>;
|
||||
}
|
||||
Reference in New Issue
Block a user