diff --git a/compiler/packages/babel-plugin-react-compiler/src/Inference/InferReferenceEffects.ts b/compiler/packages/babel-plugin-react-compiler/src/Inference/InferReferenceEffects.ts index 3724c8d6f1c..520684c026b 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/Inference/InferReferenceEffects.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/Inference/InferReferenceEffects.ts @@ -1103,13 +1103,56 @@ function inferBlock( break; } case "JsxExpression": { - valueKind = { + if (instrValue.tag.kind === "Identifier") { + state.referenceAndRecordEffects( + instrValue.tag, + Effect.Freeze, + ValueReason.JsxCaptured, + functionEffects + ); + } + if (instrValue.children !== null) { + for (const child of instrValue.children) { + state.referenceAndRecordEffects( + child, + Effect.Freeze, + ValueReason.JsxCaptured, + functionEffects + ); + } + } + for (const attr of instrValue.props) { + if (attr.kind === "JsxSpreadAttribute") { + state.referenceAndRecordEffects( + attr.argument, + Effect.Freeze, + ValueReason.JsxCaptured, + functionEffects + ); + } else { + const propEffects: Array = []; + state.referenceAndRecordEffects( + attr.place, + Effect.Freeze, + ValueReason.JsxCaptured, + propEffects + ); + functionEffects.push( + ...propEffects.filter( + (propEffect) => propEffect.kind !== "GlobalMutation" + ) + ); + } + } + + state.initialize(instrValue, { kind: ValueKind.Frozen, reason: new Set([ValueReason.Other]), context: new Set(), - }; - effect = { kind: Effect.Freeze, reason: ValueReason.JsxCaptured }; - break; + }); + state.define(instr.lvalue, instrValue); + instr.lvalue.effect = Effect.ConditionallyMutate; + continue; } case "JsxFragment": { valueKind = { diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/allow-modify-global-in-callback-jsx.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/allow-modify-global-in-callback-jsx.expect.md new file mode 100644 index 00000000000..43ab697f87b --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/allow-modify-global-in-callback-jsx.expect.md @@ -0,0 +1,86 @@ + +## Input + +```javascript +import { useMemo } from "react"; + +const someGlobal = { value: 0 }; + +function Component({ value }) { + const onClick = () => { + someGlobal.value = value; + }; + return useMemo(() => { + return
{someGlobal.value}
; + }, []); +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{ value: 0 }], + sequentialRenders: [ + { value: 1 }, + { value: 1 }, + { value: 42 }, + { value: 42 }, + { value: 0 }, + ], +}; + +``` + +## Code + +```javascript +import { c as _c } from "react/compiler-runtime"; +import { useMemo } from "react"; + +const someGlobal = { value: 0 }; + +function Component(t0) { + const $ = _c(4); + const { value } = t0; + let t1; + if ($[0] !== value) { + t1 = () => { + someGlobal.value = value; + }; + $[0] = value; + $[1] = t1; + } else { + t1 = $[1]; + } + const onClick = t1; + let t2; + let t3; + if ($[2] !== onClick) { + t3 =
{someGlobal.value}
; + $[2] = onClick; + $[3] = t3; + } else { + t3 = $[3]; + } + t2 = t3; + return t2; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{ value: 0 }], + sequentialRenders: [ + { value: 1 }, + { value: 1 }, + { value: 42 }, + { value: 42 }, + { value: 0 }, + ], +}; + +``` + +### Eval output +(kind: ok)
0
+
0
+
0
+
0
+
0
\ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/allow-modify-global-in-callback-jsx.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/allow-modify-global-in-callback-jsx.js new file mode 100644 index 00000000000..7404b4b6c01 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/allow-modify-global-in-callback-jsx.js @@ -0,0 +1,24 @@ +import { useMemo } from "react"; + +const someGlobal = { value: 0 }; + +function Component({ value }) { + const onClick = () => { + someGlobal.value = value; + }; + return useMemo(() => { + return
{someGlobal.value}
; + }, []); +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{ value: 0 }], + sequentialRenders: [ + { value: 1 }, + { value: 1 }, + { value: 42 }, + { value: 42 }, + { value: 0 }, + ], +}; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/allow-reassignment-to-global-function-jsx-prop.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/allow-reassignment-to-global-function-jsx-prop.expect.md new file mode 100644 index 00000000000..eaa2834a497 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/allow-reassignment-to-global-function-jsx-prop.expect.md @@ -0,0 +1,53 @@ + +## Input + +```javascript +function Component() { + const onClick = () => { + // Cannot assign to globals + someUnknownGlobal = true; + moduleLocal = true; + }; + // It's possible that this could be an event handler / effect function, + // but we don't know that and optimistically assume it will only be + // called by an event handler or effect, where it is allowed to modify globals + return
; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{}], +}; + +``` + +## Code + +```javascript +import { c as _c } from "react/compiler-runtime"; +function Component() { + const $ = _c(1); + let t0; + if ($[0] === Symbol.for("react.memo_cache_sentinel")) { + const onClick = () => { + someUnknownGlobal = true; + moduleLocal = true; + }; + + t0 =
; + $[0] = t0; + } else { + t0 = $[0]; + } + return t0; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{}], +}; + +``` + +### Eval output +(kind: ok)
\ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/allow-reassignment-to-global-function-jsx-prop.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/allow-reassignment-to-global-function-jsx-prop.js new file mode 100644 index 00000000000..8b9da9eb311 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/allow-reassignment-to-global-function-jsx-prop.js @@ -0,0 +1,16 @@ +function Component() { + const onClick = () => { + // Cannot assign to globals + someUnknownGlobal = true; + moduleLocal = true; + }; + // It's possible that this could be an event handler / effect function, + // but we don't know that and optimistically assume it will only be + // called by an event handler or effect, where it is allowed to modify globals + return
; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{}], +}; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.assign-global-in-component-tag-function.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.assign-global-in-component-tag-function.expect.md new file mode 100644 index 00000000000..5553f235a08 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.assign-global-in-component-tag-function.expect.md @@ -0,0 +1,27 @@ + +## Input + +```javascript +function Component() { + const Foo = () => { + someGlobal = true; + }; + return ; +} + +``` + + +## Error + +``` + 1 | function Component() { + 2 | const Foo = () => { +> 3 | someGlobal = true; + | ^^^^^^^^^^ InvalidReact: Unexpected reassignment of a variable which was defined outside of the component. Components and hooks should be pure and side-effect free, but variable reassignment is a form of side-effect. If this variable is used in rendering, use useState instead. (https://react.dev/reference/rules/components-and-hooks-must-be-pure#side-effects-must-run-outside-of-render) (3:3) + 4 | }; + 5 | return ; + 6 | } +``` + + \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.assign-global-in-component-tag-function.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.assign-global-in-component-tag-function.js new file mode 100644 index 00000000000..2982fdf7085 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.assign-global-in-component-tag-function.js @@ -0,0 +1,6 @@ +function Component() { + const Foo = () => { + someGlobal = true; + }; + return ; +} diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.assign-global-in-jsx-children.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.assign-global-in-jsx-children.expect.md new file mode 100644 index 00000000000..d380137836c --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.assign-global-in-jsx-children.expect.md @@ -0,0 +1,30 @@ + +## Input + +```javascript +function Component() { + const foo = () => { + someGlobal = true; + }; + // Children are generally access/called during render, so + // modifying a global in a children function is almost + // certainly a mistake. + return {foo}; +} + +``` + + +## Error + +``` + 1 | function Component() { + 2 | const foo = () => { +> 3 | someGlobal = true; + | ^^^^^^^^^^ InvalidReact: Unexpected reassignment of a variable which was defined outside of the component. Components and hooks should be pure and side-effect free, but variable reassignment is a form of side-effect. If this variable is used in rendering, use useState instead. (https://react.dev/reference/rules/components-and-hooks-must-be-pure#side-effects-must-run-outside-of-render) (3:3) + 4 | }; + 5 | // Children are generally access/called during render, so + 6 | // modifying a global in a children function is almost +``` + + \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.assign-global-in-jsx-children.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.assign-global-in-jsx-children.js new file mode 100644 index 00000000000..82554e8ac43 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.assign-global-in-jsx-children.js @@ -0,0 +1,9 @@ +function Component() { + const foo = () => { + someGlobal = true; + }; + // Children are generally access/called during render, so + // modifying a global in a children function is almost + // certainly a mistake. + return {foo}; +} diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.assign-global-in-jsx-spread-attribute.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.assign-global-in-jsx-spread-attribute.expect.md new file mode 100644 index 00000000000..3861b16e90d --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.assign-global-in-jsx-spread-attribute.expect.md @@ -0,0 +1,27 @@ + +## Input + +```javascript +function Component() { + const foo = () => { + someGlobal = true; + }; + return
; +} + +``` + + +## Error + +``` + 1 | function Component() { + 2 | const foo = () => { +> 3 | someGlobal = true; + | ^^^^^^^^^^ InvalidReact: Unexpected reassignment of a variable which was defined outside of the component. Components and hooks should be pure and side-effect free, but variable reassignment is a form of side-effect. If this variable is used in rendering, use useState instead. (https://react.dev/reference/rules/components-and-hooks-must-be-pure#side-effects-must-run-outside-of-render) (3:3) + 4 | }; + 5 | return
; + 6 | } +``` + + \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.assign-global-in-jsx-spread-attribute.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.assign-global-in-jsx-spread-attribute.js new file mode 100644 index 00000000000..1eea9267b50 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.assign-global-in-jsx-spread-attribute.js @@ -0,0 +1,6 @@ +function Component() { + const foo = () => { + someGlobal = true; + }; + return
; +} diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.reassignment-to-global-function-prop.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.reassignment-to-global-function-prop.expect.md deleted file mode 100644 index 56132c34c14..00000000000 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.reassignment-to-global-function-prop.expect.md +++ /dev/null @@ -1,32 +0,0 @@ - -## Input - -```javascript -function Component() { - const foo = () => { - // Cannot assign to globals - someUnknownGlobal = true; - moduleLocal = true; - }; - // It's possible that this could be an event handler / effect function, - // but we don't know that and conservatively assume it's a render helper - // where it's disallowed to modify globals - return ; -} - -``` - - -## Error - -``` - 2 | const foo = () => { - 3 | // Cannot assign to globals -> 4 | someUnknownGlobal = true; - | ^^^^^^^^^^^^^^^^^ InvalidReact: Unexpected reassignment of a variable which was defined outside of the component. Components and hooks should be pure and side-effect free, but variable reassignment is a form of side-effect. If this variable is used in rendering, use useState instead. (https://react.dev/reference/rules/components-and-hooks-must-be-pure#side-effects-must-run-outside-of-render) (4:4) - 5 | moduleLocal = true; - 6 | }; - 7 | // It's possible that this could be an event handler / effect function, -``` - - \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.reassignment-to-global-function-prop.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.reassignment-to-global-function-prop.js deleted file mode 100644 index 926f4c048fd..00000000000 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.reassignment-to-global-function-prop.js +++ /dev/null @@ -1,11 +0,0 @@ -function Component() { - const foo = () => { - // Cannot assign to globals - someUnknownGlobal = true; - moduleLocal = true; - }; - // It's possible that this could be an event handler / effect function, - // but we don't know that and conservatively assume it's a render helper - // where it's disallowed to modify globals - return ; -}