From fc85f24865a62f0cd3395fadf387b4f467be7f0c Mon Sep 17 00:00:00 2001 From: Joe Savona Date: Fri, 10 Nov 2023 17:07:21 -0800 Subject: [PATCH] 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. --- .../src/HIR/Environment.ts | 8 +++++++ .../Validation/ValidateNoRefAccesInRender.ts | 24 ++++++++++--------- ...n-callback-invoked-during-render.expect.md | 2 +- ...d-ref-in-callback-invoked-during-render.js | 2 +- ...rrent-aliased-not-added-to-dep-2.expect.md | 3 ++- .../ref-current-aliased-not-added-to-dep-2.js | 1 + .../ref-current-not-added-to-dep-2.expect.md | 3 ++- .../ref-current-not-added-to-dep-2.js | 1 + 8 files changed, 29 insertions(+), 15 deletions(-) diff --git a/compiler/packages/babel-plugin-react-forget/src/HIR/Environment.ts b/compiler/packages/babel-plugin-react-forget/src/HIR/Environment.ts index 2cd06f100f..25cd81de3f 100644 --- a/compiler/packages/babel-plugin-react-forget/src/HIR/Environment.ts +++ b/compiler/packages/babel-plugin-react-forget/src/HIR/Environment.ts @@ -122,6 +122,14 @@ const EnvironmentConfigSchema = z.object({ // Validate that ref values (`ref.current`) are not accessed during render. validateRefAccessDuringRender: z.boolean().default(false), + /** + * Extension of validateRefAccessDuringRender that validates that refs are not accessed during + * render indirectly by calling function expressions which access the ref. + * + * This validation has known issues and is not yet recommended + */ + 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. diff --git a/compiler/packages/babel-plugin-react-forget/src/Validation/ValidateNoRefAccesInRender.ts b/compiler/packages/babel-plugin-react-forget/src/Validation/ValidateNoRefAccesInRender.ts index 0e4cffc48d..1844e3ea90 100644 --- a/compiler/packages/babel-plugin-react-forget/src/Validation/ValidateNoRefAccesInRender.ts +++ b/compiler/packages/babel-plugin-react-forget/src/Validation/ValidateNoRefAccesInRender.ts @@ -63,17 +63,19 @@ export function validateNoRefAccessInRender(fn: HIRFunction): void { } case "ObjectMethod": case "FunctionExpression": { - /* - * functions are allowed to capture refs, so long as the function is not called - * during render. see AnalyzeFunctions for how we ensure that functions which - * capture refs get assigned a mutable range so we know here whether the function - * is called or not - */ - const mutableRange = instr.lvalue.identifier.mutableRange; - if (mutableRange.end > mutableRange.start + 1) { - for (const operand of eachInstructionValueOperand(instr.value)) { - validateNonRefValue(error, operand); - validateNonRefObject(error, operand); + if (fn.env.config.validateRefAccessDuringRenderFunctionExpressions) { + /* + * functions are allowed to capture refs, so long as the function is not called + * during render. see AnalyzeFunctions for how we ensure that functions which + * capture refs get assigned a mutable range so we know here whether the function + * is called or not + */ + const mutableRange = instr.lvalue.identifier.mutableRange; + if (mutableRange.end > mutableRange.start + 1) { + for (const operand of eachInstructionValueOperand(instr.value)) { + validateNonRefValue(error, operand); + validateNonRefObject(error, operand); + } } } break; diff --git a/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/error.invalid-ref-in-callback-invoked-during-render.expect.md b/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/error.invalid-ref-in-callback-invoked-during-render.expect.md index f8d25f7b51..4f8e34ddea 100644 --- a/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/error.invalid-ref-in-callback-invoked-during-render.expect.md +++ b/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/error.invalid-ref-in-callback-invoked-during-render.expect.md @@ -2,7 +2,7 @@ ## Input ```javascript -// @validateRefAccessDuringRender +// @validateRefAccessDuringRender @validateRefAccessDuringRenderFunctionExpressions function Component(props) { const ref = useRef(null); const renderItem = (item) => { diff --git a/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/error.invalid-ref-in-callback-invoked-during-render.js b/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/error.invalid-ref-in-callback-invoked-during-render.js index df786b271a..04f48326be 100644 --- a/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/error.invalid-ref-in-callback-invoked-during-render.js +++ b/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/error.invalid-ref-in-callback-invoked-during-render.js @@ -1,4 +1,4 @@ -// @validateRefAccessDuringRender +// @validateRefAccessDuringRender @validateRefAccessDuringRenderFunctionExpressions function Component(props) { const ref = useRef(null); const renderItem = (item) => { diff --git a/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/ref-current-aliased-not-added-to-dep-2.expect.md b/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/ref-current-aliased-not-added-to-dep-2.expect.md index 3725d01815..25d9ff2cc9 100644 --- a/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/ref-current-aliased-not-added-to-dep-2.expect.md +++ b/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/ref-current-aliased-not-added-to-dep-2.expect.md @@ -2,6 +2,7 @@ ## Input ```javascript +// @validateRefAccessDuringRender:false function Foo({ a }) { const ref = useRef(); const val = ref.current; @@ -15,7 +16,7 @@ function Foo({ a }) { ## Code ```javascript -import { unstable_useMemoCache as useMemoCache } from "react"; +import { unstable_useMemoCache as useMemoCache } from "react"; // @validateRefAccessDuringRender:false function Foo(t20) { const $ = useMemoCache(4); const { a } = t20; diff --git a/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/ref-current-aliased-not-added-to-dep-2.js b/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/ref-current-aliased-not-added-to-dep-2.js index 1bdd040c67..d8004a2540 100644 --- a/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/ref-current-aliased-not-added-to-dep-2.js +++ b/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/ref-current-aliased-not-added-to-dep-2.js @@ -1,3 +1,4 @@ +// @validateRefAccessDuringRender:false function Foo({ a }) { const ref = useRef(); const val = ref.current; diff --git a/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/ref-current-not-added-to-dep-2.expect.md b/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/ref-current-not-added-to-dep-2.expect.md index d9c0147aa5..d8717af18e 100644 --- a/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/ref-current-not-added-to-dep-2.expect.md +++ b/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/ref-current-not-added-to-dep-2.expect.md @@ -2,6 +2,7 @@ ## Input ```javascript +// @validateRefAccessDuringRender:false function Foo({ a }) { const ref = useRef(); const x = { a, val: ref.current }; @@ -14,7 +15,7 @@ function Foo({ a }) { ## Code ```javascript -import { unstable_useMemoCache as useMemoCache } from "react"; +import { unstable_useMemoCache as useMemoCache } from "react"; // @validateRefAccessDuringRender:false function Foo(t17) { const $ = useMemoCache(4); const { a } = t17; diff --git a/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/ref-current-not-added-to-dep-2.js b/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/ref-current-not-added-to-dep-2.js index 80ced49a9d..98902acfa9 100644 --- a/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/ref-current-not-added-to-dep-2.js +++ b/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/ref-current-not-added-to-dep-2.js @@ -1,3 +1,4 @@ +// @validateRefAccessDuringRender:false function Foo({ a }) { const ref = useRef(); const x = { a, val: ref.current };