Angular 21 - Turned on strictTemplates flag

[cdavalos7]

Motivation for features / changes

Turns on strictTemplates. The flag was already set intsconfig.json, but it never reached the Angular compiler because of a gap in the Bazel build tooling. Fixing that bring up a batch of template type errors that this PR resolves.

Technical description of changes

• Patched @bazel/concatjs so angularCompilerOptions from the root tsconfig.json reaches the compiler. This is what made strictTemplates actually apply.

• strictTemplates regulated template validation in 38 source files. Essentially a clean up was made with correct typing and unused code:

  • Type corrections, mostly event handler params and inputs or outputs whose declared types never matched what callers passed.
  • Five real bugs, all previously silent. A dialog reading a field that does not exist, a misspelled event property, two trackBy functions receiving the index instead of the item, an observable field assigned a plain value, and an input never passed down from its container.
  • Dead template code removed, all verified inert at runtime. Bindings to non-existent outputs and members, plus a Material floatLabel="never" that stopped being valid several versions ago.

• strictNullInputTypes is false here, since AsyncPipe is typed T | null and rejects all 166 async bindings. A follow-up PR moves those containers onto signals and turns it back on.

Verification

• //tensorboard/webapp/... builds clean, 691 targets.
• Karma green, 2014 specs plus 66 in feature_flag.

[arcra]

I think it would be more straightforward to just have the separate types on a flat level:

number[][] | Array<null> | null

(I tried internally and I got a lint about using null[] type, saying Array type using 'T[]' is forbidden for non-simple types. Use 'Array<T>' instead)

Is Array<null> necessary? Can it be simply number[][] | null ?

[cdavalos7]

Good point. Thank you I just created a dry-run CL to review the PR. And yes, it's easier to read with Array<number[] | null> | null.

number[][] | Array<null> | null can not be use because doesn't allow to mix different values in the same array. All the elements needs to be number[] or null... we need to allow different values like [[1,2], null, [3]]

[arcra]

I was thinking we can use startWith instead, in the source observable, so we would have something like:

this.width$ = this.store.select(getSideBarWidthInPercent).pipe(
      startWith(0), // I added this line.
      combineLatestWith(this.runsTableFullScreen$),
      map(([percentageWidth, fullScreen]) => {
        return fullScreen ? 100 : percentageWidth;
      })
    );

Hmmm... but actually, I'm not sure if this would allow getting rid of the async pipe, maybe not... nor whether TS would be able to tell that it will have a non-null / non-undefined value.

To me, this seemed simpler to reason about and handle, but maybe this is fine. You can check if that works and decide.

[cdavalos7]

Good catch, but even with startWith(0) on the source, AsyncPipe.transform still types the result as T | null, so the template would still see number | null and > 0 would break.

[arcra]

How can we be sure this is not undefined or null?

Is having the matAutocompleteDisabled attribute the intended way to support this?

[cdavalos7]

Yes, that's the guard. When matAutocomplete is undefined, matAutocompleteDisabled is also true, so Material actually never touches that ! value.

[arcra]

Isn't this redundant with the ? at the end of the name? Should that be removed, then?

[cdavalos7]

Thank you, yes that was redundant.

[arcra]

Maybe we can ensure this function returns a number (e.g. fall back to zero)?

[arcra]

Can we make the default value be an empty array instead? And we can remove the null checks above, in the template.

[cdavalos7]

Thank you, good simplification