Skip to content

[compiler] fix source location for return statements#444

Closed
everettbu wants to merge 2 commits into
mainfrom
nathanmarks/fix-compiler-return-sourcelocs
Closed

[compiler] fix source location for return statements#444
everettbu wants to merge 2 commits into
mainfrom
nathanmarks/fix-compiler-return-sourcelocs

Conversation

@everettbu

Copy link
Copy Markdown

Mirror of facebook/react#35660
Original author: nathanmarks


Fixes missing source locations for ReturnStatement nodes in generated ast. Simple change using existing pattern, only required changes to the codegen step, no other pipeline changes.

Most file changes are new lines in generated code. First commit has the relevant changes, second commit has the noisy snap updates.

I added an exception to the validator to not report an error when a return statement will be optimized to an implicit return by codegen, as there's no separate return statement to instrument anyways in the final ast. An edge case when it comes to preserving source locations for instrumentation that is likely not as common for most babel transforms since they are not doing optimizations.

@everettbu
everettbu marked this pull request as ready for review January 29, 2026 21:17
@greptile-apps

greptile-apps Bot commented Jan 29, 2026

Copy link
Copy Markdown

Greptile Overview

Greptile Summary

This PR adds source location tracking to ReturnStatement nodes in the compiler's code generation step. The implementation follows the existing pattern using the withLoc wrapper and correctly handles two cases: return statements with values and implicit undefined returns.

Key changes:

  • Modified CodegenReactiveFunction.ts to use createReturnStatement (which preserves source locations) instead of calling t.returnStatement() directly
  • Added a validation exception in ValidateSourceLocations.ts for return statements that will be optimized away (arrow functions like () => { return expr } become () => expr)
  • The validation exception correctly checks that the arrow function body is a single-statement block with no directives before skipping validation
  • Test snapshots show the fix working correctly: error count for missing source locations dropped from 25 to 22

The validation logic at lines 2103-2107 in CodegenReactiveFunction.ts matches the exception at lines 156-167 in ValidateSourceLocations.ts, ensuring consistency between optimization and validation.

Confidence Score: 5/5

  • This PR is safe to merge with minimal risk
  • The implementation is straightforward, follows existing patterns consistently, and includes proper validation logic. The changes are minimal (only 4 lines of actual code) with extensive test coverage updates confirming correctness
  • No files require special attention

Important Files Changed

Filename Overview
compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/CodegenReactiveFunction.ts Added source location tracking to ReturnStatement nodes using existing withLoc pattern
compiler/packages/babel-plugin-react-compiler/src/Validation/ValidateSourceLocations.ts Added validation exception for return statements optimized to implicit arrow function returns
compiler/packages/babel-plugin-react-compiler/src/tests/fixtures/compiler/error.todo-missing-source-locations.expect.md Error count reduced from 25 to 22, removing 3 ReturnStatement source location errors

@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

@everettbu

Copy link
Copy Markdown
Author

Upstream PR was closed or merged. Code is synced via branch mirror.

@everettbu everettbu closed this Jan 30, 2026
@everettbu
everettbu deleted the nathanmarks/fix-compiler-return-sourcelocs branch January 30, 2026 17:25
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