Skip to content

Commit ff889d9

Browse files
committed
[compiler] Fix invalid Array.map type
Summary: Test Plan: Reviewers: Subscribers: Tasks: Tags:
1 parent 59006b0 commit ff889d9

File tree

4 files changed

+46
-43
lines changed

4 files changed

+46
-43
lines changed

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

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -545,8 +545,16 @@ addObject(BUILTIN_SHAPES, BuiltInMixedReadonlyId, [
545545
[
546546
'map',
547547
addFunction(BUILTIN_SHAPES, [], {
548+
/**
549+
* Note `map`'s arguments are annotated as Effect.ConditionallyMutate as
550+
* calling `<array>.map(fn)` might invoke `fn`, which means replaying its
551+
* effects.
552+
*
553+
* (Note that -- Effect.Read / Effect.Capture on a function type means
554+
* potential data dependency or aliasing respectively.)
555+
*/
548556
positionalParams: [],
549-
restParam: Effect.Read,
557+
restParam: Effect.ConditionallyMutate,
550558
returnType: {kind: 'Object', shapeId: BuiltInArrayId},
551559
calleeEffect: Effect.ConditionallyMutate,
552560
returnValueKind: ValueKind.Mutable,
Lines changed: 34 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,9 @@ import {
3636
function Component({extraJsx}) {
3737
const x = makeArray();
3838
const items = useFragment();
39+
// This closure has the following effects that must be replayed:
40+
// - MaybeFreeze / Capture of `items`
41+
// - ConditionalMutate of x
3942
const jsx = items.a.map((item, i) => {
4043
arrayPush(x, 2);
4144
return <Stringify item={item} key={i} />;
@@ -97,60 +100,47 @@ import {
97100
*/
98101

99102
function Component(t0) {
100-
const $ = _c(9);
103+
const $ = _c(6);
101104
const { extraJsx } = t0;
102-
let t1;
103-
if ($[0] === Symbol.for("react.memo_cache_sentinel")) {
104-
t1 = makeArray();
105-
$[0] = t1;
106-
} else {
107-
t1 = $[0];
108-
}
109-
const x = t1;
105+
const x = makeArray();
110106
const items = useFragment();
111-
let jsx;
112-
if ($[1] !== extraJsx || $[2] !== items.a) {
113-
jsx = items.a.map((item, i) => {
114-
arrayPush(x, 2);
115-
return <Stringify item={item} key={i} />;
116-
});
117-
const offset = jsx.length;
118-
for (let i_0 = 0; i_0 < extraJsx; i_0++) {
119-
jsx.push(<Stringify item={0} key={i_0 + offset} />);
120-
}
121-
$[1] = extraJsx;
122-
$[2] = items.a;
123-
$[3] = jsx;
124-
} else {
125-
jsx = $[3];
107+
108+
const jsx = items.a.map((item, i) => {
109+
arrayPush(x, 2);
110+
return <Stringify item={item} key={i} />;
111+
});
112+
const offset = jsx.length;
113+
for (let i_0 = 0; i_0 < extraJsx; i_0++) {
114+
jsx.push(<Stringify item={0} key={i_0 + offset} />);
126115
}
127116

128117
const count = jsx.length;
129118
identity(count);
130-
let t2;
131-
if ($[4] !== count) {
132-
t2 = <Stringify x={x} count={count} />;
133-
$[4] = count;
134-
$[5] = t2;
119+
let t1;
120+
if ($[0] !== count || $[1] !== x) {
121+
t1 = <Stringify x={x} count={count} />;
122+
$[0] = count;
123+
$[1] = x;
124+
$[2] = t1;
135125
} else {
136-
t2 = $[5];
126+
t1 = $[2];
137127
}
138-
const t3 = jsx[0];
139-
let t4;
140-
if ($[6] !== t2 || $[7] !== t3) {
141-
t4 = (
128+
const t2 = jsx[0];
129+
let t3;
130+
if ($[3] !== t1 || $[4] !== t2) {
131+
t3 = (
142132
<>
133+
{t1}
143134
{t2}
144-
{t3}
145135
</>
146136
);
147-
$[6] = t2;
148-
$[7] = t3;
149-
$[8] = t4;
137+
$[3] = t1;
138+
$[4] = t2;
139+
$[5] = t3;
150140
} else {
151-
t4 = $[8];
141+
t3 = $[5];
152142
}
153-
return t4;
143+
return t3;
154144
}
155145

156146
export const FIXTURE_ENTRYPOINT = {
@@ -160,4 +150,7 @@ export const FIXTURE_ENTRYPOINT = {
160150
};
161151

162152
```
163-
153+
154+
### Eval output
155+
(kind: ok) <div>{"x":[2,2,2],"count":3}</div><div>{"item":1}</div>
156+
<div>{"x":[2,2,2],"count":4}</div><div>{"item":1}</div>
Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,9 @@ import {
3232
function Component({extraJsx}) {
3333
const x = makeArray();
3434
const items = useFragment();
35+
// This closure has the following effects that must be replayed:
36+
// - MaybeFreeze / Capture of `items`
37+
// - ConditionalMutate of x
3538
const jsx = items.a.map((item, i) => {
3639
arrayPush(x, 2);
3740
return <Stringify item={item} key={i} />;

compiler/packages/snap/src/SproutTodoFilter.ts

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -486,7 +486,6 @@ const skipFilter = new Set([
486486
'bug-aliased-capture-mutate',
487487
'bug-functiondecl-hoisting',
488488
'bug-try-catch-maybe-null-dependency',
489-
'bug-invalid-mixedreadonly-map-shape',
490489
'bug-type-inference-control-flow',
491490
'reduce-reactive-deps/bug-infer-function-cond-access-not-hoisted',
492491
'bug-invalid-phi-as-dependency',

0 commit comments

Comments
 (0)