Validate unconditional setState in render
Lauren Tan committed
Jul 17, 2023 at 13:00 UTC
5ac909172e1b0a2f1d12aa1ce94a5fcdc18c8350
10 files changed
+222
-1
compiler/forget/packages/babel-plugin-react-forget/src/Entrypoint/Pipeline.ts
+8
@@ -55,6 +55,7 @@ import {
55
validateFrozenLambdas,
56
validateHooksUsage,
57
validateNoRefAccessInRender,
58
+ validateNoSetStateInRender,
59
validateUnconditionalHooks,
60
} from "../Validation";
61
@@ -136,6 +137,13 @@ export function* run(
137
validateNoRefAccessInRender(hir);
138
}
139
140
+ const noSetStateInRenderResult = validateNoSetStateInRender(hir).unwrap();
141
+ yield log({
142
+ kind: "debug",
143
+ name: "ValidateNoSetStateInRender",
144
+ value: noSetStateInRenderResult.debug(),
145
+ });
146
+
147
leaveSSA(hir);
148
yield log({ kind: "hir", name: "LeaveSSA", value: hir });
149
compiler/forget/packages/babel-plugin-react-forget/src/Inference/AnalyseFunctions.ts
+6
-1
@@ -12,6 +12,7 @@ import {
12
HIRFunction,
13
Identifier,
14
isRefValueType,
15
+ isSetStateType,
16
isUseRefType,
17
mergeConsecutiveBlocks,
18
Place,
@@ -147,7 +148,11 @@ function infer(
148
name = dep.identifier.name;
149
}
150
150
- if (isUseRefType(dep.identifier) || isRefValueType(dep.identifier)) {
151
+ if (
152
+ isUseRefType(dep.identifier) ||
153
+ isRefValueType(dep.identifier) ||
154
+ isSetStateType(dep.identifier)
155
+ ) {
156
// TODO: this is a hack to ensure we treat functions which reference refs
157
// as having a capture and therefore being considered mutable. this ensures
158
// the function gets a mutable range which accounts for anywhere that it
compiler/forget/packages/babel-plugin-react-forget/src/Validation/ValidateNoSetStateInRender.ts
new
+80
@@ -0,0 +1,80 @@
1
+import { CompilerError, ErrorSeverity } from "../CompilerError";
2
+import {
3
+ BlockId,
4
+ HIRFunction,
5
+ Place,
6
+ computePostDominatorTree,
7
+ isSetStateType,
8
+} from "../HIR";
9
+import { PostDominator } from "../HIR/Dominator";
10
+import { eachInstructionValueOperand } from "../HIR/visitors";
11
+import { findBlocksWithBackEdges } from "../Optimization/DeadCodeElimination";
12
+import { Err, Ok, Result } from "../Utils/Result";
13
+
14
+export function validateNoSetStateInRender(
15
+ fn: HIRFunction
16
+): Result<PostDominator<BlockId>, CompilerError> {
17
+ // Construct the set of blocks that is always reachable from the entry block.
18
+ const unconditionalBlocks = new Set<BlockId>();
19
+ const blocksWithBackEdges = findBlocksWithBackEdges(fn);
20
+ const dominators = computePostDominatorTree(fn, {
21
+ includeThrowsAsExitNode: false,
22
+ });
23
+ const exit = dominators.exit;
24
+ let current: BlockId | null = fn.body.entry;
25
+ while (
26
+ current !== null &&
27
+ current !== exit &&
28
+ !blocksWithBackEdges.has(current)
29
+ ) {
30
+ unconditionalBlocks.add(current);
31
+ current = dominators.get(current);
32
+ }
33
+
34
+ const errors = new CompilerError();
35
+ for (const [, block] of fn.body.blocks) {
36
+ if (unconditionalBlocks.has(block.id)) {
37
+ for (const instr of block.instructions) {
38
+ switch (instr.value.kind) {
39
+ case "FunctionExpression": {
40
+ /**
41
+ * TODO: setState's return value is considered Frozen, so the lambda's mutable range
42
+ * does not get extended even if the lambda is called in render. The below only catches
43
+ * setStates where the lambda has another instruction that extends its mutable range
44
+ */
45
+ const mutableRange = instr.lvalue.identifier.mutableRange;
46
+ if (mutableRange.end > mutableRange.start + 1) {
47
+ for (const operand of eachInstructionValueOperand(instr.value)) {
48
+ validateNonSetState(errors, operand);
49
+ }
50
+ }
51
+ break;
52
+ }
53
+ case "CallExpression": {
54
+ validateNonSetState(errors, instr.value.callee);
55
+ break;
56
+ }
57
+ }
58
+ }
59
+ }
60
+ }
61
+
62
+ if (errors.hasErrors()) {
63
+ return Err(errors);
64
+ } else {
65
+ return Ok(dominators);
66
+ }
67
+}
68
+
69
+function validateNonSetState(errors: CompilerError, operand: Place): void {
70
+ if (isSetStateType(operand.identifier)) {
71
+ errors.push({
72
+ reason:
73
+ "This is an unconditional set state during render, which will trigger an infinite loop. (https://react.dev/reference/react/useState)",
74
+ description: null,
75
+ severity: ErrorSeverity.InvalidReact,
76
+ loc: typeof operand.loc !== "symbol" ? operand.loc : null,
77
+ suggestions: null,
78
+ });
79
+ }
80
+}
compiler/forget/packages/babel-plugin-react-forget/src/Validation/index.ts
+1
@@ -8,4 +8,5 @@
8
export { validateFrozenLambdas } from "./ValidateFrozenLambdas";
9
export { validateHooksUsage } from "./ValidateHooksUsage";
10
export { validateNoRefAccessInRender } from "./ValidateNoRefAccesInRender";
11
+export { validateNoSetStateInRender } from "./ValidateNoSetStateInRender";
12
export { validateUnconditionalHooks } from "./ValidateUnconditionalHooks";
compiler/forget/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/conditional-set-state-in-render.expect.md
new
+39
@@ -0,0 +1,39 @@
1
+
2
+## Input
3
+
4
+```javascript
5
+function Component(props) {
6
+ const [x, setX] = useState(0);
7
+
8
+ const foo = () => {
9
+ setX(1);
10
+ };
11
+
12
+ if (props.cond) {
13
+ setX(2);
14
+ foo();
15
+ }
16
+
17
+ return x;
18
+}
19
+
20
+```
21
+
22
+## Code
23
+
24
+```javascript
25
+function Component(props) {
26
+ const [x, setX] = useState(0);
27
+
28
+ const foo = () => {
29
+ setX(1);
30
+ };
31
+ if (props.cond) {
32
+ setX(2);
33
+ foo();
34
+ }
35
+ return x;
36
+}
37
+
38
+```
39
+
\ No newline at end of file
compiler/forget/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/conditional-set-state-in-render.js
new
+14
@@ -0,0 +1,14 @@
1
+function Component(props) {
2
+ const [x, setX] = useState(0);
3
+
4
+ const foo = () => {
5
+ setX(1);
6
+ };
7
+
8
+ if (props.cond) {
9
+ setX(2);
10
+ foo();
11
+ }
12
+
13
+ return x;
14
+}
compiler/forget/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/error.invalid-unconditional-set-state-in-render.expect.md
new
+26
@@ -0,0 +1,26 @@
1
+
2
+## Input
3
+
4
+```javascript
5
+function Component(props) {
6
+ const [x, setX] = useState(0);
7
+ const aliased = setX;
8
+
9
+ setX(1);
10
+ aliased(2);
11
+
12
+ return x;
13
+}
14
+
15
+```
16
+
17
+
18
+## Error
19
+
20
+```
21
+[ReactForget] InvalidReact: This is an unconditional set state during render, which will trigger an infinite loop. (https://react.dev/reference/react/useState) (5:5)
22
+
23
+[ReactForget] InvalidReact: This is an unconditional set state during render, which will trigger an infinite loop. (https://react.dev/reference/react/useState) (6:6)
24
+```
25
+
26
+
\ No newline at end of file
compiler/forget/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/error.invalid-unconditional-set-state-in-render.js
new
+9
@@ -0,0 +1,9 @@
1
+function Component(props) {
2
+ const [x, setX] = useState(0);
3
+ const aliased = setX;
4
+
5
+ setX(1);
6
+ aliased(2);
7
+
8
+ return x;
9
+}
compiler/forget/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/error.todo-unconditional-set-state-lambda.expect.md
new
+27
@@ -0,0 +1,27 @@
1
+
2
+## Input
3
+
4
+```javascript
5
+function Component(props) {
6
+ let y = 0;
7
+ const [x, setX] = useState(0);
8
+
9
+ const foo = () => {
10
+ setX(1);
11
+ y = 1; // TODO: force foo's mutable range to extend, but ideally we can just remove this line
12
+ };
13
+ foo();
14
+
15
+ return [x, y];
16
+}
17
+
18
+```
19
+
20
+
21
+## Error
22
+
23
+```
24
+[ReactForget] InvalidReact: This is an unconditional set state during render, which will trigger an infinite loop. (https://react.dev/reference/react/useState) (6:6)
25
+```
26
+
27
+
\ No newline at end of file
compiler/forget/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/error.todo-unconditional-set-state-lambda.js
new
+12
@@ -0,0 +1,12 @@
1
+function Component(props) {
2
+ let y = 0;
3
+ const [x, setX] = useState(0);
4
+
5
+ const foo = () => {
6
+ setX(1);
7
+ y = 1; // TODO: force foo's mutable range to extend, but ideally we can just remove this line
8
+ };
9
+ foo();
10
+
11
+ return [x, y];
12
+}