Compare commits
1 Commits
eslint-plu
...
pr35134
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
cd1d0dc2dc |
@@ -22,6 +22,7 @@ import {
|
|||||||
BasicBlock,
|
BasicBlock,
|
||||||
isUseRefType,
|
isUseRefType,
|
||||||
SourceLocation,
|
SourceLocation,
|
||||||
|
ArrayExpression,
|
||||||
} from '../HIR';
|
} from '../HIR';
|
||||||
import {eachInstructionLValue, eachInstructionOperand} from '../HIR/visitors';
|
import {eachInstructionLValue, eachInstructionOperand} from '../HIR/visitors';
|
||||||
import {isMutable} from '../ReactiveScopes/InferReactiveScopeVariables';
|
import {isMutable} from '../ReactiveScopes/InferReactiveScopeVariables';
|
||||||
@@ -36,11 +37,17 @@ type DerivationMetadata = {
|
|||||||
isStateSource: boolean;
|
isStateSource: boolean;
|
||||||
};
|
};
|
||||||
|
|
||||||
|
type EffectMetadata = {
|
||||||
|
effect: HIRFunction;
|
||||||
|
dependencies: ArrayExpression;
|
||||||
|
};
|
||||||
|
|
||||||
type ValidationContext = {
|
type ValidationContext = {
|
||||||
readonly functions: Map<IdentifierId, FunctionExpression>;
|
readonly functions: Map<IdentifierId, FunctionExpression>;
|
||||||
|
readonly candidateDependencies: Map<IdentifierId, ArrayExpression>;
|
||||||
readonly errors: CompilerError;
|
readonly errors: CompilerError;
|
||||||
readonly derivationCache: DerivationCache;
|
readonly derivationCache: DerivationCache;
|
||||||
readonly effects: Set<HIRFunction>;
|
readonly effectsCache: Map<IdentifierId, EffectMetadata>;
|
||||||
readonly setStateLoads: Map<IdentifierId, IdentifierId | null>;
|
readonly setStateLoads: Map<IdentifierId, IdentifierId | null>;
|
||||||
readonly setStateUsages: Map<IdentifierId, Set<SourceLocation>>;
|
readonly setStateUsages: Map<IdentifierId, Set<SourceLocation>>;
|
||||||
};
|
};
|
||||||
@@ -175,18 +182,20 @@ export function validateNoDerivedComputationsInEffects_exp(
|
|||||||
fn: HIRFunction,
|
fn: HIRFunction,
|
||||||
): Result<void, CompilerError> {
|
): Result<void, CompilerError> {
|
||||||
const functions: Map<IdentifierId, FunctionExpression> = new Map();
|
const functions: Map<IdentifierId, FunctionExpression> = new Map();
|
||||||
|
const candidateDependencies: Map<IdentifierId, ArrayExpression> = new Map();
|
||||||
const derivationCache = new DerivationCache();
|
const derivationCache = new DerivationCache();
|
||||||
const errors = new CompilerError();
|
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 setStateLoads: Map<IdentifierId, IdentifierId> = new Map();
|
||||||
const setStateUsages: Map<IdentifierId, Set<SourceLocation>> = new Map();
|
const setStateUsages: Map<IdentifierId, Set<SourceLocation>> = new Map();
|
||||||
|
|
||||||
const context: ValidationContext = {
|
const context: ValidationContext = {
|
||||||
functions,
|
functions,
|
||||||
|
candidateDependencies,
|
||||||
errors,
|
errors,
|
||||||
derivationCache,
|
derivationCache,
|
||||||
effects,
|
effectsCache,
|
||||||
setStateLoads,
|
setStateLoads,
|
||||||
setStateUsages,
|
setStateUsages,
|
||||||
};
|
};
|
||||||
@@ -229,8 +238,8 @@ export function validateNoDerivedComputationsInEffects_exp(
|
|||||||
isFirstPass = false;
|
isFirstPass = false;
|
||||||
} while (context.derivationCache.snapshot());
|
} while (context.derivationCache.snapshot());
|
||||||
|
|
||||||
for (const effect of effects) {
|
for (const [, effect] of effectsCache) {
|
||||||
validateEffect(effect, context);
|
validateEffect(effect.effect, effect.dependencies, context);
|
||||||
}
|
}
|
||||||
|
|
||||||
return errors.asResult();
|
return errors.asResult();
|
||||||
@@ -354,8 +363,14 @@ function recordInstructionDerivations(
|
|||||||
value.args[1].kind === 'Identifier'
|
value.args[1].kind === 'Identifier'
|
||||||
) {
|
) {
|
||||||
const effectFunction = context.functions.get(value.args[0].identifier.id);
|
const effectFunction = context.functions.get(value.args[0].identifier.id);
|
||||||
if (effectFunction != null) {
|
const deps = context.candidateDependencies.get(
|
||||||
context.effects.add(effectFunction.loweredFunc.func);
|
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) {
|
} else if (isUseStateType(lvalue.identifier) && value.args.length > 0) {
|
||||||
typeOfValue = 'fromState';
|
typeOfValue = 'fromState';
|
||||||
@@ -367,6 +382,8 @@ function recordInstructionDerivations(
|
|||||||
);
|
);
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
|
} else if (value.kind === 'ArrayExpression') {
|
||||||
|
context.candidateDependencies.set(lvalue.identifier.id, value);
|
||||||
}
|
}
|
||||||
|
|
||||||
for (const operand of eachInstructionOperand(instr)) {
|
for (const operand of eachInstructionOperand(instr)) {
|
||||||
@@ -596,6 +613,7 @@ function getFnLocalDeps(
|
|||||||
|
|
||||||
function validateEffect(
|
function validateEffect(
|
||||||
effectFunction: HIRFunction,
|
effectFunction: HIRFunction,
|
||||||
|
dependencies: ArrayExpression,
|
||||||
context: ValidationContext,
|
context: ValidationContext,
|
||||||
): void {
|
): void {
|
||||||
const seenBlocks: Set<BlockId> = new Set();
|
const seenBlocks: Set<BlockId> = new Set();
|
||||||
@@ -612,6 +630,16 @@ function validateEffect(
|
|||||||
Set<SourceLocation>
|
Set<SourceLocation>
|
||||||
> = new Map();
|
> = 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;
|
let cleanUpFunctionDeps: Set<IdentifierId> | undefined;
|
||||||
|
|
||||||
const globals: Set<IdentifierId> = new Set();
|
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