From 2ba9a6ca73aa6a1bd199f3ffc5ec58da6026abff Mon Sep 17 00:00:00 2001 From: Joe Savona Date: Wed, 25 Jun 2025 14:52:08 -0700 Subject: [PATCH] [compiler] Infer render helpers for additional validation We currently assume that any functions passes as props may be event handlers or effect functions, and thus don't check for side effects such as mutating globals. However, if a prop is a function that returns JSX that is a sure sign that it's actually a render helper and not an event handler or effect function. So we now emit a `Render` effect for any prop that is a JSX-returning function, triggering all of our render validation. This required a small fix to InferTypes: we weren't correctly populating the `return` type of function types during unification. I also improved the printing of types so we can see the inferred return types. --- .../src/HIR/PrintHIR.ts | 3 +- .../Inference/InferMutationAliasingEffects.ts | 14 ++++++++ .../src/TypeInference/InferTypes.ts | 9 +++++ ...ate-global-in-render-helper-prop.expect.md | 33 +++++++++++++++++++ ...lid-mutate-global-in-render-helper-prop.js | 12 +++++++ 5 files changed, 70 insertions(+), 1 deletion(-) create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-mutate-global-in-render-helper-prop.expect.md create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-mutate-global-in-render-helper-prop.js diff --git a/compiler/packages/babel-plugin-react-compiler/src/HIR/PrintHIR.ts b/compiler/packages/babel-plugin-react-compiler/src/HIR/PrintHIR.ts index 89591aca2d..13855ac63b 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/HIR/PrintHIR.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/HIR/PrintHIR.ts @@ -893,7 +893,8 @@ export function printType(type: Type): string { if (type.kind === 'Object' && type.shapeId != null) { return `:T${type.kind}<${type.shapeId}>`; } else if (type.kind === 'Function' && type.shapeId != null) { - return `:T${type.kind}<${type.shapeId}>`; + const returnType = printType(type.return); + return `:T${type.kind}<${type.shapeId}>()${returnType !== '' ? returnType : ''}`; } else { return `:T${type.kind}`; } diff --git a/compiler/packages/babel-plugin-react-compiler/src/Inference/InferMutationAliasingEffects.ts b/compiler/packages/babel-plugin-react-compiler/src/Inference/InferMutationAliasingEffects.ts index e875aab68a..15cdb39f4f 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/Inference/InferMutationAliasingEffects.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/Inference/InferMutationAliasingEffects.ts @@ -25,6 +25,7 @@ import { InstructionKind, InstructionValue, isArrayType, + isJsxType, isMapType, isPrimitiveType, isRefOrRefValue, @@ -1842,6 +1843,19 @@ function computeSignatureForInstruction( }); } } + for (const prop of value.props) { + if ( + prop.kind === 'JsxAttribute' && + prop.place.identifier.type.kind === 'Function' && + isJsxType(prop.place.identifier.type.return) + ) { + // Any props which return jsx are assumed to be called during render + effects.push({ + kind: 'Render', + place: prop.place, + }); + } + } } break; } diff --git a/compiler/packages/babel-plugin-react-compiler/src/TypeInference/InferTypes.ts b/compiler/packages/babel-plugin-react-compiler/src/TypeInference/InferTypes.ts index 859c871c26..9070d143be 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/TypeInference/InferTypes.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/TypeInference/InferTypes.ts @@ -712,6 +712,15 @@ class Unifier { return {kind: 'Phi', operands: type.operands.map(o => this.get(o))}; } + if (type.kind === 'Function') { + return { + kind: 'Function', + isConstructor: type.isConstructor, + shapeId: type.shapeId, + return: this.get(type.return), + }; + } + return type; } } diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-mutate-global-in-render-helper-prop.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-mutate-global-in-render-helper-prop.expect.md new file mode 100644 index 0000000000..037b2997bc --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-mutate-global-in-render-helper-prop.expect.md @@ -0,0 +1,33 @@ + +## Input + +```javascript +function Component() { + const renderItem = item => { + // Normally we assume that it's safe to mutate globals in a function passed + // as a prop, because the prop could be used as an event handler or effect. + // But if the function returns JSX we can assume it's a render helper, ie + // called during render, and thus it's unsafe to mutate globals or call + // other impure code. + global.property = true; + return ; + }; + return ; +} + +``` + + +## Error + +``` + 6 | // called during render, and thus it's unsafe to mutate globals or call + 7 | // other impure code. +> 8 | global.property = true; + | ^^^^^^ InvalidReact: Writing to a variable defined outside a component or hook is not allowed. Consider using an effect (8:8) + 9 | return ; + 10 | }; + 11 | return ; +``` + + \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-mutate-global-in-render-helper-prop.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-mutate-global-in-render-helper-prop.js new file mode 100644 index 0000000000..9355c482fb --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-mutate-global-in-render-helper-prop.js @@ -0,0 +1,12 @@ +function Component() { + const renderItem = item => { + // Normally we assume that it's safe to mutate globals in a function passed + // as a prop, because the prop could be used as an event handler or effect. + // But if the function returns JSX we can assume it's a render helper, ie + // called during render, and thus it's unsafe to mutate globals or call + // other impure code. + global.property = true; + return ; + }; + return ; +}