Skip to content

Commit f2f002c

Browse files
authored
[compiler][be] Stabilize compiler output: sort deps and decls by name (#31362)
All dependencies and declarations of a reactive scope can be reordered to scope start/end. i.e. generated code does not depend on conditional short-circuiting logic as dependencies are inferred to have no side effects. Sorting these by name helps us get higher signal compilation snapshot diffs when upgrading the compiler and testing PRs
1 parent 792fa06 commit f2f002c

File tree

154 files changed

+732
-685
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

154 files changed

+732
-685
lines changed

compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/CodegenReactiveFunction.ts

Lines changed: 49 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@ import {
3434
ReactiveInstruction,
3535
ReactiveScope,
3636
ReactiveScopeBlock,
37+
ReactiveScopeDeclaration,
3738
ReactiveScopeDependency,
3839
ReactiveTerminal,
3940
ReactiveValue,
@@ -572,7 +573,8 @@ function codegenReactiveScope(
572573
const changeExpressions: Array<t.Expression> = [];
573574
const changeExpressionComments: Array<string> = [];
574575
const outputComments: Array<string> = [];
575-
for (const dep of scope.dependencies) {
576+
577+
for (const dep of [...scope.dependencies].sort(compareScopeDependency)) {
576578
const index = cx.nextCacheIndex;
577579
changeExpressionComments.push(printDependencyComment(dep));
578580
const comparison = t.binaryExpression(
@@ -615,7 +617,10 @@ function codegenReactiveScope(
615617
);
616618
}
617619
let firstOutputIndex: number | null = null;
618-
for (const [, {identifier}] of scope.declarations) {
620+
621+
for (const [, {identifier}] of [...scope.declarations].sort(([, a], [, b]) =>
622+
compareScopeDeclaration(a, b),
623+
)) {
619624
const index = cx.nextCacheIndex;
620625
if (firstOutputIndex === null) {
621626
firstOutputIndex = index;
@@ -2566,3 +2571,45 @@ function convertIdentifier(identifier: Identifier): t.Identifier {
25662571
);
25672572
return t.identifier(identifier.name.value);
25682573
}
2574+
2575+
function compareScopeDependency(
2576+
a: ReactiveScopeDependency,
2577+
b: ReactiveScopeDependency,
2578+
): number {
2579+
CompilerError.invariant(
2580+
a.identifier.name?.kind === 'named' && b.identifier.name?.kind === 'named',
2581+
{
2582+
reason: '[Codegen] Expected named identifier for dependency',
2583+
loc: a.identifier.loc,
2584+
},
2585+
);
2586+
const aName = [
2587+
a.identifier.name.value,
2588+
...a.path.map(entry => `${entry.optional ? '?' : ''}${entry.property}`),
2589+
].join('.');
2590+
const bName = [
2591+
b.identifier.name.value,
2592+
...b.path.map(entry => `${entry.optional ? '?' : ''}${entry.property}`),
2593+
].join('.');
2594+
if (aName < bName) return -1;
2595+
else if (aName > bName) return 1;
2596+
else return 0;
2597+
}
2598+
2599+
function compareScopeDeclaration(
2600+
a: ReactiveScopeDeclaration,
2601+
b: ReactiveScopeDeclaration,
2602+
): number {
2603+
CompilerError.invariant(
2604+
a.identifier.name?.kind === 'named' && b.identifier.name?.kind === 'named',
2605+
{
2606+
reason: '[Codegen] Expected named identifier for declaration',
2607+
loc: a.identifier.loc,
2608+
},
2609+
);
2610+
const aName = a.identifier.name.value;
2611+
const bName = b.identifier.name.value;
2612+
if (aName < bName) return -1;
2613+
else if (aName > bName) return 1;
2614+
else return 0;
2615+
}

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/allocating-primitive-as-dep-nested-scope.expect.md

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@ import { identity, mutate, setProperty } from "shared-runtime";
4747
function AllocatingPrimitiveAsDepNested(props) {
4848
const $ = _c(5);
4949
let t0;
50-
if ($[0] !== props.b || $[1] !== props.a) {
50+
if ($[0] !== props.a || $[1] !== props.b) {
5151
const x = {};
5252
mutate(x);
5353
const t1 = identity(props.b) + 1;
@@ -62,8 +62,8 @@ function AllocatingPrimitiveAsDepNested(props) {
6262
const y = t2;
6363
setProperty(x, props.a);
6464
t0 = [x, y];
65-
$[0] = props.b;
66-
$[1] = props.a;
65+
$[0] = props.a;
66+
$[1] = props.b;
6767
$[2] = t0;
6868
} else {
6969
t0 = $[2];

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/array-at-effect.expect.md

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ function ArrayAtTest(props) {
4141
}
4242
const arr = t1;
4343
let t2;
44-
if ($[4] !== props.y || $[5] !== arr) {
44+
if ($[4] !== arr || $[5] !== props.y) {
4545
let t3;
4646
if ($[7] !== props.y) {
4747
t3 = bar(props.y);
@@ -51,8 +51,8 @@ function ArrayAtTest(props) {
5151
t3 = $[8];
5252
}
5353
t2 = arr.at(t3);
54-
$[4] = props.y;
55-
$[5] = arr;
54+
$[4] = arr;
55+
$[5] = props.y;
5656
$[6] = t2;
5757
} else {
5858
t2 = $[6];

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/array-expression-spread.expect.md

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -22,10 +22,10 @@ import { c as _c } from "react/compiler-runtime";
2222
function Component(props) {
2323
const $ = _c(3);
2424
let t0;
25-
if ($[0] !== props.foo || $[1] !== props.bar) {
25+
if ($[0] !== props.bar || $[1] !== props.foo) {
2626
t0 = [0, ...props.foo, null, ...props.bar, "z"];
27-
$[0] = props.foo;
28-
$[1] = props.bar;
27+
$[0] = props.bar;
28+
$[1] = props.foo;
2929
$[2] = t0;
3030
} else {
3131
t0 = $[2];

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/array-property-call.expect.md

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -24,18 +24,18 @@ export const FIXTURE_ENTRYPOINT = {
2424
import { c as _c } from "react/compiler-runtime";
2525
function Component(props) {
2626
const $ = _c(11);
27-
let t0;
2827
let a;
28+
let t0;
2929
if ($[0] !== props.a || $[1] !== props.b) {
3030
a = [props.a, props.b, "hello"];
3131
t0 = a.push(42);
3232
$[0] = props.a;
3333
$[1] = props.b;
34-
$[2] = t0;
35-
$[3] = a;
34+
$[2] = a;
35+
$[3] = t0;
3636
} else {
37-
t0 = $[2];
38-
a = $[3];
37+
a = $[2];
38+
t0 = $[3];
3939
}
4040
const x = t0;
4141
let t1;

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-invalid-phi-as-dependency.expect.md

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -70,10 +70,10 @@ function Component() {
7070
throw new Error("invariant broken");
7171
}
7272
let t1;
73-
if ($[1] !== obj || $[2] !== boxedInner) {
73+
if ($[1] !== boxedInner || $[2] !== obj) {
7474
t1 = <Stringify obj={obj} inner={boxedInner} />;
75-
$[1] = obj;
76-
$[2] = boxedInner;
75+
$[1] = boxedInner;
76+
$[2] = obj;
7777
$[3] = t1;
7878
} else {
7979
t1 = $[3];

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-object-expression-computed-key-modified-during-after-construction-hoisted-sequence-expr.expect.md

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -57,16 +57,16 @@ import { identity, mutate } from "shared-runtime";
5757
*/
5858
function Component(props) {
5959
const $ = _c(8);
60-
let t0;
6160
let key;
61+
let t0;
6262
if ($[0] === Symbol.for("react.memo_cache_sentinel")) {
6363
key = {};
6464
t0 = (mutate(key), key);
65-
$[0] = t0;
66-
$[1] = key;
65+
$[0] = key;
66+
$[1] = t0;
6767
} else {
68-
t0 = $[0];
69-
key = $[1];
68+
key = $[0];
69+
t0 = $[1];
7070
}
7171
const tmp = t0;
7272
let t1;

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/capturing-fun-alias-captured-mutate-2-iife.expect.md

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ import { mutate } from "shared-runtime";
3232
function component(foo, bar) {
3333
const $ = _c(3);
3434
let x;
35-
if ($[0] !== foo || $[1] !== bar) {
35+
if ($[0] !== bar || $[1] !== foo) {
3636
x = { foo };
3737
const y = { bar };
3838

@@ -41,8 +41,8 @@ function component(foo, bar) {
4141
a.x = b;
4242

4343
mutate(y);
44-
$[0] = foo;
45-
$[1] = bar;
44+
$[0] = bar;
45+
$[1] = foo;
4646
$[2] = x;
4747
} else {
4848
x = $[2];

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/capturing-fun-alias-captured-mutate-2.expect.md

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ import { c as _c } from "react/compiler-runtime";
2424
function component(foo, bar) {
2525
const $ = _c(3);
2626
let x;
27-
if ($[0] !== foo || $[1] !== bar) {
27+
if ($[0] !== bar || $[1] !== foo) {
2828
x = { foo };
2929
const y = { bar };
3030
const f0 = function () {
@@ -35,8 +35,8 @@ function component(foo, bar) {
3535

3636
f0();
3737
mutate(y);
38-
$[0] = foo;
39-
$[1] = bar;
38+
$[0] = bar;
39+
$[1] = foo;
4040
$[2] = x;
4141
} else {
4242
x = $[2];

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/capturing-fun-alias-captured-mutate-arr-2-iife.expect.md

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ const { mutate } = require("shared-runtime");
3232
function component(foo, bar) {
3333
const $ = _c(3);
3434
let x;
35-
if ($[0] !== foo || $[1] !== bar) {
35+
if ($[0] !== bar || $[1] !== foo) {
3636
x = { foo };
3737
const y = { bar };
3838

@@ -41,8 +41,8 @@ function component(foo, bar) {
4141
a.x = b;
4242

4343
mutate(y);
44-
$[0] = foo;
45-
$[1] = bar;
44+
$[0] = bar;
45+
$[1] = foo;
4646
$[2] = x;
4747
} else {
4848
x = $[2];

0 commit comments

Comments
 (0)