@samitouri / QOS-React-1 / commits / fc85f24865

Separate flag for ref access violation within function expressions

Adds a separate compiler flag for enabling the incomplete validation of ref access within function expressions. Unlike the previous PR for set-state-in-render validation, ref access in render can be okay in some circumstances so i'm leaving this off by default. The point of splitting this up is that our linting will be able to enable the rule without risk of false positives.

Joe Savona committed Nov 10, 2023 at 17:07 UTC fc85f24865a62f0cd3395fadf387b4f467be7f0c
8 files changed +29 -15
compiler/packages/babel-plugin-react-forget/src/HIR/Environment.ts
+8
@@ -122,6 +122,14 @@ const EnvironmentConfigSchema = z.object({
122 // Validate that ref values (`ref.current`) are not accessed during render.
123 validateRefAccessDuringRender: z.boolean().default(false),
124
125 + /**
126 + * Extension of validateRefAccessDuringRender that validates that refs are not accessed during
127 + * render indirectly by calling function expressions which access the ref.
128 + *
129 + * This validation has known issues and is not yet recommended
130 + */
131 + validateRefAccessDuringRenderFunctionExpressions: z.boolean().default(false),
132 +
133 /*
134 * Validate that mutable lambdas are not passed where a frozen value is expected, since mutable
135 * lambdas cannot be frozen. The only mutation allowed inside a frozen lambda is of ref values.
compiler/packages/babel-plugin-react-forget/src/Validation/ValidateNoRefAccesInRender.ts
+13 -11
@@ -63,17 +63,19 @@ export function validateNoRefAccessInRender(fn: HIRFunction): void {
63 }
64 case "ObjectMethod":
65 case "FunctionExpression": {
66 - /*
67 - * functions are allowed to capture refs, so long as the function is not called
68 - * during render. see AnalyzeFunctions for how we ensure that functions which
69 - * capture refs get assigned a mutable range so we know here whether the function
70 - * is called or not
71 - */
72 - const mutableRange = instr.lvalue.identifier.mutableRange;
73 - if (mutableRange.end > mutableRange.start + 1) {
74 - for (const operand of eachInstructionValueOperand(instr.value)) {
75 - validateNonRefValue(error, operand);
76 - validateNonRefObject(error, operand);
66 + if (fn.env.config.validateRefAccessDuringRenderFunctionExpressions) {
67 + /*
68 + * functions are allowed to capture refs, so long as the function is not called
69 + * during render. see AnalyzeFunctions for how we ensure that functions which
70 + * capture refs get assigned a mutable range so we know here whether the function
71 + * is called or not
72 + */
73 + const mutableRange = instr.lvalue.identifier.mutableRange;
74 + if (mutableRange.end > mutableRange.start + 1) {
75 + for (const operand of eachInstructionValueOperand(instr.value)) {
76 + validateNonRefValue(error, operand);
77 + validateNonRefObject(error, operand);
78 + }
79 }
80 }
81 break;
compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/error.invalid-ref-in-callback-invoked-during-render.expect.md
+1 -1
@@ -2,7 +2,7 @@
2 ## Input
3
4 ```javascript
5 -// @validateRefAccessDuringRender
5 +// @validateRefAccessDuringRender @validateRefAccessDuringRenderFunctionExpressions
6 function Component(props) {
7 const ref = useRef(null);
8 const renderItem = (item) => {
compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/error.invalid-ref-in-callback-invoked-during-render.js
+1 -1
@@ -1,4 +1,4 @@
1 -// @validateRefAccessDuringRender
1 +// @validateRefAccessDuringRender @validateRefAccessDuringRenderFunctionExpressions
2 function Component(props) {
3 const ref = useRef(null);
4 const renderItem = (item) => {
compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/ref-current-aliased-not-added-to-dep-2.expect.md
+2 -1
@@ -2,6 +2,7 @@
2 ## Input
3
4 ```javascript
5 +// @validateRefAccessDuringRender:false
6 function Foo({ a }) {
7 const ref = useRef();
8 const val = ref.current;
@@ -15,7 +16,7 @@ function Foo({ a }) {
16 ## Code
17
18 ```javascript
18 -import { unstable_useMemoCache as useMemoCache } from "react";
19 +import { unstable_useMemoCache as useMemoCache } from "react"; // @validateRefAccessDuringRender:false
20 function Foo(t20) {
21 const $ = useMemoCache(4);
22 const { a } = t20;
compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/ref-current-aliased-not-added-to-dep-2.js
+1
@@ -1,3 +1,4 @@
1 +// @validateRefAccessDuringRender:false
2 function Foo({ a }) {
3 const ref = useRef();
4 const val = ref.current;
compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/ref-current-not-added-to-dep-2.expect.md
+2 -1
@@ -2,6 +2,7 @@
2 ## Input
3
4 ```javascript
5 +// @validateRefAccessDuringRender:false
6 function Foo({ a }) {
7 const ref = useRef();
8 const x = { a, val: ref.current };
@@ -14,7 +15,7 @@ function Foo({ a }) {
15 ## Code
16
17 ```javascript
17 -import { unstable_useMemoCache as useMemoCache } from "react";
18 +import { unstable_useMemoCache as useMemoCache } from "react"; // @validateRefAccessDuringRender:false
19 function Foo(t17) {
20 const $ = useMemoCache(4);
21 const { a } = t17;
compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/ref-current-not-added-to-dep-2.js
+1
@@ -1,3 +1,4 @@
1 +// @validateRefAccessDuringRender:false
2 function Foo({ a }) {
3 const ref = useRef();
4 const x = { a, val: ref.current };