From 5c6971e73f0f7f5afa1cad0a6e6344c44b649152 Mon Sep 17 00:00:00 2001 From: Karel Bousson Date: Wed, 23 Sep 2026 17:00:25 +0200 Subject: [PATCH 1/3] Don't emit named-arg values in `bindInvokable` A curried keyword with named args emits a comma pair (#1068): the keyword call validates the args, and `bindInvokable(...)` produces the curried value. When a named arg's value was a call to a generic function that returns a function, like ember-set-helper's `(set this "el")`, TypeScript skipped it during `bindInvokable`'s first inference pass (`SkipGenericFunctions`). Together with the likewise-skipped `resolveForBind(...)`, that lost all of `bindInvokable`'s type parameters and collapsed the result to `Invokable<(...args: unknown[]) => unknown>`. Yielded against `WithBoundArgs`, `Args` was then inferred from the expected type and valid code was rejected. `bindInvokable` only reads the named args' keys, so the transform now emits `boundArg` for every value there. This generalizes the #1147 fix, which covered only `fn`, so the `bindPositional` special case is removed and `fn` always emits a plain call (#1247). Co-Authored-By: Claude Opus 5.5 (1M context) --- .../-private/environment/index.ts | 17 ++-- .../template/template-to-typescript.ts | 74 ++++++++--------- .../ember-tsc/types/-private/dsl/index.d.ts | 48 +++-------- .../transform/template-to-typescript.test.ts | 77 +++++++++--------- .../src/bind-generic-helper-arg.test.gts | 80 +++++++++++++++++++ .../src/fn-in-nested-args.test.gts | 9 +-- 6 files changed, 177 insertions(+), 128 deletions(-) create mode 100644 test-packages/ts-gts-7-1-app/src/bind-generic-helper-arg.test.gts diff --git a/packages/ember-tsc/src/environment-ember-template-imports/-private/environment/index.ts b/packages/ember-tsc/src/environment-ember-template-imports/-private/environment/index.ts index df5830484..ee89f7e42 100644 --- a/packages/ember-tsc/src/environment-ember-template-imports/-private/environment/index.ts +++ b/packages/ember-tsc/src/environment-ember-template-imports/-private/environment/index.ts @@ -124,17 +124,12 @@ export default function emberTemplateImportsEnvironment( // `const` signature preserves literal element types at non-contextual // positions (see `array-keyword-preserve-literals.test.gts`). // - // The RFC 470 `fn` keyword needs a special form for a subtler - // reason: its `FnHelper` type is overloaded (one overload per - // bound-argument arity), and TypeScript's nested-call re-inference - // only handles single-signature callees, so an inline `(fn ...)` - // inside another invocation's arguments (most visibly - // `{{component Foo onChange=(fn ...)}}`) collapsed the outer call's - // entire inference (#1147). The `bind-positional` form emits - // `bindPositional` in place of the real `fn` call inside the result - // half of a `{{component}}`/`{{helper}}`/`{{modifier}}` comma pair, - // and a plain `fn` call everywhere else (#1247). See - // `bindPositional` in `types/-private/dsl/index.d.ts`. (The `@ember/helper` import + // The RFC 470 `fn` keyword gets the `bind-positional` form, which + // emits a plain `fn` call so it keeps its slot's contextual type + // (#1247). Inside `{{component}}`/`{{helper}}`/`{{modifier}}`, the + // `bindInvokable` half of the comma pair doesn't emit named-arg + // values at all (#1147); see `boundArg` in + // `types/-private/dsl/index.d.ts`. (The `@ember/helper` import // below gets the same treatment unconditionally; this entry is // gated with the other built-in globals so a pre-7.1 project's own // `fn` import from elsewhere isn't hijacked.) diff --git a/packages/ember-tsc/src/transform/template/template-to-typescript.ts b/packages/ember-tsc/src/transform/template/template-to-typescript.ts index 625bf6673..e0b77b614 100644 --- a/packages/ember-tsc/src/transform/template/template-to-typescript.ts +++ b/packages/ember-tsc/src/transform/template/template-to-typescript.ts @@ -124,9 +124,6 @@ export function templateToTypescript( let { rangeForNode } = mapper; let scope = new ScopeStack([]); let inHtmlContext: 'svg' | 'math' | 'default' = 'default'; - // Depth of `bindInvokable(...)` result-copy args being emitted — see - // `emitBindInvokableExpression` and `emitBindPositionalExpression`. - let bindInvokableResultDepth = 0; emitTemplateBoilerplate(() => { for (let statement of ast?.body ?? []) { @@ -449,17 +446,19 @@ export function templateToTypescript( // Two-stage comma expression (#1068): the keyword call validates // arg types (result discarded), while bindInvokable preserves // generic type parameters via Args/T holistic capture (result used). + // + // bindInvokable only reads the named args' keys, so their values + // are emitted as a placeholder here. A real value can collapse its + // inference: TypeScript skips calls to generic functions that + // return functions — `(fn f a)`, `(set this "el")` — during its + // first inference pass, and bindInvokable's type parameters are + // then lost (#1147). mapper.text('('); emitKeywordCall(); mapper.text(', __glintDSL__.bindInvokable(__glintDSL__.resolveForBind('); emitExpression(node.params[0]); mapper.text('), '); - bindInvokableResultDepth++; - try { - emitArgs([], node.hash); - } finally { - bindInvokableResultDepth--; - } + emitBoundNamedArgs(node.hash); mapper.text('))'); } else { emitKeywordCall(); @@ -488,33 +487,18 @@ export function templateToTypescript( mapper.text('__glintDSL__.emitContent('); } - if (bindInvokableResultDepth > 0) { - // Inside the result half of a `bind-invokable` comma pair, e.g. the - // `bindInvokable(...)` call emitted for - // `{{component Foo onChange=(fn f a)}}`. Calling the overloaded - // (generic, function-returning) `fn` here makes TypeScript skip it - // during the outer call's first inference pass, which collapses - // `bindInvokable`'s inference entirely (#1147). The single-signature - // `bindPositional` computes the partially-applied type instead. - // Arguments are validated by the keyword call in the other half of - // the pair, where this same `fn` is emitted as a real call. - mapper.text('__glintDSL__.bindPositional('); - emitArgs(node.params, node.hash); - mapper.text(')'); - } else { - // Everywhere else `fn` is a real call sitting directly in its slot, - // so it gets the slot's contextual type. Return-type inference from - // that context is what lets a generic callback resolve its type - // parameters — e.g. `fn this.update "name"` where `update` is - // `(key: K, value: V) => void` - // (#1247). A comma pair would leave the validating call without a - // contextual type. - mapper.text('__glintDSL__.resolve('); - emitExpression(node.path); - mapper.text(')('); - emitArgs(node.params, node.hash); - mapper.text(')'); - } + // `fn` is a real call sitting directly in its slot, so it gets the + // slot's contextual type. Return-type inference from that context is + // what lets a generic callback resolve its type parameters — e.g. + // `fn this.update "name"` where `update` is + // `(key: K, value: V) => void` + // (#1247). A comma pair would leave the validating call without a + // contextual type. + mapper.text('__glintDSL__.resolve('); + emitExpression(node.path); + mapper.text(')('); + emitArgs(node.params, node.hash); + mapper.text(')'); if (position === 'top-level') { mapper.text(')'); @@ -1836,6 +1820,24 @@ export function templateToTypescript( } } + // Emits `{ key: __glintDSL__.boundArg, ... }` for the named args of a + // `bindInvokable(...)` call — see `emitBindInvokableExpression`. + function emitBoundNamedArgs(named: AST.Hash): void { + mapper.forNode(named, () => { + mapper.text('{ '); + + let { start } = rangeForNode(named); + for (let pair of named.pairs) { + start = template.indexOf(pair.key, start); + emitHashKey(pair.key, start); + mapper.text(': __glintDSL__.boundArg, '); + start = rangeForNode(pair.value).end; + } + + mapper.text('...__glintDSL__.NamedArgsMarker }'); + }); + } + type PathKind = 'this' | 'arg' | 'free'; function emitPath(node: AST.PathExpression): void { diff --git a/packages/ember-tsc/types/-private/dsl/index.d.ts b/packages/ember-tsc/types/-private/dsl/index.d.ts index 0ec55a3bf..a7d0864fb 100644 --- a/packages/ember-tsc/types/-private/dsl/index.d.ts +++ b/packages/ember-tsc/types/-private/dsl/index.d.ts @@ -49,52 +49,26 @@ export declare function templateExpression< ): TemplateOnlyComponent & (abstract new () => InvokableInstance & HasContext); -import { Mut } from '../intrinsics/mut'; - /* - * Computes the type of `{{fn ...}}` when it's curried into a component, - * helper or modifier via the `{{component}}`/`{{helper}}`/`{{modifier}}` - * keywords (#1147). Those keywords emit a two-stage comma expression (#1068), - * and `fn` appearing in the result half is emitted as this call instead of a - * real `fn` call: + * The value emitted for each named arg in the `bindInvokable(...)` half of a + * `{{component}}`/`{{helper}}`/`{{modifier}}` comma pair (#1068): * * {{component Foo onChange=(fn f a)}} * * becomes * * (resolve(component)(Foo, { onChange: resolve(fn)(f, a) }), - * bindInvokable(Foo, { onChange: bindPositional(f, a) })) - * - * The keyword call validates the `fn` arguments with the component's arg - * types as context, while `bindPositional` — being non-overloaded and not - * itself returning a function type — isn't skipped by TypeScript's first - * inference pass over `bindInvokable`'s arguments (`SkipGenericFunctions`). - * A real `fn` call there made TypeScript give up on ALL of `bindInvokable`'s - * type parameters, collapsing the result to - * `Invokable<(...args: unknown[]) => unknown>`. - * - * Everywhere else `fn` is emitted as a real call, so it keeps its slot's - * contextual type (#1247). + * bindInvokable(Foo, { onChange: boundArg })) * - * Because argument validation is the keyword call's job, this signature is - * deliberately lenient: a mismatched bind yields `never` here and a real, - * mapped error from the keyword call. `Bound` is `const` so literal bound - * arguments (`fn f "name"`) aren't widened before being matched against `f`'s - * parameters. A generic `f` is instantiated rather than kept generic - * (`fn identity "hi"` types as `() => unknown`, not `() => string`), and one - * whose type parameters depend on each other may yield `never`; either way - * only the curried arg's key matters to `bindInvokable`. + * `bindInvokable` only reads the keys of its named args, and the keyword call + * validates the values with the invokable's arg types as context. Emitting + * the real values there could collapse `bindInvokable`'s inference to + * `Invokable<(...args: unknown[]) => unknown>`: TypeScript skips calls to + * generic functions that return functions (`fn`, ember-set-helper's `set`) + * during an outer call's first inference pass (`SkipGenericFunctions`), and + * `bindInvokable`'s type parameters are lost with them (#1147). */ -export declare function bindPositional( - f: F, - ...bound: Bound -): F extends Mut - ? Bound extends [] - ? (value: T) => void - : () => void - : F extends (...args: [...Bound, ...infer Rest]) => infer Ret - ? (...rest: Rest) => Ret - : never; +export declare const boundArg: unknown; import { Invokable } from '@glint/template/-private/integration'; diff --git a/test-packages/package-test-core/__tests__/transform/template-to-typescript.test.ts b/test-packages/package-test-core/__tests__/transform/template-to-typescript.test.ts index b40bde154..6d977e22e 100644 --- a/test-packages/package-test-core/__tests__/transform/template-to-typescript.test.ts +++ b/test-packages/package-test-core/__tests__/transform/template-to-typescript.test.ts @@ -184,20 +184,20 @@ describe('Transform: rewriteTemplate', () => { expect(templateBody(template, { specialForms, globals: ['testComponent', 'let'] })) .toMatchInlineSnapshot(` - "{ - const __glintY__ = __glintDSL__.emitComponent(__glintDSL__.resolve(__glintDSL__.Globals.let)((__glintDSL__.resolve(__glintDSL__.Globals.testComponent)((() => __glintDSL__.resolveForBind(Inner))(), { x: 1 , ...__glintDSL__.NamedArgsMarker }), __glintDSL__.bindInvokable(__glintDSL__.resolveForBind(Inner), { x: 1 , ...__glintDSL__.NamedArgsMarker })))); - { - const [Bound] = __glintY__.blockParams["default"]; - { - const __glintY__ = __glintDSL__.emitComponent(__glintDSL__.resolve(Consumer)({ - bound: (Bound as import("@glint/template").InferenceInertInvokable), - direct: Inner, ...__glintDSL__.NamedArgsMarker })); - } - (__glintDSL__.noop(testYield), __glintDSL__.yieldToBlock(__glintRef__, "default")(Bound)); - } - __glintDSL__.Globals.let; - }" - `); + "{ + const __glintY__ = __glintDSL__.emitComponent(__glintDSL__.resolve(__glintDSL__.Globals.let)((__glintDSL__.resolve(__glintDSL__.Globals.testComponent)((() => __glintDSL__.resolveForBind(Inner))(), { x: 1 , ...__glintDSL__.NamedArgsMarker }), __glintDSL__.bindInvokable(__glintDSL__.resolveForBind(Inner), { x: __glintDSL__.boundArg, ...__glintDSL__.NamedArgsMarker })))); + { + const [Bound] = __glintY__.blockParams["default"]; + { + const __glintY__ = __glintDSL__.emitComponent(__glintDSL__.resolve(Consumer)({ + bound: (Bound as import("@glint/template").InferenceInertInvokable), + direct: Inner, ...__glintDSL__.NamedArgsMarker })); + } + (__glintDSL__.noop(testYield), __glintDSL__.yieldToBlock(__glintRef__, "default")(Bound)); + } + __glintDSL__.Globals.let; + }" + `); }); // The collapse this cast prevents requires a generic class component — @@ -219,7 +219,7 @@ describe('Transform: rewriteTemplate', () => { }), ).toMatchInlineSnapshot(` "{ - const __glintY__ = __glintDSL__.emitComponent(__glintDSL__.resolve(__glintDSL__.Globals.let)((__glintDSL__.resolve(__glintDSL__.Globals.testComponent)((() => __glintDSL__.resolveForBind(Inner))(), { x: 1 , ...__glintDSL__.NamedArgsMarker }), __glintDSL__.bindInvokable(__glintDSL__.resolveForBind(Inner), { x: 1 , ...__glintDSL__.NamedArgsMarker })))); + const __glintY__ = __glintDSL__.emitComponent(__glintDSL__.resolve(__glintDSL__.Globals.let)((__glintDSL__.resolve(__glintDSL__.Globals.testComponent)((() => __glintDSL__.resolveForBind(Inner))(), { x: 1 , ...__glintDSL__.NamedArgsMarker }), __glintDSL__.bindInvokable(__glintDSL__.resolveForBind(Inner), { x: __glintDSL__.boundArg, ...__glintDSL__.NamedArgsMarker })))); { const [Bound] = __glintY__.blockParams["default"]; { @@ -244,33 +244,32 @@ describe('Transform: rewriteTemplate', () => { expect(templateBody(template, { specialForms, globals: ['testComponent', 'let'] })) .toMatchInlineSnapshot(` - "{ - const __glintY__ = __glintDSL__.emitComponent(__glintDSL__.resolve(__glintDSL__.Globals.let)((__glintDSL__.resolve(__glintDSL__.Globals.testComponent)((() => __glintDSL__.resolveForBind(Inner))(), { x: 1 , ...__glintDSL__.NamedArgsMarker }), __glintDSL__.bindInvokable(__glintDSL__.resolveForBind(Inner), { x: 1 , ...__glintDSL__.NamedArgsMarker })))); - { - const [Bound] = __glintY__.blockParams["default"]; - { - const __glintY__ = __glintDSL__.emitComponent(__glintDSL__.resolve(__glintDSL__.Globals.let)(Other)); - { - const [Bound] = __glintY__.blockParams["default"]; - { - const __glintY__ = __glintDSL__.emitComponent(__glintDSL__.resolve(Consumer)({ - bound: Bound, ...__glintDSL__.NamedArgsMarker })); - } - } - __glintDSL__.Globals.let; - } - } - __glintDSL__.Globals.let; - }" - `); + "{ + const __glintY__ = __glintDSL__.emitComponent(__glintDSL__.resolve(__glintDSL__.Globals.let)((__glintDSL__.resolve(__glintDSL__.Globals.testComponent)((() => __glintDSL__.resolveForBind(Inner))(), { x: 1 , ...__glintDSL__.NamedArgsMarker }), __glintDSL__.bindInvokable(__glintDSL__.resolveForBind(Inner), { x: __glintDSL__.boundArg, ...__glintDSL__.NamedArgsMarker })))); + { + const [Bound] = __glintY__.blockParams["default"]; + { + const __glintY__ = __glintDSL__.emitComponent(__glintDSL__.resolve(__glintDSL__.Globals.let)(Other)); + { + const [Bound] = __glintY__.blockParams["default"]; + { + const __glintY__ = __glintDSL__.emitComponent(__glintDSL__.resolve(Consumer)({ + bound: Bound, ...__glintDSL__.NamedArgsMarker })); + } + } + __glintDSL__.Globals.let; + } + } + __glintDSL__.Globals.let; + }" + `); }); }); // The `fn` keyword emits as a plain call, so it keeps its slot's - // contextual type (#1247) — except inside the result half of a - // `bind-invokable` comma pair, where a real (overloaded, function-returning) - // `fn` call collapsed `bindInvokable`'s entire inference (#1147). There it - // emits `bindPositional` instead; the keyword half validates the args. + // contextual type (#1247). Inside a `bind-invokable` keyword it appears + // only in the keyword half of the comma pair: the `bindInvokable` half + // emits `boundArg` for every named-arg value (#1147). describe('{{fn}} (bind-positional)', () => { test('in top-level position', () => { let template = '{{testFn foo 123}}'; @@ -306,7 +305,7 @@ describe('Transform: rewriteTemplate', () => { expect( templateBody(template, { specialForms, globals: ['testFn', 'testComponent'] }), ).toMatchInlineSnapshot( - `"__glintDSL__.emitContent((__glintDSL__.resolve(__glintDSL__.Globals.testComponent)((() => __glintDSL__.resolveForBind(Foo))(), { onChange: __glintDSL__.resolve(__glintDSL__.Globals.testFn)(foo, 123) , ...__glintDSL__.NamedArgsMarker }), __glintDSL__.bindInvokable(__glintDSL__.resolveForBind(Foo), { onChange: __glintDSL__.bindPositional(foo, 123) , ...__glintDSL__.NamedArgsMarker })));"`, + `"__glintDSL__.emitContent((__glintDSL__.resolve(__glintDSL__.Globals.testComponent)((() => __glintDSL__.resolveForBind(Foo))(), { onChange: __glintDSL__.resolve(__glintDSL__.Globals.testFn)(foo, 123) , ...__glintDSL__.NamedArgsMarker }), __glintDSL__.bindInvokable(__glintDSL__.resolveForBind(Foo), { onChange: __glintDSL__.boundArg, ...__glintDSL__.NamedArgsMarker })));"`, ); }); }); diff --git a/test-packages/ts-gts-7-1-app/src/bind-generic-helper-arg.test.gts b/test-packages/ts-gts-7-1-app/src/bind-generic-helper-arg.test.gts new file mode 100644 index 000000000..23119df21 --- /dev/null +++ b/test-packages/ts-gts-7-1-app/src/bind-generic-helper-arg.test.gts @@ -0,0 +1,80 @@ +import Component from '@glimmer/component'; +import type { ComponentLike, ModifierLike, WithBoundArgs } from '@glint/template'; +import { expectTypeOf, to } from '@glint/type-test'; + +// Regression guard: a generic helper as a curried named arg. +// +// `{{component}}`/`{{helper}}`/`{{modifier}}` with named args emit a comma +// pair (#1068): a keyword call that validates the args, and a `bindInvokable` +// call whose result is the curried value. When a named arg's value was a call +// to a generic function that returns a function — like ember-set-helper's +// `(set this "el")` — TypeScript skipped that call during `bindInvokable`'s +// first inference pass (`SkipGenericFunctions`) and lost its type parameters, +// so the curried value collapsed to `Invokable<(...args: unknown[]) => unknown>`. +// Yielded against `WithBoundArgs`, `Args` was then inferred from the expected +// type instead, and valid code was rejected. `bindInvokable` only uses the +// named args' keys, so the transform no longer emits their values there. + +// Same shape as ember-set-helper's `set`. +declare function setter( + target: T, + path: K, +): (value?: unknown) => T[K]; + +declare const myModifier: ModifierLike<{ + Args: { Named: { onCreate: (element: HTMLElement) => void } }; + Element: HTMLElement; +}>; + +declare const MyComponent: ComponentLike<{ + Args: { onCreate: (element: HTMLElement) => void; label: string }; +}>; + +// A generic helper inside `(modifier)`, yielded against `WithBoundArgs`. +export class YieldModifier extends Component<{ + Blocks: { default: [WithBoundArgs] }; +}> { + el: HTMLElement | null = null; + +} + +// The same through `{{component}}`, with only some args bound. +export class YieldComponent extends Component<{ + Blocks: { default: [WithBoundArgs] }; +}> { + el: HTMLElement | null = null; + +} + +declare const boundModifier: WithBoundArgs; + +// The same bind consumed via `{{#let}}`. This used to type-check only because +// the collapsed value accepts anything; it now keeps the modifier's type. +export class LetModifier extends Component { + el: HTMLElement | null = null; + +} + +// A non-generic function in the same position. +export class YieldNonGeneric extends Component<{ + Blocks: { default: [WithBoundArgs] }; +}> { + onCreate = (_: HTMLElement): void => {}; + +} + +// A generic helper whose result doesn't fit the arg is still rejected. +export class Mismatched extends Component<{ + Blocks: { default: [WithBoundArgs] }; +}> { + el: HTMLElement | null = null; + +} diff --git a/test-packages/ts-gts-7-1-app/src/fn-in-nested-args.test.gts b/test-packages/ts-gts-7-1-app/src/fn-in-nested-args.test.gts index aaff62b11..2e13ea51b 100644 --- a/test-packages/ts-gts-7-1-app/src/fn-in-nested-args.test.gts +++ b/test-packages/ts-gts-7-1-app/src/fn-in-nested-args.test.gts @@ -13,9 +13,9 @@ import { expectTypeOf, to } from '@glint/type-test'; // re-inference only handles single-signature callees, so // `{{component Foo onChange=(fn ...)}}` produced // `Invokable<(...args: unknown[]) => unknown>` instead of the bound component -// type. The transform now emits `fn` as a comma pair — the real (overloaded) -// call for argument validation, single-signature `bindPositional` for the -// resulting type — so the outer inference sees only a resolved value. +// type. The `bindInvokable` half of the keyword's comma pair now emits a +// placeholder for every named-arg value, so `fn` only appears in the keyword +// half that validates the args. const MyComponent: TOC<{ Args: ( @@ -58,8 +58,7 @@ function noop(): void {} }>; // Direct `{{fn}}` type behavior away from nested-argument positions, plus the -// `(fn (mut ...))` forms that FnHelper's Mut overloads validate and -// `bindPositional`'s Mut branch types. +// `(fn (mut ...))` forms that FnHelper's Mut overloads type. const state = { count: 0 }; const directForms =