Fix: Handle named functions calling other named functions when defined in different order#142
Closed
everettbu wants to merge 3 commits into
Closed
Fix: Handle named functions calling other named functions when defined in different order#142everettbu wants to merge 3 commits into
everettbu wants to merge 3 commits into
Conversation
Greptile OverviewGreptile SummaryThis PR attempts to fix an issue where the React Compiler doesn't properly handle named functions calling other named functions defined later in the code. The fix adds logic to detect when a function reference occurs within another function being compiled. Key Changes:
Critical Issues Found:
Confidence Score: 1/5
Important Files ChangedFile Analysis
|
Comment on lines
+1415
to
+1417
| const parentFunction = scope.getFunctionParent(); | ||
| if (parentFunction !== null) { | ||
| const parentFnName = getFunctionName(parentFunction); |
There was a problem hiding this comment.
syntax: scope.getFunctionParent() returns a Scope, but getFunctionName() expects a NodePath. Should be parentFunction.path
Suggested change
| const parentFunction = scope.getFunctionParent(); | |
| if (parentFunction !== null) { | |
| const parentFnName = getFunctionName(parentFunction); | |
| const parentFunction = scope.getFunctionParent(); | |
| if (parentFunction !== null) { | |
| const parentFnName = getFunctionName(parentFunction.path); |
Prompt To Fix With AI
This is a comment left during a code review.
Path: compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Program.ts
Line: 1415:1417
Comment:
**syntax:** `scope.getFunctionParent()` returns a `Scope`, but `getFunctionName()` expects a `NodePath`. Should be `parentFunction.path`
```suggestion
const parentFunction = scope.getFunctionParent();
if (parentFunction !== null) {
const parentFnName = getFunctionName(parentFunction.path);
```
How can I resolve this? If you propose a fix, please make it concise.
Author
|
Upstream PR was closed or merged. Code is synced via branch mirror. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Mirror of facebook/react#34542
Original author: pritjasani08
Summary
This PR fixes issue #33689 where the React Compiler wasn't properly handling cases where named functions call other named functions when they're defined in a "weird order" (i.e., when a function is called before it's declared).
Problem
The
getFunctionReferencedBeforeDeclarationAtTopLevelfunction only checked for function references at the top level scope, but didn't account for nested function calls where one named function calls another named function that's defined later in the code.Solution
Enhanced the function to:
referencedBeforeDeclarationwhen appropriateChanges
Modified:
compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Program.tsgetFunctionReferencedBeforeDeclarationAtTopLevelfunction to handle nested function callsAdded: Test fixtures to verify the fix
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/named-functions-order.jscompiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/named-functions-order.expect.mdTest Cases
The fix handles these scenarios:
Example
This now works correctly with the React Compiler.
##Checklist
Fixes #33689