From 09197bb786344d2ede1286e7f3ec4e21b18a58f2 Mon Sep 17 00:00:00 2001 From: Sathya Gunasekaran Date: Wed, 6 Nov 2024 17:50:13 +0000 Subject: [PATCH] [compiler] Outline jsx with duplicate attributes (#31441) Previously, we would skip outlining jsx expressions that had duplicate jsx attributes as we would not rename them causing incorrect compilation. In this PR, we add outlining support for duplicate jsx attributes by renaming them. --- .../src/Optimization/OutlineJsx.ts | 18 +- ...jsx-outlining-dup-key-diff-value.expect.md | 166 ++++++++++++++++ .../jsx-outlining-dup-key-diff-value.js | 41 ++++ ...outlining-dupe-attr-after-rename.expect.md | 177 ++++++++++++++++++ .../jsx-outlining-dupe-attr-after-rename.js | 42 +++++ ...utlining-dupe-key-dupe-component.expect.md | 157 ++++++++++++++++ .../jsx-outlining-dupe-key-dupe-component.js | 37 ++++ ...=> jsx-outlining-duplicate-prop.expect.md} | 56 ++++-- ...rop.js => jsx-outlining-duplicate-prop.js} | 4 +- 9 files changed, 675 insertions(+), 23 deletions(-) create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/jsx-outlining-dup-key-diff-value.expect.md create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/jsx-outlining-dup-key-diff-value.js create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/jsx-outlining-dupe-attr-after-rename.expect.md create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/jsx-outlining-dupe-attr-after-rename.js create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/jsx-outlining-dupe-key-dupe-component.expect.md create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/jsx-outlining-dupe-key-dupe-component.js rename compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/{todo.jsx-outlining-duplicate-prop.expect.md => jsx-outlining-duplicate-prop.expect.md} (68%) rename compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/{todo.jsx-outlining-duplicate-prop.js => jsx-outlining-duplicate-prop.js} (94%) 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 6efcc16538..b5e92171c1 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/Optimization/OutlineJsx.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/Optimization/OutlineJsx.ts @@ -222,6 +222,8 @@ function collectProps( const attributes: Array = []; const jsxIds = new Set(instructions.map(i => i.lvalue.identifier.id)); const seen: Set = new Set(); + let id = 1; + for (const instr of instructions) { const {value} = instr; @@ -230,21 +232,17 @@ function collectProps( return null; } - /* - * TODO(gsn): Handle attributes that have same value across - * the outlined jsx instructions. - */ - if (seen.has(at.name)) { - return null; - } - if (at.kind === 'JsxAttribute') { - seen.add(at.name); + let newName = at.name; + while (seen.has(newName)) { + newName = `${at.name}${id++}`; + } attributes.push({ originalName: at.name, - newName: at.name, + newName, place: at.place, }); + seen.add(newName); } } 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 new file mode 100644 index 0000000000..ded6e67020 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/jsx-outlining-dup-key-diff-value.expect.md @@ -0,0 +1,166 @@ + +## Input + +```javascript +// @enableJsxOutlining +function Component({arr}) { + const x = useX(); + return ( + <> + {arr.map((i, id) => { + return ( + + + + + ); + })} + + ); +} +function Bar({x, children}) { + return ( + <> + {x} + {children} + + ); +} + +function Baz({i}) { + return i; +} + +function Foo({k}) { + return k; +} + +function useX() { + return 'x'; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{arr: ['foo', 'bar']}], +}; + +``` + +## Code + +```javascript +import { c as _c } from "react/compiler-runtime"; // @enableJsxOutlining +function Component(t0) { + const $ = _c(7); + const { arr } = t0; + const x = useX(); + let t1; + if ($[0] !== arr || $[1] !== x) { + let t2; + if ($[3] !== x) { + t2 = (i, id) => { + const T0 = _temp; + return ; + }; + $[3] = x; + $[4] = t2; + } else { + t2 = $[4]; + } + t1 = arr.map(t2); + $[0] = arr; + $[1] = x; + $[2] = t1; + } else { + t1 = $[2]; + } + let t2; + if ($[5] !== t1) { + t2 = <>{t1}; + $[5] = t1; + $[6] = t2; + } else { + t2 = $[6]; + } + return t2; +} +function _temp(t0) { + const $ = _c(8); + const { i: i, k: k, x: x } = t0; + let t1; + if ($[0] !== i) { + t1 = ; + $[0] = i; + $[1] = t1; + } else { + t1 = $[1]; + } + let t2; + if ($[2] !== k) { + t2 = ; + $[2] = k; + $[3] = t2; + } else { + t2 = $[3]; + } + let t3; + if ($[4] !== t1 || $[5] !== t2 || $[6] !== x) { + t3 = ( + + {t1} + {t2} + + ); + $[4] = t1; + $[5] = t2; + $[6] = x; + $[7] = t3; + } else { + t3 = $[7]; + } + return t3; +} + +function Bar(t0) { + const $ = _c(3); + const { x, children } = t0; + let t1; + if ($[0] !== children || $[1] !== x) { + t1 = ( + <> + {x} + {children} + + ); + $[0] = children; + $[1] = x; + $[2] = t1; + } else { + t1 = $[2]; + } + return t1; +} + +function Baz(t0) { + const { i } = t0; + return i; +} + +function Foo(t0) { + const { k } = t0; + return k; +} + +function useX() { + return "x"; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{ arr: ["foo", "bar"] }], +}; + +``` + +### Eval output +(kind: ok) xfooifoojxbaribarj \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/jsx-outlining-dup-key-diff-value.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/jsx-outlining-dup-key-diff-value.js new file mode 100644 index 0000000000..63086739a2 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/jsx-outlining-dup-key-diff-value.js @@ -0,0 +1,41 @@ +// @enableJsxOutlining +function Component({arr}) { + const x = useX(); + return ( + <> + {arr.map((i, id) => { + return ( + + + + + ); + })} + + ); +} +function Bar({x, children}) { + return ( + <> + {x} + {children} + + ); +} + +function Baz({i}) { + return i; +} + +function Foo({k}) { + return k; +} + +function useX() { + return 'x'; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{arr: ['foo', 'bar']}], +}; 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 new file mode 100644 index 0000000000..c95e23222e --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/jsx-outlining-dupe-attr-after-rename.expect.md @@ -0,0 +1,177 @@ + +## Input + +```javascript +// @enableJsxOutlining +function Component({arr}) { + const x = useX(); + return ( + <> + {arr.map((i, id) => { + return ( + + + + + + ); + })} + + ); +} +function Bar({x, children}) { + return ( + <> + {x} + {children} + + ); +} + +function Baz({k1}) { + return k1; +} + +function Foo({k}) { + return k; +} + +function useX() { + return 'x'; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{arr: ['foo', 'bar']}], +}; + +``` + +## Code + +```javascript +import { c as _c } from "react/compiler-runtime"; // @enableJsxOutlining +function Component(t0) { + const $ = _c(7); + const { arr } = t0; + const x = useX(); + let t1; + if ($[0] !== arr || $[1] !== x) { + let t2; + if ($[3] !== x) { + t2 = (i, id) => { + const T0 = _temp; + return ; + }; + $[3] = x; + $[4] = t2; + } else { + t2 = $[4]; + } + t1 = arr.map(t2); + $[0] = arr; + $[1] = x; + $[2] = t1; + } else { + t1 = $[2]; + } + let t2; + if ($[5] !== t1) { + t2 = <>{t1}; + $[5] = t1; + $[6] = t2; + } else { + t2 = $[6]; + } + return t2; +} +function _temp(t0) { + const $ = _c(11); + const { k: k, k1: k1, k12: k12, x: x } = t0; + let t1; + if ($[0] !== k) { + t1 = ; + $[0] = k; + $[1] = t1; + } else { + t1 = $[1]; + } + let t2; + if ($[2] !== k1) { + t2 = ; + $[2] = k1; + $[3] = t2; + } else { + t2 = $[3]; + } + let t3; + if ($[4] !== k12) { + t3 = ; + $[4] = k12; + $[5] = t3; + } else { + t3 = $[5]; + } + let t4; + if ($[6] !== t1 || $[7] !== t2 || $[8] !== t3 || $[9] !== x) { + t4 = ( + + {t1} + {t2} + {t3} + + ); + $[6] = t1; + $[7] = t2; + $[8] = t3; + $[9] = x; + $[10] = t4; + } else { + t4 = $[10]; + } + return t4; +} + +function Bar(t0) { + const $ = _c(3); + const { x, children } = t0; + let t1; + if ($[0] !== children || $[1] !== x) { + t1 = ( + <> + {x} + {children} + + ); + $[0] = children; + $[1] = x; + $[2] = t1; + } else { + t1 = $[2]; + } + return t1; +} + +function Baz(t0) { + const { k1 } = t0; + return k1; +} + +function Foo(t0) { + const { k } = t0; + return k; +} + +function useX() { + return "x"; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{ arr: ["foo", "bar"] }], +}; + +``` + +### Eval output +(kind: ok) xfooifoojfoojxbaribarjbarj \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/jsx-outlining-dupe-attr-after-rename.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/jsx-outlining-dupe-attr-after-rename.js new file mode 100644 index 0000000000..09a826492a --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/jsx-outlining-dupe-attr-after-rename.js @@ -0,0 +1,42 @@ +// @enableJsxOutlining +function Component({arr}) { + const x = useX(); + return ( + <> + {arr.map((i, id) => { + return ( + + + + + + ); + })} + + ); +} +function Bar({x, children}) { + return ( + <> + {x} + {children} + + ); +} + +function Baz({k1}) { + return k1; +} + +function Foo({k}) { + return k; +} + +function useX() { + return 'x'; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{arr: ['foo', 'bar']}], +}; 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 new file mode 100644 index 0000000000..a53d9d92aa --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/jsx-outlining-dupe-key-dupe-component.expect.md @@ -0,0 +1,157 @@ + +## Input + +```javascript +// @enableJsxOutlining +function Component({arr}) { + const x = useX(); + return ( + <> + {arr.map((i, id) => { + return ( + + + + + ); + })} + + ); +} +function Bar({x, children}) { + return ( + <> + {x} + {children} + + ); +} + +function Foo({k}) { + return k; +} + +function useX() { + return 'x'; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{arr: ['foo', 'bar']}], +}; + +``` + +## Code + +```javascript +import { c as _c } from "react/compiler-runtime"; // @enableJsxOutlining +function Component(t0) { + const $ = _c(7); + const { arr } = t0; + const x = useX(); + let t1; + if ($[0] !== arr || $[1] !== x) { + let t2; + if ($[3] !== x) { + t2 = (i, id) => { + const T0 = _temp; + return ; + }; + $[3] = x; + $[4] = t2; + } else { + t2 = $[4]; + } + t1 = arr.map(t2); + $[0] = arr; + $[1] = x; + $[2] = t1; + } else { + t1 = $[2]; + } + let t2; + if ($[5] !== t1) { + t2 = <>{t1}; + $[5] = t1; + $[6] = t2; + } else { + t2 = $[6]; + } + return t2; +} +function _temp(t0) { + const $ = _c(8); + const { k: k, k1: k1, x: x } = t0; + let t1; + if ($[0] !== k) { + t1 = ; + $[0] = k; + $[1] = t1; + } else { + t1 = $[1]; + } + let t2; + if ($[2] !== k1) { + t2 = ; + $[2] = k1; + $[3] = t2; + } else { + t2 = $[3]; + } + let t3; + if ($[4] !== t1 || $[5] !== t2 || $[6] !== x) { + t3 = ( + + {t1} + {t2} + + ); + $[4] = t1; + $[5] = t2; + $[6] = x; + $[7] = t3; + } else { + t3 = $[7]; + } + return t3; +} + +function Bar(t0) { + const $ = _c(3); + const { x, children } = t0; + let t1; + if ($[0] !== children || $[1] !== x) { + t1 = ( + <> + {x} + {children} + + ); + $[0] = children; + $[1] = x; + $[2] = t1; + } else { + t1 = $[2]; + } + return t1; +} + +function Foo(t0) { + const { k } = t0; + return k; +} + +function useX() { + return "x"; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{ arr: ["foo", "bar"] }], +}; + +``` + +### Eval output +(kind: ok) xfooifoojxbaribarj \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/jsx-outlining-dupe-key-dupe-component.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/jsx-outlining-dupe-key-dupe-component.js new file mode 100644 index 0000000000..aa7cb548ce --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/jsx-outlining-dupe-key-dupe-component.js @@ -0,0 +1,37 @@ +// @enableJsxOutlining +function Component({arr}) { + const x = useX(); + return ( + <> + {arr.map((i, id) => { + return ( + + + + + ); + })} + + ); +} +function Bar({x, children}) { + return ( + <> + {x} + {children} + + ); +} + +function Foo({k}) { + return k; +} + +function useX() { + return 'x'; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{arr: ['foo', 'bar']}], +}; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/todo.jsx-outlining-duplicate-prop.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/jsx-outlining-duplicate-prop.expect.md similarity index 68% rename from compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/todo.jsx-outlining-duplicate-prop.expect.md rename to compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/jsx-outlining-duplicate-prop.expect.md index c661094fdd..e6ba7dd7ee 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/todo.jsx-outlining-duplicate-prop.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/jsx-outlining-duplicate-prop.expect.md @@ -31,8 +31,8 @@ function Baz({i}) { return i; } -function Foo({k}) { - return k; +function Foo({i}) { + return i; } function useX() { @@ -58,12 +58,10 @@ function Component(t0) { if ($[0] !== arr || $[1] !== x) { let t2; if ($[3] !== x) { - t2 = (i, id) => ( - - - - - ); + t2 = (i, id) => { + const T0 = _temp; + return ; + }; $[3] = x; $[4] = t2; } else { @@ -86,6 +84,42 @@ function Component(t0) { } return t2; } +function _temp(t0) { + const $ = _c(8); + const { i: i, i1: i1, x: x } = t0; + let t1; + if ($[0] !== i) { + t1 = ; + $[0] = i; + $[1] = t1; + } else { + t1 = $[1]; + } + let t2; + if ($[2] !== i1) { + t2 = ; + $[2] = i1; + $[3] = t2; + } else { + t2 = $[3]; + } + let t3; + if ($[4] !== t1 || $[5] !== t2 || $[6] !== x) { + t3 = ( + + {t1} + {t2} + + ); + $[4] = t1; + $[5] = t2; + $[6] = x; + $[7] = t3; + } else { + t3 = $[7]; + } + return t3; +} function Bar(t0) { const $ = _c(3); @@ -113,8 +147,8 @@ function Baz(t0) { } function Foo(t0) { - const { k } = t0; - return k; + const { i } = t0; + return i; } function useX() { @@ -129,4 +163,4 @@ export const FIXTURE_ENTRYPOINT = { ``` ### Eval output -(kind: ok) xfooxbar \ No newline at end of file +(kind: ok) xfoofooxbarbar \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/todo.jsx-outlining-duplicate-prop.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/jsx-outlining-duplicate-prop.js similarity index 94% rename from compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/todo.jsx-outlining-duplicate-prop.js rename to compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/jsx-outlining-duplicate-prop.js index 20c6bd934a..0314ce8c67 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/todo.jsx-outlining-duplicate-prop.js +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/jsx-outlining-duplicate-prop.js @@ -27,8 +27,8 @@ function Baz({i}) { return i; } -function Foo({k}) { - return k; +function Foo({i}) { + return i; } function useX() {