From ab668d5b5063156a9b0fba3ae16fc07d0f458253 Mon Sep 17 00:00:00 2001 From: Mike Vitousek Date: Thu, 15 Aug 2024 14:25:24 -0400 Subject: [PATCH] [compiler] Don't error on ref-in-render on StartMemoize Test Plan: Fixes the previous issue: ref enforcement ignores memoization marker instructions [ghstack-poisoned] --- .../Validation/ValidateNoRefAccesInRender.ts | 3 + ...ified-later-preserve-memoization.expect.md | 20 ++----- ...ia-function-preserve-memoization.expect.md | 24 +++----- .../error.useCallback-ref-in-render.expect.md | 41 ------------- .../useCallback-ref-in-render.expect.md | 58 +++++++++++++++++++ ...render.js => useCallback-ref-in-render.js} | 0 6 files changed, 73 insertions(+), 73 deletions(-) delete mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.useCallback-ref-in-render.expect.md create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/useCallback-ref-in-render.expect.md rename compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/{error.useCallback-ref-in-render.js => useCallback-ref-in-render.js} (100%) diff --git a/compiler/packages/babel-plugin-react-compiler/src/Validation/ValidateNoRefAccesInRender.ts b/compiler/packages/babel-plugin-react-compiler/src/Validation/ValidateNoRefAccesInRender.ts index b4fb2a618ada4..b17c495230784 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/Validation/ValidateNoRefAccesInRender.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/Validation/ValidateNoRefAccesInRender.ts @@ -184,6 +184,9 @@ function validateNoRefAccessInRenderImpl( } break; } + case 'StartMemoize': + case 'FinishMemoize': + break; default: { for (const operand of eachInstructionValueOperand(instr.value)) { validateNoRefValueAccess(errors, refAccessingFunctions, operand); diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.todo-useCallback-set-ref-nested-property-ref-modified-later-preserve-memoization.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.todo-useCallback-set-ref-nested-property-ref-modified-later-preserve-memoization.expect.md index c79ceca9efcee..bff8f0e01f582 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.todo-useCallback-set-ref-nested-property-ref-modified-later-preserve-memoization.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.todo-useCallback-set-ref-nested-property-ref-modified-later-preserve-memoization.expect.md @@ -31,23 +31,13 @@ export const FIXTURE_ENTRYPOINT = { ## Error ``` - 5 | const ref = useRef({inner: null}); - 6 | -> 7 | const onChange = useCallback(event => { - | ^^^^^^^^^^ -> 8 | // The ref should still be mutable here even though function deps are frozen in - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ -> 9 | // @enablePreserveExistingMemoizationGuarantees mode - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ -> 10 | ref.current.inner = event.target.value; - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ -> 11 | }); - | ^^^^ InvalidReact: Ref values (the `current` property) may not be accessed during render. (https://react.dev/reference/react/useRef). Cannot access ref value at freeze $44:TObject (7:11) - -InvalidReact: Ref values (the `current` property) may not be accessed during render. (https://react.dev/reference/react/useRef) (14:14) 12 | 13 | // The ref is modified later, extending its range and preventing memoization of onChange - 14 | ref.current.inner = null; +> 14 | ref.current.inner = null; + | ^^^^^^^^^^^^^^^^^ InvalidReact: Ref values (the `current` property) may not be accessed during render. (https://react.dev/reference/react/useRef) (14:14) + 15 | + 16 | return ; + 17 | } ``` \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.useCallback-accesses-ref-mutated-later-via-function-preserve-memoization.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.useCallback-accesses-ref-mutated-later-via-function-preserve-memoization.expect.md index 045ac8c3d61cb..9014de67d4478 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.useCallback-accesses-ref-mutated-later-via-function-preserve-memoization.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.useCallback-accesses-ref-mutated-later-via-function-preserve-memoization.expect.md @@ -34,25 +34,15 @@ export const FIXTURE_ENTRYPOINT = { ## Error ``` - 5 | const ref = useRef({inner: null}); - 6 | -> 7 | const onChange = useCallback(event => { - | ^^^^^^^^^^ -> 8 | // The ref should still be mutable here even though function deps are frozen in - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ -> 9 | // @enablePreserveExistingMemoizationGuarantees mode - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ -> 10 | ref.current.inner = event.target.value; - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ -> 11 | }); - | ^^^^ InvalidReact: Ref values (the `current` property) may not be accessed during render. (https://react.dev/reference/react/useRef). Cannot access ref value at freeze $53:TObject (7:11) - -InvalidReact: This function accesses a ref value (the `current` property), which may not be accessed during render. (https://react.dev/reference/react/useRef). Function mutate? $77[20:22]:TObject accesses a ref (17:17) + 15 | ref.current.inner = null; + 16 | }; +> 17 | reset(); + | ^^^^^ InvalidReact: This function accesses a ref value (the `current` property), which may not be accessed during render. (https://react.dev/reference/react/useRef). Function mutate? $77[20:22]:TObject accesses a ref (17:17) InvalidReact: Ref values (the `current` property) may not be accessed during render. (https://react.dev/reference/react/useRef) (17:17) - 12 | - 13 | // The ref is modified later, extending its range and preventing memoization of onChange - 14 | const reset = () => { + 18 | + 19 | return ; + 20 | } ``` \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.useCallback-ref-in-render.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.useCallback-ref-in-render.expect.md deleted file mode 100644 index ad1c0e7c3fe3a..0000000000000 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.useCallback-ref-in-render.expect.md +++ /dev/null @@ -1,41 +0,0 @@ - -## Input - -```javascript -// @flow @validateRefAccessDuringRender @validatePreserveExistingMemoizationGuarantees - -component Foo() { - const ref = useRef(); - - const s = useCallback(() => { - return ref.current; - }); - - return ; -} - -export const FIXTURE_ENTRYPOINT = { - fn: Foo, - params: [], -}; - -``` - - -## Error - -``` - 4 | const ref = useRef(); - 5 | -> 6 | const s = useCallback(() => { - | ^^^^^^^ -> 7 | return ref.current; - | ^^^^^^^^^^^^^^^^^^^^^^^ -> 8 | }); - | ^^^^ InvalidReact: Ref values (the `current` property) may not be accessed during render. (https://react.dev/reference/react/useRef). Cannot access ref value at read $27:TObject (6:8) - 9 | - 10 | return ; - 11 | } -``` - - \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/useCallback-ref-in-render.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/useCallback-ref-in-render.expect.md new file mode 100644 index 0000000000000..90f0877b9d42d --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/useCallback-ref-in-render.expect.md @@ -0,0 +1,58 @@ + +## Input + +```javascript +// @flow @validateRefAccessDuringRender @validatePreserveExistingMemoizationGuarantees + +component Foo() { + const ref = useRef(); + + const s = useCallback(() => { + return ref.current; + }); + + return ; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Foo, + params: [], +}; + +``` + +## Code + +```javascript +import { c as _c } from "react/compiler-runtime"; + +function Foo() { + const $ = _c(2); + const ref = useRef(); + let t0; + if ($[0] === Symbol.for("react.memo_cache_sentinel")) { + t0 = () => ref.current; + $[0] = t0; + } else { + t0 = $[0]; + } + const s = t0; + let t1; + if ($[1] === Symbol.for("react.memo_cache_sentinel")) { + t1 = ; + $[1] = t1; + } else { + t1 = $[1]; + } + return t1; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Foo, + params: [], +}; + +``` + +### Eval output +(kind: exception) useRef is not defined \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.useCallback-ref-in-render.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/useCallback-ref-in-render.js similarity index 100% rename from compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.useCallback-ref-in-render.js rename to compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/useCallback-ref-in-render.js