diff --git a/compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Pipeline.ts b/compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Pipeline.ts index 89df46b095..6c8c394c88 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Pipeline.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Pipeline.ts @@ -224,11 +224,6 @@ function* runWithEnvironment( yield log({kind: 'hir', name: 'InstructionReordering', value: hir}); } - if (env.config.enableInlineSingleReturnJSX) { - inlineSingleReturnJSX(hir); - yield log({kind: 'hir', name: 'InlineSingleReturnJSX', value: hir}); - } - pruneMaybeThrows(hir); yield log({kind: 'hir', name: 'PruneMaybeThrows', value: hir}); @@ -332,6 +327,11 @@ function* runWithEnvironment( assertValidBlockNesting(hir); + if (env.config.enableInlineSingleReturnJSX) { + inlineSingleReturnJSX(hir); + yield log({kind: 'hir', name: 'InlineSingleReturnJSX', value: hir}); + } + flattenReactiveLoopsHIR(hir); yield log({ kind: 'hir', diff --git a/compiler/packages/babel-plugin-react-compiler/src/HIR/BuildReactiveScopeTerminalsHIR.ts b/compiler/packages/babel-plugin-react-compiler/src/HIR/BuildReactiveScopeTerminalsHIR.ts index 0999a3492b..4f3eda44a8 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/HIR/BuildReactiveScopeTerminalsHIR.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/HIR/BuildReactiveScopeTerminalsHIR.ts @@ -176,6 +176,12 @@ export function buildReactiveScopeTerminalsHIR(fn: HIRFunction): void { * Step 5: * Fix scope and identifier ranges to account for renumbered instructions */ + updateScopeRangesAfterRenumberingInstructions(fn); +} + +export function updateScopeRangesAfterRenumberingInstructions( + fn: HIRFunction, +): void { for (const [, block] of fn.body.blocks) { const terminal = block.terminal; if (terminal.kind === 'scope' || terminal.kind === 'pruned-scope') { diff --git a/compiler/packages/babel-plugin-react-compiler/src/Optimization/InlineSingleReturnJSX.ts b/compiler/packages/babel-plugin-react-compiler/src/Optimization/InlineSingleReturnJSX.ts index a74363927c..d395316799 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/Optimization/InlineSingleReturnJSX.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/Optimization/InlineSingleReturnJSX.ts @@ -5,73 +5,114 @@ * LICENSE file in the root directory of this source tree. */ +import {CompilerError} from '..'; import { BlockId, Effect, HIRFunction, Instruction, - isFunctionComponentType, makeInstructionId, ObjectProperty, Place, + ReactiveScope, SpreadPattern, } from '../HIR'; +import {updateScopeRangesAfterRenumberingInstructions} from '../HIR/BuildReactiveScopeTerminalsHIR'; import {createTemporaryPlace, markInstructionIds} from '../HIR/HIRBuilder'; import {BuiltInPropsId} from '../HIR/ObjectShape'; +import {retainWhere} from '../Utils/utils'; export function inlineSingleReturnJSX(fn: HIRFunction): void { if (fn.fnType !== 'Component') { // This optimization only applies to function components return; } - const returnValues: Array<{block: BlockId; place: Place}> = []; + const returnValues: Array = []; for (const [, block] of fn.body.blocks) { if (block.terminal.kind === 'return') { - returnValues.push({ - block: block.id, - place: block.terminal.value, - }); + returnValues.push(block.terminal.value); } } if (returnValues.length !== 1) { - // This optimization only applies if the component always returns - // the same element, multiple returns are rarely the same element. - // Note that in theory you could return the same tag+props+children - // in two different places, but that's pretty strange and not worth - // optimizing for. + /* + * This optimization only applies if the component always returns + * the same element, multiple returns are rarely the same element. + * Note that in theory you could return the same tag+props+children + * in two different places, but that's pretty strange and not worth + * optimizing for. + */ return; } const returnValue = returnValues[0]!; - const returnBlock = fn.body.blocks.get(returnValue.block)!; - let nextInstructions: Array | null = null; - for (let i = 0; i < returnBlock.instructions.length; i++) { - const instr = returnBlock.instructions[i]!; - if (instr.lvalue.identifier.id !== returnValue.place.identifier.id) { - if (nextInstructions !== null) { - nextInstructions.push(instr); + const scopes: Array<{start: BlockId; end: BlockId; scope: ReactiveScope}> = + []; + for (const [, block] of fn.body.blocks) { + retainWhere(scopes, scope => scope.end !== block.id); + let nextInstructions: Array | null = null; + for (let i = 0; i < block.instructions.length; i++) { + const instr = block.instructions[i]!; + if (instr.lvalue.identifier.id !== returnValue.identifier.id) { + if (nextInstructions !== null) { + nextInstructions.push(instr); + } + continue; } - continue; - } - const value = instr.value; + const value = instr.value; - if ( - value.kind !== 'JsxExpression' || - value.tag.kind !== 'Identifier' || - value.tag.reactive - // || isFunctionComponentType(value.tag.identifier) - ) { - return; - } + if ( + value.kind !== 'JsxExpression' || + value.tag.kind !== 'Identifier' || + value.tag.reactive || + value.props.some( + prop => + prop.kind === 'JsxAttribute' && + prop.name === 'key' && + prop.place.reactive, + ) || + scopes.length !== 1 + /* + * || + * scopes[0].scope.reassignments.size !== 0 || + * scopes[0].scope.declarations.size !== 1 || + * !Iterable_some( + * scopes[0].scope.declarations.values(), + * decl => decl.identifier.id === returnValue.identifier.id, + * ) || + * !isFunctionComponentType(value.tag.identifier) + */ + ) { + return; + } - nextInstructions ??= returnBlock.instructions.slice(0, i); - const propsTemp = createTemporaryPlace(fn.env, value.loc); - propsTemp.effect = Effect.Freeze; - propsTemp.identifier.type = {kind: 'Object', shapeId: BuiltInPropsId}; + const scopeStart = fn.body.blocks.get(scopes[0].start)!; + CompilerError.invariant( + scopeStart.terminal.kind === 'scope' || + scopeStart.terminal.kind === 'pruned-scope', + { + reason: 'wip', + loc: scopeStart.terminal.loc, + }, + ); + scopeStart.terminal = { + kind: 'label', + block: scopeStart.terminal.block, + fallthrough: scopeStart.terminal.fallthrough, + id: scopeStart.terminal.id, + loc: scopeStart.terminal.loc, + }; - const properties: Array = value.props.map( - attr => { + nextInstructions ??= block.instructions.slice(0, i); + const propsTemp = createTemporaryPlace(fn.env, value.loc); + propsTemp.effect = Effect.Freeze; + propsTemp.identifier.type = {kind: 'Object', shapeId: BuiltInPropsId}; + + const properties: Array = []; + value.props.forEach(attr => { if (attr.kind === 'JsxAttribute') { - return { + if (attr.name === 'key') { + return; + } + properties.push({ kind: 'ObjectProperty', key: { kind: 'identifier', @@ -79,76 +120,87 @@ export function inlineSingleReturnJSX(fn: HIRFunction): void { }, place: attr.place, type: 'property', - } satisfies ObjectProperty; + } satisfies ObjectProperty); } else { - return { + properties.push({ kind: 'Spread', place: attr.argument, - } satisfies SpreadPattern; + } satisfies SpreadPattern); } - }, - ); - if (value.children !== null) { - if (value.children.length === 1) { - properties.push({ - kind: 'ObjectProperty', - key: { - kind: 'identifier', - name: 'children', - }, - place: {...value.children[0]!}, - type: 'property', - }); - } else { - const childrenTemp = createTemporaryPlace(fn.env, value.loc); - childrenTemp.effect = Effect.Freeze; - nextInstructions.push({ - id: makeInstructionId(0), - lvalue: {...childrenTemp}, - value: { - kind: 'ArrayExpression', - elements: value.children, + }); + if (value.children !== null) { + if (value.children.length === 1) { + properties.push({ + kind: 'ObjectProperty', + key: { + kind: 'identifier', + name: 'children', + }, + place: {...value.children[0]!}, + type: 'property', + }); + } else { + const childrenTemp = createTemporaryPlace(fn.env, value.loc); + childrenTemp.effect = Effect.Freeze; + nextInstructions.push({ + id: makeInstructionId(0), + lvalue: {...childrenTemp}, + value: { + kind: 'ArrayExpression', + elements: value.children, + loc: value.loc, + }, loc: value.loc, - }, - loc: value.loc, - }); - properties.push({ - kind: 'ObjectProperty', - key: { - kind: 'identifier', - name: 'children', - }, - place: {...childrenTemp}, - type: 'property', - }); + }); + properties.push({ + kind: 'ObjectProperty', + key: { + kind: 'identifier', + name: 'children', + }, + place: {...childrenTemp}, + type: 'property', + }); + } } - } - nextInstructions.push({ - id: makeInstructionId(0), - lvalue: {...propsTemp}, - value: { - kind: 'ObjectExpression', - properties, + nextInstructions.push({ + id: makeInstructionId(0), + lvalue: {...propsTemp}, + value: { + kind: 'ObjectExpression', + properties, + loc: value.loc, + }, loc: value.loc, - }, - loc: value.loc, - }); - nextInstructions.push({ - id: makeInstructionId(0), - lvalue: instr.lvalue, - value: { - kind: 'CallExpression', - callee: {...value.tag}, - args: [{...propsTemp}], + }); + nextInstructions.push({ + id: makeInstructionId(0), + lvalue: instr.lvalue, + value: { + kind: 'CallExpression', + callee: {...value.tag}, + args: [{...propsTemp}], + loc: value.loc, + }, loc: value.loc, - }, - loc: value.loc, - }); - } - if (nextInstructions !== null) { - returnBlock.instructions = nextInstructions; + }); + } + if (nextInstructions !== null) { + block.instructions = nextInstructions; + } + if ( + block.terminal.kind === 'scope' || + block.terminal.kind === 'pruned-scope' + ) { + scopes.push({ + start: block.id, + end: block.terminal.fallthrough, + scope: block.terminal.scope, + }); + } } markInstructionIds(fn.body); + updateScopeRangesAfterRenumberingInstructions(fn); } diff --git a/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/InferReactiveScopeVariables.ts b/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/InferReactiveScopeVariables.ts index bfb23d23e1..a179b459f7 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/InferReactiveScopeVariables.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/InferReactiveScopeVariables.ts @@ -231,8 +231,10 @@ function mayAllocate(env: Environment, instruction: Instruction): boolean { case 'ObjectExpression': { const type = instruction.lvalue.identifier.type; if (type.kind === 'Object' && type.shapeId === BuiltInPropsId) { - // If this is an object literal for inlined JSX, we don't need to memoize - // it. The props object is always assumed to have changed. + /* + * If this is an object literal for inlined JSX, we don't need to memoize + * it. The props object is always assumed to have changed. + */ return false; } return true; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/inline-single-return-jsx/inline-single-return-jsx.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/inline-single-return-jsx/inline-single-return-jsx.expect.md index e8d4b411fe..92a34f8245 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/inline-single-return-jsx/inline-single-return-jsx.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/inline-single-return-jsx/inline-single-return-jsx.expect.md @@ -3,15 +3,9 @@ ```javascript // @enableInlineSingleReturnJSX -function Component({a, b}) { - return ( - -
{b}
- - ); -} function Child({value, children}) { + 'use no forget'; return (
{value} @@ -20,6 +14,14 @@ function Child({value, children}) { ); } +function Component({a, b}) { + return ( + +
{b}
+ + ); +} + export const FIXTURE_ENTRYPOINT = { fn: Component, params: [{a: 0, b: 0}], @@ -39,6 +41,17 @@ export const FIXTURE_ENTRYPOINT = { ```javascript import { c as _c } from "react/compiler-runtime"; // @enableInlineSingleReturnJSX + +function Child({ value, children }) { + "use no forget"; + return ( +
+ {value} + {children} +
+ ); +} + function Component(t0) { const $ = _c(2); const { a, b } = t0; @@ -53,34 +66,6 @@ function Component(t0) { return Child({ value: a, children: t1 }); } -function Child(t0) { - const $ = _c(5); - const { value, children } = t0; - let t1; - if ($[0] !== value) { - t1 = {value}; - $[0] = value; - $[1] = t1; - } else { - t1 = $[1]; - } - let t2; - if ($[2] !== t1 || $[3] !== children) { - t2 = ( -
- {t1} - {children} -
- ); - $[2] = t1; - $[3] = children; - $[4] = t2; - } else { - t2 = $[4]; - } - return t2; -} - export const FIXTURE_ENTRYPOINT = { fn: Component, params: [{ a: 0, b: 0 }], diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/inline-single-return-jsx/inline-single-return-jsx.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/inline-single-return-jsx/inline-single-return-jsx.js index cb9ec370e0..9df8f41e19 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/inline-single-return-jsx/inline-single-return-jsx.js +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/inline-single-return-jsx/inline-single-return-jsx.js @@ -1,13 +1,7 @@ // @enableInlineSingleReturnJSX -function Component({a, b}) { - return ( - -
{b}
- - ); -} function Child({value, children}) { + 'use no forget'; return (
{value} @@ -16,6 +10,14 @@ function Child({value, children}) { ); } +function Component({a, b}) { + return ( + +
{b}
+ + ); +} + export const FIXTURE_ENTRYPOINT = { fn: Component, params: [{a: 0, b: 0}], diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/inline-single-return-jsx/no-inline-single-return-jsx-reactive-key.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/inline-single-return-jsx/no-inline-single-return-jsx-reactive-key.expect.md new file mode 100644 index 0000000000..2ceb680d5f --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/inline-single-return-jsx/no-inline-single-return-jsx-reactive-key.expect.md @@ -0,0 +1,138 @@ + +## Input + +```javascript +// @enableInlineSingleReturnJSX +import {useEffect, useState} from 'react'; + +function Component({a, b}) { + return ; +} + +function Child({value, children}) { + const [state, setState] = useState(value); + useEffect(() => { + if (state === 0 && value === 0) { + setState(1); + } + }, [state]); + return ( +
+ {state} + {value} + {children} +
+ ); +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{a: 0, b: 0}], + sequentialRenders: [ + {a: 0, b: 0}, + {a: 1, b: 0}, + {a: 1, b: 1}, + {a: 0, b: 1}, + {a: 0, b: 0}, + {a: 1, b: 1}, + ], +}; + +``` + +## Code + +```javascript +import { c as _c } from "react/compiler-runtime"; // @enableInlineSingleReturnJSX +import { useEffect, useState } from "react"; + +function Component(t0) { + const $ = _c(3); + const { a, b } = t0; + let t1; + if ($[0] !== b || $[1] !== a) { + t1 = ; + $[0] = b; + $[1] = a; + $[2] = t1; + } else { + t1 = $[2]; + } + return t1; +} + +function Child(t0) { + const $ = _c(11); + const { value, children } = t0; + const [state, setState] = useState(value); + let t1; + if ($[0] !== state || $[1] !== value) { + t1 = () => { + if (state === 0 && value === 0) { + setState(1); + } + }; + $[0] = state; + $[1] = value; + $[2] = t1; + } else { + t1 = $[2]; + } + let t2; + if ($[3] !== state) { + t2 = [state]; + $[3] = state; + $[4] = t2; + } else { + t2 = $[4]; + } + useEffect(t1, t2); + let t3; + if ($[5] !== value) { + t3 = {value}; + $[5] = value; + $[6] = t3; + } else { + t3 = $[6]; + } + let t4; + if ($[7] !== state || $[8] !== t3 || $[9] !== children) { + t4 = ( +
+ {state} + {t3} + {children} +
+ ); + $[7] = state; + $[8] = t3; + $[9] = children; + $[10] = t4; + } else { + t4 = $[10]; + } + return t4; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{ a: 0, b: 0 }], + sequentialRenders: [ + { a: 0, b: 0 }, + { a: 1, b: 0 }, + { a: 1, b: 1 }, + { a: 0, b: 1 }, + { a: 0, b: 0 }, + { a: 1, b: 1 }, + ], +}; + +``` + +### Eval output +(kind: ok)
10
+
11
+
11
+
10
+
10
+
11
\ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/inline-single-return-jsx/no-inline-single-return-jsx-reactive-key.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/inline-single-return-jsx/no-inline-single-return-jsx-reactive-key.js new file mode 100644 index 0000000000..4b61a93129 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/inline-single-return-jsx/no-inline-single-return-jsx-reactive-key.js @@ -0,0 +1,35 @@ +// @enableInlineSingleReturnJSX +import {useEffect, useState} from 'react'; + +function Component({a, b}) { + return ; +} + +function Child({value, children}) { + const [state, setState] = useState(value); + useEffect(() => { + if (state === 0 && value === 0) { + setState(1); + } + }, [state]); + return ( +
+ {state} + {value} + {children} +
+ ); +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{a: 0, b: 0}], + sequentialRenders: [ + {a: 0, b: 0}, + {a: 1, b: 0}, + {a: 1, b: 1}, + {a: 0, b: 1}, + {a: 0, b: 0}, + {a: 1, b: 1}, + ], +}; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/inline-single-return-jsx/no-inline-single-return-jsx-reactive-tag.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/inline-single-return-jsx/no-inline-single-return-jsx-reactive-tag.expect.md new file mode 100644 index 0000000000..437402040c --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/inline-single-return-jsx/no-inline-single-return-jsx-reactive-tag.expect.md @@ -0,0 +1,173 @@ + +## Input + +```javascript +// @enableInlineSingleReturnJSX +import {useEffect, useState} from 'react'; + +function Component({a, b}) { + let Tag = a === 0 ? Child1 : Child2; + return ( + +
{b}
+ + ); +} + +function Child1(props) { + 'use no forget'; + return ; +} + +function Child2(props) { + 'use no forget'; + return ; +} + +function Child({value, children}) { + const [state, setState] = useState(value); + useEffect(() => { + if (state === 0 && value === 0) { + setState(1); + } + }, [state]); + return ( +
+ {state} + {value} + {children} +
+ ); +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{a: 0, b: 0}], + sequentialRenders: [ + {a: 0, b: 0}, + {a: 1, b: 0}, + {a: 1, b: 1}, + {a: 0, b: 1}, + {a: 0, b: 0}, + {a: 1, b: 1}, + ], +}; + +``` + +## Code + +```javascript +import { c as _c } from "react/compiler-runtime"; // @enableInlineSingleReturnJSX +import { useEffect, useState } from "react"; + +function Component(t0) { + const $ = _c(6); + const { a, b } = t0; + const Tag = a === 0 ? Child1 : Child2; + let t1; + if ($[0] !== b) { + t1 =
{b}
; + $[0] = b; + $[1] = t1; + } else { + t1 = $[1]; + } + let t2; + if ($[2] !== Tag || $[3] !== a || $[4] !== t1) { + t2 = {t1}; + $[2] = Tag; + $[3] = a; + $[4] = t1; + $[5] = t2; + } else { + t2 = $[5]; + } + return t2; +} + +function Child1(props) { + "use no forget"; + return ; +} + +function Child2(props) { + "use no forget"; + return ; +} + +function Child(t0) { + const $ = _c(11); + const { value, children } = t0; + const [state, setState] = useState(value); + let t1; + if ($[0] !== state || $[1] !== value) { + t1 = () => { + if (state === 0 && value === 0) { + setState(1); + } + }; + $[0] = state; + $[1] = value; + $[2] = t1; + } else { + t1 = $[2]; + } + let t2; + if ($[3] !== state) { + t2 = [state]; + $[3] = state; + $[4] = t2; + } else { + t2 = $[4]; + } + useEffect(t1, t2); + let t3; + if ($[5] !== value) { + t3 = {value}; + $[5] = value; + $[6] = t3; + } else { + t3 = $[6]; + } + let t4; + if ($[7] !== state || $[8] !== t3 || $[9] !== children) { + t4 = ( +
+ {state} + {t3} + {children} +
+ ); + $[7] = state; + $[8] = t3; + $[9] = children; + $[10] = t4; + } else { + t4 = $[10]; + } + return t4; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{ a: 0, b: 0 }], + sequentialRenders: [ + { a: 0, b: 0 }, + { a: 1, b: 0 }, + { a: 1, b: 1 }, + { a: 0, b: 1 }, + { a: 0, b: 0 }, + { a: 1, b: 1 }, + ], +}; + +``` + +### Eval output +(kind: ok)
10
0
+
11
0
+
11
1
+
10
1
+
10
0
+
11
1
\ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/inline-single-return-jsx/no-inline-single-return-jsx-reactive-tag.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/inline-single-return-jsx/no-inline-single-return-jsx-reactive-tag.js new file mode 100644 index 0000000000..1d0555af9d --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/inline-single-return-jsx/no-inline-single-return-jsx-reactive-tag.js @@ -0,0 +1,50 @@ +// @enableInlineSingleReturnJSX +import {useEffect, useState} from 'react'; + +function Component({a, b}) { + let Tag = a === 0 ? Child1 : Child2; + return ( + +
{b}
+ + ); +} + +function Child1(props) { + 'use no forget'; + return ; +} + +function Child2(props) { + 'use no forget'; + return ; +} + +function Child({value, children}) { + const [state, setState] = useState(value); + useEffect(() => { + if (state === 0 && value === 0) { + setState(1); + } + }, [state]); + return ( +
+ {state} + {value} + {children} +
+ ); +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{a: 0, b: 0}], + sequentialRenders: [ + {a: 0, b: 0}, + {a: 1, b: 0}, + {a: 1, b: 1}, + {a: 0, b: 1}, + {a: 0, b: 0}, + {a: 1, b: 1}, + ], +};