From 18e4165a942b2645321a65b2d9637cefe212b647 Mon Sep 17 00:00:00 2001 From: Joe Savona Date: Sat, 3 May 2025 09:58:15 +0900 Subject: [PATCH 01/10] [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-poisoned] --- ...-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 0000000000000..6b244323a304d --- /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 0000000000000..2e8b7827d09d9 --- /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'; +} From 0ee6891337d8e48a0fd45eb946246b982ee90e2e Mon Sep 17 00:00:00 2001 From: Joe Savona Date: Sat, 3 May 2025 16:25:52 +0900 Subject: [PATCH 02/10] Update on "[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-poisoned] From e7fcefc3dc7fc4f6d35c300e0f98a08d133e9c2b Mon Sep 17 00:00:00 2001 From: Joe Savona Date: Wed, 7 May 2025 17:45:31 -0700 Subject: [PATCH 03/10] Update on "[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-poisoned] From 5a976c6ededa5ce5e25c4b273bdb8fffb1f8cc37 Mon Sep 17 00:00:00 2001 From: Joe Savona Date: Thu, 8 May 2025 12:45:09 -0700 Subject: [PATCH 04/10] Update on "[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-poisoned] From 0fd33c44b2ce634d10d20dff3488a49c0b0e4473 Mon Sep 17 00:00:00 2001 From: Joe Savona Date: Thu, 8 May 2025 13:03:58 -0700 Subject: [PATCH 05/10] Update on "[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-poisoned] From a0f7bca2593618561eb7d7f9ef04d3472e53166d Mon Sep 17 00:00:00 2001 From: Joe Savona Date: Thu, 8 May 2025 16:24:17 -0700 Subject: [PATCH 06/10] Update on "[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-poisoned] From 8362c0458eabc045167b3982241f9aaeb38878d7 Mon Sep 17 00:00:00 2001 From: Joe Savona Date: Fri, 9 May 2025 09:01:05 -0700 Subject: [PATCH 07/10] Update on "[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-poisoned] From b2b384a976af51ee3d744b93b39bdb7eb49ebbbc Mon Sep 17 00:00:00 2001 From: Joe Savona Date: Fri, 9 May 2025 10:36:02 -0700 Subject: [PATCH 08/10] Update on "[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-poisoned] From 4de20da296b8f6e22c767d429ce22ee1bcf64220 Mon Sep 17 00:00:00 2001 From: Joe Savona Date: Fri, 9 May 2025 10:40:45 -0700 Subject: [PATCH 09/10] Update on "[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-poisoned] From 8da1df5ba0473e18055c6466b039b5a65f61f22b Mon Sep 17 00:00:00 2001 From: Joe Savona Date: Fri, 9 May 2025 10:42:39 -0700 Subject: [PATCH 10/10] Update on "[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-poisoned]