fix and-narrowing (#1169)#1170
Conversation
| ? ({ | ||
| eq: '===', | ||
| neq: '!==', | ||
| and: '&&', |
There was a problem hiding this comment.
having and / or be in this list makes me nervous, because it's a deviation from runtime / the RFC
There was a problem hiding this comment.
Note
This reply is from Claude
The nervousness is justified, so here's the precise scope of the deviation — it's narrower and more asymmetric than "and/or now deviate from runtime":
Value position: no deviation. This PR only emits the operator in an if/unless condition. In every other position and/or still emit as the helper invocation, so their return type (FirstFalsy/FirstTruthy) matches the RFC / ember-truth-helpers exactly — empty array falsy, isTruthy honored, (or [] x) types as x. That behavior is unchanged by this PR.
Condition position: a bounded truthiness deviation only. {{#if (and a b)}} becomes if (a && b), which uses JavaScript truthiness — so an empty array reads as truthy, where Ember's toBool would read it as falsy. Two things bound how much this matters:
- It's the same approximation glint already makes for every condition.
{{#if anArray}}already emitsif (anArray)(JS-truthy empty array), not EmbertoBool. Soand/orin a condition deviate to exactly the degreeif/unlessalready do — this PR doesn't introduce a new kind of inaccuracy, it extends an existing one. - The deviation never co-occurs with the feature it buys. Narrowing only happens when the operands are type-guard
booleans, and JS vs. Ember truthiness agree on booleans. The empty-array/proxy divergence only appears with non-boolean operands — i.e. the cases where you weren't narrowing anyway.
Why the operator is unavoidable for the fix. Narrowing is a control-flow property of the emitted TS, and a TS type predicate (x is T) can only narrow a function's own parameter. When and is a helper call, the tested variable lives inside its argument, already collapsed to boolean — so no and signature (generic, callback, assertion) can recover it. Native && in the emitted condition is the only construct TS will narrow through. (Same reason eq/neq are emitted as ===/!== rather than typed helpers.)
So the real question to decide is narrow: is the existing {{#if}} truthiness approximation acceptable to extend to and/or conditions, in exchange for narrowing? If not, the alternative is to keep and/or as helpers everywhere and close #1169 as "use nested {{#if}}s" — there's no third option where a helper call also narrows.
|
Note This reply is from Claude Some context for reviewers who don't work in glint's internals — what this PR is doing and why it's shaped the way it is. How glint type-checks a templateglint never runs a template to check it. It transpiles each template into an intermediate TypeScript snippet and hands that to A "special form" ( Why
|
| eq: '===', | ||
| neq: '!==', | ||
| and: '&&', | ||
| or: '||', |
There was a problem hiding this comment.
Note
This reply is from Claude
This is the line that actually fixes #1169. specialForms maps a template keyword to a piece of TypeScript syntax the transpiler emits instead of a normal helper call — eq/neq were already mapped to ===/!== here. Adding and/or → &&/|| is what lets TypeScript's control-flow analysis see isNumber(x.a) && isNumber(x.b) and narrow x.a/x.b inside the block.
It has to be a native operator: a boolean-returning helper can't narrow, because a TS type predicate (x is T) only narrows a function's own parameter — and the variable being tested lives inside the helper's argument, not as a parameter of and itself.
| ); | ||
|
|
||
| if (!inTestPosition || node.params.length < 2) { | ||
| // Outside a condition we emit the keyword as a helper invocation |
There was a problem hiding this comment.
Note
This reply is from Claude
This branch is the heart of the design, and the subtlety worth understanding:
-
!inTestPosition→ emit the helper. Native&&/||use JavaScript truthiness, but Ember'sand/orusetoBool(empty array is falsy, proxies honorisTruthy), and they also return a value, not a boolean. Emitting the helper here keeps the runtime-accurate return type (FirstFalsy/FirstTruthy). Only anif/unlesscondition setsinTestPosition = true(see theemitExpression(node.params[0], true)calls), so in every other position the emit is identical to the pre-PR helper behavior. -
params.length < 2→ also emit the helper.and/orrequire at least two operands. A one-armed&&/||would be a syntax error, so we fall back to the helper, whose type already requires ≥ 2 args. This is a deliberate behavior change: the "too few arguments" error now comes from the helper's type (an ordinary TS error, suppressible with@glint-expect-error) instead of a transform-time error. That's why the two "wrong number of parameters" transform tests were updated to expect no transform error.
Net effect: narrowing in conditions, runtime-accurate types everywhere else. The cost is that an empty array is treated as truthy in a condition — the same approximation glint already makes for plain {{#if anArray}}.
| for (const [index, param] of node.params.entries()) { | ||
| emitExpression(param); | ||
| if (index === 0 && formInfo.requiresConsumption) { | ||
| // Ride the keyword consumption on the first operand |
There was a problem hiding this comment.
Note
This reply is from Claude
A glint detail that makes this look stranger than it is. For a documented keyword, glint emits a throwaway reference to it (__glintDSL__.noop(Globals.and)) so that editor hover / go-to-definition still work on and even though the runtime form is just an operator.
The non-obvious part is where that reference goes. Wrapping the whole chain as (noop(and), (a && b)) would put the operator chain on the right of a comma — and TypeScript will not narrow an &&-chain of type guards through a comma expression (it does narrow a single guard or an ===, which is why eq/not don't need this dance). So the consumption rides on just the first operand — (noop(and), a) && b — keeping the && chain at the top level where control-flow analysis still narrows it.
Resolves: #1169
by... potentially lying to the type system -- still investigating
ideally we have the type system match the runtime... since... that's the whole point