diff --git a/compiler/packages/babel-plugin-react-compiler/src/Optimization/OutlineJsx.ts b/compiler/packages/babel-plugin-react-compiler/src/Optimization/OutlineJsx.ts index a38256896b1a..e6e2d5cf1228 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/Optimization/OutlineJsx.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/Optimization/OutlineJsx.ts @@ -6,6 +6,7 @@ */ import invariant from 'invariant'; +import {isValidIdentifier} from '@babel/types'; import {Environment} from '../HIR'; import { BasicBlock, @@ -215,6 +216,18 @@ type OutlinedJsxAttribute = { place: Place; }; +/** + * Returns true when the original JSX attribute name contains non-identifier + * characters (e.g. `aria-label`) but is still a valid JSX attribute name. + * These props should keep their original name in the outlined JSX call site + * and use a quoted key in the destructuring pattern. + */ +function isHyphenatedJsxProp(originalName: string): boolean { + return ( + !isValidIdentifier(originalName) && /^[a-zA-Z_][\w.-]*$/.test(originalName) + ); +} + function collectProps( env: Environment, instructions: Array, @@ -222,9 +235,19 @@ function collectProps( let id = 1; function generateName(oldName: string): string { - let newName = oldName; + // Sanitize names that aren't valid JS identifiers (e.g. "aria-label" -> "ariaLabel") + let baseName = oldName; + if (!isValidIdentifier(baseName)) { + baseName = baseName.replace(/[^a-zA-Z0-9$_]+(.)?/g, (_, char) => + char != null ? char.toUpperCase() : '', + ); + if (!isValidIdentifier(baseName)) { + baseName = `_${baseName}`; + } + } + let newName = baseName; while (seen.has(newName)) { - newName = `${oldName}${id++}`; + newName = `${baseName}${id++}`; } seen.add(newName); env.programContext.addNewReference(newName); @@ -280,7 +303,13 @@ function emitOutlinedJsx( ): Array { const props: Array = outlinedProps.map(p => ({ kind: 'JsxAttribute', - name: p.newName, + /* + * Use the original name when it's a valid JSX attribute name (e.g. `aria-label` + * is a valid JSX attr even though it's not a valid JS identifier). + * Fall back to newName for internal temp names (e.g. `#t16`) and for + * deduplicated props where originalName would conflict. + */ + name: isHyphenatedJsxProp(p.originalName) ? p.originalName : p.newName, place: p.place, })); @@ -494,12 +523,18 @@ function emitDestructureProps( ): Instruction { const properties: Array = []; for (const [_, prop] of oldToNewProps) { + /* + * When the original prop name is a valid identifier (e.g. `disabled`), + * use newName as the key (handles deduplication like `k` → `k1`). + * When the original prop name is NOT a valid identifier (e.g. `aria-label`), + * use originalName as the string key so we get `'aria-label': ariaLabel` + * instead of `ariaLabel: ariaLabel`. + */ properties.push({ kind: 'ObjectProperty', - key: { - kind: 'string', - name: prop.newName, - }, + key: isHyphenatedJsxProp(prop.originalName) + ? {kind: 'string', name: prop.originalName} + : {kind: 'identifier', name: prop.newName}, type: 'property', place: prop.place, }); diff --git a/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/CodegenReactiveFunction.ts b/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/CodegenReactiveFunction.ts index 486773d5eb91..70fea5b30134 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/CodegenReactiveFunction.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/CodegenReactiveFunction.ts @@ -1104,7 +1104,9 @@ function codegenInstructionNullable( }); CompilerError.invariant(value?.type === 'FunctionExpression', { reason: 'Expected a function as a function declaration value', - description: `Got ${value == null ? String(value) : value.type} at ${printInstruction(instr)}`, + description: `Got ${ + value == null ? String(value) : value.type + } at ${printInstruction(instr)}`, loc: instr.value.loc, }); return createFunctionDeclaration( @@ -1963,34 +1965,35 @@ function codegenInstructionValue( instruction, })), ).body; - const expressions = body.map(stmt => { + const expressions = body.flatMap(stmt => { if (stmt.type === 'ExpressionStatement') { - return stmt.expression; + return [stmt.expression]; + } 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'), + ); + } + }); } else { - if (t.isVariableDeclaration(stmt)) { - const declarator = stmt.declarations[0]; - cx.recordError( - new CompilerErrorDetail({ - reason: `(CodegenReactiveFunction::codegenInstructionValue) Cannot declare variables in a value block, tried to declare '${ - (declarator.id as t.Identifier).name - }'`, - category: ErrorCategory.Todo, - loc: declarator.loc ?? null, - suggestions: null, - }), - ); - return t.stringLiteral(`TODO handle ${declarator.id}`); - } else { - cx.recordError( - new CompilerErrorDetail({ - reason: `(CodegenReactiveFunction::codegenInstructionValue) Handle conversion of ${stmt.type} to expression`, - category: ErrorCategory.Todo, - loc: stmt.loc ?? null, - suggestions: null, - }), - ); - return t.stringLiteral(`TODO handle ${stmt.type}`); - } + cx.recordError( + new CompilerErrorDetail({ + reason: `(CodegenReactiveFunction::codegenInstructionValue) Handle conversion of ${stmt.type} to expression`, + category: ErrorCategory.Todo, + loc: stmt.loc ?? null, + suggestions: null, + }), + ); + return [t.stringLiteral(`TODO handle ${stmt.type}`)]; } }); if (expressions.length === 0) { diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/jsx-outlining-child-stored-in-id.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/jsx-outlining-child-stored-in-id.expect.md index 108c6725f7e8..49a255f4e5fa 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/jsx-outlining-child-stored-in-id.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/jsx-outlining-child-stored-in-id.expect.md @@ -74,7 +74,7 @@ function Component(t0) { } function _temp(t0) { const $ = _c(5); - const { i: i, x: x } = t0; + const { i, x } = t0; let t1; if ($[0] !== i) { t1 = ; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/jsx-outlining-dup-key-diff-value.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/jsx-outlining-dup-key-diff-value.expect.md index ded6e67020fa..e20053a98f76 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/jsx-outlining-dup-key-diff-value.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/jsx-outlining-dup-key-diff-value.expect.md @@ -86,7 +86,7 @@ function Component(t0) { } function _temp(t0) { const $ = _c(8); - const { i: i, k: k, x: x } = t0; + const { i, k, x } = t0; let t1; if ($[0] !== i) { t1 = ; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/jsx-outlining-dupe-attr-after-rename.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/jsx-outlining-dupe-attr-after-rename.expect.md index c95e23222ed7..0f41a57e1529 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/jsx-outlining-dupe-attr-after-rename.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/jsx-outlining-dupe-attr-after-rename.expect.md @@ -87,7 +87,7 @@ function Component(t0) { } function _temp(t0) { const $ = _c(11); - const { k: k, k1: k1, k12: k12, x: x } = t0; + const { k, k1, k12, x } = t0; let t1; if ($[0] !== k) { t1 = ; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/jsx-outlining-dupe-key-dupe-component.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/jsx-outlining-dupe-key-dupe-component.expect.md index a53d9d92aa67..2852bf90b26a 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/jsx-outlining-dupe-key-dupe-component.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/jsx-outlining-dupe-key-dupe-component.expect.md @@ -82,7 +82,7 @@ function Component(t0) { } function _temp(t0) { const $ = _c(8); - const { k: k, k1: k1, x: x } = t0; + const { k, k1, x } = t0; let t1; if ($[0] !== k) { t1 = ; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/jsx-outlining-duplicate-prop.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/jsx-outlining-duplicate-prop.expect.md index e6ba7dd7ee7c..b0c1e68de9e5 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/jsx-outlining-duplicate-prop.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/jsx-outlining-duplicate-prop.expect.md @@ -86,7 +86,7 @@ function Component(t0) { } function _temp(t0) { const $ = _c(8); - const { i: i, i1: i1, x: x } = t0; + const { i, i1, x } = t0; let t1; if ($[0] !== i) { t1 = ; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/jsx-outlining-jsx-stored-in-id.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/jsx-outlining-jsx-stored-in-id.expect.md index 284491e6fb6f..76e32af1434c 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/jsx-outlining-jsx-stored-in-id.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/jsx-outlining-jsx-stored-in-id.expect.md @@ -85,7 +85,7 @@ function Component(t0) { } function _temp(t0) { const $ = _c(5); - const { i: i, x: x } = t0; + const { i, x } = t0; let t1; if ($[0] !== i) { t1 = ; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/jsx-outlining-separate-nested.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/jsx-outlining-separate-nested.expect.md index 9d2b254c063d..659acfe6c90b 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/jsx-outlining-separate-nested.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/jsx-outlining-separate-nested.expect.md @@ -91,7 +91,7 @@ function Component(t0) { } function _temp(t0) { const $ = _c(11); - const { i: i, j: j, k: k, x: x } = t0; + const { i, j, k, x } = t0; let t1; if ($[0] !== i) { t1 = ; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/jsx-outlining-simple.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/jsx-outlining-simple.expect.md index 09323f5ac65e..d8d396f2b2e5 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/jsx-outlining-simple.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/jsx-outlining-simple.expect.md @@ -81,7 +81,7 @@ function Component(t0) { } function _temp(t0) { const $ = _c(5); - const { i: i, x: x } = t0; + const { i, x } = t0; let t1; if ($[0] !== i) { t1 = ; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/jsx-outlining-variable-declaration-in-sequence-expression.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/jsx-outlining-variable-declaration-in-sequence-expression.expect.md new file mode 100644 index 000000000000..d74ebe3f4b80 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/jsx-outlining-variable-declaration-in-sequence-expression.expect.md @@ -0,0 +1,71 @@ + +## Input + +```javascript +// @enableJsxOutlining +function Component() { + const [isSubmitting] = useState(false); + + return ssoProviders.map(provider => { + return ( +
+ +
+ ); + }); +} + +``` + +## Code + +```javascript +import { c as _c } from "react/compiler-runtime"; // @enableJsxOutlining +function Component() { + const $ = _c(2); + const [isSubmitting] = useState(false); + let t0; + if ($[0] !== isSubmitting) { + t0 = ssoProviders.map((provider) => { + const T0 = _temp; + return ( + + ); + }); + $[0] = isSubmitting; + $[1] = t0; + } else { + t0 = $[1]; + } + return t0; +} +function _temp(t0) { + const $ = _c(3); + const { disabled, "aria-label": ariaLabel } = t0; + let t1; + if ($[0] !== ariaLabel || $[1] !== disabled) { + t1 = ( +
+ +
+ ); + $[0] = ariaLabel; + $[1] = disabled; + $[2] = t1; + } else { + t1 = $[2]; + } + return t1; +} + +``` + +### Eval output +(kind: exception) Fixture not implemented \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/jsx-outlining-variable-declaration-in-sequence-expression.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/jsx-outlining-variable-declaration-in-sequence-expression.js new file mode 100644 index 000000000000..b49b55fb2892 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/jsx-outlining-variable-declaration-in-sequence-expression.js @@ -0,0 +1,15 @@ +// @enableJsxOutlining +function Component() { + const [isSubmitting] = useState(false); + + return ssoProviders.map(provider => { + return ( +
+ +
+ ); + }); +} diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/jsx-outlining-with-non-jsx-children.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/jsx-outlining-with-non-jsx-children.expect.md index 5408ea83a831..d8178ab783ec 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/jsx-outlining-with-non-jsx-children.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/jsx-outlining-with-non-jsx-children.expect.md @@ -93,7 +93,7 @@ function Component(t0) { } function _temp(t0) { const $ = _c(9); - const { i: i, t: t, k: k, x: x } = t0; + const { i, t, k, x } = t0; let t1; if ($[0] !== i || $[1] !== t) { t1 = {t};