[TS-01] No strictNullChecks: ~65 type annotations lie, hiding null-safety bugs at runtime

[mairas]

Severity: high | Confidence: confirmed | Area: type-safety | Source: codebase review 2026-06-30, finding TS-01

Problem

Neither tsconfig.json nor any extended config (src/tsconfig.app.json, src/tsconfig.spec.json) enables strict, strictNullChecks, noImplicitAny, or strictPropertyInitialization. The only strictness options present are Angular-specific template checks (strictTemplates and strictInjectionParameters). As a result, approximately 65 non-test declarations annotate a non-nullable type such as string or number while being initialised to null — for example let currentValue: string = null and private serverEndpoint: string = null. TypeScript silently accepts these, so the annotations actively misrepresent the runtime state: a variable typed string legitimately holds null, and every downstream operation that trusts the annotation (string methods, arithmetic, comparisons) can fail at runtime without any compile-time warning.

Impact

In a marine dashboard context, Signal K data paths frequently emit null on timeout, sensor loss, or initialisation. Without strictNullChecks, the type system provides zero protection at those boundaries: null values flow through code annotated as non-nullable, reaching operations that assume a valid string or number, which can produce silent NaN results, TypeError exceptions, or corrupt display values. For a fork targeting long-term reliability in a safety-relevant marine environment, enabling strict null checking would surface real latent bugs mechanically rather than requiring manual audit.

Affected code

• tsconfig.json:1-26
• src/tsconfig.app.json
• src/tsconfig.spec.json
• src/app/core/services/data.service.ts:247-248
• src/app/core/services/storage.service.ts:39-43
• src/app/core/services/signalk-delta.service.ts:58
• src/app/widgets/minichart/minichart.component.ts:39-48

Suggested direction

Enable strictNullChecks incrementally: introduce a tsconfig.strict.json that extends the base config and enables strictNullChecks, wire it into CI as a ratchet (fail on regression, do not require 100% compliance on day one), then work through the ~65 lying annotations changing them to proper union types (string | null, number | null, etc.). Once that is clean, enable the remaining strict flags and add noImplicitReturns and noFallthroughCasesInSwitch. This approach is largely mechanical and surfaces real data-flow bugs rather than introducing new behaviour.

Verification

Verified against the codebase: all four cited = null declarations confirmed verbatim; repo-wide grep of all tsconfig*.json and angular.json found only strictTemplates and strictInjectionParameters — no strict, strictNullChecks, noImplicitAny, or strictPropertyInitialization anywhere (confidence: confirmed).

[mairas]

Ratchet candidates surfaced by the #23 fix and the #27 characterization pass (PRs #146/#147) — de-facto-nullable declarations that strictNullChecks will force, listed here so the ratchet slices can claim them:

• ISkPathData.type (app-interfaces.ts:60) — declared string, de facto string | undefined: data.service.ts assigns undefined into it (null first value; meta without units) and explicitly branches on pathItem.type === undefined (~line 417).
• ISkMetadata.units (signalk-interfaces.ts:184) — declared required, de facto optional: typeFromUnits guards !units and real SK metadata frequently omits units.
• remote-dashboards.service.ts:88 — let screensPayload: IScreensPayload = undefined (later null); the null-as-clear sentinel that setScreensOnRemote handles is undocumented in the type.
• Unguarded toSignal(...) dereferences that only avoid a TypeError because DataService.subscribePath happens to emit synchronously (BehaviorSubject-backed): app-service.ts:77,101 and remote-dashboards.service.ts:135. An async-emitting source would crash these effects; strict null checks will surface both.

[mairas]

Approach revised after measuring scope

Enabling strictNullChecks on the app sources surfaces 959 issues across 73 files, not ~65 — the "~65 lying annotations" are only the tip; the cascade (possibly-null derefs, argument mismatches) dominates, concentrated in the ~46 widgets. So this is delivered as an incremental ratchet, not a single flip:

• strictNullChecks stays off in the app build; tsconfig.strict.json enables it for npm run snc and the ratchet.
• betterer commits a .betterer.results baseline and fails CI only on new issues. Each slice ratchets the count down; nothing can regress.

Progress

• Ratchet infrastructure + baseline (PR Add strictNullChecks ratchet + fix the core service layer (#6) #208)
• Core service layer — 116 fixed, 959 → 843 (PR Add strictNullChecks ratchet + fix the core service layer (#6) #208). Includes the ratchet candidates from the comment above: ISkPathData.type widened, ISkMetadata.units made optional, screensPayload sentinel typed honestly, toSignal derefs guarded.
• Widgets & remaining components (843 remaining) — follow-up slices. widget-data-chart (164) and widget-windtrends-chart (100) dominate.
• Once at zero: enable the rest of strict, plus noImplicitReturns / noFallthroughCasesInSwitch.

Fixes are honest — no !, casts, or suppressions; genuinely-nullable values are widened and handled at use.