Skip to content

Commit 8797b91

Browse files
committed
[compiler] Improve ShallowMutable detection for frozen sources
The compiler currently fails to preserve manual memoization when components use rest spread destructuring for props: ```js // OK function Component(props) { const value = useMemo(() => compute(props.foo), [props.foo]); } // Manual memo could not be preserved function Component({...props}) { const value = useMemo(() => compute(props.foo), [props.foo]); } ``` The issue is that property accesses on rest-spread objects are treated as fully mutable. This PR introduces a new `ShallowMutable` ValueKind for rest spreads that come specifically from props. When we access properties from a `ShallowMutable` value, they're treated as Frozen. Closes #34313
1 parent 7697a9f commit 8797b91

16 files changed

+832
-7
lines changed

compiler/packages/babel-plugin-react-compiler/src/HIR/HIR.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1445,6 +1445,7 @@ export enum ValueKind {
14451445
Primitive = 'primitive',
14461446
Global = 'global',
14471447
Mutable = 'mutable',
1448+
ShallowMutable = 'shallowmutable',
14481449
Context = 'context',
14491450
}
14501451

@@ -1454,6 +1455,7 @@ export const ValueKindSchema = z.enum([
14541455
ValueKind.Primitive,
14551456
ValueKind.Global,
14561457
ValueKind.Mutable,
1458+
ValueKind.ShallowMutable,
14571459
ValueKind.Context,
14581460
]);
14591461

compiler/packages/babel-plugin-react-compiler/src/Inference/InferMutationAliasingEffects.ts

Lines changed: 38 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -591,8 +591,14 @@ function applyEffect(
591591
};
592592
context.effectInstructionValueCache.set(effect, value);
593593
}
594+
595+
const outputKind =
596+
fromValue.kind === ValueKind.ShallowMutable
597+
? ValueKind.Frozen
598+
: fromValue.kind;
599+
594600
state.initialize(value, {
595-
kind: fromValue.kind,
601+
kind: outputKind,
596602
reason: new Set(fromValue.reason),
597603
});
598604
state.define(effect.into, value);
@@ -607,10 +613,11 @@ function applyEffect(
607613
});
608614
break;
609615
}
616+
case ValueKind.ShallowMutable:
610617
case ValueKind.Frozen: {
611618
effects.push({
612619
kind: 'Create',
613-
value: fromValue.kind,
620+
value: outputKind,
614621
into: effect.into,
615622
reason: [...fromValue.reason][0] ?? ValueReason.Other,
616623
});
@@ -720,11 +727,14 @@ function applyEffect(
720727
* copy-on-write semantics, then we can prune the effect
721728
*/
722729
const intoKind = state.kind(effect.into).kind;
730+
const fromKind = state.kind(effect.from).kind;
731+
723732
let isMutableDesination: boolean;
724733
switch (intoKind) {
725734
case ValueKind.Context:
726735
case ValueKind.Mutable:
727-
case ValueKind.MaybeFrozen: {
736+
case ValueKind.MaybeFrozen:
737+
case ValueKind.ShallowMutable: {
728738
isMutableDesination = true;
729739
break;
730740
}
@@ -733,14 +743,14 @@ function applyEffect(
733743
break;
734744
}
735745
}
736-
const fromKind = state.kind(effect.from).kind;
737746
let isMutableReferenceType: boolean;
738747
switch (fromKind) {
739748
case ValueKind.Global:
740749
case ValueKind.Primitive: {
741750
isMutableReferenceType = false;
742751
break;
743752
}
753+
case ValueKind.ShallowMutable:
744754
case ValueKind.Frozen: {
745755
isMutableReferenceType = false;
746756
applyEffect(
@@ -781,6 +791,7 @@ function applyEffect(
781791
const fromValue = state.kind(effect.from);
782792
const fromKind = fromValue.kind;
783793
switch (fromKind) {
794+
case ValueKind.ShallowMutable:
784795
case ValueKind.Frozen: {
785796
applyEffect(
786797
context,
@@ -1267,6 +1278,7 @@ class InferenceState {
12671278
switch (value.kind) {
12681279
case ValueKind.Context:
12691280
case ValueKind.Mutable:
1281+
case ValueKind.ShallowMutable:
12701282
case ValueKind.MaybeFrozen: {
12711283
const values = this.values(place);
12721284
for (const instrValue of values) {
@@ -1315,13 +1327,30 @@ class InferenceState {
13151327
if (isRefOrRefValue(place.identifier)) {
13161328
return 'mutate-ref';
13171329
}
1318-
const kind = this.kind(place).kind;
1330+
const abstractValue = this.kind(place);
1331+
const kind = abstractValue.kind;
1332+
1333+
// Downgrade ShallowMutable to Mutable when mutated
1334+
if (kind === ValueKind.ShallowMutable) {
1335+
const values = this.values(place);
1336+
for (const value of values) {
1337+
const valueInfo = this.#values.get(value);
1338+
if (valueInfo && valueInfo.kind === ValueKind.ShallowMutable) {
1339+
this.#values.set(value, {
1340+
kind: ValueKind.Mutable,
1341+
reason: valueInfo.reason,
1342+
});
1343+
}
1344+
}
1345+
}
1346+
13191347
switch (variant) {
13201348
case 'MutateConditionally':
13211349
case 'MutateTransitiveConditionally': {
13221350
switch (kind) {
13231351
case ValueKind.Mutable:
1324-
case ValueKind.Context: {
1352+
case ValueKind.Context:
1353+
case ValueKind.ShallowMutable: {
13251354
return 'mutate';
13261355
}
13271356
default: {
@@ -1333,7 +1362,8 @@ class InferenceState {
13331362
case 'MutateTransitive': {
13341363
switch (kind) {
13351364
case ValueKind.Mutable:
1336-
case ValueKind.Context: {
1365+
case ValueKind.Context:
1366+
case ValueKind.ShallowMutable: {
13371367
return 'mutate';
13381368
}
13391369
case ValueKind.Primitive: {
@@ -2349,6 +2379,7 @@ function areArgumentsImmutableAndNonMutating(
23492379
const kind = state.kind(place).kind;
23502380
switch (kind) {
23512381
case ValueKind.Primitive:
2382+
case ValueKind.ShallowMutable:
23522383
case ValueKind.Frozen: {
23532384
/*
23542385
* Only immutable values, or frozen lambdas are allowed.
Lines changed: 90 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,90 @@
1+
2+
## Input
3+
4+
```javascript
5+
import {useMemo} from 'react';
6+
import {ValidateMemoization} from 'shared-runtime';
7+
8+
function useData() {
9+
return ['a', 'b', 'c'];
10+
}
11+
12+
function Component() {
13+
const [first, ...rest] = useData();
14+
15+
const result = useMemo(() => {
16+
return rest.join('-');
17+
}, [rest]);
18+
19+
return <ValidateMemoization inputs={[rest]} output={result} />;
20+
}
21+
22+
export const FIXTURE_ENTRYPOINT = {
23+
fn: Component,
24+
params: [],
25+
isComponent: true,
26+
};
27+
```
28+
29+
## Code
30+
31+
```javascript
32+
import { c as _c } from "react/compiler-runtime";
33+
import { useMemo } from "react";
34+
import { ValidateMemoization } from "shared-runtime";
35+
36+
function useData() {
37+
const $ = _c(1);
38+
let t0;
39+
if ($[0] === Symbol.for("react.memo_cache_sentinel")) {
40+
t0 = ["a", "b", "c"];
41+
$[0] = t0;
42+
} else {
43+
t0 = $[0];
44+
}
45+
return t0;
46+
}
47+
48+
function Component() {
49+
const $ = _c(7);
50+
const t0 = useData();
51+
let rest;
52+
if ($[0] !== t0) {
53+
[, ...rest] = t0;
54+
$[0] = t0;
55+
$[1] = rest;
56+
} else {
57+
rest = $[1];
58+
}
59+
60+
const result = rest.join("-");
61+
let t1;
62+
if ($[2] !== rest) {
63+
t1 = [rest];
64+
$[2] = rest;
65+
$[3] = t1;
66+
} else {
67+
t1 = $[3];
68+
}
69+
let t2;
70+
if ($[4] !== result || $[5] !== t1) {
71+
t2 = <ValidateMemoization inputs={t1} output={result} />;
72+
$[4] = result;
73+
$[5] = t1;
74+
$[6] = t2;
75+
} else {
76+
t2 = $[6];
77+
}
78+
return t2;
79+
}
80+
81+
export const FIXTURE_ENTRYPOINT = {
82+
fn: Component,
83+
params: [],
84+
isComponent: true,
85+
};
86+
87+
```
88+
89+
### Eval output
90+
(kind: ok) <div>{"inputs":[["b","c"]],"output":"b-c"}</div>
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
import {useMemo} from 'react';
2+
import {ValidateMemoization} from 'shared-runtime';
3+
4+
function useData() {
5+
return ['a', 'b', 'c'];
6+
}
7+
8+
function Component() {
9+
const [first, ...rest] = useData();
10+
11+
const result = useMemo(() => {
12+
return rest.join('-');
13+
}, [rest]);
14+
15+
return <ValidateMemoization inputs={[rest]} output={result} />;
16+
}
17+
18+
export const FIXTURE_ENTRYPOINT = {
19+
fn: Component,
20+
params: [],
21+
isComponent: true,
22+
};
Lines changed: 120 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,120 @@
1+
2+
## Input
3+
4+
```javascript
5+
import {useMemo} from 'react';
6+
7+
function useTheme() {
8+
return {primary: '#blue', secondary: '#green'};
9+
}
10+
11+
function computeStyles(
12+
specialProp: string | undefined,
13+
restProps: any,
14+
theme: any,
15+
) {
16+
return {
17+
color: specialProp ? theme.primary : theme.secondary,
18+
...restProps.style,
19+
};
20+
}
21+
22+
export function SpecialButton({
23+
specialProp,
24+
...restProps
25+
}: {
26+
specialProp?: string;
27+
style?: Record<string, string>;
28+
onClick?: () => void;
29+
}) {
30+
const theme = useTheme();
31+
32+
const styles = useMemo(
33+
() => computeStyles(specialProp, restProps, theme),
34+
[specialProp, restProps, theme],
35+
);
36+
37+
return (
38+
<button style={styles} onClick={restProps.onClick}>
39+
Click me
40+
</button>
41+
);
42+
}
43+
44+
export const FIXTURE_ENTRYPOINT = {
45+
fn: SpecialButton,
46+
params: [{specialProp: 'test', style: {fontSize: '16px'}, onClick: () => {}}],
47+
isComponent: true,
48+
};
49+
50+
```
51+
52+
## Code
53+
54+
```javascript
55+
import { c as _c } from "react/compiler-runtime";
56+
import { useMemo } from "react";
57+
58+
function useTheme() {
59+
const $ = _c(1);
60+
let t0;
61+
if ($[0] === Symbol.for("react.memo_cache_sentinel")) {
62+
t0 = { primary: "#blue", secondary: "#green" };
63+
$[0] = t0;
64+
} else {
65+
t0 = $[0];
66+
}
67+
return t0;
68+
}
69+
70+
function computeStyles(specialProp, restProps, theme) {
71+
const $ = _c(3);
72+
73+
const t0 = specialProp ? theme.primary : theme.secondary;
74+
let t1;
75+
if ($[0] !== restProps.style || $[1] !== t0) {
76+
t1 = { color: t0, ...restProps.style };
77+
$[0] = restProps.style;
78+
$[1] = t0;
79+
$[2] = t1;
80+
} else {
81+
t1 = $[2];
82+
}
83+
return t1;
84+
}
85+
86+
export function SpecialButton(t0) {
87+
const $ = _c(3);
88+
const { specialProp, ...restProps } = t0;
89+
90+
const theme = useTheme();
91+
92+
const styles = computeStyles(specialProp, restProps, theme);
93+
let t1;
94+
if ($[0] !== restProps.onClick || $[1] !== styles) {
95+
t1 = (
96+
<button style={styles} onClick={restProps.onClick}>
97+
Click me
98+
</button>
99+
);
100+
$[0] = restProps.onClick;
101+
$[1] = styles;
102+
$[2] = t1;
103+
} else {
104+
t1 = $[2];
105+
}
106+
return t1;
107+
}
108+
109+
export const FIXTURE_ENTRYPOINT = {
110+
fn: SpecialButton,
111+
params: [
112+
{ specialProp: "test", style: { fontSize: "16px" }, onClick: () => {} },
113+
],
114+
isComponent: true,
115+
};
116+
117+
```
118+
119+
### Eval output
120+
(kind: ok) <button style="font-size: 16px;">Click me</button>

0 commit comments

Comments
 (0)