Skip to content

Activated flag strictNullInputTypes on Angular 21 #7151

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
cdavalos7 wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Activated flag strictNullInputTypes on Angular 21 #7151

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
17c0630
Activated flag strictNullInputTypes, causing breaking async pipes is …
cdavalos7 Aug 26, 2026
a9eb642
Removed redundancy, avoid subscribe twice to same observable
cdavalos7 Sep 1, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 12 additions & 14 deletions tensorboard/plugins/debugger_v2/tf_debugger_v2_plugin/debugger_container.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ import {
OnDestroy,
OnInit,
} from '@angular/core';
import {createSelector, select, Store} from '@ngrx/store';
import {createSelector, Store} from '@ngrx/store';
import {debuggerLoaded, debuggerUnloaded} from './actions';
import {getActiveRunId, getDebuggerRunListing} from './store';
import {State} from './store/debugger_types';
Expand All @@ -29,9 +29,9 @@ import {State} from './store/debugger_types';
selector: 'tf-debugger-v2',
template: `
<debugger-component
[runs]="runs$ | async"
[runIds]="runsIds$ | async"
[activeRunId]="activeRunId$ | async"
[runs]="runs()"
[runIds]="runsIds()"
[activeRunId]="activeRunId()"
></debugger-component>
`,
styles: [
Expand All @@ -44,22 +44,20 @@ import {State} from './store/debugger_types';
],
})
export class DebuggerContainer implements OnInit, OnDestroy {
readonly runs$;
readonly runs;

readonly runsIds$;
readonly runsIds;

readonly activeRunId$;
readonly activeRunId;

constructor(private readonly store: Store<State>) {
this.runs$ = this.store.pipe(select(getDebuggerRunListing));
this.runsIds$ = this.store.pipe(
select(
createSelector(getDebuggerRunListing, (runs): string[] =>
Object.keys(runs)
)
this.runs = this.store.selectSignal(getDebuggerRunListing);
this.runsIds = this.store.selectSignal(
createSelector(getDebuggerRunListing, (runs): string[] =>
Object.keys(runs)
)
);
this.activeRunId$ = this.store.pipe(select(getActiveRunId));
this.activeRunId = this.store.selectSignal(getActiveRunId);
}

ngOnInit(): void {
Expand Down
44 changes: 21 additions & 23 deletions tensorboard/plugins/debugger_v2/tf_debugger_v2_plugin/views/alerts/alerts_container.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ See the License for the specific language governing permissions and
limitations under the License.
==============================================================================*/
import {ChangeDetectionStrategy, Component} from '@angular/core';
import {createSelector, select, Store} from '@ngrx/store';
import {createSelector, Store} from '@ngrx/store';
import {alertTypeFocusToggled} from '../../actions';
import {
getAlertsBreakdown,
Expand Down Expand Up @@ -49,40 +49,38 @@ const ALERT_TYPE_TO_DISPLAY_NAME_AND_SYMBOL: {
selector: 'tf-debugger-v2-alerts',
template: `
<alerts-component
[numAlerts]="numAlerts$ | async"
[alertsBreakdown]="alertsBreakdown$ | async"
[focusType]="focusType$ | async"
[numAlerts]="numAlerts()"
[alertsBreakdown]="alertsBreakdown()"
[focusType]="focusType()"
(onToggleFocusType)="onToggleFocusType($event)"
>
</alerts-component>
`,
changeDetection: ChangeDetectionStrategy.OnPush,
})
export class AlertsContainer {
readonly numAlerts$;
readonly numAlerts;

@arcra arcra Sep 3, 2026

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.

See my comment below, but if we're not assigning something directly to these, we should specify the type for each of them (although at that point, then I might prefer the direct assignment).

AFAIU, with a direct assignment at declaration, the static analyzer can infer the type, and thus, also signal when the variable is not used as it should be used. I don't know if it's the same for these without a type. I would imagine that the static analyzer would consider them to have type any, which would not be helpful to catch errors.

Although... since they're declared as "readonly" ... it's possible that the analyzer knows how to infer the type from the assignment in the constructor... I don't really know (google says that that's not the case).

I recognize this is also not introduced here, but that's how it was before, so maybe it's ok to leave it as is, but in the future, let's prefer assignment at declaration.


readonly alertsBreakdown$;
readonly alertsBreakdown;

readonly focusType$;
readonly focusType;

constructor(private readonly store: Store<State>) {
this.numAlerts$ = this.store.pipe(select(getNumAlerts));
this.alertsBreakdown$ = this.store.pipe(
select(
createSelector(getAlertsBreakdown, (alertsBreakdown) => {
const alertTypes = Object.keys(alertsBreakdown);
alertTypes.sort();
return alertTypes.map((alertType): AlertTypeDisplay => {
return {
type: alertType as AlertType,
...ALERT_TYPE_TO_DISPLAY_NAME_AND_SYMBOL[alertType],
count: alertsBreakdown[alertType],
};
});
})
)
this.numAlerts = this.store.selectSignal(getNumAlerts);

@arcra arcra Sep 3, 2026

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.

No need to change it in this PR if you prefer the alternative of adding the type to the variables, but in the future, the recommended pattern is to use inject() and direct assignment of the signals outside of the constructor (and the constructor can likely be removed).

this.alertsBreakdown = this.store.selectSignal(
createSelector(getAlertsBreakdown, (alertsBreakdown) => {

@arcra arcra Sep 3, 2026

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.

No need to change it in this PR, but wondering what you think about these being computed signals instead of a selector defined inline?

I guess we'd need to first have a signal for the existing selector and then a separate computed signal for the thing we actually care about... which is a bit annoying.

const alertTypes = Object.keys(alertsBreakdown);
alertTypes.sort();
return alertTypes.map((alertType): AlertTypeDisplay => {
return {
type: alertType as AlertType,
...ALERT_TYPE_TO_DISPLAY_NAME_AND_SYMBOL[alertType],
count: alertsBreakdown[alertType],
};
});
})
);
this.focusType$ = this.store.pipe(select(getAlertsFocusType));
this.focusType = this.store.selectSignal(getAlertsFocusType);
}

onToggleFocusType(alertType: AlertType) {
Expand Down
2 changes: 1 addition & 1 deletion ...lugins/debugger_v2/tf_debugger_v2_plugin/views/execution_data/execution_data_component.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ export class ExecutionDataComponent {
focusedExecutionIndex!: number;

@Input()
focusedExecutionData!: Execution;
focusedExecutionData!: Execution | null;

@Input()
tensorDebugMode: TensorDebugMode = TensorDebugMode.UNSPECIFIED;
Expand Down
159 changes: 70 additions & 89 deletions ...lugins/debugger_v2/tf_debugger_v2_plugin/views/execution_data/execution_data_container.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ See the License for the specific language governing permissions and
limitations under the License.
==============================================================================*/
import {ChangeDetectionStrategy, Component, Input} from '@angular/core';
import {createSelector, select, Store} from '@ngrx/store';
import {createSelector, Store} from '@ngrx/store';
import {getFocusedExecutionData} from '../../store';
import {Execution, State, TensorDebugMode} from '../../store/debugger_types';
import {DTYPE_ENUM_TO_NAME} from '../../tf_dtypes';
Expand All @@ -27,115 +27,96 @@ const UNKNOWN_DTYPE_NAME = 'Unknown dtype';
template: `
<execution-data-component
[focusedExecutionIndex]="focusedExecutionIndex"
[focusedExecutionData]="focusedExecutionData$ | async"
[tensorDebugMode]="tensorDebugMode$ | async"
[hasDebugTensorValues]="hasDebugTensorValues$ | async"
[debugTensorValues]="debugTensorValues$ | async"
[debugTensorDtypes]="debugTensorDtypes$ | async"
[focusedExecutionData]="focusedExecutionData()"
[tensorDebugMode]="tensorDebugMode()"
[hasDebugTensorValues]="hasDebugTensorValues()"
[debugTensorValues]="debugTensorValues()"
[debugTensorDtypes]="debugTensorDtypes()"
></execution-data-component>
`,
})
export class ExecutionDataContainer {
@Input()
focusedExecutionIndex!: number;

readonly focusedExecutionData$;
readonly focusedExecutionData;

readonly tensorDebugMode$;
readonly tensorDebugMode;

readonly hasDebugTensorValues$;
readonly hasDebugTensorValues;

readonly debugTensorValues$;
readonly debugTensorValues;

readonly debugTensorDtypes$;
readonly debugTensorDtypes;

constructor(private readonly store: Store<State>) {
this.focusedExecutionData$ = this.store.pipe(
select(getFocusedExecutionData)
this.focusedExecutionData = this.store.selectSignal(
getFocusedExecutionData
);
this.tensorDebugMode$ = this.store.pipe(
select(
createSelector(
getFocusedExecutionData,
(execution: Execution | null) => {
if (execution === null) {
return TensorDebugMode.UNSPECIFIED;
} else {
return execution.tensor_debug_mode;
}
}
)
)
this.tensorDebugMode = this.store.selectSignal(
createSelector(getFocusedExecutionData, (execution: Execution | null) => {

@arcra arcra Sep 3, 2026

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.

This one should have more clearly been a computed signal, since we already have a signal for the selector used here.

(But again, no need to change it here. I understand you might have just got for a simple code update, especially considering the amount of files modified.)

if (execution === null) {
return TensorDebugMode.UNSPECIFIED;
} else {
return execution.tensor_debug_mode;
}
})
);
this.hasDebugTensorValues$ = this.store.pipe(
select(
createSelector(
getFocusedExecutionData,
(execution: Execution | null) => {
if (execution === null || execution.debug_tensor_values === null) {
return false;
} else {
for (const singleDebugTensorValues of execution.debug_tensor_values) {
if (
singleDebugTensorValues !== null &&
singleDebugTensorValues.length > 0
) {
return true;
}
}
return false;
this.hasDebugTensorValues = this.store.selectSignal(
createSelector(getFocusedExecutionData, (execution: Execution | null) => {
if (execution === null || execution.debug_tensor_values === null) {
return false;
} else {
for (const singleDebugTensorValues of execution.debug_tensor_values) {
if (
singleDebugTensorValues !== null &&
singleDebugTensorValues.length > 0
) {
return true;
}
}
)
)
return false;
}
})
);
this.debugTensorValues$ = this.store.pipe(
select(
createSelector(
getFocusedExecutionData,
(execution: Execution | null) => {
if (execution === null) {
return null;
} else {
return execution.debug_tensor_values;
}
}
)
)
this.debugTensorValues = this.store.selectSignal(
createSelector(getFocusedExecutionData, (execution: Execution | null) => {
if (execution === null) {
return null;
} else {
return execution.debug_tensor_values;
}
})
);
this.debugTensorDtypes$ = this.store.pipe(
select(
createSelector(
getFocusedExecutionData,
(execution: Execution | null): string[] | null => {
if (execution === null || execution.debug_tensor_values === null) {
return null;
}
if (
execution.tensor_debug_mode !== TensorDebugMode.FULL_HEALTH &&
execution.tensor_debug_mode !== TensorDebugMode.SHAPE
) {
// TODO(cais): Add logic for other TensorDebugModes with dtype info.
return null;
}
const dtypes: string[] = [];
for (const tensorValue of execution.debug_tensor_values) {
if (tensorValue === null) {
dtypes.push(UNKNOWN_DTYPE_NAME);
} else {
const dtypeEnum = String(
execution.tensor_debug_mode === TensorDebugMode.FULL_HEALTH
? tensorValue[2] // tensor_debug_mode: FULL_HEALTH
: tensorValue[1] // tensor_debug_mode: SHAPE
);
dtypes.push(
DTYPE_ENUM_TO_NAME[dtypeEnum] || UNKNOWN_DTYPE_NAME
);
}
this.debugTensorDtypes = this.store.selectSignal(
createSelector(
getFocusedExecutionData,
(execution: Execution | null): string[] | null => {
if (execution === null || execution.debug_tensor_values === null) {
return null;
}
if (
execution.tensor_debug_mode !== TensorDebugMode.FULL_HEALTH &&
execution.tensor_debug_mode !== TensorDebugMode.SHAPE
) {
// TODO(cais): Add logic for other TensorDebugModes with dtype info.
return null;
}
const dtypes: string[] = [];
for (const tensorValue of execution.debug_tensor_values) {
if (tensorValue === null) {
dtypes.push(UNKNOWN_DTYPE_NAME);
} else {
const dtypeEnum = String(
execution.tensor_debug_mode === TensorDebugMode.FULL_HEALTH
? tensorValue[2] // tensor_debug_mode: FULL_HEALTH
: tensorValue[1] // tensor_debug_mode: SHAPE
);
dtypes.push(DTYPE_ENUM_TO_NAME[dtypeEnum] || UNKNOWN_DTYPE_NAME);
}
return dtypes;
}
)
return dtypes;
}
)
);
}
Expand Down
9 changes: 6 additions & 3 deletions tensorboard/plugins/debugger_v2/tf_debugger_v2_plugin/views/graph/graph_component.ng.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,10 +20,13 @@

<div class="graph-structure-container">
<div *ngIf="opInfo !== undefined && opInfo !== null; else noOpFocused">
<div *ngIf="inputOps.length > 0; else noInputs" class="inputs-container">
<div
*ngIf="(inputOps ?? []).length > 0; else noInputs"
class="inputs-container"
>
<div>
<div
*ngFor="let inputOpInfo of inputOps; let slot = index"
*ngFor="let inputOpInfo of inputOps ?? []; let slot = index"
class="input-op-section"
>
<div class="input-slot-header">Input slot {{slot}}:</div>
Expand Down Expand Up @@ -63,7 +66,7 @@
>
<div>
<div
*ngFor="let slotConsumers of consumerOps; let slot = index"
*ngFor="let slotConsumers of consumerOps ?? []; let slot = index"
class="slot-consumers-container"
>
<div class="slot-consumers-header">
Expand Down
10 changes: 5 additions & 5 deletions tensorboard/plugins/debugger_v2/tf_debugger_v2_plugin/views/graph/graph_component.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,13 +35,13 @@ import {
})
export class GraphComponent {
@Input()
opInfo!: GraphOpInfo;
opInfo!: GraphOpInfo | null;

@arcra arcra Sep 3, 2026

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.

Why does this variable have a !, and then also a | null type?

Is the ! character saying that we're expecting it to never be null or undefined? I think this needs to be updated?


@Input()
inputOps!: GraphOpInputSpec[];
inputOps!: GraphOpInputSpec[] | null;

@Input()
consumerOps!: GraphOpConsumerSpec[][];
consumerOps!: GraphOpConsumerSpec[][] | null;

@Output()
onGraphOpNavigate = new EventEmitter<{graph_id: string; op_name: string}>();
Expand All @@ -50,14 +50,14 @@ export class GraphComponent {
* Get the ID of the immediately-enclosing graph of the op.
*/
get graphId() {
return this.opInfo.graph_ids[this.opInfo.graph_ids.length - 1];
return this.opInfo!.graph_ids[this.opInfo!.graph_ids.length - 1];

@arcra arcra Sep 3, 2026

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 we shouldn't be adding these ! characters. We should either ensure that it can't be null / undefined, or handle the case when it is?

It's possible I'm missing some context. If this is necessary for some reason, let me know.

}

/**
* Total number of consumers of all output tensors of the op.
*/
get totalNumConsumers() {
return this.consumerOps.reduce((count, slotConsumers) => {
return this.consumerOps!.reduce((count, slotConsumers) => {
return count + slotConsumers.length;
}, 0);
}
Expand Down
Loading
Loading