Files
react/compiler/packages/babel-plugin-react-compiler/src/Validation/ValidateNoSetStateInRender.ts
T
Joe Savona bc8ab8ca6d [compiler][nocommit] Quick sketch of types on places
This is a quick sketch of moving types from Identifier to Places so that we can have flow-sensitive types. The intent isn't to ship this but to quickly explore in order to figure out concrete challenges, to inform a "real" implementation.

Some observations:

* ReactiveScopeDependency/Declaration now need types. We use the type of their identifier currently, so we'd have to populate a type for them instead. But if we do flow-sensitive types, there won't be one obvious correct type to use! Consider a scope that uses `x` twice, once where we can infer its a primitive and one where we can't. We should treat this like phi typing and only infer a precise type for the dep/decl if all references have the same type.

* InferMutableRanges's aliasing logic uses a `DisjointSet<Identifier>` and checks the types for some things (refs in particular). So the obvious approach is to replace that with a `DisjointSet<Place>`. While doing that I was reminded that the way we handle aliasing for phis is kind of weird. We currently delay creating an alias until we know the phi is mutated later, but we don't do the same thing for things like `x = y` (ie we eagerly alias). Switching to paths is a good chance to revisit the aliasing.

* InferTypes gets tricky because we still want different places with the same identifier to get the same type (for now, until we introduce flow-sensitive typing). But every Place has its own type instance. So for now we can basically keep a mapping of IdentifierId to a canonical Type and use this for all the inference. The actual implementation in the PR is messier than that since i started with a variant of flow-sensitive typing and then rolled it back.

For actual flow-sensitive typing (not implemented here) there's a sort of inverse phi situation. Consider a variant of mofeiZ's recent find:

```
function Component({y}) {
  let x = makeValue(y);
  let result;
  if (...cond...) {
    result = ...x... // do something with x
  } else {
    result = ...x... // do something else with x
  }
  return result;
}
```

If both branches of the if can infer `x` as a number, then it's sound to infer `makeValue(y)` as producing a number. However, if you take away the else branch then it might not be, since now there's a code path (the fallthrough) in which we're not sure of the type. This is just like a phi for variable reassignment, but at the type level. And it's also happening in reverse — the later "operands" (usages of x) flow backwards into the "phi" that is the type of x before the if/else. We'd have to build up a mapping like this and build the appropriate type equations.

ghstack-source-id: 8360182e32
Pull Request resolved: https://github.com/facebook/react/pull/31575
2024-11-19 20:45:40 -05:00

154 lines
5.2 KiB
TypeScript

/**
* 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, ErrorSeverity} from '../CompilerError';
import {HIRFunction, IdentifierId, isSetStateType} from '../HIR';
import {computeUnconditionalBlocks} from '../HIR/ComputeUnconditionalBlocks';
import {eachInstructionValueOperand} from '../HIR/visitors';
import {Err, Ok, Result} from '../Utils/Result';
/**
* Validates that the given function does not have an infinite update loop
* caused by unconditionally calling setState during render. This validation
* is conservative and cannot catch all cases of unconditional setState in
* render, but avoids false positives. Examples of cases that are caught:
*
* ```javascript
* // Direct call of setState:
* const [state, setState] = useState(false);
* setState(true);
*
* // Indirect via a function:
* const [state, setState] = useState(false);
* const setTrue = () => setState(true);
* setTrue();
* ```
*
* However, storing setState inside another value and accessing it is not yet
* validated:
*
* ```
* // false negative, not detected but will cause an infinite render loop
* const [state, setState] = useState(false);
* const x = [setState];
* const y = x.pop();
* y();
* ```
*/
export function validateNoSetStateInRender(fn: HIRFunction): void {
const unconditionalSetStateFunctions: Set<IdentifierId> = new Set();
validateNoSetStateInRenderImpl(fn, unconditionalSetStateFunctions).unwrap();
}
function validateNoSetStateInRenderImpl(
fn: HIRFunction,
unconditionalSetStateFunctions: Set<IdentifierId>,
): Result<void, CompilerError> {
const unconditionalBlocks = computeUnconditionalBlocks(fn);
let activeManualMemoId: number | null = null;
const errors = new CompilerError();
for (const [, block] of fn.body.blocks) {
for (const instr of block.instructions) {
switch (instr.value.kind) {
case 'LoadLocal': {
if (
unconditionalSetStateFunctions.has(instr.value.place.identifier.id)
) {
unconditionalSetStateFunctions.add(instr.lvalue.identifier.id);
}
break;
}
case 'StoreLocal': {
if (
unconditionalSetStateFunctions.has(instr.value.value.identifier.id)
) {
unconditionalSetStateFunctions.add(
instr.value.lvalue.place.identifier.id,
);
unconditionalSetStateFunctions.add(instr.lvalue.identifier.id);
}
break;
}
case 'ObjectMethod':
case 'FunctionExpression': {
if (
// faster-path to check if the function expression references a setState
[...eachInstructionValueOperand(instr.value)].some(
operand =>
isSetStateType(operand.type) ||
unconditionalSetStateFunctions.has(operand.identifier.id),
) &&
// if yes, does it unconditonally call it?
validateNoSetStateInRenderImpl(
instr.value.loweredFunc.func,
unconditionalSetStateFunctions,
).isErr()
) {
// This function expression unconditionally calls a setState
unconditionalSetStateFunctions.add(instr.lvalue.identifier.id);
}
break;
}
case 'StartMemoize': {
CompilerError.invariant(activeManualMemoId === null, {
reason: 'Unexpected nested StartMemoize instructions',
loc: instr.value.loc,
});
activeManualMemoId = instr.value.manualMemoId;
break;
}
case 'FinishMemoize': {
CompilerError.invariant(
activeManualMemoId === instr.value.manualMemoId,
{
reason:
'Expected FinishMemoize to align with previous StartMemoize instruction',
loc: instr.value.loc,
},
);
activeManualMemoId = null;
break;
}
case 'CallExpression': {
const callee = instr.value.callee;
if (
isSetStateType(callee.type) ||
unconditionalSetStateFunctions.has(callee.identifier.id)
) {
if (activeManualMemoId !== null) {
errors.push({
reason:
'Calling setState from useMemo may trigger an infinite loop. (https://react.dev/reference/react/useState)',
description: null,
severity: ErrorSeverity.InvalidReact,
loc: callee.loc,
suggestions: null,
});
} else if (unconditionalBlocks.has(block.id)) {
errors.push({
reason:
'This is an unconditional set state during render, which will trigger an infinite loop. (https://react.dev/reference/react/useState)',
description: null,
severity: ErrorSeverity.InvalidReact,
loc: callee.loc,
suggestions: null,
});
}
}
break;
}
}
}
}
if (errors.hasErrors()) {
return Err(errors);
} else {
return Ok(undefined);
}
}