bluerobotics / bluerobotics/cockpit
Settings changed outside the current tab never reach consumers (handleStorageChanging iterates usernames)
- 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