Skip to content

Angular 21 - Turned on strictTemplates flag - #7148

Open
cdavalos7 wants to merge 1 commit into
tensorflow:masterfrom
cdavalos7:feature/strict-templates-on-21
Open

Angular 21 - Turned on strictTemplates flag#7148
cdavalos7 wants to merge 1 commit into
tensorflow:masterfrom
cdavalos7:feature/strict-templates-on-21

Conversation

@cdavalos7

Copy link
Copy Markdown
Contributor

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.

@cdavalos7
cdavalos7 marked this pull request as ready for review August 20, 2026 00:42
@cdavalos7
cdavalos7 requested a review from arcra August 21, 2026 18:53
/** Debug tensor values under non-FULL_TENSOR debug modes. */
@Input()
debugTensorValues: number[][] | null = null;
debugTensorValues: (number[] | null)[] | null = null;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 ?

</button>
<nav
*ngIf="(width$ | async) > 0"
*ngIf="((width$ | async) ?? 0) > 0"

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

autocomplete="off"
[placeholder]="placeholder"
[matAutocomplete]="matAutocomplete"
[matAutocomplete]="matAutocomplete!"

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

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


@Input()
customFormatter?: Formatter;
customFormatter?: Formatter | undefined;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

[axisDirection]="axisDirection"
[timeSelection]="timeSelection"
[startStepAxisPosition]="getAxisPositionFromStartStep()"
[startStepAxisPosition]="$any(getAxisPositionFromStartStep())"

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

@Input() regexFilterValue!: string;
@HostBinding('class.valid') @Input() isRegexFilterValid!: boolean;
@Input() completions!: string[];
@Input() completions: string[] | null = null;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants