Skip to content

Commit 861b349

Browse files
committed
[compiler] Repro for imprecise memo due to closure capturing changes
Syncing this stack internally there is a small percentage of files that lose memoization, generally for callbacks. The repro here tries to get at the core pattern, where a parameter escapes into a mutable return value. This makes the callback appear mutable, and means that calls like array.map aren't able to optimize as well — even if the array itself is transitively immutable. The challenge is that we can't really distinguish between just capturing and true mutation right now — AnalyzeFunctions kind of has to pick one, and consider both a mutation. ghstack-source-id: 55efdd1 Pull Request resolved: facebook/react#33180
1 parent a0fa3bb commit 861b349

File tree

3 files changed

+219
-0
lines changed

3 files changed

+219
-0
lines changed
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,171 @@
1+
2+
## Input
3+
4+
```javascript
5+
import {ValidateMemoization} from 'shared-runtime';
6+
7+
const Codes = {
8+
en: {name: 'English'},
9+
ja: {name: 'Japanese'},
10+
ko: {name: 'Korean'},
11+
zh: {name: 'Chinese'},
12+
};
13+
14+
function Component(a) {
15+
let keys;
16+
if (a) {
17+
keys = Object.keys(Codes);
18+
} else {
19+
return null;
20+
}
21+
const options = keys.map(code => {
22+
const country = Codes[code];
23+
return {
24+
name: country.name,
25+
code,
26+
};
27+
});
28+
return (
29+
<>
30+
<ValidateMemoization inputs={[]} output={keys} onlyCheckCompiled={true} />
31+
<ValidateMemoization
32+
inputs={[]}
33+
output={options}
34+
onlyCheckCompiled={true}
35+
/>
36+
</>
37+
);
38+
}
39+
40+
export const FIXTURE_ENTRYPOINT = {
41+
fn: Component,
42+
params: [{a: false}],
43+
sequentialRenders: [
44+
{a: false},
45+
{a: true},
46+
{a: true},
47+
{a: false},
48+
{a: true},
49+
{a: false},
50+
],
51+
};
52+
53+
```
54+
55+
## Code
56+
57+
```javascript
58+
import { c as _c } from "react/compiler-runtime";
59+
import { ValidateMemoization } from "shared-runtime";
60+
61+
const Codes = {
62+
en: { name: "English" },
63+
ja: { name: "Japanese" },
64+
ko: { name: "Korean" },
65+
zh: { name: "Chinese" },
66+
};
67+
68+
function Component(a) {
69+
const $ = _c(13);
70+
let keys;
71+
let t0;
72+
let t1;
73+
if ($[0] !== a) {
74+
t1 = Symbol.for("react.early_return_sentinel");
75+
bb0: {
76+
if (a) {
77+
keys = Object.keys(Codes);
78+
} else {
79+
t1 = null;
80+
break bb0;
81+
}
82+
83+
t0 = keys.map(_temp);
84+
}
85+
$[0] = a;
86+
$[1] = t0;
87+
$[2] = t1;
88+
$[3] = keys;
89+
} else {
90+
t0 = $[1];
91+
t1 = $[2];
92+
keys = $[3];
93+
}
94+
if (t1 !== Symbol.for("react.early_return_sentinel")) {
95+
return t1;
96+
}
97+
const options = t0;
98+
let t2;
99+
if ($[4] === Symbol.for("react.memo_cache_sentinel")) {
100+
t2 = [];
101+
$[4] = t2;
102+
} else {
103+
t2 = $[4];
104+
}
105+
let t3;
106+
if ($[5] !== keys) {
107+
t3 = (
108+
<ValidateMemoization inputs={t2} output={keys} onlyCheckCompiled={true} />
109+
);
110+
$[5] = keys;
111+
$[6] = t3;
112+
} else {
113+
t3 = $[6];
114+
}
115+
let t4;
116+
if ($[7] === Symbol.for("react.memo_cache_sentinel")) {
117+
t4 = [];
118+
$[7] = t4;
119+
} else {
120+
t4 = $[7];
121+
}
122+
let t5;
123+
if ($[8] !== options) {
124+
t5 = (
125+
<ValidateMemoization
126+
inputs={t4}
127+
output={options}
128+
onlyCheckCompiled={true}
129+
/>
130+
);
131+
$[8] = options;
132+
$[9] = t5;
133+
} else {
134+
t5 = $[9];
135+
}
136+
let t6;
137+
if ($[10] !== t3 || $[11] !== t5) {
138+
t6 = (
139+
<>
140+
{t3}
141+
{t5}
142+
</>
143+
);
144+
$[10] = t3;
145+
$[11] = t5;
146+
$[12] = t6;
147+
} else {
148+
t6 = $[12];
149+
}
150+
return t6;
151+
}
152+
function _temp(code) {
153+
const country = Codes[code];
154+
return { name: country.name, code };
155+
}
156+
157+
export const FIXTURE_ENTRYPOINT = {
158+
fn: Component,
159+
params: [{ a: false }],
160+
sequentialRenders: [
161+
{ a: false },
162+
{ a: true },
163+
{ a: true },
164+
{ a: false },
165+
{ a: true },
166+
{ a: false },
167+
],
168+
};
169+
170+
```
171+
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
import {ValidateMemoization} from 'shared-runtime';
2+
3+
const Codes = {
4+
en: {name: 'English'},
5+
ja: {name: 'Japanese'},
6+
ko: {name: 'Korean'},
7+
zh: {name: 'Chinese'},
8+
};
9+
10+
function Component(a) {
11+
let keys;
12+
if (a) {
13+
keys = Object.keys(Codes);
14+
} else {
15+
return null;
16+
}
17+
const options = keys.map(code => {
18+
const country = Codes[code];
19+
return {
20+
name: country.name,
21+
code,
22+
};
23+
});
24+
return (
25+
<>
26+
<ValidateMemoization inputs={[]} output={keys} onlyCheckCompiled={true} />
27+
<ValidateMemoization
28+
inputs={[]}
29+
output={options}
30+
onlyCheckCompiled={true}
31+
/>
32+
</>
33+
);
34+
}
35+
36+
export const FIXTURE_ENTRYPOINT = {
37+
fn: Component,
38+
params: [{a: false}],
39+
sequentialRenders: [
40+
{a: false},
41+
{a: true},
42+
{a: true},
43+
{a: false},
44+
{a: true},
45+
{a: false},
46+
],
47+
};

compiler/packages/snap/src/SproutTodoFilter.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -483,6 +483,7 @@ const skipFilter = new Set([
483483
'todo.lower-context-access-array-destructuring',
484484
'lower-context-selector-simple',
485485
'lower-context-acess-multiple',
486+
'bug-separate-memoization-due-to-callback-capturing',
486487
]);
487488

488489
export default skipFilter;

0 commit comments

Comments
 (0)