From 34dfede6ed07c4201181045b083633a960e6318d Mon Sep 17 00:00:00 2001 From: Joe Savona Date: Sat, 3 May 2025 09:56:08 +0900 Subject: [PATCH] [compiler] Repro for false positive ValidateNoFreezingKnownMutableFunctions Found when testing the new validation from #33079 internally. I haven't fully debugged, but somehow the combination of the effect function *accessing* a ref and also calling a second function which has a purely local mutation triggers the validation. Even though the called second function only mutates local variables. If i remove the ref access in the effect function, the error goes away. Anyway I'll keep debugging, putting up a repro for now. ghstack-source-id: f22345952aab24859c08c50bd6e08797c3d7fcb9 Pull Request resolved: https://github.com/facebook/react/pull/33113 --- ...-use-effect-function-mutates-ref.expect.md | 58 +++++++++++++++++++ ...invalid-use-effect-function-mutates-ref.js | 27 +++++++++ 2 files changed, 85 insertions(+) create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-use-effect-function-mutates-ref.expect.md create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-use-effect-function-mutates-ref.js diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-use-effect-function-mutates-ref.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-use-effect-function-mutates-ref.expect.md new file mode 100644 index 0000000000..6b244323a3 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-use-effect-function-mutates-ref.expect.md @@ -0,0 +1,58 @@ + +## Input + +```javascript +// @validateNoFreezingKnownMutableFunctions + +import {useCallback, useEffect, useRef} from 'react'; +import {useHook} from 'shared-runtime'; + +function Component() { + const params = useHook(); + const update = useCallback( + partialParams => { + const nextParams = { + ...params, + ...partialParams, + }; + nextParams.param = 'value'; + console.log(nextParams); + }, + [params] + ); + const ref = useRef(null); + useEffect(() => { + if (ref.current === null) { + update(); + } + }, [update]); + + return 'ok'; +} + +``` + + +## Error + +``` + 18 | ); + 19 | const ref = useRef(null); +> 20 | useEffect(() => { + | ^^^^^^^ +> 21 | if (ref.current === null) { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +> 22 | update(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +> 23 | } + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +> 24 | }, [update]); + | ^^^^ InvalidReact: This argument is a function which modifies local variables when called, which can bypass memoization and cause the UI not to update. Functions that are returned from hooks, passed as arguments to hooks, or passed as props to components may not mutate local variables (20:24) + +InvalidReact: The function modifies a local variable here (14:14) + 25 | + 26 | return 'ok'; + 27 | } +``` + + \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-use-effect-function-mutates-ref.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-use-effect-function-mutates-ref.js new file mode 100644 index 0000000000..2e8b7827d0 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-use-effect-function-mutates-ref.js @@ -0,0 +1,27 @@ +// @validateNoFreezingKnownMutableFunctions + +import {useCallback, useEffect, useRef} from 'react'; +import {useHook} from 'shared-runtime'; + +function Component() { + const params = useHook(); + const update = useCallback( + partialParams => { + const nextParams = { + ...params, + ...partialParams, + }; + nextParams.param = 'value'; + console.log(nextParams); + }, + [params] + ); + const ref = useRef(null); + useEffect(() => { + if (ref.current === null) { + update(); + } + }, [update]); + + return 'ok'; +}