Unsafe render-time side effects (mutating globals during render)

// PoC: Demonstrates unsafe render-time mutation via a render helper prop before fix let GLOBAL_FLAG = { mutated: false }; // Component that renders by calling a function prop that returns JSX function Item({ renderItem }) { const item = { id: 1 }; return React.createElement('div', null, renderItem(item)); } function App() { const renderItem = (item) => { // This mutation happens during render via a render helper prop GLOBAL_FLAG.mutated = true; return React.createElement('span', null, `Item ${item.id}`); }; return React.createElement(Item, { renderItem }); } // In a real environment, rendering <App /> would mutate GLOBAL_FLAG.mutated during render. // Prior to this fix, such a function could be treated as an event handler or effect and // the mutation might slip past validation. The commit aims to flag such functions as render helpers // to guard against this kind of side effect during render.

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 7d2b0b234c7f..c673ac53d9b1 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/HIR/PrintHIR.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/HIR/PrintHIR.ts @@ -880,7 +880,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 36f32213eab7..8dd79409ce15 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/Inference/InferMutationAliasingEffects.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/Inference/InferMutationAliasingEffects.ts @@ -27,6 +27,7 @@ import { InstructionKind, InstructionValue, isArrayType, + isJsxType, isMapType, isPrimitiveType, isRefOrRefValue, @@ -1871,6 +1872,23 @@ function computeSignatureForInstruction( }); } } + for (const prop of value.props) { + if ( + prop.kind === 'JsxAttribute' && + prop.place.identifier.type.kind === 'Function' && + (isJsxType(prop.place.identifier.type.return) || + (prop.place.identifier.type.return.kind === 'Phi' && + prop.place.identifier.type.return.operands.some(operand => + isJsxType(operand), + ))) + ) { + // 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 488d988b9715..d3a297e2e51c 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/TypeInference/InferTypes.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/TypeInference/InferTypes.ts @@ -777,6 +777,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-phi-return-prop.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-mutate-global-in-render-helper-phi-return-prop.js new file mode 100644 index 000000000000..b6ca0a04aeea --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-mutate-global-in-render-helper-phi-return-prop.js @@ -0,0 +1,16 @@ +function Component() { + const renderItem = item => { + // Multiple returns so that the return type is a Phi (union) + if (item == null) { + return null; + } + // 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 <Item item={item} value={rand} />; + }; + return <ItemList renderItem={renderItem} />; +} 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 000000000000..9355c482fb61 --- /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 <Item item={item} value={rand} />; + }; + return <ItemList renderItem={renderItem} />; +}