spikerj/spikersoft-issues
1
0 Fork 0
Code Issues 23 Pull Requests Actions Packages Projects Releases Wiki Activity

NetworkStatusService uses Inject() instead of inject() — DI is broken #840

New Issue
Closed
opened 2026-07-25 04:03:02 +00:00 by spikerj · 2 comments
spikerj commented 2026-07-25 04:03:02 +00:00
Owner
Copy link
Copy source

Context

Found during unit-test quality audit (draft PR spikersoft-angular#574).

Bug

projects/spikersoft/src/app/_services/marks-site/network-status/network-status.service.ts assigns:

private readonly networkState: NetworkState = Inject(NetworkState);

That stores the Inject decorator/function result, not an injected NetworkState instance. It should be inject(NetworkState) (or constructor injection).

The existing spec is an it.skip placeholder with a TODO — the right behavioral tests are unclear until the DI bug is fixed (and a MockNetworkState wired correctly).

Ask

Fix DI, then un-skip / rewrite network-status.service.spec.ts to assert online/offline + connection type via a mock NetworkState.

## Context Found during unit-test quality audit (draft PR spikersoft-angular#574). ## Bug `projects/spikersoft/src/app/_services/marks-site/network-status/network-status.service.ts` assigns: ```ts private readonly networkState: NetworkState = Inject(NetworkState); ``` That stores the `Inject` decorator/function result, not an injected `NetworkState` instance. It should be `inject(NetworkState)` (or constructor injection). The existing spec is an `it.skip` placeholder with a TODO — the right behavioral tests are unclear until the DI bug is fixed (and a `MockNetworkState` wired correctly). ## Ask Fix DI, then un-skip / rewrite `network-status.service.spec.ts` to assert online/offline + connection type via a mock `NetworkState`.
spikerj commented 2026-07-29 18:21:27 +00:00
Author
Owner
Copy link
Copy source

Audited against origin/master — STILL BROKEN, description exact, plus one thing worth adding to the notes.

projects/spikersoft/src/app/_services/marks-site/network-status/network-status.service.ts:6:

private readonly networkState: NetworkState = Inject(NetworkState);

with the capital-Inject decorator imported at :1. Calling a decorator as a function in a field initializer doesn't resolve anything from DI — exactly as filed.

The addition: NetworkStatusService has no consumer. Grepping for it across non-spec .ts files returns only its own declaration at :5. Nothing injects it anywhere.

That explains why this has never surfaced at runtime, and it changes how to think about the fix: this isn't a live DI failure users are hitting, it's a service that would break the moment someone first injects it. So it's cleanup-before-use rather than a bug fix — and worth doing before anyone wires it up, since the failure at that point would look like a problem with the consumer rather than with this file.

Priority context from the same eleven-ticket sweep: #840, #792 and #808 are all latent with no production caller. The live ones are #811 (broken AES on a real network path, security-relevant, and misleadingly carrying a test(coverage) commit that only pinned it), #793, #794, #795, #797 and #799. If these get worked as a batch, the live set is where the user-visible impact is.

This one is a genuinely two-line fix (inject(NetworkState) lowercase, drop the decorator import), so it's cheap to clear regardless.

Audited against `origin/master` — **STILL BROKEN, description exact, plus one thing worth adding to the notes.** `projects/spikersoft/src/app/_services/marks-site/network-status/network-status.service.ts:6`: ```ts private readonly networkState: NetworkState = Inject(NetworkState); ``` with the capital-`Inject` **decorator** imported at `:1`. Calling a decorator as a function in a field initializer doesn't resolve anything from DI — exactly as filed. **The addition: `NetworkStatusService` has no consumer.** Grepping for it across non-spec `.ts` files returns only its own declaration at `:5`. Nothing injects it anywhere. That explains why this has never surfaced at runtime, and it changes how to think about the fix: this isn't a live DI failure users are hitting, it's a service that would break the moment someone first injects it. So it's cleanup-before-use rather than a bug fix — and worth doing *before* anyone wires it up, since the failure at that point would look like a problem with the consumer rather than with this file. **Priority context** from the same eleven-ticket sweep: **#840, #792 and #808** are all latent with no production caller. The live ones are **#811** (broken AES on a real network path, security-relevant, and misleadingly carrying a `test(coverage)` commit that only *pinned* it), **#793**, **#794**, **#795**, **#797** and **#799**. If these get worked as a batch, the live set is where the user-visible impact is. This one is a genuinely two-line fix (`inject(NetworkState)` lowercase, drop the decorator import), so it's cheap to clear regardless.
spikerj commented 2026-08-07 13:48:13 +00:00
Author
Owner
Copy link
Copy source

Migrated to spikerj/spikersoft-angular#641 as part of the umbrella-tracker breakup.

Verified 2026-08-07 against spikersoft-angular@8e5a404.

  • Code: Unchanged, exactly as filed. projects/spikersoft/src/app/_services/marks-site/network-status/network-status.service.ts:1 imports the capital-Inject decorator from @angular/core; :6 reads private readonly networkState: NetworkState = Inject(NetworkState);. :7-8 then immediately read this.networkState.isOnline / .connectionType off that non-instance, and :21-27 register listeners on it. The 2026-07-29 comment's addition also holds: the only non-spec reference to NetworkStatusService in the repo is its own declaration at :5 — nothing injects it.
  • Live: Nothing runtime-observable — the service has no consumer, so it is never instantiated in production. This is cleanup-before-use, not a live failure.

Status: not done — The two-line fix (inject(NetworkState), drop the decorator import), then un-skip / rewrite network-status.service.spec.ts against a MockNetworkState.

Closing here. Work now lives in the repo that holds the fix, so fixes #641 in a PR will auto-close it on merge. The umbrella tracker keeps cross-repo epics only.

— Opus 5 Agent

Migrated to **spikerj/spikersoft-angular#641** as part of the umbrella-tracker breakup. Verified 2026-08-07 against `spikersoft-angular@8e5a404`. - **Code:** Unchanged, exactly as filed. `projects/spikersoft/src/app/_services/marks-site/network-status/network-status.service.ts:1` imports the capital-`Inject` **decorator** from `@angular/core`; `:6` reads `private readonly networkState: NetworkState = Inject(NetworkState);`. `:7-8` then immediately read `this.networkState.isOnline` / `.connectionType` off that non-instance, and `:21-27` register listeners on it. The 2026-07-29 comment's addition also holds: the only non-spec reference to `NetworkStatusService` in the repo is its own declaration at `:5` — nothing injects it. - **Live:** Nothing runtime-observable — the service has no consumer, so it is never instantiated in production. This is cleanup-before-use, not a live failure. Status: **not done** — The two-line fix (`inject(NetworkState)`, drop the decorator import), then un-skip / rewrite `network-status.service.spec.ts` against a `MockNetworkState`. Closing here. Work now lives in the repo that holds the fix, so `fixes #641` in a PR will auto-close it on merge. The umbrella tracker keeps cross-repo epics only. — Opus 5 Agent
Sign in to join this conversation.
No Branch/Tag Specified
Labels
Clear labels
agentic
Actionable surfaced by the agentic architecture & security review
 bug
Something is not working
 duplicate
This issue or pull request already exists
 enhancement
New feature
 epic
Cross-repo epic — children live in per-repo trackers
 help wanted
Need some help
 invalid
Something is wrong
 question
More information is needed
 sonarqube
Code quality issue surfaced by SonarQube (bug / code smell)
 wontfix
This won't be fixed
No labels
Milestone
No items
No Milestone
Projects
Clear projects
No projects
Assignees
Clear assignees
spikerj (Joseph Spiker)
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: spikerj/spikersoft-issues#840