From 4161c5046544dc38491a5bbd63626183ac8ac49a Mon Sep 17 00:00:00 2001 From: Sathya Gunsasekaran Date: Mon, 28 Oct 2024 16:06:07 +0000 Subject: [PATCH 1/2] [compiler] Store original and new prop names Previously, we'd directly store the original attributes from the jsx expressions. But this isn't enough as we want to rename duplicate attributes. This PR refactors the prop collection logic to store both the original and new names for jsx attributes in the newly outlined jsx expression. For now, both the new and old names are the same. In the future, they will be different when we add support for outlining expressions with duplicate attribute names. [ghstack-poisoned] --- .../src/Optimization/OutlineJsx.ts | 76 ++++++++++++------- 1 file changed, 49 insertions(+), 27 deletions(-) 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 f10f97c425dd0..6efcc16538e06 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/Optimization/OutlineJsx.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/Optimization/OutlineJsx.ts @@ -210,10 +210,16 @@ function process( return {instrs: newInstrs, fn: outlinedFn}; } +type OutlinedJsxAttribute = { + originalName: string; + newName: string; + place: Place; +}; + function collectProps( instructions: Array, -): Array | null { - const attributes: Array = []; +): Array | null { + const attributes: Array = []; const jsxIds = new Set(instructions.map(i => i.lvalue.identifier.id)); const seen: Set = new Set(); for (const instr of instructions) { @@ -234,7 +240,11 @@ function collectProps( if (at.kind === 'JsxAttribute') { seen.add(at.name); - attributes.push(at); + attributes.push({ + originalName: at.name, + newName: at.name, + place: at.place, + }); } } @@ -252,9 +262,15 @@ function collectProps( function emitOutlinedJsx( env: Environment, instructions: Array, - props: Array, + outlinedProps: Array, outlinedTag: string, ): Array { + const props: Array = outlinedProps.map(p => ({ + kind: 'JsxAttribute', + name: p.newName, + place: p.place, + })); + const loadJsx: Instruction = { id: makeInstructionId(0), loc: GeneratedSource, @@ -290,7 +306,7 @@ function emitOutlinedJsx( function emitOutlinedFn( env: Environment, jsx: Array, - oldProps: Array, + oldProps: Array, globals: LoadGlobalMap, ): HIRFunction | null { const instructions: Array = []; @@ -299,9 +315,11 @@ function emitOutlinedFn( const propsObj: Place = createTemporaryPlace(env, GeneratedSource); promoteTemporary(propsObj.identifier); - const destructurePropsInstr = emitDestructureProps(env, propsObj, [ - ...oldToNewProps.values(), - ]); + const destructurePropsInstr = emitDestructureProps( + env, + propsObj, + oldToNewProps, + ); instructions.push(destructurePropsInstr); const updatedJsxInstructions = emitUpdatedJsx(jsx, oldToNewProps); @@ -368,7 +386,7 @@ function emitLoadGlobals( function emitUpdatedJsx( jsx: Array, - oldToNewProps: Map, + oldToNewProps: Map, ): Array { const newInstrs: Array = []; @@ -390,7 +408,8 @@ function emitUpdatedJsx( `Expected a new property for ${printIdentifier(prop.place.identifier)}`, ); newProps.push({ - ...prop, + kind: 'JsxAttribute', + name: newProp.originalName, place: newProp.place, }); } @@ -409,31 +428,21 @@ function emitUpdatedJsx( function createOldToNewPropsMapping( env: Environment, - oldProps: Array, -): Map { + oldProps: Array, +): Map { const oldToNewProps = new Map(); for (const oldProp of oldProps) { - invariant( - oldProp.kind === 'JsxAttribute', - `Expected only attributes but found ${oldProp.kind}`, - ); - // Do not read key prop in the outlined component - if (oldProp.name === 'key') { + if (oldProp.originalName === 'key') { continue; } - const newProp: ObjectProperty = { - kind: 'ObjectProperty', - key: { - kind: 'string', - name: oldProp.name, - }, - type: 'property', + const newProp: OutlinedJsxAttribute = { + ...oldProp, place: createTemporaryPlace(env, GeneratedSource), }; - newProp.place.identifier.name = makeIdentifierName(oldProp.name); + newProp.place.identifier.name = makeIdentifierName(oldProp.newName); oldToNewProps.set(oldProp.place.identifier.id, newProp); } @@ -443,8 +452,21 @@ function createOldToNewPropsMapping( function emitDestructureProps( env: Environment, propsObj: Place, - properties: Array, + oldToNewProps: Map, ): Instruction { + const properties: Array = []; + for (const [_, prop] of oldToNewProps) { + properties.push({ + kind: 'ObjectProperty', + key: { + kind: 'string', + name: prop.newName, + }, + type: 'property', + place: prop.place, + }); + } + const destructurePropsInstr: Instruction = { id: makeInstructionId(0), lvalue: createTemporaryPlace(env, GeneratedSource), From 65009020499933fccff3057414a47d3265ce65a0 Mon Sep 17 00:00:00 2001 From: Sathya Gunsasekaran Date: Mon, 28 Oct 2024 16:09:06 +0000 Subject: [PATCH 2/2] Update on "[compiler] Store original and new prop names" Previously, we'd directly store the original attributes from the jsx expressions. But this isn't enough as we want to rename duplicate attributes. This PR refactors the prop collection logic to store both the original and new names for jsx attributes in the newly outlined jsx expression. For now, both the new and old names are the same. In the future, they will be different when we add support for outlining expressions with duplicate attribute names. [ghstack-poisoned]