[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: 51283108942002a14d032613a9d0b8b665ee3a94 Pull Request resolved: https://github.com/facebook/react/pull/30797
Joe Savona committed
Aug 22, 2024 at 17:51 UTC
1b7478246d05b030a2ae7a8bb07aea8c7df7ef27
4 files changed
+149
-44
compiler/packages/babel-plugin-react-compiler/src/TypeInference/InferTypes.ts
+44
-2
@@ -25,6 +25,7 @@ import {
25
BuiltInArrayId,
26
BuiltInFunctionId,
27
BuiltInJsxId,
28
+ BuiltInMixedReadonlyId,
29
BuiltInObjectId,
30
BuiltInPropsId,
31
BuiltInRefValueId,
@@ -496,8 +497,13 @@ class Unifier {
497
if (candidateType === null) {
498
candidateType = resolved;
499
} else if (!typeEquals(resolved, candidateType)) {
499
- candidateType = null;
500
- break;
500
+ const unionType = tryUnionTypes(resolved, candidateType);
501
+ if (unionType === null) {
502
+ candidateType = null;
503
+ break;
504
+ } else {
505
+ candidateType = unionType;
506
+ }
507
} // else same type, continue
508
}
509
@@ -650,3 +656,39 @@ const RefLikeNameRE = /^(?:[a-zA-Z$_][a-zA-Z$_0-9]*)Ref$|^ref$/;
656
function isRefLikeName(t: PropType): boolean {
657
return RefLikeNameRE.test(t.objectName) && t.propertyName === 'current';
658
}
659
+
660
+function tryUnionTypes(ty1: Type, ty2: Type): Type | null {
661
+ let readonlyType: Type;
662
+ let otherType: Type;
663
+ if (ty1.kind === 'Object' && ty1.shapeId === BuiltInMixedReadonlyId) {
664
+ readonlyType = ty1;
665
+ otherType = ty2;
666
+ } else if (ty2.kind === 'Object' && ty2.shapeId === BuiltInMixedReadonlyId) {
667
+ readonlyType = ty2;
668
+ otherType = ty1;
669
+ } else {
670
+ return null;
671
+ }
672
+ if (otherType.kind === 'Primitive') {
673
+ /**
674
+ * Union(Primitive | MixedReadonly) = MixedReadonly
675
+ *
676
+ * For example, `data ?? null` could return `data`, the fact that RHS
677
+ * is a primitive doesn't guarantee the result is a primitive.
678
+ */
679
+ return readonlyType;
680
+ } else if (
681
+ otherType.kind === 'Object' &&
682
+ otherType.shapeId === BuiltInArrayId
683
+ ) {
684
+ /**
685
+ * Union(Array | MixedReadonly) = Array
686
+ *
687
+ * In practice this pattern means the result is always an array. Given
688
+ * that this behavior requires opting-in to the mixedreadonly type
689
+ * (via moduleTypeProvider) this seems like a reasonable heuristic.
690
+ */
691
+ return otherType;
692
+ }
693
+ return null;
694
+}
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.todo-repro-missing-memoization-lack-of-phi-types.expect.md
deleted
-42
@@ -1,42 +0,0 @@
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
-
28
-## Error
29
-
30
-```
31
- 8 | const flatMap = nodes.flatMap(node => node.items);
32
- 9 | const filtered = flatMap.filter(item => item != null);
33
-> 10 | const map = useMemo(() => filtered.map(), [filtered]);
34
- | ^^^^^^^^ CannotPreserveMemoization: React Compiler has skipped optimizing this component because the existing manual memoization could not be preserved. This dependency may be mutated later, which could cause the value to change unexpectedly (10:10)
35
-
36
-CannotPreserveMemoization: React Compiler has skipped optimizing this component because the existing manual memoization could not be preserved. This value was memoized in source but not in compilation output. (10:10)
37
- 11 | const index = filtered.findIndex(x => x === null);
38
- 12 |
39
- 13 | return (
40
-```
41
-
42
-
\ No newline at end of file
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/repro-missing-memoization-lack-of-phi-types.expect.md
new
+105
@@ -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
\ No newline at end of file