Skip to content

fix(compiler): use globalThis.Symbol to prevent shadowing by user components#374

Closed
everettbu wants to merge 2 commits into
mainfrom
fix/compiler-symbol-shadowing-35383
Closed

fix(compiler): use globalThis.Symbol to prevent shadowing by user components#374
everettbu wants to merge 2 commits into
mainfrom
fix/compiler-symbol-shadowing-35383

Conversation

@everettbu

Copy link
Copy Markdown

Mirror of facebook/react#35383
Original author: gowthamrdyy


Summary

Fixes a bug where React Compiler generates code that uses Symbol.for() without qualifying it with globalThis, causing runtime errors when a user-defined component is named Symbol.

The Problem:
When a user defines a component named Symbol, it shadows the global built-in Symbol. The compiler-generated code (e.g., for memo cache sentinels) tries to call Symbol.for() but instead accesses the user's component, resulting in "Symbol.for is not a function".

Playground repro: Link

The Fix:
Updated CodegenReactiveFunction.ts to use globalThis.Symbol.for() instead of bare Symbol.for() references. This ensures the built-in global is always used, regardless of local variable names.

How did you test this change?

  1. Added a new test fixture (component-named-symbol-shadows-global.js) that defines a component named Symbol.
    • Verified that this test case previously failed (or would produce broken code) and now compiles to correct code using globalThis.Symbol.
  2. Verified generated output: checked that the compiled output for cache sentinels and early return sentinels now explicitly uses globalThis.Symbol.
  3. Manual verification: Confirmed the fix addresses the issue reported in the playground.

@greptile-apps

greptile-apps Bot commented Jan 18, 2026

Copy link
Copy Markdown

Greptile Summary

This PR fixes a runtime error where React Compiler-generated code would fail when a user defines a component named Symbol. The compiler generates calls to Symbol.for() for memo cache and early return sentinels, which would break if Symbol is shadowed by a local component.

The fix changes two locations in CodegenReactiveFunction.ts to use globalThis.Symbol.for() instead of bare Symbol.for(), ensuring the global built-in is always accessed regardless of local variable naming.

Key changes:

  • Updated memo cache sentinel generation to use globalThis.Symbol.for()
  • Updated early return sentinel generation to use globalThis.Symbol.for()
  • Added test fixture with a component named Symbol to prevent regression
  • Updated 423 snapshot files to reflect the new code generation pattern

Confidence Score: 5/5

  • This PR is safe to merge with no risk
  • The fix is minimal, surgical, and correct. It changes two specific code generation sites to use globalThis.Symbol instead of bare Symbol, which is a well-established pattern to avoid shadowing issues. The change is fully covered by a new test case and all 423 snapshot tests have been updated to reflect the new output. The fix follows JavaScript best practices and has no behavioral side effects.
  • No files require special attention

Important Files Changed

Filename Overview
compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/CodegenReactiveFunction.ts Updated Symbol.for() calls to use globalThis.Symbol.for() to prevent shadowing by user-defined components
compiler/packages/babel-plugin-react-compiler/src/tests/fixtures/compiler/component-named-symbol-shadows-global.js New test fixture with component named Symbol to verify the fix prevents shadowing issues

@everettbu everettbu added the Resolution: Stale Automatically closed due to inactivity label Mar 17, 2026
@github-actions

Copy link
Copy Markdown

Closing this pull request after a prolonged period of inactivity. If this issue is still present in the latest release, please ask for this pull request to be reopened. Thank you!

@github-actions github-actions Bot closed this Mar 24, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA Signed Resolution: Stale Automatically closed due to inactivity

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants