Skip to content

Commit 522d22f

Browse files
mvitousekmvitousek
committed
[compiler] Recompute values every time
Summary: This PR expands the analysis from the previous in the stack in order to also capture when a value can incorrectly change within a single render, rather than just changing between two renders. In the case where dependencies have changed and so a new value is being computed, we now compute the value twice and compare the results. This would, for example, catch when we call Math.random() in render. The generated code is a little convoluted, because we don't want to have to traverse the generated code and substitute variable names with new ones. Instead, we save the initial value to the cache as normal, then run the computation block again and compare the resulting values to the cached ones. Then, to make sure that the cached values are identical to the computed ones, we reassign the cached values into the output variables. ghstack-source-id: d0f11a4 Pull Request resolved: #29657
1 parent c69211a commit 522d22f

File tree

8 files changed

+215
-123
lines changed

8 files changed

+215
-123
lines changed

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

Lines changed: 104 additions & 96 deletions
Original file line numberDiff line numberDiff line change
@@ -461,6 +461,11 @@ function codegenReactiveScope(
461461
): void {
462462
const cacheStoreStatements: Array<t.Statement> = [];
463463
const cacheLoadStatements: Array<t.Statement> = [];
464+
const cacheLoads: Array<{
465+
name: t.Identifier;
466+
index: number;
467+
value: t.Expression;
468+
}> = [];
464469
const changeExpressions: Array<t.Expression> = [];
465470
const changeExpressionComments: Array<string> = [];
466471
const outputComments: Array<string> = [];
@@ -488,6 +493,10 @@ function codegenReactiveScope(
488493
} else {
489494
changeExpressions.push(comparison);
490495
}
496+
/*
497+
* Adding directly to cacheStoreStatements rather than cacheLoads, because there
498+
* is no corresponding cacheLoadStatement for dependencies
499+
*/
491500
cacheStoreStatements.push(
492501
t.expressionStatement(
493502
t.assignmentExpression(
@@ -523,32 +532,7 @@ function codegenReactiveScope(
523532
t.variableDeclaration("let", [t.variableDeclarator(name)])
524533
);
525534
}
526-
cacheStoreStatements.push(
527-
t.expressionStatement(
528-
t.assignmentExpression(
529-
"=",
530-
t.memberExpression(
531-
t.identifier(cx.synthesizeName("$")),
532-
t.numericLiteral(index),
533-
true
534-
),
535-
wrapCacheDep(cx, name)
536-
)
537-
)
538-
);
539-
cacheLoadStatements.push(
540-
t.expressionStatement(
541-
t.assignmentExpression(
542-
"=",
543-
name,
544-
t.memberExpression(
545-
t.identifier(cx.synthesizeName("$")),
546-
t.numericLiteral(index),
547-
true
548-
)
549-
)
550-
)
551-
);
535+
cacheLoads.push({ name, index, value: wrapCacheDep(cx, name) });
552536
cx.declare(identifier);
553537
}
554538
for (const reassignment of scope.reassignments) {
@@ -558,34 +542,9 @@ function codegenReactiveScope(
558542
}
559543
const name = convertIdentifier(reassignment);
560544
outputComments.push(name.name);
561-
562-
cacheStoreStatements.push(
563-
t.expressionStatement(
564-
t.assignmentExpression(
565-
"=",
566-
t.memberExpression(
567-
t.identifier(cx.synthesizeName("$")),
568-
t.numericLiteral(index),
569-
true
570-
),
571-
wrapCacheDep(cx, name)
572-
)
573-
)
574-
);
575-
cacheLoadStatements.push(
576-
t.expressionStatement(
577-
t.assignmentExpression(
578-
"=",
579-
name,
580-
t.memberExpression(
581-
t.identifier(cx.synthesizeName("$")),
582-
t.numericLiteral(index),
583-
true
584-
)
585-
)
586-
)
587-
);
545+
cacheLoads.push({ name, index, value: wrapCacheDep(cx, name) });
588546
}
547+
589548
let testCondition = (changeExpressions as Array<t.Expression>).reduce(
590549
(acc: t.Expression | null, ident: t.Expression) => {
591550
if (acc == null) {
@@ -632,67 +591,116 @@ function codegenReactiveScope(
632591
);
633592
}
634593
let computationBlock = codegenBlock(cx, block);
594+
635595
let memoStatement;
636-
const memoBlock = t.blockStatement(cacheLoadStatements);
637596
if (
638597
cx.env.config.enableChangeDetectionForDebugging != null &&
639598
changeExpressions.length > 0
640599
) {
641600
const detectionFunction =
642601
cx.env.config.enableChangeDetectionForDebugging.importSpecifierName;
602+
const cacheLoadOldValueStatements: Array<t.Statement> = [];
643603
const changeDetectionStatements: Array<t.Statement> = [];
644-
const oldVarDeclarationStatements: Array<t.Statement> = [];
645-
memoBlock.body.forEach((stmt) => {
646-
if (
647-
stmt.type === "ExpressionStatement" &&
648-
stmt.expression.type === "AssignmentExpression" &&
649-
stmt.expression.left.type === "Identifier"
650-
) {
651-
const name = stmt.expression.left.name;
652-
const loadName = cx.synthesizeName(`old$${name}`);
653-
oldVarDeclarationStatements.push(
654-
t.variableDeclaration("let", [
655-
t.variableDeclarator(t.identifier(loadName)),
604+
const idempotenceDetectionStatements: Array<t.Statement> = [];
605+
606+
for (const { name, index, value } of cacheLoads) {
607+
const loadName = cx.synthesizeName(`old$${name.name}`);
608+
const slot = t.memberExpression(
609+
t.identifier(cx.synthesizeName("$")),
610+
t.numericLiteral(index),
611+
true
612+
);
613+
cacheStoreStatements.push(
614+
t.expressionStatement(t.assignmentExpression("=", slot, value))
615+
);
616+
cacheLoadOldValueStatements.push(
617+
t.variableDeclaration("let", [
618+
t.variableDeclarator(t.identifier(loadName), slot),
619+
])
620+
);
621+
changeDetectionStatements.push(
622+
t.expressionStatement(
623+
t.callExpression(t.identifier(detectionFunction), [
624+
t.identifier(loadName),
625+
name,
626+
t.stringLiteral(name.name),
627+
t.stringLiteral(cx.fnName),
628+
t.stringLiteral("cached"),
656629
])
657-
);
658-
stmt.expression.left = t.identifier(loadName);
659-
changeDetectionStatements.push(
660-
t.expressionStatement(
661-
t.callExpression(t.identifier(detectionFunction), [
662-
t.identifier(loadName),
663-
t.identifier(name),
664-
t.stringLiteral(name),
665-
t.stringLiteral(cx.fnName),
666-
])
667-
)
668-
);
669-
changeDetectionStatements.push(
670-
t.expressionStatement(
671-
t.assignmentExpression(
672-
"=",
673-
t.identifier(name),
674-
t.identifier(loadName)
675-
)
676-
)
677-
);
678-
}
679-
});
630+
)
631+
);
632+
idempotenceDetectionStatements.push(
633+
t.expressionStatement(
634+
t.callExpression(t.identifier(detectionFunction), [
635+
slot,
636+
name,
637+
t.stringLiteral(name.name),
638+
t.stringLiteral(cx.fnName),
639+
t.stringLiteral("recomputed"),
640+
])
641+
)
642+
);
643+
idempotenceDetectionStatements.push(
644+
t.expressionStatement(t.assignmentExpression("=", name, slot))
645+
);
646+
}
647+
const condition = cx.synthesizeName("condition");
680648
memoStatement = t.blockStatement([
681649
...computationBlock.body,
650+
t.variableDeclaration("let", [
651+
t.variableDeclarator(t.identifier(condition), testCondition),
652+
]),
682653
t.ifStatement(
683-
t.unaryExpression("!", testCondition),
654+
t.unaryExpression("!", t.identifier(condition)),
684655
t.blockStatement([
685-
...oldVarDeclarationStatements,
686-
...memoBlock.body,
656+
...cacheLoadOldValueStatements,
687657
...changeDetectionStatements,
688658
])
689659
),
690660
...cacheStoreStatements,
661+
t.ifStatement(
662+
t.identifier(condition),
663+
t.blockStatement([
664+
...computationBlock.body,
665+
...idempotenceDetectionStatements,
666+
])
667+
),
691668
]);
692669
} else {
670+
for (const { name, index, value } of cacheLoads) {
671+
cacheStoreStatements.push(
672+
t.expressionStatement(
673+
t.assignmentExpression(
674+
"=",
675+
t.memberExpression(
676+
t.identifier(cx.synthesizeName("$")),
677+
t.numericLiteral(index),
678+
true
679+
),
680+
value
681+
)
682+
)
683+
);
684+
cacheLoadStatements.push(
685+
t.expressionStatement(
686+
t.assignmentExpression(
687+
"=",
688+
name,
689+
t.memberExpression(
690+
t.identifier(cx.synthesizeName("$")),
691+
t.numericLiteral(index),
692+
true
693+
)
694+
)
695+
)
696+
);
697+
}
693698
computationBlock.body.push(...cacheStoreStatements);
694-
695-
memoStatement = t.ifStatement(testCondition, computationBlock, memoBlock);
699+
memoStatement = t.ifStatement(
700+
testCondition,
701+
computationBlock,
702+
t.blockStatement(cacheLoadStatements)
703+
);
696704
}
697705

698706
if (cx.env.config.enableMemoizationComments) {
@@ -734,9 +742,9 @@ function codegenReactiveScope(
734742
true
735743
);
736744
}
737-
if (memoBlock.body.length > 0) {
745+
if (cacheLoadStatements.length > 0) {
738746
t.addComment(
739-
memoBlock.body[0]!,
747+
cacheLoadStatements[0]!,
740748
"leading",
741749
` Inputs did not change, use cached value`,
742750
true

‎compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/change-detect-reassign.expect.md‎

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
1+
2+
## Input
3+
4+
```javascript
5+
// @enableChangeDetectionForDebugging
6+
function Component(props) {
7+
let x = null;
8+
if (props.cond) {
9+
x = [];
10+
x.push(props.value);
11+
}
12+
return x;
13+
}
14+
15+
```
16+
17+
## Code
18+
19+
```javascript
20+
import { $structuralCheck } from "react-compiler-runtime";
21+
import { c as _c } from "react/compiler-runtime"; // @enableChangeDetectionForDebugging
22+
function Component(props) {
23+
const $ = _c(2);
24+
let x = null;
25+
if (props.cond) {
26+
{
27+
x = [];
28+
x.push(props.value);
29+
let condition = $[0] !== props.value;
30+
if (!condition) {
31+
let old$x = $[1];
32+
$structuralCheck(old$x, x, "x", "Component", "cached");
33+
}
34+
$[0] = props.value;
35+
$[1] = x;
36+
if (condition) {
37+
x = [];
38+
x.push(props.value);
39+
$structuralCheck($[1], x, "x", "Component", "recomputed");
40+
x = $[1];
41+
}
42+
}
43+
}
44+
return x;
45+
}
46+
47+
```
48+

‎compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/change-detect-reassign.js‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
// @enableChangeDetectionForDebugging
2+
function Component(props) {
3+
let x = null;
4+
if (props.cond) {
5+
x = [];
6+
x.push(props.value);
7+
}
8+
return x;
9+
}

‎compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/useState-and-other-hook-unpruned-dependency.expect.md‎

Lines changed: 18 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -43,29 +43,37 @@ function Component(props) {
4343
let t0;
4444
{
4545
t0 = f(props.x);
46-
if (!($[0] !== props.x)) {
47-
let old$t0;
48-
old$t0 = $[1];
49-
$structuralCheck(old$t0, t0, "t0", "Component");
50-
t0 = old$t0;
46+
let condition = $[0] !== props.x;
47+
if (!condition) {
48+
let old$t0 = $[1];
49+
$structuralCheck(old$t0, t0, "t0", "Component", "cached");
5150
}
5251
$[0] = props.x;
5352
$[1] = t0;
53+
if (condition) {
54+
t0 = f(props.x);
55+
$structuralCheck($[1], t0, "t0", "Component", "recomputed");
56+
t0 = $[1];
57+
}
5458
}
5559
const w = t0;
5660
const z = useOther(w);
5761
const [x] = useState(z);
5862
let t1;
5963
{
6064
t1 = <div>{x}</div>;
61-
if (!($[2] !== x)) {
62-
let old$t1;
63-
old$t1 = $[3];
64-
$structuralCheck(old$t1, t1, "t1", "Component");
65-
t1 = old$t1;
65+
let condition = $[2] !== x;
66+
if (!condition) {
67+
let old$t1 = $[3];
68+
$structuralCheck(old$t1, t1, "t1", "Component", "cached");
6669
}
6770
$[2] = x;
6871
$[3] = t1;
72+
if (condition) {
73+
t1 = <div>{x}</div>;
74+
$structuralCheck($[3], t1, "t1", "Component", "recomputed");
75+
t1 = $[3];
76+
}
6977
}
7078
return t1;
7179
}

‎compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/useState-pruned-dependency-change-detect.expect.md‎

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -32,14 +32,18 @@ function Component(props) {
3232
let t1;
3333
{
3434
t1 = <div>{x}</div>;
35-
if (!($[1] !== x)) {
36-
let old$t1;
37-
old$t1 = $[2];
38-
$structuralCheck(old$t1, t1, "t1", "Component");
39-
t1 = old$t1;
35+
let condition = $[1] !== x;
36+
if (!condition) {
37+
let old$t1 = $[2];
38+
$structuralCheck(old$t1, t1, "t1", "Component", "cached");
4039
}
4140
$[1] = x;
4241
$[2] = t1;
42+
if (condition) {
43+
t1 = <div>{x}</div>;
44+
$structuralCheck($[2], t1, "t1", "Component", "recomputed");
45+
t1 = $[2];
46+
}
4347
}
4448
return t1;
4549
}

0 commit comments

Comments
 (0)