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

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

New Issue
Open
opened 2026-07-25 04:03:02 +00:00 by spikerj · 1 comment
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.
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
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
 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