From d2d5350e6522aeedfecf5f963248bdd9d43ddeb6 Mon Sep 17 00:00:00 2001
From: Mofei Zhang <feifei0@meta.com>
Date: Thu, 13 Mar 2025 11:52:25 -0400
Subject: [PATCH] [compiler][optim] more shapes for mixedreadonly

- Add `at`, `indexOf`, and `includes`
- Optimize MixedReadOnly which is currently only used by hook return values. Hook return values are typed as Frozen, this change propagates that to return values of aliasing function calls (such as `at`). One potential issue is that  developers may pass `enableAssumeHooksFollowRulesOfReact:false` and set `transitiveMixedData`, expecting their transitive mixed data to be mutable. This is a bit of an edge case and already doesn't have clear semantics.
---
 .../src/HIR/ObjectShape.ts                    | 58 ++++++++++++++++++-
 1 file changed, 56 insertions(+), 2 deletions(-)

diff --git a/compiler/packages/babel-plugin-react-compiler/src/HIR/ObjectShape.ts b/compiler/packages/babel-plugin-react-compiler/src/HIR/ObjectShape.ts
index f51768c586..22ae261867 100644
--- a/compiler/packages/babel-plugin-react-compiler/src/HIR/ObjectShape.ts
+++ b/compiler/packages/babel-plugin-react-compiler/src/HIR/ObjectShape.ts
@@ -535,6 +535,30 @@ addObject(BUILTIN_SHAPES, BuiltInRefValueId, [
   ['*', {kind: 'Object', shapeId: BuiltInRefValueId}],
 ]);
 
+/**
+ * MixedReadOnly =
+ *   | primitive
+ *   | simple objects (Record<string, MixedReadOnly>)
+ *   | Array<MixedReadOnly>
+ *
+ * APIs such as Relay — but also Flux and other data stores — often return a
+ * union of types with some interesting properties in terms of analysis.
+ *
+ * Given this constraint, if data came from Relay, then we should be able to
+ * infer things like `data.items.map(): Array`. That may seem like a leap at
+ * first but remember, we assume you're not patching builtins. Thus the only way
+ * data.items.map can exist and be a function, given the above set of data types
+ * and builtin JS methods, is if `data.items` was an Array, and `data.items.map`
+ * is therefore calling Array.prototype.map. Then we know that function returns
+ * an Array as well. This relies on the fact that map() is being called, so if
+ * data.items was some other type it would error at runtime - so it's sound.
+ *
+ * Note that this shape is currently only used for hook return values, which
+ * means that it's safe to type aliasing method-call return kinds as `Frozen`.
+ *
+ * Also note that all newly created arrays from method-calls (e.g. `.map`)
+ * have the appropriate mutable `BuiltInArray` shape
+ */
 addObject(BUILTIN_SHAPES, BuiltInMixedReadonlyId, [
   [
     'toString',
@@ -546,6 +570,36 @@ addObject(BUILTIN_SHAPES, BuiltInMixedReadonlyId, [
       returnValueKind: ValueKind.Primitive,
     }),
   ],
+  [
+    'indexOf',
+    addFunction(BUILTIN_SHAPES, [], {
+      positionalParams: [],
+      restParam: Effect.Read,
+      returnType: {kind: 'Primitive'},
+      calleeEffect: Effect.Read,
+      returnValueKind: ValueKind.Primitive,
+    }),
+  ],
+  [
+    'includes',
+    addFunction(BUILTIN_SHAPES, [], {
+      positionalParams: [],
+      restParam: Effect.Read,
+      returnType: {kind: 'Primitive'},
+      calleeEffect: Effect.Read,
+      returnValueKind: ValueKind.Primitive,
+    }),
+  ],
+  [
+    'at',
+    addFunction(BUILTIN_SHAPES, [], {
+      positionalParams: [Effect.Read],
+      restParam: null,
+      returnType: {kind: 'Object', shapeId: BuiltInMixedReadonlyId},
+      calleeEffect: Effect.Capture,
+      returnValueKind: ValueKind.Frozen,
+    }),
+  ],
   [
     'map',
     addFunction(BUILTIN_SHAPES, [], {
@@ -642,9 +696,9 @@ addObject(BUILTIN_SHAPES, BuiltInMixedReadonlyId, [
     addFunction(BUILTIN_SHAPES, [], {
       positionalParams: [],
       restParam: Effect.ConditionallyMutate,
-      returnType: {kind: 'Poly'},
+      returnType: {kind: 'Object', shapeId: BuiltInMixedReadonlyId},
       calleeEffect: Effect.ConditionallyMutate,
-      returnValueKind: ValueKind.Mutable,
+      returnValueKind: ValueKind.Frozen,
       noAlias: true,
       mutableOnlyIfOperandsAreMutable: true,
     }),