Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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 `<X />`, not `<x.Y />` / `<x:y />`). 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<IdentifierId> {
const set = new Set<IdentifierId>();
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<IdentifierId> = new Set(),
): void {
while (true) {
const haveTerminalsChanged = applyConstantPropagation(fn, constants);
const haveTerminalsChanged = applyConstantPropagation(
fn,
constants,
jsxSimpleTagPlaces,
);
if (!haveTerminalsChanged) {
break;
}
Expand Down Expand Up @@ -107,6 +153,7 @@ function constantPropagationImpl(fn: HIRFunction, constants: Constants): void {
function applyConstantPropagation(
fn: HIRFunction,
constants: Constants,
jsxSimpleTagPlaces: Set<IdentifierId>,
): boolean {
let hasChanges = false;
for (const [, block] of fn.body.blocks) {
Expand All @@ -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);
}
Expand Down Expand Up @@ -224,6 +271,7 @@ function evaluatePhi(phi: Phi, constants: Constants): Constant | null {
function evaluateInstruction(
constants: Constants,
instr: Instruction,
jsxSimpleTagPlaces: Set<IdentifierId>,
): Constant | null {
const value = instr.value;
switch (value.kind) {
Expand Down Expand Up @@ -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 `<Comp />` (component reference) into `<base />` (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;
Expand All @@ -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': {
Expand Down
Original file line number Diff line number Diff line change
@@ -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 `<Comp />` (component reference) into `<base />` (HTML element).

const base = 'div';

function Component() {
const Comp = base;
return <Comp />;
}

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 `<Comp />` (component reference) into `<base />` (HTML element).

const base = "div";

function Component() {
const $ = _c(1);
const Comp = base;
let t0;
if ($[0] === Symbol.for("react.memo_cache_sentinel")) {
t0 = <Comp />;
$[0] = t0;
} else {
t0 = $[0];
}
return t0;
}

export const FIXTURE_ENTRYPOINT = {
fn: Component,
params: [],
};

```

### Eval output
(kind: ok) <div></div>
Original file line number Diff line number Diff line change
@@ -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 `<Comp />` (component reference) into `<base />` (HTML element).

const base = 'div';

function Component() {
const Comp = base;
return <Comp />;
}

export const FIXTURE_ENTRYPOINT = {
fn: Component,
params: [],
};