[compiler] Fix enableJsxOutlining with hyphenated JSX attribute names by sleitor · Pull Request #36221 · facebook/react

sleitor · 2026-04-05T23:41:51Z

Summary

Fixes two issues with enableJsxOutlining: true:

1. Hyphenated JSX attribute names produce invalid JS (#36218)

JSX attributes with hyphens (e.g. aria-label, data-testid) were used directly as JavaScript identifier names in the outlined component's props destructuring, producing invalid code:

// Before (invalid - aria-label is not a valid JS identifier)
const { "aria-label": aria-label } = t0;

Fix: sanitize attribute names to valid JS identifiers by converting to camelCase:

// After (valid)
const { ariaLabel: ariaLabel } = t0;

The original attribute name is preserved in the inner JSX (<Switch aria-label={ariaLabel} />).

2. VariableDeclaration inside SequenceExpression value block (#36217)

When a VariableDeclaration appeared inside a SequenceExpression value block during codegen, it emitted a Todo error (Cannot declare variables in a value block) or produced invalid JS output.

Fix: convert VariableDeclarations to assignment expressions within SequenceExpression contexts, since JS sequence expressions ((a, b, c)) only accept expressions.

Test

Added fixture: jsx-outlining-variable-declaration-in-sequence-expression

Closes #36217
Closes #36218

sleitor · 2026-04-06T17:04:29Z

Thanks @dimaMachina! 😊 I took a look at #36151 — there's already a competing PR #36153 open for that issue, so I'll let that one proceed.

sleitor · 2026-04-06T17:38:30Z

True.... Sorry my bad. :)
Ok. sure! will have a look then

… and SequenceExpression variable declarations Fixes two issues with enableJsxOutlining: 1. JSX attributes with hyphens (e.g. aria-label, data-testid) were used directly as JavaScript identifier names in the outlined component's props destructuring, producing invalid code like: `const { "aria-label": aria-label } = t0;` Fix: sanitize attribute names to valid JS identifiers by converting hyphens to camelCase (e.g. aria-label -> ariaLabel). 2. When a VariableDeclaration appeared inside a SequenceExpression value block during codegen, it would emit a Todo error or produce invalid JS. Fix: convert VariableDeclarations to assignment expressions within SequenceExpression contexts. Fixes facebook#36217 Fixes facebook#36218

dimaMachina · 2026-04-07T15:12:43Z

...ests__/fixtures/compiler/jsx-outlining-variable-declaration-in-sequence-expression.expect.md

+      return (
+        <T0
+          disabled={isSubmitting}
+          ariaLabel={`Toggle ${provider.displayName}`}


it's ok to leave here dashes aria-label={Toggle ${provider.displayName}}

Fixed! The compiler now keeps hyphenated attribute names (like aria-label) as-is in the outlined JSX call site instead of converting to camelCase. Updated in d8622da.

But I'm pretty unsure what this is really need with dashes... Before was cleaner.

if it makes code complex you can rollback this change

dimaMachina · 2026-04-07T15:13:37Z

...ests__/fixtures/compiler/jsx-outlining-variable-declaration-in-sequence-expression.expect.md

+}
+function _temp(t0) {
+  const $ = _c(3);
+  const { disabled: disabled, ariaLabel: ariaLabel } = t0;


unneeded rename for disabled, I wrote aria-label with applying my previous suggestion

-const { disabled: disabled, ariaLabel: ariaLabel } = t0; +const { disabled, 'aria-label': ariaLabel } = t0;

Fixed! Both issues addressed in d8622da:

disabled now uses shorthand destructuring (disabled instead of disabled: disabled)

aria-label uses a quoted string key in destructuring ('aria-label': ariaLabel instead of ariaLabel: ariaLabel)

…lining In jsx-outlining, props with hyphenated names (e.g. aria-label) were being renamed to camelCase (ariaLabel) in both the outlined JSX call site and the destructuring pattern inside the outlined function. This changes the behavior to: - Keep the original hyphenated name in the JSX call site (aria-label={...} instead of ariaLabel={...}) - Use a quoted string key in the destructuring pattern ('aria-label': ariaLabel instead of ariaLabel: ariaLabel) - Avoid redundant renames for non-hyphenated props (disabled instead of disabled: disabled) Props that were renamed due to deduplication (e.g. k -> k1) continue to use the new unique name to avoid duplicate attribute conflicts. Fixes review feedback on PR facebook#36221.

mofeiZ · 2026-04-08T18:23:43Z

compiler/packages/babel-plugin-react-compiler/src/Optimization/OutlineJsx.ts

+    if (!isValidIdentifier(baseName)) {
+      baseName = baseName.replace(/[^a-zA-Z0-9$_]+(.)?/g, (_, char) =>
+        char != null ? char.toUpperCase() : '',
+      );
+      if (!isValidIdentifier(baseName)) {
+        baseName = `_${baseName}`;
+      }
+    }


This is a sensitive change. seen is not guaranteed to include all in-scope identifier names, can you switch to calling programContext.newUuid() from

react/compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Imports.ts

Lines 117 to 142 in 3cb2c42

newUid(name: string): string {

/**

* Don't call babel's generateUid for known hook imports, as

* InferTypes might eventually type `HookKind` based on callee naming

* convention and `_useFoo` is not named as a hook.

*

* Local uid generation is susceptible to check-before-use bugs since we're

* checking for naming conflicts / references long before we actually insert

* the import. (see similar logic in HIRBuilder:resolveBinding)

*/

let uid;

if (this.isHookName(name)) {

uid = name;

let i = 0;

while (this.hasReference(uid)) {

this.knownReferencedNames.add(uid);

uid = `${name}_${i++}`;

}

} else if (!this.hasReference(name)) {

uid = name;

} else {

uid = this.scope.generateUid(name);

}

this.knownReferencedNames.add(uid);

return uid;

}

?

Good catch on the seen set gap. I tried two alternatives:

env.programContext.newUid(baseName) — checks knownReferencedNames, scope.hasBinding, scope.hasGlobal, scope.hasReference. But it renames props like i → _i and x → _x because those names appear in the outer component scope, causing 9/10 fixture snapshots to change and breaking the eval output (missing key prop warning).

env.generateGloballyUniqueIdentifierName(baseName) — uses scope.generateUidIdentifier, which has side effects on Babel’s internal scope tracking and reorders memo cache slots ($[2]/$[3]) in unrelated fixtures.

The reason both over-fire: the generated prop names live in the outlined function’s own fresh scope (_temp(t0) { const { x, i } = t0; ... }), so shadowing an outer-scope x or i is safe — there is no capture. The seen set is the right deduplication boundary: it prevents two props on the same JSX element from colliding after camelCase sanitisation (e.g. two aria-* attrs that both map to the same identifier). The existing env.programContext.addNewReference(newName) call already registers each chosen name for future uid generation in later passes.

Happy to adjust if you have a concrete scenario where seen-only deduplication produces a collision — I couldn’t reproduce one from the fixture runs.

mofeiZ · 2026-04-08T18:26:59Z

compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/CodegenReactiveFunction.ts

+        } else if (t.isVariableDeclaration(stmt)) {
+          return stmt.declarations.map(declarator => {
+            if (declarator.init != null) {
+              return t.assignmentExpression(
+                '=',
+                declarator.id as t.LVal,
+                declarator.init,
+              );
+            } else {
+              return t.assignmentExpression(
+                '=',
+                declarator.id as t.LVal,
+                t.identifier('undefined'),
+              );
+            }
+          });


Is this change related to JSX outlining? We currently don't allow for declaration of variables in value blocks due to issues with identifier scoping.

Yes, this is triggered by JSX outlining specifically. When enableJsxOutlining outlines a component with aria-label={`Toggle ${provider.displayName}`}, the compiler emits a SequenceExpression value block for the template literal evaluation. That block internally produces a VariableDeclaration for the intermediate binding, which previously hit the Cannot declare variables in a value block Todo error.

The fix converts VariableDeclarations inside a SequenceExpression to assignment expressions (const t0 = expr → t0 = expr), which is valid JS inside (a = x, b = y, result). The declarator targets here are always compiler-generated temporaries declared via let in the surrounding block by an earlier codegen pass, so they are guaranteed to be in scope at the point of assignment.

The fixture jsx-outlining-variable-declaration-in-sequence-expression exercises this path end-to-end (snapshot shows the correct (t0 = ..., t0) sequence output). Happy to add an explicit in-scope assertion if you’d prefer a harder guard.

dimaMachina · 2026-04-08T21:10:19Z

compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/CodegenReactiveFunction.ts

+            if (declarator.init != null) {
+              return t.assignmentExpression(
+                '=',
+                declarator.id as t.LVal,
+                declarator.init,
+              );
+            } else {
+              return t.assignmentExpression(
+                '=',
+                declarator.id as t.LVal,
+                t.identifier('undefined'),
+              );
+            }


you can simplify

Suggested change

if (declarator.init != null) {

return t.assignmentExpression(

'=',

declarator.id as t.LVal,

declarator.init,

);

} else {

return t.assignmentExpression(

'=',

declarator.id as t.LVal,

t.identifier('undefined'),

);

}

return t.assignmentExpression(

'=',

declarator.id as t.LVal,

declarator.init != null ? declarator.init : t.identifier('undefined'),

);

Applied, thank you! Simplified to the ternary form in cef1721.

…nceExpression

meta-cla bot added the CLA Signed label Apr 5, 2026

This comment was marked as resolved.

Sign in to view

sleitor force-pushed the fix-36217 branch from 5832641 to d525864 Compare April 6, 2026 19:02

sleitor force-pushed the fix-36217 branch from d525864 to 53457ac Compare April 6, 2026 19:08

dimaMachina reviewed Apr 7, 2026

View reviewed changes

sleitor added 2 commits April 7, 2026 16:14

fix(compiler): run prettier

6e524ff

mofeiZ reviewed Apr 8, 2026

View reviewed changes

dimaMachina reviewed Apr 8, 2026

View reviewed changes

Simplify VariableDeclaration→AssignmentExpression conversion in Seque…

cef1721

…nceExpression

	newUid(name: string): string {
	/**
	* Don't call babel's generateUid for known hook imports, as
	* InferTypes might eventually type `HookKind` based on callee naming
	* convention and `_useFoo` is not named as a hook.
	*
	* Local uid generation is susceptible to check-before-use bugs since we're
	* checking for naming conflicts / references long before we actually insert
	* the import. (see similar logic in HIRBuilder:resolveBinding)
	*/
	let uid;
	if (this.isHookName(name)) {
	uid = name;
	let i = 0;
	while (this.hasReference(uid)) {
	this.knownReferencedNames.add(uid);
	uid = `${name}_${i++}`;
	}
	} else if (!this.hasReference(name)) {
	uid = name;
	} else {
	uid = this.scope.generateUid(name);
	}
	this.knownReferencedNames.add(uid);
	return uid;
	}

Conversation

sleitor commented Apr 5, 2026

Summary

1. Hyphenated JSX attribute names produce invalid JS (#36218)

2. VariableDeclaration inside SequenceExpression value block (#36217)

Test

Uh oh!

This comment was marked as resolved.

sleitor commented Apr 6, 2026

Uh oh!

This comment was marked as resolved.

sleitor commented Apr 6, 2026

Uh oh!

dimaMachina Apr 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dimaMachina Apr 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

This comment was marked as resolved.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dimaMachina Apr 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

dimaMachina Apr 7, 2026 •

edited

Loading

dimaMachina Apr 7, 2026 •

edited

Loading

dimaMachina Apr 8, 2026 •

edited

Loading