From 4a36b8379611bfaf4bc628eb45542bc3946961cb Mon Sep 17 00:00:00 2001 From: Joe Savona Date: Tue, 20 Jun 2023 10:11:56 -0700 Subject: [PATCH] Ref validation allows passing refs as props Updates ValidateNoRefAccessInRender to allow passing refs to JSX, but continue disallowing passing ref values (so `ref` is okay but not `ref.current`) --- .../src/HIR/ValidateNoRefAccesInRender.ts | 14 ++++++-- .../allow-passing-refs-as-props.expect.md | 32 +++++++++++++++++++ .../compiler/allow-passing-refs-as-props.js | 4 +++ ...error.invalid-ref-value-as-props.expect.md | 19 +++++++++++ .../error.invalid-ref-value-as-props.js | 4 +++ 5 files changed, 70 insertions(+), 3 deletions(-) create mode 100644 compiler/forget/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/allow-passing-refs-as-props.expect.md create mode 100644 compiler/forget/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/allow-passing-refs-as-props.js create mode 100644 compiler/forget/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/error.invalid-ref-value-as-props.expect.md create mode 100644 compiler/forget/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/error.invalid-ref-value-as-props.js diff --git a/compiler/forget/packages/babel-plugin-react-forget/src/HIR/ValidateNoRefAccesInRender.ts b/compiler/forget/packages/babel-plugin-react-forget/src/HIR/ValidateNoRefAccesInRender.ts index 180febfded..2f9cd040e2 100644 --- a/compiler/forget/packages/babel-plugin-react-forget/src/HIR/ValidateNoRefAccesInRender.ts +++ b/compiler/forget/packages/babel-plugin-react-forget/src/HIR/ValidateNoRefAccesInRender.ts @@ -49,10 +49,18 @@ export function validateNoRefAccessInRender(fn: HIRFunction): void { // for these instructions so they ensure we have a complete analysis. break; } + case "JsxExpression": { + // It's okay to pass refs to JSX, but not ref *values* + for (const operand of eachInstructionValueOperand(instr.value)) { + validateNonRefValue(error, operand); + } + break; + } case "FunctionExpression": { - // For now we assume *all* function expressions are safe, eventually we can - // be more precise and disallow ref access in functions that may be called - // during render + // 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)) { diff --git a/compiler/forget/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/allow-passing-refs-as-props.expect.md b/compiler/forget/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/allow-passing-refs-as-props.expect.md new file mode 100644 index 0000000000..022d17f16f --- /dev/null +++ b/compiler/forget/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/allow-passing-refs-as-props.expect.md @@ -0,0 +1,32 @@ + +## Input + +```javascript +function Component(props) { + const ref = useRef(null); + return ; +} + +``` + +## Code + +```javascript +import { unstable_useMemoCache as useMemoCache } from "react"; +function Component(props) { + const $ = useMemoCache(2); + const ref = useRef(null); + const c_0 = $[0] !== ref; + let t0; + if (c_0) { + t0 = ; + $[0] = ref; + $[1] = t0; + } else { + t0 = $[1]; + } + return t0; +} + +``` + \ No newline at end of file diff --git a/compiler/forget/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/allow-passing-refs-as-props.js b/compiler/forget/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/allow-passing-refs-as-props.js new file mode 100644 index 0000000000..a820b0d60e --- /dev/null +++ b/compiler/forget/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/allow-passing-refs-as-props.js @@ -0,0 +1,4 @@ +function Component(props) { + const ref = useRef(null); + return ; +} diff --git a/compiler/forget/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/error.invalid-ref-value-as-props.expect.md b/compiler/forget/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/error.invalid-ref-value-as-props.expect.md new file mode 100644 index 0000000000..40ea3775b3 --- /dev/null +++ b/compiler/forget/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/error.invalid-ref-value-as-props.expect.md @@ -0,0 +1,19 @@ + +## Input + +```javascript +function Component(props) { + const ref = useRef(null); + return ; +} + +``` + + +## Error + +``` +[ReactForget] InvalidInput: Ref values (the `current` property) may not be accessed during render. Cannot access ref value at freeze $20:TObject (3:3) +``` + + \ No newline at end of file diff --git a/compiler/forget/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/error.invalid-ref-value-as-props.js b/compiler/forget/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/error.invalid-ref-value-as-props.js new file mode 100644 index 0000000000..3cf0b2aaf8 --- /dev/null +++ b/compiler/forget/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/error.invalid-ref-value-as-props.js @@ -0,0 +1,4 @@ +function Component(props) { + const ref = useRef(null); + return ; +}