First steps towards template type-checking in ngtsc - #26203

Why not isComponent?

Done.

There does not seem to be an assertion. If you checking that it does not throw, say so.

driveMain asserts that the compiler ran successfully. This pattern is used in all the tests.

The semantic for NonNullable doesn't seem right for NgIf, should be some kind of NonFalsy instead, like:

type Falsy = false | null | undefined | 0 | ''

class NgIf {
  static ngTemplateGuard_ngIf<E>(dir: NgIf, expr: E | Falsy): expr is E
  // or
  static ngTemplateGuard_ngIf<E>(dir: NgIf, expr: E): expr is Exclude<E, Falsy>
}

I agree in principle. For the real NgIf implementation, we may do something like this. However, the important case is to capture the narrowing to NonNullable, because that supports strictNullChecks in the generated type-checking code.

Narrowing other false-y values is useful only in very specific situations (calling methods which only accept truthy types). I expect this to be almost non-existent in practice.

BTW, only your latter version of ngTemplateGuard_ngIf will actually work. The former works well for inferred values of E, but fails if passed an expr with an explicitly nullable type.

The mechanism doesn't seem to be general enough, how could NgIf guard injected template, ngIfThen and ngIfElse simultaneously? Should at least be possible to write:

class NgIf {
  static ngTemplateGuard_ngIf<E>(...)
  static ngTemplateGuard_ngIf_ngIfThen<E>(...)
  static ngTemplateGuard_ngIf_ngIfElse<E>(...)
}

Sure, you can narrow multiple expressions:

class NgIf {
  static ngTemplateGuard_ngIf<E>(...)
  static ngTemplateGuard_ngIfThen<E>(...)
  static ngTemplateGuard_ngIfElse<E>(...)
}

However in practice this would be useless. Narrowing inside a template scope only happens for the <ng-template> that the directive is on. There is no association between NgIf and any other templates (even if they're passed in as input values to ngIfElse, etc).

Such an association would be difficult to build into the system.

Sure, you can narrow multiple expressions

The problem is not about multiple expressions, but multiple <ng-template>:

<!-- items: number[] | null -->
<ng-container *ngIf="items then foo else bar"></ng-container>
<ng-template #foo>{{ items.length }}<!-- No error here --></ng-template>
<ng-template #bar>{{ items.length }}<!-- Throw error here --></ng-template>

And to support that there need to be type guard per @Input() and per TemplateRef, the pattern above I suggested is:

static ngTemplateGuard_(propertyNameForExpressionToNarrowWith)_(proprtyNameForTemplateRef)

There is no association between NgIf and any other templates

The association were built when assignment happens, so the type-checking can happens there as well.

Such an association would be difficult to build into the system.

I agree it require some extra work so may not be added in this PR, but it's still quite possible. The basic model is to construct an anonymous type according to actual usage:

type anonymous_0 = { items: { length: unknown } }

And when the assignment happens check whether the actual type is assignable to usage type, I have made an informative example here.

You are correct - your example would not be type-checked at all in the current design. This is a non-trivial problem to solve.

Is anonymous_0 generated from the expressions within the template itself? That's an interesting idea. Off the top of my head it seems very limited and error prone, not to mention exceedingly complicated to generate in the first place.

I like your approach of having additional guard functions for property bindings that happen to take templates. With that approach, even local usage of ngTemplateOutlet could be correctly inferred. The challenge is that the location an <ng-template> is declared can be very different from where it's passed into an input to a directive. For example:

<div *ngIf="blah">
  <!-- deep inside some other structure, we use ngIf -->
  <ng-container *ngIf="items then foo else bar"></ng-container>
</div>

<!-- somewhere up the tree, the templates are actually declared -->
<ng-template #foo>{{ items.length }}<!-- No error here --></ng-template>
<ng-template #bar>{{ items.length }}<!-- Throw error here --></ng-template>

This is totally legal, and tricky to handle - the guards you want around the template TCBs aren't available at the location in the template where they're defined.

What I think we'd have to do is have static methods with two dimensions like you suggest. When evaluating the *ngIf directive on the <ng-container> above, we'd notice that template references are passed to the directive and that narrowing applies. We would assume at that point that the directive intends to render the template, and move the template's TCB inside the appropriate guard, as if it were written at that location anyway.

I think this is a very doable solution, but it's not going to be a priority for now.

I have made a more expressive example of the nesting case above (need to turn on strictNullChecks).

The idea of this is being a side-path for checking the assignment of TemplateRef to @Input(), which is unrelated to the real template instruction, renamed the template functions to _type_checking.

Due to the static nature of context variable, compiler could understand which context it's the variable belongs (e.g. nextContext(1)), so we'll always have a maximum type to be the declared context type. Given the derived type of expressions from nextContext(N), so inside the template we can determine that real expression type *_Expr_NextContext_N to be the intersection of these two:

type App_Context = { blah: ({ items: number[] | null }) | null }
type Container_Expr_NextContext_1 = { blah: { items: unknown } } & App_Context

This approach should be independent of templates nesting as error always happens on input assignment, not template itself. And conceptually being canonical, could even handle the case of same template being used in several places.

_{(I guess the idea being more close to Flow rather than TypeScript?)}

Also I totally agree this is not an easy work and not of a high priority.

_{(Also TypeScript seems to have bug for { items: unknown } & false not narrow to never)}

Why is the param optional, now we have to cheat here?

That's the original API - we can refactor this more in the future. In general, this cast is marking the expectation that callbackCtxt fits into T. If they're using the optional parameter, then T should be |undefined.

incomplete comment?

Fixed.

incomplete comment

Fixed.

If it's missing, its value can be seen as being undefined in which case we should use false as type guard?

EDIT Hmm, depends on the directive's own (default) value of course.

No, we just exclude the guard from the list. Not having a matching guard function just means no narrowing of that input expression takes place.

If no directive guards, why generate if (true) { ... }, where just { ... } would suffice to create a new block?

No particular reason. This way all the blocks get generated with the same structure.

incomplete comment

Fixed.

N is not used?

Fixed.

Did TS not infer non-nullable node.name here? It is guarded at the top.

Good point. Fixed.

It is currently not tested if the cached SF was already transformed, is it?

Very good catch - fixed.

Why not JSON.stringify this whole object literal?

No reason - this was moved code, I didn't modify the semantics.

@alxhub whoops, this is very much broken now ;)

Instead of a set, I would insert a cache entry such as { transformed: false, sourceFile: sourceFile } into the sfCache instead. Seems more efficient, I'd expect.

D'oh, of course it is. That's what I get for going too fast.

Fixed for real this time.