-
Notifications
You must be signed in to change notification settings - Fork 0
[compiler] Fix gating export misplaced on _unoptimized variant with TypeScript overload signatures #696
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
[compiler] Fix gating export misplaced on _unoptimized variant with TypeScript overload signatures #696
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,86 @@ | ||
|
|
||
| ## Input | ||
|
|
||
| ```javascript | ||
| // @gating @compilationMode:"annotation" | ||
| import {useStore} from 'shared-runtime'; | ||
|
|
||
| interface Session { | ||
| user: string; | ||
| } | ||
|
|
||
| // Overload signatures | ||
| export function useSession(): Session | null; | ||
| export function useSession<T>(selector: (session: Session) => T): T | null; | ||
| // Implementation | ||
| export function useSession<T>( | ||
| selector?: (session: Session) => T, | ||
| ): Session | T | null { | ||
| 'use forget'; | ||
| return useStore((s: {session: Session | null}) => { | ||
| const session = s.session; | ||
| if (!session) return null; | ||
| return selector ? selector(session) : session; | ||
| }); | ||
| } | ||
|
|
||
| export const FIXTURE_ENTRYPOINT = { | ||
| fn: eval('useSession'), | ||
| params: [[]], | ||
| }; | ||
|
|
||
| ``` | ||
|
|
||
| ## Code | ||
|
|
||
| ```javascript | ||
| import { c as _c } from "react/compiler-runtime"; | ||
| import { isForgetEnabled_Fixtures } from "ReactForgetFeatureFlag"; // @gating @compilationMode:"annotation" | ||
| import { useStore } from "shared-runtime"; | ||
|
|
||
| interface Session { | ||
| user: string; | ||
| } | ||
|
|
||
| // Overload signatures | ||
| export function useSession(): Session | null; | ||
| export function useSession<T>(selector: (session: Session) => T): T | null; | ||
| // Implementation | ||
| export const useSession = isForgetEnabled_Fixtures() | ||
| ? function useSession(selector) { | ||
| "use forget"; | ||
| const $ = _c(2); | ||
| let t0; | ||
| if ($[0] !== selector) { | ||
| t0 = (s) => { | ||
| const session = s.session; | ||
| if (!session) { | ||
| return null; | ||
| } | ||
| return selector ? selector(session) : session; | ||
| }; | ||
| $[0] = selector; | ||
| $[1] = t0; | ||
| } else { | ||
| t0 = $[1]; | ||
| } | ||
| return useStore(t0); | ||
| } | ||
|
Comment on lines
+64
to
+68
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Unoptimized variant is missing The unoptimized function expression in the expected output uses This is a pre-existing limitation of 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 Prompt To Fix 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: 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. |
||
| : function useSession(selector?: (session: Session) => T) { | ||
| "use forget"; | ||
| return useStore((s: { session: Session | null }) => { | ||
| const session = s.session; | ||
| if (!session) return null; | ||
| return selector ? selector(session) : session; | ||
| }); | ||
| }; | ||
|
|
||
| export const FIXTURE_ENTRYPOINT = { | ||
| fn: eval("useSession"), | ||
| params: [[]], | ||
| }; | ||
|
|
||
| ``` | ||
|
|
||
| ### Eval output | ||
| (kind: exception) (0 , _sharedRuntime.useStore) is not a function | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,26 @@ | ||
| // @gating @compilationMode:"annotation" | ||
| import {useStore} from 'shared-runtime'; | ||
|
|
||
| interface Session { | ||
| user: string; | ||
| } | ||
|
|
||
| // Overload signatures | ||
| export function useSession(): Session | null; | ||
| export function useSession<T>(selector: (session: Session) => T): T | null; | ||
| // Implementation | ||
| export function useSession<T>( | ||
| selector?: (session: Session) => T, | ||
| ): Session | T | null { | ||
| 'use forget'; | ||
| return useStore((s: {session: Session | null}) => { | ||
| const session = s.session; | ||
| if (!session) return null; | ||
| return selector ? selector(session) : session; | ||
| }); | ||
| } | ||
|
|
||
| export const FIXTURE_ENTRYPOINT = { | ||
| fn: eval('useSession'), | ||
| params: [[]], | ||
| }; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Orphaned overload signatures produce invalid TypeScript output
The compiled output retains the two
TSDeclareFunctionoverload signatures (lines 46–47) but follows them withexport const useSession = ...(line 49). TypeScript requires overload signatures to be immediately followed by a function declaration implementation; aconstvariable declaration does not satisfy this requirement and will produce:This means the output file, while correct at runtime (type-stripping tools like
@babel/preset-typescriptortsc --noEmit falsewill strip the signatures before execution), is technically invalid TypeScript and will fail IDE type-checking or a stricttscpass over the transformed files.A complete fix would require
insertGatedFunctionDeclaration(or its call site in the compile loop) to detect any precedingTSDeclareFunctionsiblings and remove or rewrite them when the implementation is converted to aconst. That said, this is a pre-existing limitation ofbuildFunctionExpressioninGating.tsand may be intentionally out of scope for this PR — worth tracking as a follow-up.Prompt To Fix With AI