@samitouri / QOS-React-1 / commits / 6e038f9699

Partially enable validateNoSetStateInRender

The approach i initially took to validating function expressions was to try to extend the mutable range if they are called during render, and then use the mutable range of a function to determine if it's called during render later. However there are cases where the range can be extended for other reasons, as @poteto discovered, so we can't rely on the range extension. We've had several of our validations completely off as a result of this. In this PR i'm re-enabling @poteto's ValidateNoSetStateInRender pass by default, but making the function expression checking use a separate compiler flag. This means we'll have some false negatives, but should guarantee that we avoid false positives. This means we can definitely catch things like: ``` const [state, setState] = useState(false); setState(true); ``` Which we would have allowed by default before.

Joe Savona committed Nov 10, 2023 at 16:52 UTC 6e038f96990d18d06ca9480b2c16c15437de51cf
6 files changed +26 -14
compiler/packages/babel-plugin-react-forget/src/HIR/Environment.ts
+9 -1
@@ -132,7 +132,15 @@ const EnvironmentConfigSchema = z.object({
132 * Validates that setState is not unconditionally called during render, as it can lead to
133 * infinite loops.
134 */
135 - validateNoSetStateInRender: z.boolean().default(false),
135 + validateNoSetStateInRender: z.boolean().default(true),
136 +
137 + /**
138 + * Extension of validateNoSetStateInRender which also validates that setState is not
139 + * called indirectly via a function expression.
140 + *
141 + * NOTE: this validation has known issues and is not yet recommended.
142 + */
143 + validateNoSetStateInRenderFunctionExpressions: z.boolean().default(false),
144
145 /*
146 * When enabled, the compiler assumes that hooks follow the Rules of React:
compiler/packages/babel-plugin-react-forget/src/Validation/ValidateNoSetStateInRender.ts
+13 -9
@@ -45,15 +45,19 @@ export function validateNoSetStateInRender(
45 switch (instr.value.kind) {
46 case "ObjectMethod":
47 case "FunctionExpression": {
48 - /*
49 - * TODO: setState's return value is considered Frozen, so the lambda's mutable range
50 - * does not get extended even if the lambda is called in render. The below only catches
51 - * setStates where the lambda has another instruction that extends its mutable range
52 - */
53 - const mutableRange = instr.lvalue.identifier.mutableRange;
54 - if (mutableRange.end > mutableRange.start + 1) {
55 - for (const operand of eachInstructionValueOperand(instr.value)) {
56 - validateNonSetState(errors, operand);
48 + if (fn.env.config.validateNoSetStateInRenderFunctionExpressions) {
49 + /*
50 + * TODO: setState's return value is considered Frozen, so the lambda's mutable range
51 + * does not get extended even if the lambda is called in render. The below only catches
52 + * setStates where the lambda has another instruction that extends its mutable range
53 + */
54 + const mutableRange = instr.lvalue.identifier.mutableRange;
55 + if (mutableRange.end > mutableRange.start + 1) {
56 + for (const operand of eachInstructionValueOperand(
57 + instr.value
58 + )) {
59 + validateNonSetState(errors, operand);
60 + }
61 }
62 }
63 break;
compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/error.bug-validate-no-set-state-not-all-mutable-range-extensions-are-bad.expect.md
+1 -1
@@ -2,7 +2,7 @@
2 ## Input
3
4 ```javascript
5 -// @validateNoSetStateInRender
5 +// @validateNoSetStateInRenderFunctionExpressions
6 function Component(props) {
7 const logEvent = useLogging(props.appId);
8 const [currentStep, setCurrentStep] = useState(0);
compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/error.bug-validate-no-set-state-not-all-mutable-range-extensions-are-bad.js
+1 -1
@@ -1,4 +1,4 @@
1 -// @validateNoSetStateInRender
1 +// @validateNoSetStateInRenderFunctionExpressions
2 function Component(props) {
3 const logEvent = useLogging(props.appId);
4 const [currentStep, setCurrentStep] = useState(0);
compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/error.todo-unconditional-set-state-lambda.expect.md
+1 -1
@@ -2,7 +2,7 @@
2 ## Input
3
4 ```javascript
5 -// @validateNoSetStateInRender
5 +// @validateNoSetStateInRenderFunctionExpressions
6 function Component(props) {
7 let y = 0;
8 const [x, setX] = useState(0);
compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/error.todo-unconditional-set-state-lambda.js
+1 -1
@@ -1,4 +1,4 @@
1 -// @validateNoSetStateInRender
1 +// @validateNoSetStateInRenderFunctionExpressions
2 function Component(props) {
3 let y = 0;
4 const [x, setX] = useState(0);