From 61941a9420be08fd9e9b3b53c06099e15fd8863f Mon Sep 17 00:00:00 2001 From: Jack Pope Date: Tue, 3 Sep 2024 12:08:23 -0400 Subject: [PATCH] Pass through unmodified props spread --- .../src/Optimization/InlineJsxTransform.ts | 63 ++++++++---- .../compiler/inline-jsx-transform.expect.md | 97 ++++++++++++------- .../fixtures/compiler/inline-jsx-transform.js | 11 ++- 3 files changed, 115 insertions(+), 56 deletions(-) diff --git a/compiler/packages/babel-plugin-react-compiler/src/Optimization/InlineJsxTransform.ts b/compiler/packages/babel-plugin-react-compiler/src/Optimization/InlineJsxTransform.ts index 344159d0b6072..0fe516da977e3 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/Optimization/InlineJsxTransform.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/Optimization/InlineJsxTransform.ts @@ -23,6 +23,7 @@ import { markPredecessors, reversePostorderBlocks, } from '../HIR/HIRBuilder'; +import {CompilerError} from '..'; function createSymbolProperty( fn: HIRFunction, @@ -151,6 +152,16 @@ function createPropsProperties( let refProperty: ObjectProperty | undefined; let keyProperty: ObjectProperty | undefined; const props: Array = []; + const jsxAttributesWithoutKeyAndRef = propAttributes.filter( + p => p.kind === 'JsxAttribute' && p.name !== 'key' && p.name !== 'ref', + ); + const jsxSpreadAttributes = propAttributes.filter( + p => p.kind === 'JsxSpreadAttribute', + ); + const spreadPropsOnly = + jsxAttributesWithoutKeyAndRef.length === 0 && + jsxSpreadAttributes.length === 1; + propAttributes.forEach(prop => { switch (prop.kind) { case 'JsxAttribute': { @@ -180,7 +191,6 @@ function createPropsProperties( break; } case 'JsxSpreadAttribute': { - // TODO: Optimize spreads to pass object directly if none of its properties are mutated props.push({ kind: 'Spread', place: {...prop.argument}, @@ -189,6 +199,7 @@ function createPropsProperties( } } }); + const propsPropertyPlace = createTemporaryPlace(fn.env, instr.value.loc); if (children) { let childrenPropProperty: ObjectProperty; @@ -268,23 +279,39 @@ function createPropsProperties( nextInstructions.push(keyInstruction); } - const propsInstruction: Instruction = { - id: makeInstructionId(0), - lvalue: {...propsPropertyPlace, effect: Effect.Mutate}, - value: { - kind: 'ObjectExpression', - properties: props, - loc: instr.value.loc, - }, - loc: instr.loc, - }; - const propsProperty: ObjectProperty = { - kind: 'ObjectProperty', - key: {name: 'props', kind: 'string'}, - type: 'property', - place: {...propsPropertyPlace, effect: Effect.Capture}, - }; - nextInstructions.push(propsInstruction); + let propsProperty: ObjectProperty; + if (spreadPropsOnly) { + const spreadProp = jsxSpreadAttributes[0]; + CompilerError.invariant(spreadProp.kind === 'JsxSpreadAttribute', { + reason: 'Spread prop attribute must be of kind JSXSpreadAttribute', + loc: instr.loc, + }); + propsProperty = { + kind: 'ObjectProperty', + key: {name: 'props', kind: 'string'}, + type: 'property', + place: {...spreadProp.argument, effect: Effect.Mutate}, + }; + } else { + const propsInstruction: Instruction = { + id: makeInstructionId(0), + lvalue: {...propsPropertyPlace, effect: Effect.Mutate}, + value: { + kind: 'ObjectExpression', + properties: props, + loc: instr.value.loc, + }, + loc: instr.loc, + }; + propsProperty = { + kind: 'ObjectProperty', + key: {name: 'props', kind: 'string'}, + type: 'property', + place: {...propsPropertyPlace, effect: Effect.Capture}, + }; + nextInstructions.push(propsInstruction); + } + return {refProperty, keyProperty, propsProperty}; } diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/inline-jsx-transform.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/inline-jsx-transform.expect.md index f399317925c64..6dd899d5c7408 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/inline-jsx-transform.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/inline-jsx-transform.expect.md @@ -28,9 +28,9 @@ function ParentAndRefAndKey(props) { function ParentAndChildren(props) { return ( - + - + ); @@ -38,7 +38,12 @@ function ParentAndChildren(props) { const propsToSpread = {a: 'a', b: 'b', c: 'c'}; function PropsSpread() { - return ; + return ( + <> + + + + ); } export const FIXTURE_ENTRYPOINT = { @@ -146,54 +151,59 @@ function ParentAndRefAndKey(props) { } function ParentAndChildren(props) { - const $ = _c2(3); + const $ = _c2(7); let t0; - if ($[0] === Symbol.for("react.memo_cache_sentinel")) { + if ($[0] !== props) { t0 = { $$typeof: Symbol.for("react.transitional.element"), type: Child, ref: null, key: "a", - props: {}, + props: props, }; - $[0] = t0; + $[0] = props; + $[1] = t0; } else { - t0 = $[0]; + t0 = $[1]; } let t1; - if ($[1] !== props.foo) { + if ($[2] !== props) { t1 = { $$typeof: Symbol.for("react.transitional.element"), - type: Parent, + type: Child, ref: null, - key: null, + key: "b", props: { - children: [ - t0, - { - $$typeof: Symbol.for("react.transitional.element"), - type: Child, - ref: null, - key: "b", - props: { - children: { - $$typeof: Symbol.for("react.transitional.element"), - type: GrandChild, - ref: null, - key: null, - props: { className: props.foo }, - }, - }, - }, - ], + children: { + $$typeof: Symbol.for("react.transitional.element"), + type: GrandChild, + ref: null, + key: null, + props: { className: props.foo, ...props }, + }, }, }; - $[1] = props.foo; - $[2] = t1; + $[2] = props; + $[3] = t1; } else { - t1 = $[2]; + t1 = $[3]; } - return t1; + let t2; + if ($[4] !== t0 || $[5] !== t1) { + t2 = { + $$typeof: Symbol.for("react.transitional.element"), + type: Parent, + ref: null, + key: null, + props: { children: [t0, t1] }, + }; + $[4] = t0; + $[5] = t1; + $[6] = t2; + } else { + t2 = $[6]; + } + return t2; } const propsToSpread = { a: "a", b: "b", c: "c" }; @@ -203,10 +213,27 @@ function PropsSpread() { if ($[0] === Symbol.for("react.memo_cache_sentinel")) { t0 = { $$typeof: Symbol.for("react.transitional.element"), - type: Test, + type: Symbol.for("react.fragment"), ref: null, key: null, - props: { ...propsToSpread }, + props: { + children: [ + { + $$typeof: Symbol.for("react.transitional.element"), + type: Test, + ref: null, + key: null, + props: propsToSpread, + }, + { + $$typeof: Symbol.for("react.transitional.element"), + type: Test, + ref: null, + key: null, + props: { ...propsToSpread, a: "z" }, + }, + ], + }, }; $[0] = t0; } else { diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/inline-jsx-transform.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/inline-jsx-transform.js index 29acaf60d276f..4a9d53b6f4b96 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/inline-jsx-transform.js +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/inline-jsx-transform.js @@ -24,9 +24,9 @@ function ParentAndRefAndKey(props) { function ParentAndChildren(props) { return ( - + - + ); @@ -34,7 +34,12 @@ function ParentAndChildren(props) { const propsToSpread = {a: 'a', b: 'b', c: 'c'}; function PropsSpread() { - return ; + return ( + <> + + + + ); } export const FIXTURE_ENTRYPOINT = {