[compiler] Fix gating export misplaced on _unoptimized variant with TypeScript overload signatures#696
Conversation
…ypeScript overload signatures When a function has TypeScript overload signatures (TSDeclareFunction nodes), the overload identifiers were treated as runtime references by Babel's isReferencedIdentifier(). This caused getFunctionReferencedBeforeDeclarationAtTopLevel to incorrectly mark the implementation as 'referenced before declaration', triggering the insertAdditionalFunctionDeclaration path in insertGatedFunctionDeclaration. In that path, the original function is renamed to <fn>_unoptimized in-place, keeping any parent ExportNamedDeclaration wrapper — so the export ended up on the wrong (unoptimized) function. The new dispatcher function was inserted without export, making the exported name wrong at runtime. Fix: skip TSDeclareFunction nodes in the top-level reference traversal, so overload signatures are not treated as runtime references, and the standard (non-referencedBeforeDeclaration) gating path is used instead, which correctly replaces the entire export with `export const useSession = gating() ? ... : ...`. Fixes #35991 Test plan: added compiler fixture gating-ts-overload-export.tsx
Greptile SummaryThis PR fixes a bug where TypeScript overload signatures ( Key changes:
Notable limitation in the generated output: The compiler currently preserves the Confidence Score: 4/5
Important Files Changed
Prompt To Fix All With AIThis is a comment left during a code review.
Path: compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/gating/gating-ts-overload-export.expect.md
Line: 46-49
Comment:
**Orphaned overload signatures produce invalid TypeScript output**
The compiled output retains the two `TSDeclareFunction` overload signatures (lines 46–47) but follows them with `export const useSession = ...` (line 49). TypeScript requires overload signatures to be **immediately followed by a function declaration implementation**; a `const` variable declaration does not satisfy this requirement and will produce:
> TS2391: Function implementation is missing or not immediately following the declaration.
This means the output file, while correct at runtime (type-stripping tools like `@babel/preset-typescript` or `tsc --noEmit false` will strip the signatures before execution), is technically invalid TypeScript and will fail IDE type-checking or a strict `tsc` pass over the transformed files.
A complete fix would require `insertGatedFunctionDeclaration` (or its call site in the compile loop) to detect any preceding `TSDeclareFunction` siblings and remove or rewrite them when the implementation is converted to a `const`. That said, this is a pre-existing limitation of `buildFunctionExpression` in `Gating.ts` and may be intentionally out of scope for this PR — worth tracking as a follow-up.
How can I resolve this? If you propose a fix, please make it concise.Last reviewed commit: af8e139 |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| } else { | ||
| t0 = $[1]; | ||
| } | ||
| return useStore(t0); | ||
| } |
There was a problem hiding this comment.
Unoptimized variant is missing <T> type parameter
The unoptimized function expression in the expected output uses T in its parameter type (selector?: (session: Session) => T) without declaring <T> as a type parameter on the function expression itself. This would produce a TypeScript "Cannot find name 'T'" error if a downstream tsc type-check pass runs over this emitted code.
This is a pre-existing limitation of buildFunctionExpression in Gating.ts, which constructs the FunctionExpression node manually without copying the typeParameters field from the source FunctionDeclaration:
const fn: t.FunctionExpression = {
type: 'FunctionExpression',
async: node.async,
generator: node.generator,
loc: node.loc ?? null,
id: node.id ?? null,
params: node.params, // type annotations referencing T are preserved…
body: node.body,
// typeParameters: node.typeParameters ← missing
};This PR correctly funnels generic TypeScript functions with overload signatures into the buildFunctionExpression path for the first time, making the gap more visible. Worth tracking as a follow-up: either copy typeParameters in buildFunctionExpression, or strip TypeScript annotations from the unoptimized branch before emitting.
Prompt To Fix With AI
This is a comment left during a code review.
Path: compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/gating/gating-ts-overload-export.expect.md
Line: 64-68
Comment:
**Unoptimized variant is missing `<T>` type parameter**
The unoptimized function expression in the expected output uses `T` in its parameter type (`selector?: (session: Session) => T`) without declaring `<T>` as a type parameter on the function expression itself. This would produce a TypeScript "Cannot find name 'T'" error if a downstream `tsc` type-check pass runs over this emitted code.
This is a pre-existing limitation of `buildFunctionExpression` in `Gating.ts`, which constructs the `FunctionExpression` node manually without copying the `typeParameters` field from the source `FunctionDeclaration`:
```ts
const fn: t.FunctionExpression = {
type: 'FunctionExpression',
async: node.async,
generator: node.generator,
loc: node.loc ?? null,
id: node.id ?? null,
params: node.params, // type annotations referencing T are preserved…
body: node.body,
// typeParameters: node.typeParameters ← missing
};
```
This PR correctly funnels generic TypeScript functions with overload signatures into the `buildFunctionExpression` path for the first time, making the gap more visible. Worth tracking as a follow-up: either copy `typeParameters` in `buildFunctionExpression`, or strip TypeScript annotations from the unoptimized branch before emitting.
How can I resolve this? If you propose a fix, please make it concise.
| export function useSession(): Session | null; | ||
| export function useSession<T>(selector: (session: Session) => T): T | null; | ||
| // Implementation | ||
| export const useSession = isForgetEnabled_Fixtures() |
There was a problem hiding this comment.
Orphaned overload signatures produce invalid TypeScript output
The compiled output retains the two TSDeclareFunction overload signatures (lines 46–47) but follows them with export const useSession = ... (line 49). TypeScript requires overload signatures to be immediately followed by a function declaration implementation; a const variable declaration does not satisfy this requirement and will produce:
TS2391: Function implementation is missing or not immediately following the declaration.
This means the output file, while correct at runtime (type-stripping tools like @babel/preset-typescript or tsc --noEmit false will strip the signatures before execution), is technically invalid TypeScript and will fail IDE type-checking or a strict tsc pass over the transformed files.
A complete fix would require insertGatedFunctionDeclaration (or its call site in the compile loop) to detect any preceding TSDeclareFunction siblings and remove or rewrite them when the implementation is converted to a const. That said, this is a pre-existing limitation of buildFunctionExpression in Gating.ts and may be intentionally out of scope for this PR — worth tracking as a follow-up.
Prompt To Fix With AI
This is a comment left during a code review.
Path: compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/gating/gating-ts-overload-export.expect.md
Line: 46-49
Comment:
**Orphaned overload signatures produce invalid TypeScript output**
The compiled output retains the two `TSDeclareFunction` overload signatures (lines 46–47) but follows them with `export const useSession = ...` (line 49). TypeScript requires overload signatures to be **immediately followed by a function declaration implementation**; a `const` variable declaration does not satisfy this requirement and will produce:
> TS2391: Function implementation is missing or not immediately following the declaration.
This means the output file, while correct at runtime (type-stripping tools like `@babel/preset-typescript` or `tsc --noEmit false` will strip the signatures before execution), is technically invalid TypeScript and will fail IDE type-checking or a strict `tsc` pass over the transformed files.
A complete fix would require `insertGatedFunctionDeclaration` (or its call site in the compile loop) to detect any preceding `TSDeclareFunction` siblings and remove or rewrite them when the implementation is converted to a `const`. That said, this is a pre-existing limitation of `buildFunctionExpression` in `Gating.ts` and may be intentionally out of scope for this PR — worth tracking as a follow-up.
How can I resolve this? If you propose a fix, please make it concise.2 participants
Mirror of facebook/react#36020
Original author: sleitor
Summary
When a function has TypeScript overload signatures (
TSDeclareFunctionnodes), the overload identifier names were treated as runtime references by Babel'sisReferencedIdentifier(). This causedgetFunctionReferencedBeforeDeclarationAtTopLevelto incorrectly mark the implementation as referenced before declaration, triggering theinsertAdditionalFunctionDeclarationpath ininsertGatedFunctionDeclaration.In that path, the original function is renamed to
<fn>_unoptimizedin-place, keeping any parentExportNamedDeclarationwrapper — so theexportended up on the wrong (unoptimized) function. The new dispatcher function was inserted withoutexport, breaking the exported binding at runtime.Before:
After:
Fix
Skip
TSDeclareFunctionnodes in the top-level reference traversal (the same wayTSTypeAliasDeclarationis already skipped). Overload signatures are not runtime references and should not influence the reference-before-declaration detection.Fixes #35991
Test Plan
Added compiler fixture
gating-ts-overload-export.tsxcovering the exported function with TypeScript overload signatures + gating + annotation mode.