Facebook/react
1
0
Fork 0
mirror of https://github.com/facebook/react.git synced 2025-11-01 09:12:30 +00:00
Code Issues Packages Projects Releases Wiki Activity

[be] Remove ValidateFrozenLambdas

Browse Source 
This pass doesn't really make sense in light of 
`@enableTransitivelyFreezeFunctionExpressions`. The original idea of 
ValidateFrozenLambdas was that trying to pass a "mutable" lambda to a frozen 
value was invalid. But since then we've realized that the better heuristic is 
that freezing a lambda is transitive.
This commit is contained in:
Joe Savona
2024-02-13 16:45:16 -08:00
parent 5f1b8fd57f
commit bde30f0284
13 changed files with 3 additions and 328 deletions
Download Patch File Download Diff File Expand all files Collapse all files
compiler/packages/babel-plugin-react-forget/src/Entrypoint/Pipeline.ts
-5
View File
@@ -70,7 +70,6 @@ import {
import { assertExhaustive } from "../Utils/utils";
import {
validateContextVariableLValues,
validateFrozenLambdas,
validateHooksUsage,
validateMemoizedEffectDependencies,
validateNoRefAccessInRender,
@@ -161,10 +160,6 @@ function* runWithEnvironment(
inferReferenceEffects(hir);
yield log({ kind: "hir", name: "InferReferenceEffects", value: hir });
if (env.config.validateFrozenLambdas) {
validateFrozenLambdas(hir);
}
// Note: Has to come after infer reference effects because "dead" code may still affect inference
deadCodeElimination(hir);
yield log({ kind: "hir", name: "DeadCodeElimination", value: hir });
compiler/packages/babel-plugin-react-forget/src/HIR/Environment.ts
-6
View File
@@ -180,12 +180,6 @@ const EnvironmentConfigSchema = z.object({
*/
validateRefAccessDuringRenderFunctionExpressions: z.boolean().default(false),
/*
* Validate that mutable lambdas are not passed where a frozen value is expected, since mutable
* lambdas cannot be frozen. The only mutation allowed inside a frozen lambda is of ref values.
*/
validateFrozenLambdas: z.boolean().default(false),
/*
* Validates that setState is not unconditionally called during render, as it can lead to
* infinite loops.
compiler/packages/babel-plugin-react-forget/src/Validation/ValidateFrozenLambdas.ts
-159
View File
@@ -1,159 +0,0 @@
/*
* 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,
CompilerErrorDetail,
ErrorSeverity,
} from "../CompilerError";
import {
Effect,
FunctionExpression,
HIRFunction,
IdentifierId,
ObjectMethod,
Place,
isRefValueType,
isUseRefType,
} from "../HIR/HIR";
import {
eachInstructionValueOperand,
eachTerminalOperand,
} from "../HIR/visitors";
/*
* Various APIs in React take ownership of the values passed to them, such that it is invalid
* to subsequently modify those values. Examples include:
* - Passing a value as a prop to JSX. Subsequently mutating this value will result in undefined
* behavior, since the mutation may or may not be observed depending on when the child re-renders.
* In addition, the value may be used as an input to memoization in children, and mutation could
* invalidate that memoization.
* - Passing a value to `useState()`, for the same reason.
* - Passing a value to a hook, for the same reason.
*
* Most "normal" data types (objects, arrays, etc) can be "frozen" when passed to a React API simply
* by not calling any mutating methods on them. However, mutable lambdas are an exception: if a lambda
* has side-effects, there is no way to do something to the lambda that would allow calling it without
* triggering those side effects. The only thing a developer could do is not call the lambda, but developers
* also have no way of knowing that they can't call the lambda.
*
* From a type system perspective, the above APIs that "take ownership" of their values really accept
* *already frozen* values as input. Thus it is invalid to pass a value that cannot be frozen to these APIs,
* and it is therefore invalid to pass a mutable lambda.
*
* This pass validates the above rule. Note that this validation can by bypassed by storing a mutable lambda
* inside some value (eg as an array element or object property). In these cases we trust that the developer
* is not breaking the rules. The goal of this validation is to find cases that are provably wrong and help
* the developer fix the mistake earlier.
*/
export function validateFrozenLambdas(fn: HIRFunction): void {
const state = new State();
const errors = new CompilerError();
for (const [, block] of fn.body.blocks) {
for (const phi of block.phis) {
for (const [, operand] of phi.operands) {
const resolvedId = state.temporaries.get(operand.id) ?? operand.id;
const lambda = state.lambdas.get(resolvedId);
if (lambda !== undefined) {
state.lambdas.set(phi.id.id, lambda);
break;
}
}
}
for (const instr of block.instructions) {
switch (instr.value.kind) {
case "ObjectMethod":
case "FunctionExpression": {
if (
instr.value.loweredFunc.dependencies.some(
(place) =>
place.effect === Effect.Mutate &&
!isRefValueType(place.identifier) &&
!isUseRefType(place.identifier)
)
) {
state.lambdas.set(instr.lvalue.identifier.id, instr.value);
}
break;
}
case "LoadLocal": {
const resolvedId =
state.temporaries.get(instr.value.place.identifier.id) ??
instr.value.place.identifier.id;
state.temporaries.set(instr.lvalue.identifier.id, resolvedId);
break;
}
case "StoreLocal": {
const resolvedId =
state.temporaries.get(instr.value.value.identifier.id) ??
instr.value.value.identifier.id;
state.temporaries.set(
instr.value.lvalue.place.identifier.id,
resolvedId
);
break;
}
default: {
for (const operand of eachInstructionValueOperand(instr.value)) {
const operandError = validateOperand(operand, state);
if (operandError !== null) {
errors.pushErrorDetail(operandError);
}
}
}
}
}
for (const operand of eachTerminalOperand(block.terminal)) {
const operandError = validateOperand(operand, state);
if (operandError !== null) {
errors.pushErrorDetail(operandError);
}
}
}
if (errors.hasErrors()) {
throw errors;
}
}
class State {
lambdas: Map<IdentifierId, FunctionExpression | ObjectMethod> = new Map();
temporaries: Map<IdentifierId, IdentifierId> = new Map();
}
function validateOperand(
operand: Place,
state: State
): CompilerErrorDetail | null {
if (operand.effect === Effect.Freeze) {
const operandId =
state.temporaries.get(operand.identifier.id) ?? operand.identifier.id;
const lambda = state.lambdas.get(operandId);
if (lambda !== undefined) {
/*
* TODO: these seem to always be null, we should try to preserve original
* names from source
* TODO: figure out how to print object methods as they don't have names
*/
const description =
lambda.kind === "FunctionExpression" &&
lambda.name !== null &&
operand.identifier.name !== null
? `\`${lambda.name}\` is a function that may mutate \`${operand.identifier.name}\`. If you must mutate \`${operand.identifier.name}\` try using a React API like useState and use its setter function instead`
: null;
return new CompilerErrorDetail({
description,
loc: typeof operand.loc !== "symbol" ? operand.loc : null,
reason:
"This mutates a variable that is managed by React, where an immutable value or a function was expected",
severity: ErrorSeverity.InvalidReact,
suggestions: null,
});
}
}
return null;
}
compiler/packages/babel-plugin-react-forget/src/Validation/index.ts
-1
View File
@@ -6,7 +6,6 @@
*/
export { validateContextVariableLValues } from "./ValidateContextVariableLValues";
export { validateFrozenLambdas } from "./ValidateFrozenLambdas";
export { validateHooksUsage } from "./ValidateHooksUsage";
export { validateMemoizedEffectDependencies } from "./ValidateMemoizedEffectDependencies";
export { validateNoRefAccessInRender } from "./ValidateNoRefAccesInRender";
compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/error.invalid-capture-func-passed-to-jsx.expect.md
-27
View File
@@ -1,27 +0,0 @@
## Input
```javascript
// @validateFrozenLambdas
function component(a, b) {
let y = { b };
let z = { a };
let x = function () {
z.a = 2;
y.b;
};
let t = <Foo x={x}></Foo>;
mutate(x); // x should be frozen here
return t;
}
```
## Error
```
[ReactForget] InvalidReact: This mutates a variable that is managed by React, where an immutable value or a function was expected (9:9)
```
compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/error.invalid-capture-func-passed-to-jsx.js
-12
View File
@@ -1,12 +0,0 @@
// @validateFrozenLambdas
function component(a, b) {
let y = { b };
let z = { a };
let x = function () {
z.a = 2;
y.b;
};
let t = <Foo x={x}></Foo>;
mutate(x); // x should be frozen here
return t;
}
compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/error.invalid-freeze-conditionally-mutable-lambda.expect.md
-32
View File
@@ -1,32 +0,0 @@
## Input
```javascript
// @validateFrozenLambdas
function Component(props) {
const x = {};
let fn;
if (props.cond) {
// mutable
fn = () => {
x.value = props.value;
};
} else {
// immutable
fn = () => {
x.value;
};
}
return fn;
}
```
## Error
```
[ReactForget] InvalidReact: This mutates a variable that is managed by React, where an immutable value or a function was expected (16:16)
```
compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/error.invalid-freeze-conditionally-mutable-lambda.js
-17
View File
@@ -1,17 +0,0 @@
// @validateFrozenLambdas
function Component(props) {
const x = {};
let fn;
if (props.cond) {
// mutable
fn = () => {
x.value = props.value;
};
} else {
// immutable
fn = () => {
x.value;
};
}
return fn;
}
compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/error.invalid-freeze-mutable-lambda-mutate-local.expect.md
-25
View File
@@ -1,25 +0,0 @@
## Input
```javascript
// @validateFrozenLambdas
function Component(props) {
const x = {};
const onChange = (e) => {
// INVALID! should use copy-on-write and pass the new value
x.value = e.target.value;
setX(x);
};
return <input value={x.value} onChange={onChange} />;
}
```
## Error
```
[ReactForget] InvalidReact: This mutates a variable that is managed by React, where an immutable value or a function was expected (9:9)
```
compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/error.invalid-freeze-mutable-lambda-mutate-local.js
-10
View File
@@ -1,10 +0,0 @@
// @validateFrozenLambdas
function Component(props) {
const x = {};
const onChange = (e) => {
// INVALID! should use copy-on-write and pass the new value
x.value = e.target.value;
setX(x);
};
return <input value={x.value} onChange={onChange} />;
}
compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/error.invalid-freeze-mutable-lambda-reassign-local.expect.md
-23
View File
@@ -1,23 +0,0 @@
## Input
```javascript
// @validateFrozenLambdas
function Component(props) {
let x = "";
const onChange = (e) => {
x = e.target.value;
};
return <input value={x} onChange={onChange} />;
}
```
## Error
```
[ReactForget] InvalidReact: This mutates a variable that is managed by React, where an immutable value or a function was expected (7:7)
```
compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/error.invalid-freeze-mutable-lambda-reassign-local.js
-8
View File
@@ -1,8 +0,0 @@
// @validateFrozenLambdas
function Component(props) {
let x = "";
const onChange = (e) => {
x = e.target.value;
};
return <input value={x} onChange={onChange} />;
}
compiler/packages/babel-plugin-react-forget/src/__tests__/parseConfigPragma-test.ts
+3 -3
View File
@@ -14,16 +14,16 @@ describe("parseConfigPragma()", () => {
// Validate defaults first to make sure that the parser is getting the value from the pragma,
// and not just missing it and getting the default value
expect(defaultConfig.enableForest).toBe(false);
expect(defaultConfig.validateFrozenLambdas).toBe(false);
expect(defaultConfig.validateRefAccessDuringRender).toBe(false);
expect(defaultConfig.memoizeJsxElements).toBe(true);
const config = parseConfigPragma(
"@enableForest @validateFrozenLambdas:true @memoizeJsxElements:false"
"@enableForest @validateRefAccessDuringRender:true @memoizeJsxElements:false"
);
expect(config).toEqual({
...defaultConfig,
enableForest: true,
validateFrozenLambdas: true,
validateRefAccessDuringRender: true,
memoizeJsxElements: false,
});
});