Skip to content

Commit a47270f

Browse files
committed
[compiler] Special-case phi inference for mixed readonly type
This allows us to handle common operations such as `useFragment(...).edges.nodes ?? []` where we have a `Phi(MixedReadonly, Array)`. The underlying pattern remains general-purpose and not Relay-specific, and any API that returns transitively "mixed" data (primitives, arrays, plain objects) can benefit from the same type refinement. ghstack-source-id: 27c9465 Pull Request resolved: #30797
1 parent 1e23e9d commit a47270f

File tree

4 files changed

+151
-44
lines changed

4 files changed

+151
-44
lines changed

compiler/packages/babel-plugin-react-compiler/src/TypeInference/InferTypes.ts

Lines changed: 46 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,9 @@ import {
1414
Identifier,
1515
IdentifierId,
1616
Instruction,
17+
isArrayType,
1718
makeType,
19+
ObjectType,
1820
PropType,
1921
Type,
2022
typeEquals,
@@ -25,6 +27,7 @@ import {
2527
BuiltInArrayId,
2628
BuiltInFunctionId,
2729
BuiltInJsxId,
30+
BuiltInMixedReadonlyId,
2831
BuiltInObjectId,
2932
BuiltInPropsId,
3033
BuiltInRefValueId,
@@ -496,8 +499,13 @@ class Unifier {
496499
if (candidateType === null) {
497500
candidateType = resolved;
498501
} else if (!typeEquals(resolved, candidateType)) {
499-
candidateType = null;
500-
break;
502+
const unionType = tryUnionTypes(resolved, candidateType);
503+
if (unionType === null) {
504+
candidateType = null;
505+
break;
506+
} else {
507+
candidateType = unionType;
508+
}
501509
} // else same type, continue
502510
}
503511

@@ -650,3 +658,39 @@ const RefLikeNameRE = /^(?:[a-zA-Z$_][a-zA-Z$_0-9]*)Ref$|^ref$/;
650658
function isRefLikeName(t: PropType): boolean {
651659
return RefLikeNameRE.test(t.objectName) && t.propertyName === 'current';
652660
}
661+
662+
function tryUnionTypes(ty1: Type, ty2: Type): Type | null {
663+
let readonlyType: Type;
664+
let otherType: Type;
665+
if (ty1.kind === 'Object' && ty1.shapeId === BuiltInMixedReadonlyId) {
666+
readonlyType = ty1;
667+
otherType = ty2;
668+
} else if (ty2.kind === 'Object' && ty2.shapeId === BuiltInMixedReadonlyId) {
669+
readonlyType = ty2;
670+
otherType = ty1;
671+
} else {
672+
return null;
673+
}
674+
if (otherType.kind === 'Primitive') {
675+
/**
676+
* Union(Primitive | MixedReadonly) = MixedReadonly
677+
*
678+
* For example, `data ?? null` could return `data`, the fact that RHS
679+
* is a primitive doesn't guarantee the result is a primitive.
680+
*/
681+
return readonlyType;
682+
} else if (
683+
otherType.kind === 'Object' &&
684+
otherType.shapeId === BuiltInArrayId
685+
) {
686+
/**
687+
* Union(Array | MixedReadonly) = Array
688+
*
689+
* In practice this pattern means the result is always an array. Given
690+
* that this behavior requires opting-in to the mixedreadonly type
691+
* (via moduleTypeProvider) this seems like a reasonable heuristic.
692+
*/
693+
return otherType;
694+
}
695+
return null;
696+
}

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.todo-repro-missing-memoization-lack-of-phi-types.expect.md

Lines changed: 0 additions & 42 deletions
This file was deleted.
Lines changed: 105 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,105 @@
1+
2+
## Input
3+
4+
```javascript
5+
// @flow @validatePreserveExistingMemoizationGuarantees
6+
import {useMemo} from 'react';
7+
import {useFragment} from 'shared-runtime';
8+
9+
function Component() {
10+
const data = useFragment();
11+
const nodes = data.nodes ?? [];
12+
const flatMap = nodes.flatMap(node => node.items);
13+
const filtered = flatMap.filter(item => item != null);
14+
const map = useMemo(() => filtered.map(), [filtered]);
15+
const index = filtered.findIndex(x => x === null);
16+
17+
return (
18+
<div>
19+
{map}
20+
{index}
21+
</div>
22+
);
23+
}
24+
25+
```
26+
27+
## Code
28+
29+
```javascript
30+
import { c as _c } from "react/compiler-runtime";
31+
import { useMemo } from "react";
32+
import { useFragment } from "shared-runtime";
33+
34+
function Component() {
35+
const $ = _c(11);
36+
const data = useFragment();
37+
let t0;
38+
if ($[0] !== data.nodes) {
39+
t0 = data.nodes ?? [];
40+
$[0] = data.nodes;
41+
$[1] = t0;
42+
} else {
43+
t0 = $[1];
44+
}
45+
const nodes = t0;
46+
let t1;
47+
if ($[2] !== nodes) {
48+
t1 = nodes.flatMap(_temp);
49+
$[2] = nodes;
50+
$[3] = t1;
51+
} else {
52+
t1 = $[3];
53+
}
54+
const flatMap = t1;
55+
let t2;
56+
if ($[4] !== flatMap) {
57+
t2 = flatMap.filter(_temp2);
58+
$[4] = flatMap;
59+
$[5] = t2;
60+
} else {
61+
t2 = $[5];
62+
}
63+
const filtered = t2;
64+
let t3;
65+
let t4;
66+
if ($[6] !== filtered) {
67+
t4 = filtered.map();
68+
$[6] = filtered;
69+
$[7] = t4;
70+
} else {
71+
t4 = $[7];
72+
}
73+
t3 = t4;
74+
const map = t3;
75+
const index = filtered.findIndex(_temp3);
76+
let t5;
77+
if ($[8] !== map || $[9] !== index) {
78+
t5 = (
79+
<div>
80+
{map}
81+
{index}
82+
</div>
83+
);
84+
$[8] = map;
85+
$[9] = index;
86+
$[10] = t5;
87+
} else {
88+
t5 = $[10];
89+
}
90+
return t5;
91+
}
92+
function _temp3(x) {
93+
return x === null;
94+
}
95+
function _temp2(item) {
96+
return item != null;
97+
}
98+
function _temp(node) {
99+
return node.items;
100+
}
101+
102+
```
103+
104+
### Eval output
105+
(kind: exception) Fixture not implemented
File renamed without changes.

0 commit comments

Comments
 (0)