[compiler][ez] Patch hoistability for ObjectMethods

Extends #31066 to ObjectMethods (somehow missed this before). -
facebook · Oct 24, 2024 · cbd12f5 · cbd12f5
1 parent 69d4b80
commit cbd12f5
Show file tree

Hide file tree

Showing 3 changed files with 114 additions and 2 deletions.
diff --git a/compiler/packages/babel-plugin-react-compiler/src/HIR/CollectHoistablePropertyLoads.ts b/compiler/packages/babel-plugin-react-compiler/src/HIR/CollectHoistablePropertyLoads.ts
@@ -348,7 +348,8 @@ function collectNonNullsInBlocks(
         assumedNonNullObjects.add(maybeNonNull);
       }
       if (
-        instr.value.kind === 'FunctionExpression' &&
+        (instr.value.kind === 'FunctionExpression' ||
+          instr.value.kind === 'ObjectMethod') &&
         !fn.env.config.enableTreatFunctionDepsAsConditional
       ) {
         const innerFn = instr.value.loweredFunc;
@@ -591,7 +592,10 @@ function collectFunctionExpressionFakeLoads(
 
   for (const [_, block] of fn.body.blocks) {
     for (const {lvalue, value} of block.instructions) {
-      if (value.kind === 'FunctionExpression') {
+      if (
+        value.kind === 'FunctionExpression' ||
+        value.kind === 'ObjectMethod'
+      ) {
         for (const reference of value.loweredFunc.dependencies) {
           let curr: IdentifierId | undefined = reference.identifier.id;
           while (curr != null) {

diff --git a/...ope-deps-hir-fork/reduce-reactive-deps/infer-objectmethod-cond-access.expect.md b/...ope-deps-hir-fork/reduce-reactive-deps/infer-objectmethod-cond-access.expect.md
@@ -0,0 +1,82 @@
+
+## Input
+
+```javascript
+// @enablePropagateDepsInHIR
+import {Stringify} from 'shared-runtime';
+
+function Foo({a, shouldReadA}) {
+  return (
+    <Stringify
+      objectMethod={{
+        method() {
+          if (shouldReadA) return a.b.c;
+          return null;
+        },
+      }}
+      shouldInvokeFns={true}
+    />
+  );
+}
+
+export const FIXTURE_ENTRYPOINT = {
+  fn: Foo,
+  params: [{a: null, shouldReadA: true}],
+  sequentialRenders: [
+    {a: null, shouldReadA: true},
+    {a: null, shouldReadA: false},
+    {a: {b: {c: 4}}, shouldReadA: true},
+  ],
+};
+
+```
+
+## Code
+
+```javascript
+import { c as _c } from "react/compiler-runtime"; // @enablePropagateDepsInHIR
+import { Stringify } from "shared-runtime";
+
+function Foo(t0) {
+  const $ = _c(3);
+  const { a, shouldReadA } = t0;
+  let t1;
+  if ($[0] !== shouldReadA || $[1] !== a) {
+    t1 = (
+      <Stringify
+        objectMethod={{
+          method() {
+            if (shouldReadA) {
+              return a.b.c;
+            }
+            return null;
+          },
+        }}
+        shouldInvokeFns={true}
+      />
+    );
+    $[0] = shouldReadA;
+    $[1] = a;
+    $[2] = t1;
+  } else {
+    t1 = $[2];
+  }
+  return t1;
+}
+
+export const FIXTURE_ENTRYPOINT = {
+  fn: Foo,
+  params: [{ a: null, shouldReadA: true }],
+  sequentialRenders: [
+    { a: null, shouldReadA: true },
+    { a: null, shouldReadA: false },
+    { a: { b: { c: 4 } }, shouldReadA: true },
+  ],
+};
+
+```
+      
+### Eval output
+(kind: ok) [[ (exception in render) TypeError: Cannot read properties of null (reading 'b') ]]
+<div>{"objectMethod":{"method":{"kind":"Function","result":null}},"shouldInvokeFns":true}</div>
+<div>{"objectMethod":{"method":{"kind":"Function","result":4}},"shouldInvokeFns":true}</div>
diff --git a/...iler/propagate-scope-deps-hir-fork/reduce-reactive-deps/infer-objectmethod-cond-access.js b/...iler/propagate-scope-deps-hir-fork/reduce-reactive-deps/infer-objectmethod-cond-access.js
@@ -0,0 +1,26 @@
+// @enablePropagateDepsInHIR
+import {Stringify} from 'shared-runtime';
+
+function Foo({a, shouldReadA}) {
+  return (
+    <Stringify
+      objectMethod={{
+        method() {
+          if (shouldReadA) return a.b.c;
+          return null;
+        },
+      }}
+      shouldInvokeFns={true}
+    />
+  );
+}
+
+export const FIXTURE_ENTRYPOINT = {
+  fn: Foo,
+  params: [{a: null, shouldReadA: true}],
+  sequentialRenders: [
+    {a: null, shouldReadA: true},
+    {a: null, shouldReadA: false},
+    {a: {b: {c: 4}}, shouldReadA: true},
+  ],
+};