[compiler] Inferred effect dependencies now include optional chains

Inferred effect dependencies now include optional chains.

This is a temporary solution while https://github.com/facebook/react/pull/32099 and its followups are worked on. Ideally, we should model reactive scope dependencies in the IR similarly to `ComputeIR` -- dependencies should be hoisted and all references rewritten to use the hoisted dependencies.

`

compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Pipeline.ts

@@ -130,6 +130,7 @@ function run(
     mode,
     config,
     contextIdentifiers,
+    func,
     logger,
     filename,
     code,

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

@@ -70,12 +70,14 @@ import {BuiltInArrayId} from './ObjectShape';
 export function lower(
   func: NodePath<t.Function>,
   env: Environment,
+  // Bindings captured from the outer function, in case lower() is called recursively (for lambdas)
   bindings: Bindings | null = null,
   capturedRefs: Array<t.Identifier> = [],
-  // the outermost function being compiled, in case lower() is called recursively (for lambdas)
-  parent: NodePath<t.Function> | null = null,
 ): Result<HIRFunction, CompilerError> {
-  const builder = new HIRBuilder(env, parent ?? func, bindings, capturedRefs);
+  const builder = new HIRBuilder(env, {
+    bindings,
+    context: capturedRefs,
+  });
   const context: HIRFunction['context'] = [];
 
   for (const ref of capturedRefs ?? []) {
@@ -215,7 +217,7 @@ export function lower(
   return Ok({
     id,
     params,
-    fnType: parent == null ? env.fnType : 'Other',
+    fnType: bindings == null ? env.fnType : 'Other',
     returnTypeAnnotation: null, // TODO: extract the actual return type node if present
     returnType: makeType(),
     body: builder.build(),
@@ -3417,7 +3419,7 @@ function lowerFunction(
     | t.ObjectMethod
   >,
 ): LoweredFunction | null {
-  const componentScope: Scope = builder.parentFunction.scope;
+  const componentScope: Scope = builder.environment.parentFunction.scope;
   const capturedContext = gatherCapturedContext(expr, componentScope);
 
   /*
@@ -3428,13 +3430,10 @@ function lowerFunction(
    * This isn't a problem in practice because use Babel's scope analysis to
    * identify the correct references.
    */
-  const lowering = lower(
-    expr,
-    builder.environment,
-    builder.bindings,
-    [...builder.context, ...capturedContext],
-    builder.parentFunction,
-  );
+  const lowering = lower(expr, builder.environment, builder.bindings, [
+    ...builder.context,
+    ...capturedContext,
+  ]);
   let loweredFunc: HIRFunction;
   if (lowering.isErr()) {
     lowering
@@ -3456,7 +3455,7 @@ function lowerExpressionToTemporary(
   return lowerValueToTemporary(builder, value);
 }
 
-function lowerValueToTemporary(
+export function lowerValueToTemporary(
   builder: HIRBuilder,
   value: InstructionValue,
 ): Place {

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

@@ -241,7 +241,10 @@ type PropertyPathNode =
 class PropertyPathRegistry {
   roots: Map<IdentifierId, RootNode> = new Map();
 
-  getOrCreateIdentifier(identifier: Identifier): PropertyPathNode {
+  getOrCreateIdentifier(
+    identifier: Identifier,
+    reactive: boolean,
+  ): PropertyPathNode {
     /**
      * Reads from a statically scoped variable are always safe in JS,
      * with the exception of TDZ (not addressed by this pass).
@@ -255,12 +258,19 @@ class PropertyPathRegistry {
         optionalProperties: new Map(),
         fullPath: {
           identifier,
+          reactive,
           path: [],
         },
         hasOptional: false,
         parent: null,
       };
       this.roots.set(identifier.id, rootNode);
+    } else {
+      CompilerError.invariant(reactive === rootNode.fullPath.reactive, {
+        reason:
+          '[HoistablePropertyLoads] Found inconsistencies in `reactive` flag when deduping identifier reads within the same scope',
+        loc: identifier.loc,
+      });
     }
     return rootNode;
   }
@@ -278,6 +288,7 @@ class PropertyPathRegistry {
         parent: parent,
         fullPath: {
           identifier: parent.fullPath.identifier,
+          reactive: parent.fullPath.reactive,
           path: parent.fullPath.path.concat(entry),
         },
         hasOptional: parent.hasOptional || entry.optional,
@@ -293,7 +304,7 @@ class PropertyPathRegistry {
      * so all subpaths of a PropertyLoad should already exist
      * (e.g. a.b is added before a.b.c),
      */
-    let currNode = this.getOrCreateIdentifier(n.identifier);
+    let currNode = this.getOrCreateIdentifier(n.identifier, n.reactive);
     if (n.path.length === 0) {
       return currNode;
     }
@@ -315,10 +326,11 @@ function getMaybeNonNullInInstruction(
   instr: InstructionValue,
   context: CollectHoistablePropertyLoadsContext,
 ): PropertyPathNode | null {
-  let path = null;
+  let path: ReactiveScopeDependency | null = null;
   if (instr.kind === 'PropertyLoad') {
     path = context.temporaries.get(instr.object.identifier.id) ?? {
       identifier: instr.object.identifier,
+      reactive: instr.object.reactive,
       path: [],
     };
   } else if (instr.kind === 'Destructure') {
@@ -381,7 +393,7 @@ function collectNonNullsInBlocks(
   ) {
     const identifier = fn.params[0].identifier;
     knownNonNullIdentifiers.add(
-      context.registry.getOrCreateIdentifier(identifier),
+      context.registry.getOrCreateIdentifier(identifier, true),
     );
   }
   const nodes = new Map<
@@ -616,9 +628,11 @@ function reduceMaybeOptionalChains(
     changed = false;
 
     for (const original of optionalChainNodes) {
-      let {identifier, path: origPath} = original.fullPath;
-      let currNode: PropertyPathNode =
-        registry.getOrCreateIdentifier(identifier);
+      let {identifier, path: origPath, reactive} = original.fullPath;
+      let currNode: PropertyPathNode = registry.getOrCreateIdentifier(
+        identifier,
+        reactive,
+      );
       for (let i = 0; i < origPath.length; i++) {
         const entry = origPath[i];
         // If the base is known to be non-null, replace with a non-optional load

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

@@ -290,6 +290,7 @@ function traverseOptionalBlock(
     );
     baseObject = {
       identifier: maybeTest.instructions[0].value.place.identifier,
+      reactive: maybeTest.instructions[0].value.place.reactive,
       path,
     };
     test = maybeTest.terminal;
@@ -391,6 +392,7 @@ function traverseOptionalBlock(
   );
   const load = {
     identifier: baseObject.identifier,
+    reactive: baseObject.reactive,
     path: [
       ...baseObject.path,
       {

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

@@ -25,8 +25,9 @@ export class ReactiveScopeDependencyTreeHIR {
    * `identifier.path`, or `identifier?.path` is in this map, it is safe to
    * evaluate (non-optional) PropertyLoads from.
    */
-  #hoistableObjects: Map<Identifier, HoistableNode> = new Map();
-  #deps: Map<Identifier, DependencyNode> = new Map();
+  #hoistableObjects: Map<Identifier, HoistableNode & {reactive: boolean}> =
+    new Map();
+  #deps: Map<Identifier, DependencyNode & {reactive: boolean}> = new Map();
 
   /**
    * @param hoistableObjects a set of paths from which we can safely evaluate
@@ -35,9 +36,10 @@ export class ReactiveScopeDependencyTreeHIR {
    * duplicates when traversing the CFG.
    */
   constructor(hoistableObjects: Iterable<ReactiveScopeDependency>) {
-    for (const {path, identifier} of hoistableObjects) {
+    for (const {path, identifier, reactive} of hoistableObjects) {
       let currNode = ReactiveScopeDependencyTreeHIR.#getOrCreateRoot(
         identifier,
+        reactive,
         this.#hoistableObjects,
         path.length > 0 && path[0].optional ? 'Optional' : 'NonNull',
       );
@@ -70,7 +72,8 @@ export class ReactiveScopeDependencyTreeHIR {
 
   static #getOrCreateRoot<T extends string>(
     identifier: Identifier,
-    roots: Map<Identifier, TreeNode<T>>,
+    reactive: boolean,
+    roots: Map<Identifier, TreeNode<T> & {reactive: boolean}>,
     defaultAccessType: T,
   ): TreeNode<T> {
     // roots can always be accessed unconditionally in JS
@@ -79,9 +82,16 @@ export class ReactiveScopeDependencyTreeHIR {
     if (rootNode === undefined) {
       rootNode = {
         properties: new Map(),
+        reactive,
         accessType: defaultAccessType,
       };
       roots.set(identifier, rootNode);
+    } else {
+      CompilerError.invariant(reactive === rootNode.reactive, {
+        reason: '[DeriveMinimalDependenciesHIR] Conflicting reactive root flag',
+        description: `Identifier ${printIdentifier(identifier)}`,
+        loc: GeneratedSource,
+      });
     }
     return rootNode;
   }
@@ -92,9 +102,10 @@ export class ReactiveScopeDependencyTreeHIR {
    * safe-to-evaluate subpath
    */
   addDependency(dep: ReactiveScopeDependency): void {
-    const {identifier, path} = dep;
+    const {identifier, reactive, path} = dep;
     let depCursor = ReactiveScopeDependencyTreeHIR.#getOrCreateRoot(
       identifier,
+      reactive,
       this.#deps,
       PropertyAccessType.UnconditionalAccess,
     );
@@ -172,7 +183,13 @@ export class ReactiveScopeDependencyTreeHIR {
   deriveMinimalDependencies(): Set<ReactiveScopeDependency> {
     const results = new Set<ReactiveScopeDependency>();
     for (const [rootId, rootNode] of this.#deps.entries()) {
-      collectMinimalDependenciesInSubtree(rootNode, rootId, [], results);
+      collectMinimalDependenciesInSubtree(
+        rootNode,
+        rootNode.reactive,
+        rootId,
+        [],
+        results,
+      );
     }
 
     return results;
@@ -294,25 +311,24 @@ type HoistableNode = TreeNode<'Optional' | 'NonNull'>;
 type DependencyNode = TreeNode<PropertyAccessType>;
 
 /**
- * TODO: this is directly pasted from DeriveMinimalDependencies. Since we no
- * longer have conditionally accessed nodes, we can simplify
- *
  * Recursively calculates minimal dependencies in a subtree.
  * @param node DependencyNode representing a dependency subtree.
  * @returns a minimal list of dependencies in this subtree.
  */
 function collectMinimalDependenciesInSubtree(
   node: DependencyNode,
+  reactive: boolean,
   rootIdentifier: Identifier,
   path: Array<DependencyPathEntry>,
   results: Set<ReactiveScopeDependency>,
 ): void {
   if (isDependency(node.accessType)) {
-    results.add({identifier: rootIdentifier, path});
+    results.add({identifier: rootIdentifier, reactive, path});
   } else {
     for (const [childName, childNode] of node.properties) {
       collectMinimalDependenciesInSubtree(
         childNode,
+        reactive,
         rootIdentifier,
         [
           ...path,

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

@@ -47,7 +47,7 @@ import {
   ShapeRegistry,
   addHook,
 } from './ObjectShape';
-import {Scope as BabelScope} from '@babel/traverse';
+import {Scope as BabelScope, NodePath} from '@babel/traverse';
 import {TypeSchema} from './TypeSchema';
 
 export const ReactElementSymbolSchema = z.object({
@@ -675,6 +675,7 @@ export class Environment {
 
   #contextIdentifiers: Set<t.Identifier>;
   #hoistedIdentifiers: Set<t.Identifier>;
+  parentFunction: NodePath<t.Function>;
 
   constructor(
     scope: BabelScope,
@@ -682,6 +683,7 @@ export class Environment {
     compilerMode: CompilerMode,
     config: EnvironmentConfig,
     contextIdentifiers: Set<t.Identifier>,
+    parentFunction: NodePath<t.Function>, // the outermost function being compiled
     logger: Logger | null,
     filename: string | null,
     code: string | null,
@@ -740,6 +742,7 @@ export class Environment {
       this.#moduleTypes.set(REANIMATED_MODULE_NAME, reanimatedModuleType);
     }
 
+    this.parentFunction = parentFunction;
     this.#contextIdentifiers = contextIdentifiers;
     this.#hoistedIdentifiers = new Set();
   }

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

@@ -1568,6 +1568,18 @@ export type DependencyPathEntry = {
 export type DependencyPath = Array<DependencyPathEntry>;
 export type ReactiveScopeDependency = {
   identifier: Identifier;
+  /**
+   * Reflects whether the base identifier is reactive. Note that some reactive
+   * objects may have non-reactive properties, but we do not currently track
+   * this.
+   *
+   * ```js
+   * // Technically, result[0] is reactive and result[1] is not.
+   * // Currently, both dependencies would be marked as reactive.
+   * const result = useState();
+   * ```
+   */
+  reactive: boolean;
   path: DependencyPath;
 };
 

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

@@ -110,7 +110,6 @@ export default class HIRBuilder {
   #bindings: Bindings;
   #env: Environment;
   #exceptionHandlerStack: Array<BlockId> = [];
-  parentFunction: NodePath<t.Function>;
   errors: CompilerError = new CompilerError();
   /**
    * Traversal context: counts the number of `fbt` tag parents
@@ -136,16 +135,17 @@ export default class HIRBuilder {
 
   constructor(
     env: Environment,
-    parentFunction: NodePath<t.Function>, // the outermost function being compiled
-    bindings: Bindings | null = null,
-    context: Array<t.Identifier> | null = null,
+    options?: {
+      bindings?: Bindings | null;
+      context?: Array<t.Identifier>;
+      entryBlockKind?: BlockKind;
+    },
   ) {
     this.#env = env;
-    this.#bindings = bindings ?? new Map();
-    this.parentFunction = parentFunction;
-    this.#context = context ?? [];
+    this.#bindings = options?.bindings ?? new Map();
+    this.#context = options?.context ?? [];
     this.#entry = makeBlockId(env.nextBlockId);
-    this.#current = newBlock(this.#entry, 'block');
+    this.#current = newBlock(this.#entry, options?.entryBlockKind ?? 'block');
   }
 
   currentBlockKind(): BlockKind {
@@ -239,7 +239,7 @@ export default class HIRBuilder {
 
     // Check if the binding is from module scope
     const outerBinding =
-      this.parentFunction.scope.parent.getBinding(originalName);
+      this.#env.parentFunction.scope.parent.getBinding(originalName);
     if (babelBinding === outerBinding) {
       const path = babelBinding.path;
       if (path.isImportDefaultSpecifier()) {
@@ -293,7 +293,7 @@ export default class HIRBuilder {
     const binding = this.#resolveBabelBinding(path);
     if (binding) {
       // Check if the binding is from module scope, if so return null
-      const outerBinding = this.parentFunction.scope.parent.getBinding(
+      const outerBinding = this.#env.parentFunction.scope.parent.getBinding(
         path.node.name,
       );
       if (binding === outerBinding) {
@@ -376,7 +376,7 @@ export default class HIRBuilder {
   }
 
   // Terminate the current block w the given terminal, and start a new block
-  terminate(terminal: Terminal, nextBlockKind: BlockKind | null): void {
+  terminate(terminal: Terminal, nextBlockKind: BlockKind | null): BlockId {
     const {id: blockId, kind, instructions} = this.#current;
     this.#completed.set(blockId, {
       kind,
@@ -390,6 +390,7 @@ export default class HIRBuilder {
       const nextId = this.#env.nextBlockId;
       this.#current = newBlock(nextId, nextBlockKind);
     }
+    return blockId;
   }
 
   /*
@@ -746,6 +747,11 @@ function getReversePostorderedBlocks(func: HIR): HIR['blocks'] {
      * (eg bb2 then bb1), we ensure that they get reversed back to the correct order.
      */
     const block = func.blocks.get(blockId)!;
+    CompilerError.invariant(block != null, {
+      reason: '[HIRBuilder] Unexpected null block',
+      description: `expected block ${blockId} to exist`,
+      loc: GeneratedSource,
+    });
     const successors = [...eachTerminalSuccessor(block.terminal)].reverse();
     const fallthrough = terminalFallthrough(block.terminal);
 

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

@@ -316,6 +316,7 @@ function collectTemporariesSidemapImpl(
         ) {
           temporaries.set(lvalue.identifier.id, {
             identifier: value.place.identifier,
+            reactive: value.place.reactive,
             path: [],
           });
         }
@@ -369,11 +370,13 @@ function getProperty(
   if (resolvedDependency == null) {
     property = {
       identifier: object.identifier,
+      reactive: object.reactive,
       path: [{property: propertyName, optional}],
     };
   } else {
     property = {
       identifier: resolvedDependency.identifier,
+      reactive: resolvedDependency.reactive,
       path: [...resolvedDependency.path, {property: propertyName, optional}],
     };
   }
@@ -532,6 +535,7 @@ export class DependencyCollectionContext {
     this.visitDependency(
       this.#temporaries.get(place.identifier.id) ?? {
         identifier: place.identifier,
+        reactive: place.reactive,
         path: [],
       },
     );
@@ -596,6 +600,7 @@ export class DependencyCollectionContext {
     ) {
       maybeDependency = {
         identifier: maybeDependency.identifier,
+        reactive: maybeDependency.reactive,
         path: [],
       };
     }
@@ -617,7 +622,11 @@ export class DependencyCollectionContext {
         identifier =>
           identifier.declarationId === place.identifier.declarationId,
       ) &&
-      this.#checkValidDependency({identifier: place.identifier, path: []})
+      this.#checkValidDependency({
+        identifier: place.identifier,
+        reactive: place.reactive,
+        path: [],
+      })
     ) {
       currentScope.reassignments.add(place.identifier);
     }

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

@@ -0,0 +1,283 @@
+import {
+  Place,
+  ReactiveScopeDependency,
+  Identifier,
+  makeInstructionId,
+  InstructionKind,
+  GeneratedSource,
+  BlockId,
+  makeTemporaryIdentifier,
+  Effect,
+  GotoVariant,
+  HIR,
+} from './HIR';
+import {CompilerError} from '../CompilerError';
+import {Environment} from './Environment';
+import HIRBuilder from './HIRBuilder';
+import {lowerValueToTemporary} from './BuildHIR';
+
+type DependencyInstructions = {
+  place: Place;
+  value: HIR;
+  exitBlockId: BlockId;
+};
+
+export function buildDependencyInstructions(
+  dep: ReactiveScopeDependency,
+  env: Environment,
+): DependencyInstructions {
+  const builder = new HIRBuilder(env, {
+    entryBlockKind: 'value',
+  });
+  let dependencyValue: Identifier;
+  if (dep.path.every(path => !path.optional)) {
+    dependencyValue = writeNonOptionalDependency(dep, env, builder);
+  } else {
+    dependencyValue = writeOptionalDependency(dep, builder, null);
+  }
+
+  const exitBlockId = builder.terminate(
+    {
+      kind: 'unsupported',
+      loc: GeneratedSource,
+      id: makeInstructionId(0),
+    },
+    null,
+  );
+  return {
+    place: {
+      kind: 'Identifier',
+      identifier: dependencyValue,
+      effect: Effect.Freeze,
+      reactive: dep.reactive,
+      loc: GeneratedSource,
+    },
+    value: builder.build(),
+    exitBlockId,
+  };
+}
+
+/**
+ * Write instructions for a simple dependency (without optional chains)
+ */
+function writeNonOptionalDependency(
+  dep: ReactiveScopeDependency,
+  env: Environment,
+  builder: HIRBuilder,
+): Identifier {
+  const loc = dep.identifier.loc;
+  let curr: Identifier = makeTemporaryIdentifier(env.nextIdentifierId, loc);
+  builder.push({
+    lvalue: {
+      identifier: curr,
+      kind: 'Identifier',
+      effect: Effect.Mutate,
+      reactive: dep.reactive,
+      loc,
+    },
+    value: {
+      kind: 'LoadLocal',
+      place: {
+        identifier: dep.identifier,
+        kind: 'Identifier',
+        effect: Effect.Freeze,
+        reactive: dep.reactive,
+        loc,
+      },
+      loc,
+    },
+    id: makeInstructionId(1),
+    loc: loc,
+  });
+
+  /**
+   * Iteratively build up dependency instructions by reading from the last written
+   * instruction.
+   */
+  for (const path of dep.path) {
+    const next = makeTemporaryIdentifier(env.nextIdentifierId, loc);
+    builder.push({
+      lvalue: {
+        identifier: next,
+        kind: 'Identifier',
+        effect: Effect.Mutate,
+        reactive: dep.reactive,
+        loc,
+      },
+      value: {
+        kind: 'PropertyLoad',
+        object: {
+          identifier: curr,
+          kind: 'Identifier',
+          effect: Effect.Freeze,
+          reactive: dep.reactive,
+          loc,
+        },
+        property: path.property,
+        loc,
+      },
+      id: makeInstructionId(1),
+      loc: loc,
+    });
+    curr = next;
+  }
+  return curr;
+}
+
+/**
+ * Write a dependency into optional blocks.
+ *
+ * e.g. `a.b?.c.d` is written to an optional block that tests `a.b` and
+ * conditionally evaluates `c.d`.
+ */
+function writeOptionalDependency(
+  dep: ReactiveScopeDependency,
+  builder: HIRBuilder,
+  parentAlternate: BlockId | null,
+): Identifier {
+  const env = builder.environment;
+  /**
+   * Reserve an identifier which will be used to store the result of this
+   * dependency.
+   */
+  const dependencyValue: Place = {
+    kind: 'Identifier',
+    identifier: makeTemporaryIdentifier(env.nextIdentifierId, GeneratedSource),
+    effect: Effect.Mutate,
+    reactive: dep.reactive,
+    loc: GeneratedSource,
+  };
+
+  /**
+   * Reserve a block which is the fallthrough (and transitive successor) of this
+   * optional chain.
+   */
+  const continuationBlock = builder.reserve(builder.currentBlockKind());
+  let alternate;
+  if (parentAlternate != null) {
+    alternate = parentAlternate;
+  } else {
+    /**
+     * If an outermost alternate block has not been reserved, write one
+     *
+     * $N = Primitive undefined
+     * $M = StoreLocal $OptionalResult = $N
+     * goto fallthrough
+     */
+    alternate = builder.enter('value', () => {
+      const temp = lowerValueToTemporary(builder, {
+        kind: 'Primitive',
+        value: undefined,
+        loc: GeneratedSource,
+      });
+      lowerValueToTemporary(builder, {
+        kind: 'StoreLocal',
+        lvalue: {kind: InstructionKind.Const, place: {...dependencyValue}},
+        value: {...temp},
+        type: null,
+        loc: GeneratedSource,
+      });
+      return {
+        kind: 'goto',
+        variant: GotoVariant.Break,
+        block: continuationBlock.id,
+        id: makeInstructionId(0),
+        loc: GeneratedSource,
+      };
+    });
+  }
+
+  // Reserve the consequent block, which is the successor of the test block
+  const consequent = builder.reserve('value');
+
+  let testIdentifier: Identifier | null = null;
+  const testBlock = builder.enter('value', () => {
+    const testDependency = {
+      ...dep,
+      path: dep.path.slice(0, dep.path.length - 1),
+    };
+    const firstOptional = dep.path.findIndex(path => path.optional);
+    CompilerError.invariant(firstOptional !== -1, {
+      reason:
+        '[ScopeDependencyUtils] Internal invariant broken: expected optional path',
+      loc: dep.identifier.loc,
+      description: null,
+      suggestions: null,
+    });
+    if (firstOptional === dep.path.length - 1) {
+      // Base case: the test block is simple
+      testIdentifier = writeNonOptionalDependency(testDependency, env, builder);
+    } else {
+      // Otherwise, the test block is a nested optional chain
+      testIdentifier = writeOptionalDependency(
+        testDependency,
+        builder,
+        alternate,
+      );
+    }
+
+    return {
+      kind: 'branch',
+      test: {
+        identifier: testIdentifier,
+        effect: Effect.Freeze,
+        kind: 'Identifier',
+        loc: GeneratedSource,
+        reactive: dep.reactive,
+      },
+      consequent: consequent.id,
+      alternate,
+      id: makeInstructionId(0),
+      loc: GeneratedSource,
+      fallthrough: continuationBlock.id,
+    };
+  });
+
+  builder.enterReserved(consequent, () => {
+    CompilerError.invariant(testIdentifier !== null, {
+      reason: 'Satisfy type checker',
+      description: null,
+      loc: null,
+      suggestions: null,
+    });
+
+    lowerValueToTemporary(builder, {
+      kind: 'StoreLocal',
+      lvalue: {kind: InstructionKind.Const, place: {...dependencyValue}},
+      value: lowerValueToTemporary(builder, {
+        kind: 'PropertyLoad',
+        object: {
+          identifier: testIdentifier,
+          kind: 'Identifier',
+          effect: Effect.Freeze,
+          reactive: dep.reactive,
+          loc: GeneratedSource,
+        },
+        property: dep.path.at(-1)!.property,
+        loc: GeneratedSource,
+      }),
+      type: null,
+      loc: GeneratedSource,
+    });
+    return {
+      kind: 'goto',
+      variant: GotoVariant.Break,
+      block: continuationBlock.id,
+      id: makeInstructionId(0),
+      loc: GeneratedSource,
+    };
+  });
+  builder.terminateWithContinuation(
+    {
+      kind: 'optional',
+      optional: dep.path.at(-1)!.optional,
+      test: testBlock,
+      fallthrough: continuationBlock.id,
+      id: makeInstructionId(0),
+      loc: GeneratedSource,
+    },
+    continuationBlock,
+  );
+
+  return dependencyValue.identifier;
+}

compiler/packages/babel-plugin-react-compiler/src/Inference/InferEffectDependencies.ts

@@ -10,7 +10,6 @@ import {CompilerError, SourceLocation} from '..';
 import {
   ArrayExpression,
   Effect,
-  Environment,
   FunctionExpression,
   GeneratedSource,
   HIRFunction,
@@ -29,6 +28,9 @@ import {
   isSetStateType,
   isFireFunctionType,
   makeScopeId,
+  HIR,
+  BasicBlock,
+  BlockId,
 } from '../HIR';
 import {collectHoistablePropertyLoadsInInnerFn} from '../HIR/CollectHoistablePropertyLoads';
 import {collectOptionalChainSidemap} from '../HIR/CollectOptionalChainDependencies';
@@ -38,13 +40,20 @@ import {
   createTemporaryPlace,
   fixScopeAndIdentifierRanges,
   markInstructionIds,
+  markPredecessors,
+  reversePostorderBlocks,
 } from '../HIR/HIRBuilder';
 import {
   collectTemporariesSidemap,
   DependencyCollectionContext,
   handleInstruction,
 } from '../HIR/PropagateScopeDependenciesHIR';
-import {eachInstructionOperand, eachTerminalOperand} from '../HIR/visitors';
+import {buildDependencyInstructions} from '../HIR/ScopeDependencyUtils';
+import {
+  eachInstructionOperand,
+  eachTerminalOperand,
+  terminalFallthrough,
+} from '../HIR/visitors';
 import {empty} from '../Utils/Stack';
 import {getOrInsertWith} from '../Utils/utils';
 
@@ -53,7 +62,6 @@ import {getOrInsertWith} from '../Utils/utils';
  * a second argument to the useEffect call if no dependency array is provided.
  */
 export function inferEffectDependencies(fn: HIRFunction): void {
-  let hasRewrite = false;
   const fnExpressions = new Map<
     IdentifierId,
     TInstruction<FunctionExpression>
@@ -86,6 +94,7 @@ export function inferEffectDependencies(fn: HIRFunction): void {
    * reactive(Identifier i) = Union_{reference of i}(reactive(reference))
    */
   const reactiveIds = inferReactiveIdentifiers(fn);
+  const rewriteBlocks: Array<BasicBlock> = [];
 
   for (const [, block] of fn.body.blocks) {
     if (block.terminal.kind === 'scope') {
@@ -101,7 +110,7 @@ export function inferEffectDependencies(fn: HIRFunction): void {
         );
       }
     }
-    const rewriteInstrs = new Map<InstructionId, Array<Instruction>>();
+    const rewriteInstrs: Array<SpliceInfo> = [];
     for (const instr of block.instructions) {
       const {value, lvalue} = instr;
       if (value.kind === 'FunctionExpression') {
@@ -165,7 +174,6 @@ export function inferEffectDependencies(fn: HIRFunction): void {
         ) {
           // We have a useEffect call with no deps array, so we need to infer the deps
           const effectDeps: Array<Place> = [];
-          const newInstructions: Array<Instruction> = [];
           const deps: ArrayExpression = {
             kind: 'ArrayExpression',
             elements: effectDeps,
@@ -196,24 +204,28 @@ export function inferEffectDependencies(fn: HIRFunction): void {
              */
 
             const usedDeps = [];
-            for (const dep of minimalDeps) {
+            for (const maybeDep of minimalDeps) {
               if (
-                ((isUseRefType(dep.identifier) ||
-                  isSetStateType(dep.identifier)) &&
-                  !reactiveIds.has(dep.identifier.id)) ||
-                isFireFunctionType(dep.identifier)
+                ((isUseRefType(maybeDep.identifier) ||
+                  isSetStateType(maybeDep.identifier)) &&
+                  !reactiveIds.has(maybeDep.identifier.id)) ||
+                isFireFunctionType(maybeDep.identifier)
               ) {
                 // exclude non-reactive hook results, which will never be in a memo block
                 continue;
               }
 
-              const {place, instructions} = writeDependencyToInstructions(
+              const dep = truncateDepAtCurrent(maybeDep);
+              const {place, value, exitBlockId} = buildDependencyInstructions(
                 dep,
-                reactiveIds.has(dep.identifier.id),
                 fn.env,
-                fnExpr.loc,
               );
-              newInstructions.push(...instructions);
+              rewriteInstrs.push({
+                kind: 'block',
+                location: instr.id,
+                value,
+                exitBlockId: exitBlockId,
+              });
               effectDeps.push(place);
               usedDeps.push(dep);
             }
@@ -234,27 +246,32 @@ export function inferEffectDependencies(fn: HIRFunction): void {
               });
             }
 
-            newInstructions.push({
-              id: makeInstructionId(0),
-              loc: GeneratedSource,
-              lvalue: {...depsPlace, effect: Effect.Mutate},
-              value: deps,
-            });
-
             // Step 2: push the inferred deps array as an argument of the useEffect
+            rewriteInstrs.push({
+              kind: 'instr',
+              location: instr.id,
+              value: {
+                id: makeInstructionId(0),
+                loc: GeneratedSource,
+                lvalue: {...depsPlace, effect: Effect.Mutate},
+                value: deps,
+              },
+            });
             value.args.push({...depsPlace, effect: Effect.Freeze});
-            rewriteInstrs.set(instr.id, newInstructions);
             fn.env.inferredEffectLocations.add(callee.loc);
           } else if (loadGlobals.has(value.args[0].identifier.id)) {
             // Global functions have no reactive dependencies, so we can insert an empty array
-            newInstructions.push({
-              id: makeInstructionId(0),
-              loc: GeneratedSource,
-              lvalue: {...depsPlace, effect: Effect.Mutate},
-              value: deps,
+            rewriteInstrs.push({
+              kind: 'instr',
+              location: instr.id,
+              value: {
+                id: makeInstructionId(0),
+                loc: GeneratedSource,
+                lvalue: {...depsPlace, effect: Effect.Mutate},
+                value: deps,
+              },
             });
             value.args.push({...depsPlace, effect: Effect.Freeze});
-            rewriteInstrs.set(instr.id, newInstructions);
             fn.env.inferredEffectLocations.add(callee.loc);
           }
         } else if (
@@ -285,85 +302,164 @@ export function inferEffectDependencies(fn: HIRFunction): void {
         }
       }
     }
-    if (rewriteInstrs.size > 0) {
-      hasRewrite = true;
-      const newInstrs = [];
-      for (const instr of block.instructions) {
-        const newInstr = rewriteInstrs.get(instr.id);
-        if (newInstr != null) {
-          newInstrs.push(...newInstr, instr);
-        } else {
-          newInstrs.push(instr);
-        }
-      }
-      block.instructions = newInstrs;
-    }
+    rewriteSplices(block, rewriteInstrs, rewriteBlocks);
   }
-  if (hasRewrite) {
+
+  if (rewriteBlocks.length > 0) {
+    for (const block of rewriteBlocks) {
+      fn.body.blocks.set(block.id, block);
+    }
+
+    /**
+     * Fixup the HIR to restore RPO, ensure correct predecessors, and renumber
+     * instructions.
+     */
+    reversePostorderBlocks(fn.body);
+    markPredecessors(fn.body);
     // Renumber instructions and fix scope ranges
     markInstructionIds(fn.body);
     fixScopeAndIdentifierRanges(fn.body);
+
     fn.env.hasInferredEffect = true;
   }
 }
 
-function writeDependencyToInstructions(
+function truncateDepAtCurrent(
   dep: ReactiveScopeDependency,
-  reactive: boolean,
-  env: Environment,
-  loc: SourceLocation,
-): {place: Place; instructions: Array<Instruction>} {
-  const instructions: Array<Instruction> = [];
-  let currValue = createTemporaryPlace(env, GeneratedSource);
-  currValue.reactive = reactive;
-  instructions.push({
-    id: makeInstructionId(0),
-    loc: GeneratedSource,
-    lvalue: {...currValue, effect: Effect.Mutate},
-    value: {
-      kind: 'LoadLocal',
-      place: {
-        kind: 'Identifier',
-        identifier: dep.identifier,
-        effect: Effect.Capture,
-        reactive,
-        loc: loc,
-      },
-      loc: loc,
-    },
-  });
-  for (const path of dep.path) {
-    if (path.optional) {
-      /**
-       * TODO: instead of truncating optional paths, reuse
-       * instructions from hoisted dependencies block(s)
-       */
-      break;
-    }
-    if (path.property === 'current') {
-      /*
-       * Prune ref.current accesses. This may over-capture for non-ref values with
-       * a current property, but that's fine.
-       */
-      break;
-    }
-    const nextValue = createTemporaryPlace(env, GeneratedSource);
-    nextValue.reactive = reactive;
-    instructions.push({
-      id: makeInstructionId(0),
-      loc: GeneratedSource,
-      lvalue: {...nextValue, effect: Effect.Mutate},
-      value: {
-        kind: 'PropertyLoad',
-        object: {...currValue, effect: Effect.Capture},
-        property: path.property,
-        loc: loc,
-      },
-    });
-    currValue = nextValue;
+): ReactiveScopeDependency {
+  const idx = dep.path.findIndex(path => path.property === 'current');
+  if (idx === -1) {
+    return dep;
+  } else {
+    return {...dep, path: dep.path.slice(0, idx)};
   }
-  currValue.effect = Effect.Freeze;
-  return {place: currValue, instructions};
+}
+
+type SpliceInfo =
+  | {kind: 'instr'; location: InstructionId; value: Instruction}
+  | {
+      kind: 'block';
+      location: InstructionId;
+      value: HIR;
+      exitBlockId: BlockId;
+    };
+
+function rewriteSplices(
+  originalBlock: BasicBlock,
+  splices: Array<SpliceInfo>,
+  rewriteBlocks: Array<BasicBlock>,
+): void {
+  if (splices.length === 0) {
+    return;
+  }
+  /**
+   * Splice instructions or value blocks into the original block.
+   * --- original block ---
+   * bb_original
+   *   instr1
+   *   ...
+   *   instr2 <-- splice location
+   *   instr3
+   *   ...
+   *   <original terminal>
+   *
+   * If there is more than one block in the splice, this means that we're
+   * splicing in a set of value-blocks of the following structure:
+   * --- blocks we're splicing in ---
+   * bb_entry:
+   *   instrEntry
+   *   ...
+   *   <splice terminal> fallthrough=bb_exit
+   *
+   * bb1(value):
+   *   ...
+   *
+   * bb_exit:
+   *   instrExit
+   *   ...
+   *   <synthetic terminal>
+   *
+   *
+   * --- rewritten blocks ---
+   * bb_original
+   *   instr1
+   *   ... (original instructions)
+   *   instr2
+   *   instrEntry
+   *   ... (spliced instructions)
+   *   <splice terminal> fallthrough=bb_exit
+   *
+   * bb1(value):
+   *   ...
+   *
+   * bb_exit:
+   *   instrExit
+   *   ... (spliced instructions)
+   *   instr3
+   *   ... (original instructions)
+   *   <original terminal>
+   */
+  const originalInstrs = originalBlock.instructions;
+  let currBlock: BasicBlock = {...originalBlock, instructions: []};
+  rewriteBlocks.push(currBlock);
+
+  let cursor = 0;
+  for (const rewrite of splices) {
+    while (originalInstrs[cursor].id < rewrite.location) {
+      CompilerError.invariant(
+        originalInstrs[cursor].id < originalInstrs[cursor + 1].id,
+        {
+          reason:
+            '[InferEffectDependencies] Internal invariant broken: expected block instructions to be sorted',
+          loc: originalInstrs[cursor].loc,
+        },
+      );
+      currBlock.instructions.push(originalInstrs[cursor]);
+      cursor++;
+    }
+    CompilerError.invariant(originalInstrs[cursor].id === rewrite.location, {
+      reason:
+        '[InferEffectDependencies] Internal invariant broken: splice location not found',
+      loc: originalInstrs[cursor].loc,
+    });
+
+    if (rewrite.kind === 'instr') {
+      currBlock.instructions.push(rewrite.value);
+    } else {
+      const {entry, blocks} = rewrite.value;
+      const entryBlock = blocks.get(entry)!;
+      // splice in all instructions from the entry block
+      currBlock.instructions.push(...entryBlock.instructions);
+      if (blocks.size > 1) {
+        /**
+         * We're splicing in a set of value-blocks, which means we need
+         * to push new blocks and update terminals.
+         */
+        CompilerError.invariant(
+          terminalFallthrough(entryBlock.terminal) === rewrite.exitBlockId,
+          {
+            reason:
+              '[InferEffectDependencies] Internal invariant broken: expected entry block to have a fallthrough',
+            loc: entryBlock.terminal.loc,
+          },
+        );
+        const originalTerminal = currBlock.terminal;
+        currBlock.terminal = entryBlock.terminal;
+
+        for (const [id, block] of blocks) {
+          if (id === entry) {
+            continue;
+          }
+          if (id === rewrite.exitBlockId) {
+            block.terminal = originalTerminal;
+            currBlock = block;
+          }
+          rewriteBlocks.push(block);
+        }
+      }
+    }
+  }
+  currBlock.instructions.push(...originalInstrs.slice(cursor));
 }
 
 function inferReactiveIdentifiers(fn: HIRFunction): Set<IdentifierId> {

compiler/packages/babel-plugin-react-compiler/src/Inference/InferReactivePlaces.ts

@@ -26,6 +26,7 @@ import {
 import {PostDominator} from '../HIR/Dominator';
 import {
   eachInstructionLValue,
+  eachInstructionOperand,
   eachInstructionValueOperand,
   eachTerminalOperand,
 } from '../HIR/visitors';
@@ -292,7 +293,7 @@ export function inferReactivePlaces(fn: HIRFunction): void {
         let hasReactiveInput = false;
         /*
          * NOTE: we want to mark all operands as reactive or not, so we
-         * avoid short-circuting here
+         * avoid short-circuiting here
          */
         for (const operand of eachInstructionValueOperand(value)) {
           const reactive = reactiveIdentifiers.isReactive(operand);
@@ -375,6 +376,41 @@ export function inferReactivePlaces(fn: HIRFunction): void {
       }
     }
   } while (reactiveIdentifiers.snapshot());
+
+  function propagateReactivityToInnerFunctions(
+    fn: HIRFunction,
+    isOutermost: boolean,
+  ): void {
+    for (const [, block] of fn.body.blocks) {
+      for (const instr of block.instructions) {
+        if (!isOutermost) {
+          for (const operand of eachInstructionOperand(instr)) {
+            reactiveIdentifiers.isReactive(operand);
+          }
+        }
+        if (
+          instr.value.kind === 'ObjectMethod' ||
+          instr.value.kind === 'FunctionExpression'
+        ) {
+          propagateReactivityToInnerFunctions(
+            instr.value.loweredFunc.func,
+            false,
+          );
+        }
+      }
+      if (!isOutermost) {
+        for (const operand of eachTerminalOperand(block.terminal)) {
+          reactiveIdentifiers.isReactive(operand);
+        }
+      }
+    }
+  }
+
+  /**
+   * Propagate reactivity for inner functions, as we eventually hoist and dedupe
+   * dependency instructions for scopes.
+   */
+  propagateReactivityToInnerFunctions(fn, true);
 }
 
 /*

compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/MergeReactiveScopesThatInvalidateTogether.ts

@@ -456,6 +456,7 @@ function canMergeScopes(
       new Set(
         [...current.scope.declarations.values()].map(declaration => ({
           identifier: declaration.identifier,
+          reactive: true,
           path: [],
         })),
       ),

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies/bailout-retry/mutate-after-useeffect-optional-chain.expect.md

@@ -0,0 +1,58 @@
+
+## Input
+
+```javascript
+// @inferEffectDependencies @panicThreshold:"none" @loggerTestOnly
+import {useEffect} from 'react';
+import {print} from 'shared-runtime';
+
+function Component({foo}) {
+  const arr = [];
+  // Taking either arr[0].value or arr as a dependency is reasonable
+  // as long as developers know what to expect.
+  useEffect(() => print(arr[0]?.value));
+  arr.push({value: foo});
+  return arr;
+}
+
+export const FIXTURE_ENTRYPOINT = {
+  fn: Component,
+  params: [{foo: 1}],
+};
+
+```
+
+## Code
+
+```javascript
+// @inferEffectDependencies @panicThreshold:"none" @loggerTestOnly
+import { useEffect } from "react";
+import { print } from "shared-runtime";
+
+function Component(t0) {
+  const { foo } = t0;
+  const arr = [];
+
+  useEffect(() => print(arr[0]?.value), [arr[0]?.value]);
+  arr.push({ value: foo });
+  return arr;
+}
+
+export const FIXTURE_ENTRYPOINT = {
+  fn: Component,
+  params: [{ foo: 1 }],
+};
+
+```
+
+## Logs
+
+```
+{"kind":"CompileError","fnLoc":{"start":{"line":5,"column":0,"index":139},"end":{"line":12,"column":1,"index":384},"filename":"mutate-after-useeffect-optional-chain.ts"},"detail":{"reason":"This mutates a variable that React considers immutable","description":null,"loc":{"start":{"line":10,"column":2,"index":345},"end":{"line":10,"column":5,"index":348},"filename":"mutate-after-useeffect-optional-chain.ts","identifierName":"arr"},"suggestions":null,"severity":"InvalidReact"}}
+{"kind":"AutoDepsDecorations","fnLoc":{"start":{"line":9,"column":2,"index":304},"end":{"line":9,"column":39,"index":341},"filename":"mutate-after-useeffect-optional-chain.ts"},"decorations":[{"start":{"line":9,"column":24,"index":326},"end":{"line":9,"column":27,"index":329},"filename":"mutate-after-useeffect-optional-chain.ts","identifierName":"arr"}]}
+{"kind":"CompileSuccess","fnLoc":{"start":{"line":5,"column":0,"index":139},"end":{"line":12,"column":1,"index":384},"filename":"mutate-after-useeffect-optional-chain.ts"},"fnName":"Component","memoSlots":0,"memoBlocks":0,"memoValues":0,"prunedMemoBlocks":0,"prunedMemoValues":0}
+```
+      
+### Eval output
+(kind: ok) [{"value":1}]
+logs: [1]

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies/bailout-retry/mutate-after-useeffect-optional-chain.js

@@ -0,0 +1,17 @@
+// @inferEffectDependencies @panicThreshold:"none" @loggerTestOnly
+import {useEffect} from 'react';
+import {print} from 'shared-runtime';
+
+function Component({foo}) {
+  const arr = [];
+  // Taking either arr[0].value or arr as a dependency is reasonable
+  // as long as developers know what to expect.
+  useEffect(() => print(arr[0]?.value));
+  arr.push({value: foo});
+  return arr;
+}
+
+export const FIXTURE_ENTRYPOINT = {
+  fn: Component,
+  params: [{foo: 1}],
+};

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies/bailout-retry/mutate-after-useeffect-ref-access.expect.md

@@ -2,7 +2,7 @@
 ## Input
 
 ```javascript
-// @inferEffectDependencies @panicThreshold:"none"
+// @inferEffectDependencies @panicThreshold:"none" @loggerTestOnly
 
 import {useEffect, useRef} from 'react';
 import {print} from 'shared-runtime';
@@ -14,12 +14,17 @@ function Component({arrRef}) {
   return arrRef;
 }
 
+export const FIXTURE_ENTRYPOINT = {
+  fn: Component,
+  params: [{arrRef: {current: {val: 'initial ref value'}}}],
+};
+
 ```
 
 ## Code
 
 ```javascript
-// @inferEffectDependencies @panicThreshold:"none"
+// @inferEffectDependencies @panicThreshold:"none" @loggerTestOnly
 
 import { useEffect, useRef } from "react";
 import { print } from "shared-runtime";
@@ -32,7 +37,21 @@ function Component(t0) {
   return arrRef;
 }
 
+export const FIXTURE_ENTRYPOINT = {
+  fn: Component,
+  params: [{ arrRef: { current: { val: "initial ref value" } } }],
+};
+
+```
+
+## Logs
+
+```
+{"kind":"CompileError","fnLoc":{"start":{"line":6,"column":0,"index":148},"end":{"line":11,"column":1,"index":311},"filename":"mutate-after-useeffect-ref-access.ts"},"detail":{"reason":"Mutating component props or hook arguments is not allowed. Consider using a local variable instead","description":null,"loc":{"start":{"line":9,"column":2,"index":269},"end":{"line":9,"column":16,"index":283},"filename":"mutate-after-useeffect-ref-access.ts"},"suggestions":null,"severity":"InvalidReact"}}
+{"kind":"AutoDepsDecorations","fnLoc":{"start":{"line":8,"column":2,"index":227},"end":{"line":8,"column":40,"index":265},"filename":"mutate-after-useeffect-ref-access.ts"},"decorations":[{"start":{"line":8,"column":24,"index":249},"end":{"line":8,"column":30,"index":255},"filename":"mutate-after-useeffect-ref-access.ts","identifierName":"arrRef"}]}
+{"kind":"CompileSuccess","fnLoc":{"start":{"line":6,"column":0,"index":148},"end":{"line":11,"column":1,"index":311},"filename":"mutate-after-useeffect-ref-access.ts"},"fnName":"Component","memoSlots":0,"memoBlocks":0,"memoValues":0,"prunedMemoBlocks":0,"prunedMemoValues":0}
 ```
       
 ### Eval output
-(kind: exception) Fixture not implemented
+(kind: ok) {"current":{"val":2}}
+logs: [{ val: 2 }]

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies/bailout-retry/mutate-after-useeffect-ref-access.js

@@ -1,4 +1,4 @@
-// @inferEffectDependencies @panicThreshold:"none"
+// @inferEffectDependencies @panicThreshold:"none" @loggerTestOnly
 
 import {useEffect, useRef} from 'react';
 import {print} from 'shared-runtime';
@@ -9,3 +9,8 @@ function Component({arrRef}) {
   arrRef.current.val = 2;
   return arrRef;
 }
+
+export const FIXTURE_ENTRYPOINT = {
+  fn: Component,
+  params: [{arrRef: {current: {val: 'initial ref value'}}}],
+};

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies/bailout-retry/mutate-after-useeffect.expect.md

@@ -2,33 +2,55 @@
 ## Input
 
 ```javascript
-// @inferEffectDependencies @panicThreshold:"none"
+// @inferEffectDependencies @panicThreshold:"none" @loggerTestOnly
 import {useEffect} from 'react';
 
 function Component({foo}) {
   const arr = [];
-  useEffect(() => arr.push(foo));
+  useEffect(() => {
+    arr.push(foo);
+  });
   arr.push(2);
   return arr;
 }
 
+export const FIXTURE_ENTRYPOINT = {
+  fn: Component,
+  params: [{foo: 1}],
+};
+
 ```
 
 ## Code
 
 ```javascript
-// @inferEffectDependencies @panicThreshold:"none"
+// @inferEffectDependencies @panicThreshold:"none" @loggerTestOnly
 import { useEffect } from "react";
 
 function Component(t0) {
   const { foo } = t0;
   const arr = [];
-  useEffect(() => arr.push(foo), [arr, foo]);
+  useEffect(() => {
+    arr.push(foo);
+  }, [arr, foo]);
   arr.push(2);
   return arr;
 }
 
+export const FIXTURE_ENTRYPOINT = {
+  fn: Component,
+  params: [{ foo: 1 }],
+};
+
+```
+
+## Logs
+
+```
+{"kind":"CompileError","fnLoc":{"start":{"line":4,"column":0,"index":101},"end":{"line":11,"column":1,"index":222},"filename":"mutate-after-useeffect.ts"},"detail":{"reason":"This mutates a variable that React considers immutable","description":null,"loc":{"start":{"line":9,"column":2,"index":194},"end":{"line":9,"column":5,"index":197},"filename":"mutate-after-useeffect.ts","identifierName":"arr"},"suggestions":null,"severity":"InvalidReact"}}
+{"kind":"AutoDepsDecorations","fnLoc":{"start":{"line":6,"column":2,"index":149},"end":{"line":8,"column":4,"index":190},"filename":"mutate-after-useeffect.ts"},"decorations":[{"start":{"line":7,"column":4,"index":171},"end":{"line":7,"column":7,"index":174},"filename":"mutate-after-useeffect.ts","identifierName":"arr"},{"start":{"line":7,"column":4,"index":171},"end":{"line":7,"column":7,"index":174},"filename":"mutate-after-useeffect.ts","identifierName":"arr"},{"start":{"line":7,"column":13,"index":180},"end":{"line":7,"column":16,"index":183},"filename":"mutate-after-useeffect.ts","identifierName":"foo"}]}
+{"kind":"CompileSuccess","fnLoc":{"start":{"line":4,"column":0,"index":101},"end":{"line":11,"column":1,"index":222},"filename":"mutate-after-useeffect.ts"},"fnName":"Component","memoSlots":0,"memoBlocks":0,"memoValues":0,"prunedMemoBlocks":0,"prunedMemoValues":0}
 ```
       
 ### Eval output
-(kind: exception) Fixture not implemented
+(kind: ok) [2]

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies/bailout-retry/mutate-after-useeffect.js

@@ -1,9 +1,16 @@
-// @inferEffectDependencies @panicThreshold:"none"
+// @inferEffectDependencies @panicThreshold:"none" @loggerTestOnly
 import {useEffect} from 'react';
 
 function Component({foo}) {
   const arr = [];
-  useEffect(() => arr.push(foo));
+  useEffect(() => {
+    arr.push(foo);
+  });
   arr.push(2);
   return arr;
 }
+
+export const FIXTURE_ENTRYPOINT = {
+  fn: Component,
+  params: [{foo: 1}],
+};

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies/reactive-optional-chain-complex.expect.md

@@ -0,0 +1,99 @@
+
+## Input
+
+```javascript
+// @inferEffectDependencies
+import {useEffect} from 'react';
+import {print, shallowCopy} from 'shared-runtime';
+
+function ReactiveMemberExpr({cond, propVal}) {
+  const obj = {a: cond ? {b: propVal} : null, c: null};
+  const other = shallowCopy({a: {b: {c: {d: {e: {f: propVal + 1}}}}}});
+  const primitive = shallowCopy(propVal);
+  useEffect(() =>
+    print(obj.a?.b, other?.a?.b?.c?.d?.e.f, primitive.a?.b.c?.d?.e.f)
+  );
+}
+
+export const FIXTURE_ENTRYPOINT = {
+  fn: ReactiveMemberExpr,
+  params: [{cond: true, propVal: 1}],
+};
+
+```
+
+## Code
+
+```javascript
+import { c as _c } from "react/compiler-runtime"; // @inferEffectDependencies
+import { useEffect } from "react";
+import { print, shallowCopy } from "shared-runtime";
+
+function ReactiveMemberExpr(t0) {
+  const $ = _c(13);
+  const { cond, propVal } = t0;
+  let t1;
+  if ($[0] !== cond || $[1] !== propVal) {
+    t1 = cond ? { b: propVal } : null;
+    $[0] = cond;
+    $[1] = propVal;
+    $[2] = t1;
+  } else {
+    t1 = $[2];
+  }
+  let t2;
+  if ($[3] !== t1) {
+    t2 = { a: t1, c: null };
+    $[3] = t1;
+    $[4] = t2;
+  } else {
+    t2 = $[4];
+  }
+  const obj = t2;
+  const t3 = propVal + 1;
+  let t4;
+  if ($[5] !== t3) {
+    t4 = shallowCopy({ a: { b: { c: { d: { e: { f: t3 } } } } } });
+    $[5] = t3;
+    $[6] = t4;
+  } else {
+    t4 = $[6];
+  }
+  const other = t4;
+  let t5;
+  if ($[7] !== propVal) {
+    t5 = shallowCopy(propVal);
+    $[7] = propVal;
+    $[8] = t5;
+  } else {
+    t5 = $[8];
+  }
+  const primitive = t5;
+  let t6;
+  if (
+    $[9] !== obj.a?.b ||
+    $[10] !== other?.a?.b?.c?.d?.e.f ||
+    $[11] !== primitive.a?.b.c?.d?.e.f
+  ) {
+    t6 = () =>
+      print(obj.a?.b, other?.a?.b?.c?.d?.e.f, primitive.a?.b.c?.d?.e.f);
+    $[9] = obj.a?.b;
+    $[10] = other?.a?.b?.c?.d?.e.f;
+    $[11] = primitive.a?.b.c?.d?.e.f;
+    $[12] = t6;
+  } else {
+    t6 = $[12];
+  }
+  useEffect(t6, [obj.a?.b, other?.a?.b?.c?.d?.e.f, primitive.a?.b.c?.d?.e.f]);
+}
+
+export const FIXTURE_ENTRYPOINT = {
+  fn: ReactiveMemberExpr,
+  params: [{ cond: true, propVal: 1 }],
+};
+
+```
+      
+### Eval output
+(kind: ok) 
+logs: [1,2,undefined]

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies/reactive-optional-chain-complex.js

@@ -0,0 +1,17 @@
+// @inferEffectDependencies
+import {useEffect} from 'react';
+import {print, shallowCopy} from 'shared-runtime';
+
+function ReactiveMemberExpr({cond, propVal}) {
+  const obj = {a: cond ? {b: propVal} : null, c: null};
+  const other = shallowCopy({a: {b: {c: {d: {e: {f: propVal + 1}}}}}});
+  const primitive = shallowCopy(propVal);
+  useEffect(() =>
+    print(obj.a?.b, other?.a?.b?.c?.d?.e.f, primitive.a?.b.c?.d?.e.f)
+  );
+}
+
+export const FIXTURE_ENTRYPOINT = {
+  fn: ReactiveMemberExpr,
+  params: [{cond: true, propVal: 1}],
+};

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies/reactive-optional-chain.expect.md

@@ -6,12 +6,17 @@
 import {useEffect} from 'react';
 import {print} from 'shared-runtime';
 
-// TODO: take optional chains as dependencies
 function ReactiveMemberExpr({cond, propVal}) {
-  const obj = {a: cond ? {b: propVal} : null};
+  const obj = {a: cond ? {b: propVal} : null, c: null};
   useEffect(() => print(obj.a?.b));
+  useEffect(() => print(obj.c?.d));
 }
 
+export const FIXTURE_ENTRYPOINT = {
+  fn: ReactiveMemberExpr,
+  params: [{cond: true, propVal: 1}],
+};
+
 ```
 
 ## Code
@@ -21,9 +26,8 @@ import { c as _c } from "react/compiler-runtime"; // @inferEffectDependencies
 import { useEffect } from "react";
 import { print } from "shared-runtime";
 
-// TODO: take optional chains as dependencies
 function ReactiveMemberExpr(t0) {
-  const $ = _c(7);
+  const $ = _c(9);
   const { cond, propVal } = t0;
   let t1;
   if ($[0] !== cond || $[1] !== propVal) {
@@ -36,7 +40,7 @@ function ReactiveMemberExpr(t0) {
   }
   let t2;
   if ($[3] !== t1) {
-    t2 = { a: t1 };
+    t2 = { a: t1, c: null };
     $[3] = t1;
     $[4] = t2;
   } else {
@@ -51,10 +55,25 @@ function ReactiveMemberExpr(t0) {
   } else {
     t3 = $[6];
   }
-  useEffect(t3, [obj.a]);
+  useEffect(t3, [obj.a?.b]);
+  let t4;
+  if ($[7] !== obj.c?.d) {
+    t4 = () => print(obj.c?.d);
+    $[7] = obj.c?.d;
+    $[8] = t4;
+  } else {
+    t4 = $[8];
+  }
+  useEffect(t4, [obj.c?.d]);
 }
 
+export const FIXTURE_ENTRYPOINT = {
+  fn: ReactiveMemberExpr,
+  params: [{ cond: true, propVal: 1 }],
+};
+
 ```
       
 ### Eval output
-(kind: exception) Fixture not implemented
+(kind: ok) 
+logs: [1,undefined]

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies/reactive-optional-chain.js

@@ -2,8 +2,13 @@
 import {useEffect} from 'react';
 import {print} from 'shared-runtime';
 
-// TODO: take optional chains as dependencies
 function ReactiveMemberExpr({cond, propVal}) {
-  const obj = {a: cond ? {b: propVal} : null};
+  const obj = {a: cond ? {b: propVal} : null, c: null};
   useEffect(() => print(obj.a?.b));
+  useEffect(() => print(obj.c?.d));
 }
+
+export const FIXTURE_ENTRYPOINT = {
+  fn: ReactiveMemberExpr,
+  params: [{cond: true, propVal: 1}],
+};