Skip to content

Conversation

@PaiavullaNikhil
Copy link

Summary

This PR fixes a React Compiler bug where a local variable capturing a global value was incorrectly optimized away when the global was later mutated using a compound assignment (e.g. +=) inside a useEffect.

In this scenario, the compiler treated the captured local variable as a constant LoadGlobal and replaced it with direct references to the global variable. This caused cleanup functions to observe the mutated value instead of the originally captured one, resulting in incorrect behavior in StrictMode.

This change preserves correct JavaScript closure semantics by ensuring captured values remain stable across effect and cleanup boundaries.

Closes #34899

What was happening

Given code like:

let i = 0;

useEffect(() => {
  const runNumber = i;
  console.log("effect run", runNumber);
  i += 1;
  return () => {
    console.log("cleanup run", runNumber);
  };
}, []);

The compiler optimized away runNumber and replaced it with i, causing the cleanup to log 1 instead of the expected 0.

Root cause

The constant propagation pass did not handle StoreGlobal instructions generated by compound assignments (+=, -=, etc.). When a global was mutated, constants derived from that global were not invalidated, allowing the optimizer to incorrectly eliminate captured local variables.

Fix

This PR adds explicit handling for StoreGlobal instructions in the constant propagation pass:

  • Detects mutations of global variables
  • Invalidates constants derived from LoadGlobal of the same symbol
  • Ensures fixpoint iteration correctly tracks invalidations

This prevents captured locals from being optimized away after a global mutation.

Changes made

  • Added StoreGlobal handling to ConstantPropagation.evaluateInstruction
  • Invalidated constants derived from mutated globals
  • Updated fixpoint iteration to account for invalidations
  • Added a compiler fixture:

global-capture-before-assignment.tsx reproducing issue #34899

How did I test this change?

Expected behavior after fix

  • Captured locals remain stable snapshots
  • Cleanup functions use the captured value instead of the mutated global
  • Correct StrictMode output:
effect run 0
cleanup run 0
effect run 1

Files changed

react/compiler/packages/babel-plugin-react-compiler/src/Optimization/ConstantPropagation.ts

react/compiler/packages/babel-plugin-react-compiler/src/tests/fixtures/compiler/global-capture-before-assignment.tsx

@meta-cla meta-cla bot added the CLA Signed label Jan 8, 2026
Copy link
Member

@josephsavona josephsavona left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the contribution! A simpler and safer option would be to do a first pass over the HIR to determine all globals that are reassigned somewhere, then prevent propagation of those constants at LoadGlobal sites.

Also, you need to run the tests to generate the expect file for the new test.

@PaiavullaNikhil
Copy link
Author

Thanks for the feedback!

That makes sense — doing a conservative pre-pass to identify all globals
that are reassigned and disabling constant propagation at LoadGlobal sites
is much simpler and safer.

I’ll refactor the implementation to:

  • Add a first pass over the HIR to collect reassigned globals
  • Prevent propagation of LoadGlobal values for those globals
  • Remove the StoreGlobal invalidation logic from the constant propagation pass

I’ll also run the compiler tests and commit the generated expected output
for the new fixture.

Thanks for the guidance!

@PaiavullaNikhil PaiavullaNikhil closed this by deleting the head repository Jan 9, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Compiler Bug]: Incorrect JS output when copying and updating global variable with += in useEffect

2 participants