From 451b00eb09e4f84f1764dfb305bd89a77b007c0e 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: 0edcf31d31ff1ef8ea386ddf9c87bc4c7f51a8a8 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'; +}