Consolidate ValidateNoSetStateInRender flags
I ran the plugin with the extended version of ValidateNoSetStateInRender enabled (incl. function expressions) and there are no false positives. Let's remove the flag for the function expression case since the whole rule is working accurately.
Joe Savona committed
Nov 15, 2023 at 13:05 UTC
7b6b3706386f9416c4556e7249f36f841f3d964d
10 files changed
+25
-35
compiler/packages/babel-plugin-react-forget/src/HIR/Environment.ts
-8
@@ -142,14 +142,6 @@ const EnvironmentConfigSchema = z.object({
142
*/
143
validateNoSetStateInRender: z.boolean().default(false),
144
145
- /**
146
- * Extension of validateNoSetStateInRender which also validates that setState is not
147
- * called indirectly via a function expression.
148
- *
149
- * NOTE: this validation has known issues and is not yet recommended.
150
- */
151
- validateNoSetStateInRenderFunctionExpressions: z.boolean().default(false),
152
-
145
/*
146
* When enabled, the compiler assumes that hooks follow the Rules of React:
147
* - Hooks may memoize computation based on any of their parameters, thus
compiler/packages/babel-plugin-react-forget/src/Validation/ValidateNoSetStateInRender.ts
+15
-17
@@ -106,23 +106,21 @@ function validateNoSetStateInRenderImpl(
106
}
107
case "ObjectMethod":
108
case "FunctionExpression": {
109
- if (fn.env.config.validateNoSetStateInRenderFunctionExpressions) {
110
- if (
111
- // faster-path to check if the function expression references a setState
112
- [...eachInstructionValueOperand(instr.value)].some(
113
- (operand) =>
114
- isSetStateType(operand.identifier) ||
115
- unconditionalSetStateFunctions.has(operand.identifier.id)
116
- ) &&
117
- // if yes, does it unconditonally call it?
118
- validateNoSetStateInRenderImpl(
119
- instr.value.loweredFunc.func,
120
- unconditionalSetStateFunctions
121
- ).isErr()
122
- ) {
123
- // This function expression unconditionally calls a setState
124
- unconditionalSetStateFunctions.add(instr.lvalue.identifier.id);
125
- }
109
+ if (
110
+ // faster-path to check if the function expression references a setState
111
+ [...eachInstructionValueOperand(instr.value)].some(
112
+ (operand) =>
113
+ isSetStateType(operand.identifier) ||
114
+ unconditionalSetStateFunctions.has(operand.identifier.id)
115
+ ) &&
116
+ // if yes, does it unconditonally call it?
117
+ validateNoSetStateInRenderImpl(
118
+ instr.value.loweredFunc.func,
119
+ unconditionalSetStateFunctions
120
+ ).isErr()
121
+ ) {
122
+ // This function expression unconditionally calls a setState
123
+ unconditionalSetStateFunctions.add(instr.lvalue.identifier.id);
124
}
125
break;
126
}
compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/error.unconditional-set-state-lambda.expect.md
+1
-1
@@ -2,7 +2,7 @@
2
## Input
3
4
```javascript
5
-// @validateNoSetStateInRender @validateNoSetStateInRenderFunctionExpressions
5
+// @validateNoSetStateInRender
6
function Component(props) {
7
const [x, setX] = useState(0);
8
compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/error.unconditional-set-state-lambda.js
+1
-1
@@ -1,4 +1,4 @@
1
-// @validateNoSetStateInRender @validateNoSetStateInRenderFunctionExpressions
1
+// @validateNoSetStateInRender
2
function Component(props) {
3
const [x, setX] = useState(0);
4
compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/error.unconditional-set-state-nested-function-expressions.expect.md
+1
-1
@@ -2,7 +2,7 @@
2
## Input
3
4
```javascript
5
-// @validateNoSetStateInRender @validateNoSetStateInRenderFunctionExpressions
5
+// @validateNoSetStateInRender
6
function Component(props) {
7
const [x, setX] = useState(0);
8
compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/error.unconditional-set-state-nested-function-expressions.js
+1
-1
@@ -1,4 +1,4 @@
1
-// @validateNoSetStateInRender @validateNoSetStateInRenderFunctionExpressions
1
+// @validateNoSetStateInRender
2
function Component(props) {
3
const [x, setX] = useState(0);
4
compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/validate-no-set-state-in-render-uncalled-function-with-mutable-range-is-valid.expect.md
+2
-2
@@ -2,7 +2,7 @@
2
## Input
3
4
```javascript
5
-// @validateNoSetStateInRenderFunctionExpressions
5
+//
6
function Component(props) {
7
const logEvent = useLogging(props.appId);
8
const [currentStep, setCurrentStep] = useState(0);
@@ -33,7 +33,7 @@ function Component(props) {
33
## Code
34
35
```javascript
36
-import { unstable_useMemoCache as useMemoCache } from "react"; // @validateNoSetStateInRenderFunctionExpressions
36
+import { unstable_useMemoCache as useMemoCache } from "react"; //
37
function Component(props) {
38
const $ = useMemoCache(3);
39
const logEvent = useLogging(props.appId);
compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/validate-no-set-state-in-render-uncalled-function-with-mutable-range-is-valid.js
+1
-1
@@ -1,4 +1,4 @@
1
-// @validateNoSetStateInRenderFunctionExpressions
1
+//
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/validate-no-set-state-in-render-unconditional-lambda-which-conditionally-sets-state-ok.expect.md
+2
-2
@@ -2,7 +2,7 @@
2
## Input
3
4
```javascript
5
-// @validateNoSetStateInRender @validateNoSetStateInRenderFunctionExpressions
5
+// @validateNoSetStateInRender
6
function Component(props) {
7
const [x, setX] = useState(0);
8
@@ -35,7 +35,7 @@ export const FIXTURE_ENTRYPOINT = {
35
## Code
36
37
```javascript
38
-import { unstable_useMemoCache as useMemoCache } from "react"; // @validateNoSetStateInRender @validateNoSetStateInRenderFunctionExpressions
38
+import { unstable_useMemoCache as useMemoCache } from "react"; // @validateNoSetStateInRender
39
function Component(props) {
40
const $ = useMemoCache(2);
41
const [x, setX] = useState(0);
compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/validate-no-set-state-in-render-unconditional-lambda-which-conditionally-sets-state-ok.js
+1
-1
@@ -1,4 +1,4 @@
1
-// @validateNoSetStateInRender @validateNoSetStateInRenderFunctionExpressions
1
+// @validateNoSetStateInRender
2
function Component(props) {
3
const [x, setX] = useState(0);
4