bluerobotics / bluerobotics/cockpit

Settings changed outside the current tab never reach consumers (handleStorageChanging iterates usernames)

Open Beginner friendly
#2,913 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
TypeScript
Stars
198
Forks
63
Avg merge
2d 7h
Merged PRs (30d)
57

Description

`SettingsManager.handleStorageChanging` is what reacts to the local settings being changed from outside the current context (another tab, another window, or a direct edit). It never notifies anyone, because it iterates the wrong map:

```ts
const newSettings = this.getLocalSettings() // LocalSyncedSettings -> keys are USERNAMES
const userVehicleSettings = this.getSettingsForUserAndVehicle(this.currentUsername, this.currentVehicleId)
...
Object.keys(newSettings).forEach((key) => { // key is a username
if (userVehicleSettings[key] !== this.lastLocalUserVehicleSettings[key]) {
this.notifyListenersAboutKeyChange(key, userVehicleSettings[key])
}
})
```

`getLocalSettings()` returns `LocalSyncedSettings`, keyed by username (`src/types/settings-management.ts:51-56`), while `userVehicleSettings` is a `SettingsPackage` keyed by setting name. So:

- `userVehicleSettings[]` is always `undefined`, so the comparison is `undefined !== undefined` and the branch is normally not even entered.
- When it is entered, `this.listeners[]` is empty, so no consumer is ever notified.
- It also passes `undefined` where `notifyListenersAboutKeyChange` declares a non-optional `CockpitSetting`. `tsc` does not catch it because `noUncheckedIndexedAccess` is off.

Two consequences: a settings change made in another tab never reaches the running one until a reload, and the type violation is invisible.

The fix looks like one word — iterate `Object.keys(userVehicleSettings)` — but it turns a currently dead notification path live, so it wants its own testing rather than riding along in an unrelated fix.

Found while reviewing #2912.

Contributor guide

No contributing guide indexed for this repository

Research direction

Start at SettingsManager.handleStorageChanging and inspect the LocalSyncedSettings definition at src/types/settings-management.ts:51-56, then trace notifyListenersAboutKeyChange and the current user/vehicle settings lookup. Add focused tests for a settings change made outside the current context and verify that the relevant consumer is notified without undefined values.

Written by the indexing model from the issue text.

Assessment

Tech stack
typescript
Domain
frontend
Issue type
Bug
Difficulty
2/5
Estimated time
1-3 hours
Activity status
Quiet
Clarity
Clearly specified
Newbie friendliness
76/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.