diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/todo.invalid-reassign-local-variable-in-effect.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/todo.invalid-reassign-local-variable-in-effect.expect.md new file mode 100644 index 0000000000..ede6507782 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/todo.invalid-reassign-local-variable-in-effect.expect.md @@ -0,0 +1,98 @@ + +## Input + +```javascript +import { useEffect } from "react"; + +function Component() { + let local; + + const reassignLocal = (newValue) => { + local = newValue; + }; + + const onMount = (newValue) => { + reassignLocal("hello"); + + if (local === newValue) { + // Without React Compiler, `reassignLocal` is freshly created + // on each render, capturing a binding to the latest `local`, + // such that invoking reassignLocal will reassign the same + // binding that we are observing in the if condition, and + // we reach this branch + console.log("`local` was updated!"); + } else { + // With React Compiler enabled, `reassignLocal` is only created + // once, capturing a binding to `local` in that render pass. + // Therefore, calling `reassignLocal` will reassign the wrong + // version of `local`, and not update the binding we are checking + // in the if condition. + // + // To protect against this, we disallow reassigning locals from + // functions that escape + throw new Error("`local` not updated!"); + } + }; + + useEffect(() => { + onMount(); + }, [onMount]); + + return "ok"; +} + +``` + +## Code + +```javascript +import { c as _c } from "react/compiler-runtime"; +import { useEffect } from "react"; + +function Component() { + const $ = _c(4); + let local; + let t0; + if ($[0] === Symbol.for("react.memo_cache_sentinel")) { + t0 = (newValue) => { + local = newValue; + }; + $[0] = t0; + } else { + t0 = $[0]; + } + const reassignLocal = t0; + let t1; + if ($[1] === Symbol.for("react.memo_cache_sentinel")) { + t1 = (newValue_0) => { + reassignLocal("hello"); + if (local === newValue_0) { + console.log("`local` was updated!"); + } else { + throw new Error("`local` not updated!"); + } + }; + $[1] = t1; + } else { + t1 = $[1]; + } + const onMount = t1; + let t2; + let t3; + if ($[2] === Symbol.for("react.memo_cache_sentinel")) { + t2 = () => { + onMount(); + }; + t3 = [onMount]; + $[2] = t2; + $[3] = t3; + } else { + t2 = $[2]; + t3 = $[3]; + } + useEffect(t2, t3); + return "ok"; +} + +``` + \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/todo.invalid-reassign-local-variable-in-effect.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/todo.invalid-reassign-local-variable-in-effect.js new file mode 100644 index 0000000000..9d9f3b7f32 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/todo.invalid-reassign-local-variable-in-effect.js @@ -0,0 +1,38 @@ +import { useEffect } from "react"; + +function Component() { + let local; + + const reassignLocal = (newValue) => { + local = newValue; + }; + + const onMount = (newValue) => { + reassignLocal("hello"); + + if (local === newValue) { + // Without React Compiler, `reassignLocal` is freshly created + // on each render, capturing a binding to the latest `local`, + // such that invoking reassignLocal will reassign the same + // binding that we are observing in the if condition, and + // we reach this branch + console.log("`local` was updated!"); + } else { + // With React Compiler enabled, `reassignLocal` is only created + // once, capturing a binding to `local` in that render pass. + // Therefore, calling `reassignLocal` will reassign the wrong + // version of `local`, and not update the binding we are checking + // in the if condition. + // + // To protect against this, we disallow reassigning locals from + // functions that escape + throw new Error("`local` not updated!"); + } + }; + + useEffect(() => { + onMount(); + }, [onMount]); + + return "ok"; +} diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/todo.invalid-reassign-local-variable-in-hook-argument.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/todo.invalid-reassign-local-variable-in-hook-argument.expect.md new file mode 100644 index 0000000000..e0da1fd2a0 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/todo.invalid-reassign-local-variable-in-hook-argument.expect.md @@ -0,0 +1,96 @@ + +## Input + +```javascript +import { useEffect } from "react"; +import { useIdentity } from "shared-runtime"; + +function Component() { + let local; + + const reassignLocal = (newValue) => { + local = newValue; + }; + + const callback = (newValue) => { + reassignLocal("hello"); + + if (local === newValue) { + // Without React Compiler, `reassignLocal` is freshly created + // on each render, capturing a binding to the latest `local`, + // such that invoking reassignLocal will reassign the same + // binding that we are observing in the if condition, and + // we reach this branch + console.log("`local` was updated!"); + } else { + // With React Compiler enabled, `reassignLocal` is only created + // once, capturing a binding to `local` in that render pass. + // Therefore, calling `reassignLocal` will reassign the wrong + // version of `local`, and not update the binding we are checking + // in the if condition. + // + // To protect against this, we disallow reassigning locals from + // functions that escape + throw new Error("`local` not updated!"); + } + }; + + useIdentity(() => { + callback(); + }); + + return "ok"; +} + +``` + +## Code + +```javascript +import { c as _c } from "react/compiler-runtime"; +import { useEffect } from "react"; +import { useIdentity } from "shared-runtime"; + +function Component() { + const $ = _c(3); + let local; + let t0; + if ($[0] === Symbol.for("react.memo_cache_sentinel")) { + t0 = (newValue) => { + local = newValue; + }; + $[0] = t0; + } else { + t0 = $[0]; + } + const reassignLocal = t0; + let t1; + if ($[1] === Symbol.for("react.memo_cache_sentinel")) { + t1 = (newValue_0) => { + reassignLocal("hello"); + if (local === newValue_0) { + console.log("`local` was updated!"); + } else { + throw new Error("`local` not updated!"); + } + }; + $[1] = t1; + } else { + t1 = $[1]; + } + const callback = t1; + let t2; + if ($[2] === Symbol.for("react.memo_cache_sentinel")) { + t2 = () => { + callback(); + }; + $[2] = t2; + } else { + t2 = $[2]; + } + useIdentity(t2); + return "ok"; +} + +``` + \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/todo.invalid-reassign-local-variable-in-hook-argument.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/todo.invalid-reassign-local-variable-in-hook-argument.js new file mode 100644 index 0000000000..a46e58608b --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/todo.invalid-reassign-local-variable-in-hook-argument.js @@ -0,0 +1,39 @@ +import { useEffect } from "react"; +import { useIdentity } from "shared-runtime"; + +function Component() { + let local; + + const reassignLocal = (newValue) => { + local = newValue; + }; + + const callback = (newValue) => { + reassignLocal("hello"); + + if (local === newValue) { + // Without React Compiler, `reassignLocal` is freshly created + // on each render, capturing a binding to the latest `local`, + // such that invoking reassignLocal will reassign the same + // binding that we are observing in the if condition, and + // we reach this branch + console.log("`local` was updated!"); + } else { + // With React Compiler enabled, `reassignLocal` is only created + // once, capturing a binding to `local` in that render pass. + // Therefore, calling `reassignLocal` will reassign the wrong + // version of `local`, and not update the binding we are checking + // in the if condition. + // + // To protect against this, we disallow reassigning locals from + // functions that escape + throw new Error("`local` not updated!"); + } + }; + + useIdentity(() => { + callback(); + }); + + return "ok"; +} diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/todo.invalid-reassign-local-variable-in-jsx-callback.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/todo.invalid-reassign-local-variable-in-jsx-callback.expect.md new file mode 100644 index 0000000000..af2b89700a --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/todo.invalid-reassign-local-variable-in-jsx-callback.expect.md @@ -0,0 +1,77 @@ + +## Input + +```javascript +function Component() { + let local; + + const reassignLocal = (newValue) => { + local = newValue; + }; + + const onClick = (newValue) => { + reassignLocal("hello"); + + if (local === newValue) { + // Without React Compiler, `reassignLocal` is freshly created + // on each render, capturing a binding to the latest `local`, + // such that invoking reassignLocal will reassign the same + // binding that we are observing in the if condition, and + // we reach this branch + console.log("`local` was updated!"); + } else { + // With React Compiler enabled, `reassignLocal` is only created + // once, capturing a binding to `local` in that render pass. + // Therefore, calling `reassignLocal` will reassign the wrong + // version of `local`, and not update the binding we are checking + // in the if condition. + // + // To protect against this, we disallow reassigning locals from + // functions that escape + throw new Error("`local` not updated!"); + } + }; + + return ; +} + +``` + +## Code + +```javascript +import { c as _c } from "react/compiler-runtime"; +function Component() { + const $ = _c(2); + let local; + let t0; + if ($[0] === Symbol.for("react.memo_cache_sentinel")) { + t0 = (newValue) => { + local = newValue; + }; + $[0] = t0; + } else { + t0 = $[0]; + } + const reassignLocal = t0; + let t1; + if ($[1] === Symbol.for("react.memo_cache_sentinel")) { + const onClick = (newValue_0) => { + reassignLocal("hello"); + if (local === newValue_0) { + console.log("`local` was updated!"); + } else { + throw new Error("`local` not updated!"); + } + }; + + t1 = ; + $[1] = t1; + } else { + t1 = $[1]; + } + return t1; +} + +``` + \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/todo.invalid-reassign-local-variable-in-jsx-callback.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/todo.invalid-reassign-local-variable-in-jsx-callback.js new file mode 100644 index 0000000000..47cefa6708 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/todo.invalid-reassign-local-variable-in-jsx-callback.js @@ -0,0 +1,32 @@ +function Component() { + let local; + + const reassignLocal = (newValue) => { + local = newValue; + }; + + const onClick = (newValue) => { + reassignLocal("hello"); + + if (local === newValue) { + // Without React Compiler, `reassignLocal` is freshly created + // on each render, capturing a binding to the latest `local`, + // such that invoking reassignLocal will reassign the same + // binding that we are observing in the if condition, and + // we reach this branch + console.log("`local` was updated!"); + } else { + // With React Compiler enabled, `reassignLocal` is only created + // once, capturing a binding to `local` in that render pass. + // Therefore, calling `reassignLocal` will reassign the wrong + // version of `local`, and not update the binding we are checking + // in the if condition. + // + // To protect against this, we disallow reassigning locals from + // functions that escape + throw new Error("`local` not updated!"); + } + }; + + return ; +} diff --git a/compiler/packages/snap/src/SproutTodoFilter.ts b/compiler/packages/snap/src/SproutTodoFilter.ts index 815285fed0..c4bc6776da 100644 --- a/compiler/packages/snap/src/SproutTodoFilter.ts +++ b/compiler/packages/snap/src/SproutTodoFilter.ts @@ -483,6 +483,11 @@ const skipFilter = new Set([ "rules-of-hooks/rules-of-hooks-93dc5d5e538a", "rules-of-hooks/rules-of-hooks-69521d94fa03", + // should error + "todo.invalid-reassign-local-variable-in-jsx-callback", + "todo.invalid-reassign-local-variable-in-hook-argument", + "todo.invalid-reassign-local-variable-in-effect", + // bugs "bug-invalid-hoisting-functionexpr", "original-reactive-scopes-fork/bug-nonmutating-capture-in-unsplittable-memo-block",