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..e3861531b 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,20 +124,16 @@ 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 - // 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.) + // The RFC 470 `fn` keyword gets the `bind-positional` form. + // That form emits a plain `fn` call, + // so the call keeps its slot's contextual type (#1247). + // Inside `{{component}}`/`{{helper}}`/`{{modifier}}`, + // the `bindInvokable` half of the comma pair emits no named-arg values (#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. ...(hasEmber71BuiltIns() ? ({ eq: '===', 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..34e925584 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 bindInvokable's inference. + // During its first inference pass, TypeScript skips calls + // to generic functions that return functions, such as `(fn f a)` and `(set this "el")`. + // 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 + // lets a generic callback resolve its type parameters. + // Take `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..14762d31d 100644 --- a/packages/ember-tsc/types/-private/dsl/index.d.ts +++ b/packages/ember-tsc/types/-private/dsl/index.d.ts @@ -49,52 +49,30 @@ 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 of a `bindInvokable(...)` call. + * `{{component}}`, `{{helper}}` and `{{modifier}}` emit that call + * as the second half of a 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>`. + * bindInvokable(Foo, { onChange: boundArg })) * - * Everywhere else `fn` is emitted as a real call, so it keeps its slot's - * contextual type (#1247). + * `bindInvokable` only reads the keys of its named args. + * The keyword call validates the values, + * with the invokable's arg types as context. * - * 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`. + * Emitting the real values there could collapse `bindInvokable`'s inference + * to `Invokable<(...args: unknown[]) => unknown>`. + * During an outer call's first inference pass (`SkipGenericFunctions`), + * TypeScript skips calls to generic functions that return functions, + * such as `fn` and ember-set-helper's `set`. + * `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..e4560e28b --- /dev/null +++ b/test-packages/ts-gts-7-1-app/src/bind-generic-helper-arg.test.gts @@ -0,0 +1,85 @@ +import Component from '@glimmer/component'; +import type { ComponentLike, ModifierLike, WithBoundArgs } from '@glint/template'; +import { expectTypeOf, to } from '@glint/type-test'; + +// Regression guard for https://github.com/typed-ember/glint/pull/1249 +// +// `{{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. +// `bindInvokable` only uses the named args' keys, +// so the transform emits a placeholder for their values. +// +// A real value there can collapse the curried value's type. +// Take a call to a generic function that returns a function, +// like ember-set-helper's `(set this "el")`. +// TypeScript skips that call during `bindInvokable`'s first inference pass +// (`SkipGenericFunctions`) and loses its type parameters. +// The curried value then collapses to `Invokable<(...args: unknown[]) => unknown>`. +// Yielded against `WithBoundArgs`, `Args` is inferred from the expected type instead, +// and valid code is rejected. + +// 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}}`. +// A collapsed value would accept anything here, +// so this checks that `m` 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..2f68e9486 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 =