diff --git a/compiler/packages/babel-plugin-react-compiler/src/Optimization/ConstantPropagation.ts b/compiler/packages/babel-plugin-react-compiler/src/Optimization/ConstantPropagation.ts index 52a5a8b8c769..de85dfeabd24 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/Optimization/ConstantPropagation.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/Optimization/ConstantPropagation.ts @@ -58,12 +58,58 @@ import {eliminateRedundantPhi} from '../SSA'; */ export function constantPropagation(fn: HIRFunction): void { const constants: Constants = new Map(); - constantPropagationImpl(fn, constants); + const jsxSimpleTagPlaces = collectJsxSimpleTagPlaces(fn); + constantPropagationImpl(fn, constants, jsxSimpleTagPlaces); } -function constantPropagationImpl(fn: HIRFunction, constants: Constants): void { +/* + * Collect the `IdentifierId` of every Place that is used as a JSX element's + * tag (i.e. simple ``, not `` / ``). These places may not + * be rewritten through a LoadGlobal whose binding name has different JSX + * component-vs-intrinsic casing, because doing so changes runtime semantics + * (JSX treats lowercase tags as string intrinsics and uppercase ones as + * component references). + */ +function collectJsxSimpleTagPlaces(fn: HIRFunction): Set { + const set = new Set(); + const visit = (f: HIRFunction): void => { + for (const [, block] of f.body.blocks) { + for (const instr of block.instructions) { + const v = instr.value; + if (v.kind === 'JsxExpression' && v.tag.kind === 'Identifier') { + set.add(v.tag.identifier.id); + } + if (v.kind === 'FunctionExpression' || v.kind === 'ObjectMethod') { + visit(v.loweredFunc.func); + } + } + } + }; + visit(fn); + return set; +} + +/* + * Heuristic: identifiers whose first alphabetic character is uppercase are + * conventionally React components in JSX, while lowercase ones are intrinsic + * (HTML) element tags. + */ +function isLikelyComponentName(name: string): boolean { + const first = name.match(/[A-Za-z]/); + return first !== null && first[0] === first[0].toUpperCase(); +} + +function constantPropagationImpl( + fn: HIRFunction, + constants: Constants, + jsxSimpleTagPlaces: Set = new Set(), +): void { while (true) { - const haveTerminalsChanged = applyConstantPropagation(fn, constants); + const haveTerminalsChanged = applyConstantPropagation( + fn, + constants, + jsxSimpleTagPlaces, + ); if (!haveTerminalsChanged) { break; } @@ -107,6 +153,7 @@ function constantPropagationImpl(fn: HIRFunction, constants: Constants): void { function applyConstantPropagation( fn: HIRFunction, constants: Constants, + jsxSimpleTagPlaces: Set, ): boolean { let hasChanges = false; for (const [, block] of fn.body.blocks) { @@ -131,7 +178,7 @@ function applyConstantPropagation( continue; } const instr = block.instructions[i]!; - const value = evaluateInstruction(constants, instr); + const value = evaluateInstruction(constants, instr, jsxSimpleTagPlaces); if (value !== null) { constants.set(instr.lvalue.identifier.id, value); } @@ -224,6 +271,7 @@ function evaluatePhi(phi: Phi, constants: Constants): Constant | null { function evaluateInstruction( constants: Constants, instr: Instruction, + jsxSimpleTagPlaces: Set, ): Constant | null { const value = instr.value; switch (value.kind) { @@ -578,6 +626,30 @@ function evaluateInstruction( case 'LoadLocal': { const placeValue = read(constants, value.place); if (placeValue !== null) { + /* + * Skip rewriting when the lvalue is used as a simple JSX tag and the + * candidate constant is a LoadGlobal whose binding name has different + * JSX component-vs-intrinsic casing than the local. Propagating would + * flip `` (component reference) into `` (intrinsic + * HTML tag) or vice versa, changing runtime semantics. + */ + if ( + placeValue.kind === 'LoadGlobal' && + jsxSimpleTagPlaces.has(instr.lvalue.identifier.id) + ) { + const localName = + value.place.identifier.name?.kind === 'named' + ? value.place.identifier.name.value + : null; + const globalName = placeValue.binding.name; + if ( + localName !== null && + isLikelyComponentName(localName) !== + isLikelyComponentName(globalName) + ) { + return placeValue; + } + } instr.value = placeValue; } return placeValue; @@ -591,7 +663,11 @@ function evaluateInstruction( } case 'ObjectMethod': case 'FunctionExpression': { - constantPropagationImpl(value.loweredFunc.func, constants); + constantPropagationImpl( + value.loweredFunc.func, + constants, + jsxSimpleTagPlaces, + ); return null; } case 'StartMemoize': { diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/jsx-component-local-aliasing-lowercase-global.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/jsx-component-local-aliasing-lowercase-global.expect.md new file mode 100644 index 000000000000..86b02ebccfd9 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/jsx-component-local-aliasing-lowercase-global.expect.md @@ -0,0 +1,55 @@ + +## Input + +```javascript +// Regression test for https://github.com/facebook/react/issues/35268 +// When a component-named local alias points to a lowercase global, the +// local must not be rewritten through constant propagation — doing so +// turns `` (component reference) into `` (HTML element). + +const base = 'div'; + +function Component() { + const Comp = base; + return ; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [], +}; + +``` + +## Code + +```javascript +import { c as _c } from "react/compiler-runtime"; // Regression test for https://github.com/facebook/react/issues/35268 +// When a component-named local alias points to a lowercase global, the +// local must not be rewritten through constant propagation — doing so +// turns `` (component reference) into `` (HTML element). + +const base = "div"; + +function Component() { + const $ = _c(1); + const Comp = base; + let t0; + if ($[0] === Symbol.for("react.memo_cache_sentinel")) { + 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/jsx-component-local-aliasing-lowercase-global.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/jsx-component-local-aliasing-lowercase-global.js new file mode 100644 index 000000000000..3ae2a441e066 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/jsx-component-local-aliasing-lowercase-global.js @@ -0,0 +1,16 @@ +// Regression test for https://github.com/facebook/react/issues/35268 +// When a component-named local alias points to a lowercase global, the +// local must not be rewritten through constant propagation — doing so +// turns `` (component reference) into `` (HTML element). + +const base = 'div'; + +function Component() { + const Comp = base; + return ; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [], +};