Skip to content

[compiler] Fix Symbol shadowing by prefixing with globalThis#375

Open
everettbu wants to merge 4 commits into
mainfrom
fix/compiler-symbol-shadowing-35553
Open

[compiler] Fix Symbol shadowing by prefixing with globalThis#375
everettbu wants to merge 4 commits into
mainfrom
fix/compiler-symbol-shadowing-35553

Conversation

@everettbu

Copy link
Copy Markdown

Mirror of facebook/react#35553
Original author: hlysine


Summary

Fixes #35379

The React compiler currently emits code that produces a runtime error when the global Symbol is being shadowed by another reference named Symbol. This PR changes the emitted code to access Symbol via globalThis.Symbol to fix this issue.

Thanks to #35429 for providing a hint to the fix, but this PR also applies the fix to PropagateEarlyReturns and InlineJsxTransform.

How did you test this change?

All related test fixtures are updated with this change. Some examples of the generated code:

- if ($[0] === Symbol.for("react.memo_cache_sentinel")) {
+ if ($[0] === globalThis.Symbol.for("react.memo_cache_sentinel")) {
   if ($[0] !== props.a || $[1] !== props.b || $[2] !== props.cond) {
-    t0 = Symbol.for("react.early_return_sentinel");
+    t0 = globalThis.Symbol.for("react.early_return_sentinel");
     bb0: {

@greptile-apps

greptile-apps Bot commented Jan 18, 2026

Copy link
Copy Markdown

Greptile Summary

This PR fixes a runtime error in the React compiler when user code shadows the global Symbol (e.g., function Symbol() {} or const Symbol = ...). The compiler emits Symbol.for("react.memo_cache_sentinel") and Symbol.for("react.early_return_sentinel") in generated code, which breaks when Symbol is shadowed by a local binding.

  • Introduces a codegenGlobalAccess(cx, 'Symbol') helper in CodegenReactiveFunction.ts that checks via Babel's scope.hasBinding() whether Symbol is shadowed, and emits globalThis.Symbol instead of plain Symbol when it is.
  • Applies the fix to all 3 direct codegen sites (memo cache sentinel check, early return sentinel assignment, early return sentinel check) and to the LoadGlobal instruction handler (which covers Symbol references injected by PropagateEarlyReturns and InlineJsxTransform).
  • Adds two test fixtures: one where Symbol is shadowed by a function (triggering globalThis.Symbol in output), and one where Symbol is shadowed by a module-level const (verifying user references to the local Symbol are preserved correctly).

Confidence Score: 4/5

  • This PR is safe to merge — it's a targeted, well-tested fix for a clear bug with no impact on non-shadowing cases.
  • The change is focused and correct. The codegenGlobalAccess helper correctly uses Babel's scope API to detect shadowing, and the LoadGlobal handler properly guards on binding.kind === 'Global' to avoid rewriting user-code references. Two test fixtures cover both the shadowing case (emits globalThis.Symbol) and the local-reference case (preserves user's Symbol). The only reason for not giving a 5 is that some edge cases (e.g., globalThis itself being shadowed, or other globals being shadowed in the future) are not addressed, though these are out of scope for this fix.
  • No files require special attention.

Important Files Changed

Filename Overview
compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/CodegenReactiveFunction.ts Introduces codegenGlobalAccess helper that conditionally prefixes global references with globalThis. when shadowed by local bindings. Applied to all 3 codegen sites that emit Symbol.for(...) (memo cache sentinel, early return sentinel assignment, early return sentinel check), plus the LoadGlobal instruction handler for compiler-injected Symbol references. Clean, focused change.
compiler/packages/babel-plugin-react-compiler/src/tests/fixtures/compiler/repro-symbol-shadowing.js Test fixture with a module-level function Symbol() {} that shadows the global, exercising early return sentinel and memo cache sentinel codegen paths.
compiler/packages/babel-plugin-react-compiler/src/tests/fixtures/compiler/repro-symbol-shadowing.expect.md Expected output confirms globalThis.Symbol.for(...) is used for all compiler-internal sentinels when Symbol is shadowed, while the user's local Symbol function is preserved.
compiler/packages/babel-plugin-react-compiler/src/tests/fixtures/compiler/repro-symbol-shadowing-global.js Test fixture with const Symbol = '<symbol>' at module scope, verifying that user-code references to the local Symbol are not incorrectly rewritten to globalThis.Symbol.
compiler/packages/babel-plugin-react-compiler/src/tests/fixtures/compiler/repro-symbol-shadowing-global.expect.md Expected output confirms that user references to a locally-bound Symbol remain as-is (no globalThis prefix) since the LoadGlobal handler only intercepts bindings with kind: 'Global'.

Last reviewed commit: b295d64

@everettbu
everettbu force-pushed the fix/compiler-symbol-shadowing-35553 branch from 292b642 to 858839c Compare February 16, 2026 09:29

@greptile-apps greptile-apps Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

3 files reviewed, no comments

Edit Code Review Agent Settings | Greptile

@greptile-apps greptile-apps Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

5 files reviewed, no comments

Edit Code Review Agent Settings | Greptile

@greptile-apps greptile-apps Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

5 files reviewed, no comments

Edit Code Review Agent Settings | Greptile

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.

2 participants