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 66cfa357c..81cb6a83d 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 @@ -113,8 +113,18 @@ export default function emberTemplateImportsEnvironment( // discriminated unions — e.g. `{{#if (eq foo.kind "a")}}` narrows // `foo` to the matching variant inside the block, which a // boolean-returning helper type cannot do. + // + // The same reasoning applies to `and`/`or`: emitting them as the + // native `&&`/`||` operators lets `{{#if (and (isNumber x) ...)}}` + // propagate the type predicates of its operands into the block, which + // a value-returning helper type cannot do (typed-ember/glint#1169). ...(hasEmber71BuiltIns() - ? ({ eq: '===', neq: '!==' } satisfies GlintSpecialFormConfig['globals']) + ? ({ + eq: '===', + neq: '!==', + and: '&&', + or: '||', + } satisfies GlintSpecialFormConfig['globals']) : {}), ...additionalGlobalSpecialForms, }, 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 e7adf090c..a22118d5f 100644 --- a/packages/ember-tsc/src/transform/template/template-to-typescript.ts +++ b/packages/ember-tsc/src/transform/template/template-to-typescript.ts @@ -242,8 +242,16 @@ export function templateToTypescript( formInfo: SpecialFormInfo, node: AST.MustacheStatement | AST.SubExpression, position: InvokePosition, + inTestPosition = false, ): void { - if (formInfo.requiresConsumption) { + // Logical forms (`&&`/`||`) manage their own keyword consumption: wrapping + // the whole operator chain in a `(noop(kw), )` comma defeats + // TypeScript's control-flow narrowing of an `&&`/`||` chain of type + // guards. Instead they ride the consumption on the first operand only + // (see `emitLogicalExpression`), keeping the chain at the top level. + const isLogicalForm = formInfo.form === '&&' || formInfo.form === '||'; + + if (formInfo.requiresConsumption && !isLogicalForm) { mapper.text('(__glintDSL__.noop('); emitExpression(node.path); mapper.text('), '); @@ -281,7 +289,7 @@ export function templateToTypescript( case '&&': case '||': - emitLogicalExpression(formInfo, node); + emitLogicalExpression(formInfo, node, inTestPosition); break; case '!': @@ -293,7 +301,7 @@ export function templateToTypescript( mapper.text('undefined'); } - if (formInfo.requiresConsumption) { + if (formInfo.requiresConsumption && !isLogicalForm) { mapper.text(')'); } } @@ -424,7 +432,7 @@ export function templateToTypescript( ); mapper.text('('); - emitExpression(node.params[0]); + emitExpression(node.params[0], true); mapper.text(') ? ('); emitExpression(node.params[1]); mapper.text(') : ('); @@ -450,7 +458,7 @@ export function templateToTypescript( ); mapper.text('!('); - emitExpression(node.params[0]); + emitExpression(node.params[0], true); mapper.text(') ? ('); emitExpression(node.params[1]); mapper.text(') : ('); @@ -492,20 +500,47 @@ export function templateToTypescript( function emitLogicalExpression( formInfo: SpecialFormInfo, node: AST.MustacheStatement | AST.SubExpression, + inTestPosition: boolean, ): void { mapper.forNode(node, () => { assert( node.hash.pairs.length === 0, () => `{{${formInfo.name}}} only accepts positional parameters`, ); - assert( - node.params.length >= 2, - () => `{{${formInfo.name}}} requires at least two parameters`, - ); + if (!inTestPosition || node.params.length < 2) { + // Outside a condition we emit the keyword as a helper invocation + // rather than a native operator chain. The helper's return type + // (`FirstFalsy`/`FirstTruthy`) reproduces Ember's runtime truthiness — + // e.g. empty arrays and `{ isTruthy: false }` are falsy — which the + // JavaScript `&&`/`||` operators do not. The helper also enforces the + // "at least two operands" requirement as an ordinary type error, so it + // handles the too-few-operands case too. (This matches what the + // keyword would emit if it were a plain helper rather than a special + // form, so value-position behavior is unchanged.) + emitResolve(node, 'resolve'); + return; + } + + // In a condition, emit the native `&&`/`||` operator chain so that + // TypeScript's control-flow analysis narrows the operands' type guards + // inside the block (e.g. `{{#if (and (isNumber x) (isNumber y))}}`). mapper.text('('); for (const [index, param] of node.params.entries()) { - emitExpression(param); + if (index === 0 && formInfo.requiresConsumption) { + // Ride the keyword consumption on the first operand + // (`(noop(kw), ) && `) so the operator chain stays at + // the top level of the expression. Wrapping the whole chain in the + // comma instead would discard TypeScript's narrowing of an `&&`/`||` + // chain of type guards. + mapper.text('(__glintDSL__.noop('); + emitExpression(node.path); + mapper.text('), '); + emitExpression(param); + mapper.text(')'); + } else { + emitExpression(param); + } if (index < node.params.length - 1) { mapper.text(` ${formInfo.form} `); @@ -570,13 +605,18 @@ export function templateToTypescript( return null; } - function emitExpression(node: AST.Expression): void { + // `inTestPosition` marks expressions emitted as the condition of an + // `if`/`unless`. It only affects `and`/`or`: in a condition they emit as + // native `&&`/`||` so TypeScript narrows their operands, whereas in any + // other (value) position they emit as helper invocations to preserve their + // exact runtime return type (see `emitLogicalExpression`). + function emitExpression(node: AST.Expression, inTestPosition = false): void { switch (node.type) { case 'PathExpression': return emitPath(node); case 'SubExpression': - return emitSubExpression(node); + return emitSubExpression(node, inTestPosition); case 'BooleanLiteral': case 'NullLiteral': @@ -1157,7 +1197,7 @@ export function templateToTypescript( ); mapper.text('if ('); - emitExpression(node.params[0]); + emitExpression(node.params[0], true); mapper.text(') {'); mapper.newline(); mapper.indent(); @@ -1191,7 +1231,7 @@ export function templateToTypescript( ); mapper.text('if (!('); - emitExpression(node.params[0]); + emitExpression(node.params[0], true); mapper.text(')) {'); mapper.newline(); mapper.indent(); @@ -1309,10 +1349,10 @@ export function templateToTypescript( scope.pop(); } - function emitSubExpression(node: AST.SubExpression): void { + function emitSubExpression(node: AST.SubExpression, inTestPosition = false): void { let specialFormInfo = checkSpecialForm(node); if (specialFormInfo) { - emitSpecialFormExpression(specialFormInfo, node, 'sexpr'); + emitSpecialFormExpression(specialFormInfo, node, 'sexpr', inTestPosition); return; } 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 382a1fcd2..994ae838d 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 @@ -499,7 +499,11 @@ describe('Transform: rewriteTemplate', () => { }); describe('{{and}}', () => { - test('with two arguments', () => { + // In value position `and` emits as a helper invocation so its return type + // reproduces Ember's runtime truthiness (see truth-convert.d.ts). The + // native `&&` operator is only used in a condition (below), where its + // narrowing matters and a truthiness approximation is already in play. + test('in value position emits a helper invocation', () => { let template = stripIndent` {{log (testAnd 1 2)}} `; @@ -507,11 +511,11 @@ describe('Transform: rewriteTemplate', () => { expect( templateBody(template, { globals: ['testAnd'], specialForms: { testAnd: '&&' } }), ).toMatchInlineSnapshot( - `"__glintDSL__.emitContent(__glintDSL__.resolve(log)((__glintDSL__.noop(__glintDSL__.Globals.testAnd), (1 && 2))));"`, + `"__glintDSL__.emitContent(__glintDSL__.resolve(log)(__glintDSL__.resolve(__glintDSL__.Globals.testAnd)(1, 2)));"`, ); }); - test('with three arguments', () => { + test('with three arguments in value position emits a helper invocation', () => { let template = stripIndent` {{log (testAnd 1 2 3)}} `; @@ -519,13 +523,29 @@ describe('Transform: rewriteTemplate', () => { expect( templateBody(template, { globals: ['testAnd'], specialForms: { testAnd: '&&' } }), ).toMatchInlineSnapshot( - `"__glintDSL__.emitContent(__glintDSL__.resolve(log)((__glintDSL__.noop(__glintDSL__.Globals.testAnd), (1 && 2 && 3))));"`, + `"__glintDSL__.emitContent(__glintDSL__.resolve(log)(__glintDSL__.resolve(__glintDSL__.Globals.testAnd)(1, 2, 3)));"`, ); }); + + test('in a condition emits a native operator chain (so TS narrows operands)', () => { + let template = stripIndent` + {{#testIf (testAnd foo bar)}}{{/testIf}} + `; + + expect( + templateBody(template, { + globals: ['testIf', 'testAnd'], + specialForms: { testIf: 'if', testAnd: '&&' }, + }), + ).toMatchInlineSnapshot(` + "if (((__glintDSL__.noop(__glintDSL__.Globals.testAnd), foo) && bar)) { + }" + `); + }); }); describe('{{or}}', () => { - test('with two arguments', () => { + test('in value position emits a helper invocation', () => { let template = stripIndent` {{log (testOr 1 2)}} `; @@ -533,11 +553,11 @@ describe('Transform: rewriteTemplate', () => { expect( templateBody(template, { globals: ['testOr'], specialForms: { testOr: '||' } }), ).toMatchInlineSnapshot( - `"__glintDSL__.emitContent(__glintDSL__.resolve(log)((__glintDSL__.noop(__glintDSL__.Globals.testOr), (1 || 2))));"`, + `"__glintDSL__.emitContent(__glintDSL__.resolve(log)(__glintDSL__.resolve(__glintDSL__.Globals.testOr)(1, 2)));"`, ); }); - test('with three arguments', () => { + test('with three arguments in value position emits a helper invocation', () => { let template = stripIndent` {{log (testOr 1 2 3)}} `; @@ -545,9 +565,25 @@ describe('Transform: rewriteTemplate', () => { expect( templateBody(template, { globals: ['testOr'], specialForms: { testOr: '||' } }), ).toMatchInlineSnapshot( - `"__glintDSL__.emitContent(__glintDSL__.resolve(log)((__glintDSL__.noop(__glintDSL__.Globals.testOr), (1 || 2 || 3))));"`, + `"__glintDSL__.emitContent(__glintDSL__.resolve(log)(__glintDSL__.resolve(__glintDSL__.Globals.testOr)(1, 2, 3)));"`, ); }); + + test('in a condition emits a native operator chain (so TS narrows operands)', () => { + let template = stripIndent` + {{#testIf (testOr foo bar baz)}}{{/testIf}} + `; + + expect( + templateBody(template, { + globals: ['testIf', 'testOr'], + specialForms: { testIf: 'if', testOr: '||' }, + }), + ).toMatchInlineSnapshot(` + "if (((__glintDSL__.noop(__glintDSL__.Globals.testOr), foo) || bar || baz)) { + }" + `); + }); }); describe('{{not}}', () => { @@ -1361,18 +1397,16 @@ describe('Transform: rewriteTemplate', () => { ]); }); - test('{{and}} with wrong number of parameters', () => { + test('{{and}} with too few parameters falls back to a helper invocation', () => { + // Fewer than two operands can't form an `&&` chain, so we emit the keyword + // as a helper invocation instead. This produces no transform error; the + // keyword's own type (e.g. `AndHelper`) reports the arity requirement. let { errors } = templateToTypescript('', { typesModule: '@glint/template', specialForms: { testAnd: '&&' }, }); - expect(errors).toEqual([ - { - message: '{{testAnd}} requires at least two parameters', - location: { start: 11, end: 26 }, - }, - ]); + expect(errors).toEqual([]); }); test('{{or}} with named parameters', () => { @@ -1389,18 +1423,16 @@ describe('Transform: rewriteTemplate', () => { ]); }); - test('{{or}} with wrong number of parameters', () => { + test('{{or}} with too few parameters falls back to a helper invocation', () => { + // Fewer than two operands can't form an `||` chain, so we emit the keyword + // as a helper invocation instead. This produces no transform error; the + // keyword's own type (e.g. `OrHelper`) reports the arity requirement. let { errors } = templateToTypescript('', { typesModule: '@glint/template', specialForms: { testOr: '||' }, }); - expect(errors).toEqual([ - { - message: '{{testOr}} requires at least two parameters', - location: { start: 11, end: 25 }, - }, - ]); + expect(errors).toEqual([]); }); test('{{not}} with named parameters', () => { diff --git a/test-packages/ts-gts-7-1-app/src/globals/narrowing.test.gts b/test-packages/ts-gts-7-1-app/src/globals/narrowing.test.gts index c5dcf44ee..c1f2c3d6d 100644 --- a/test-packages/ts-gts-7-1-app/src/globals/narrowing.test.gts +++ b/test-packages/ts-gts-7-1-app/src/globals/narrowing.test.gts @@ -58,6 +58,30 @@ const foo = { x: 'a', y: 1 } as Foo; const fooA = { x: 'a', y: 1 } as { x: 'a'; y: number }; const fooB = { x: 'b', y: 's' } as { x: 'b'; y: string }; +// --------------------------------------------------------------------------- +// (5) Control-flow narrowing via `and`/`or` (typed-ember/glint#1169). These are +// emitted as the native `&&`/`||` operators, so a chain of type-predicate +// guards propagates each `arg is T` predicate into the block — something a +// value-returning helper type cannot do. +// --------------------------------------------------------------------------- +function isNumber(value: unknown): value is number { + return typeof value === 'number'; +} +type Mixed = { a: unknown; b: unknown }; +const mixed = { a: 1, b: 2 } as Mixed; + +// --------------------------------------------------------------------------- +// (6) Value-position `and`/`or` must keep Ember's runtime truthiness. Although +// they emit as native `&&`/`||` inside a condition (section 5), in value +// position they emit as helper invocations whose return type follows Ember's +// `isTruthy`/`ember-truth-helpers` rules — under which an empty array and a +// `{ isTruthy: false }` object are *falsy*, even though JavaScript's `&&`/`||` +// treat both as truthy. These assertions would fail if value position used the +// native operators. (typed-ember/glint#1169) +// --------------------------------------------------------------------------- +const emptyArray = [] as never[]; +const isFalsyBox = { isTruthy: false, tag: 'empty' } as { isTruthy: false; tag: 'empty' }; +