RccSettingsService: Rethink .get<T>, .set<T> .valueChange$<T>()
I stumbled across the following issue with RccSettingsService.valueChange$<T>()
. Trying to deal with it made me reassess .get<T>()
and .set<T>()
being generic.
RccSettingsService.valueChange$<T>(id)
returns an Observable<T>
with T
supposed to be the type of the value handled by the setting refereced to by ìd
So when using the return value things work pretty straight forward:
this.rccSettingsService.valueChange$<string>(someId)
.subscribe( (somevalue: string) => someStringOperation(somevalue) )
Typescript will not complain and neither will the linter. We could even drop the type in the subscribe line and typescript would infer the type 'correctly' as string.
What is the problem then?
The type of the value varies from setting to setting and at the moment typescript has no way of inferring the type from just the id. The method above just forcefully casts the types to whatever type we hand over. That is very convenient because from there on type checking no longer gets in our way. But that is kind of what type checking is all about :D
Consider this example:
this.rccSettingsService.valueChange$<string[]>(transmissionReceivingServiceSettingsEntryId)
.subscribe( receivingServiceIds => this.rccTransmissionService.setAllowedReceivingServices(receivingServiceIds))
In this case I expected the value of this settings entry to be an Array of strings. But for some reason I had a string stored in my browser instead. (That is not the fault of the methods at hand of course) That resulted in a TypeError at runtime, because .setAllowedReceivingServices(receivingServiceIds)
was expecting an Array and tried to perform an Array operation on what was actually a string.
This kind of error seems to be exactly what typescript should be able to prevent. And I would have expected to be warned about one of two things: either that the types do not match or that I would have to manually type check to narrow type in the code.
How to fix:
The only way I can think of is removing the type variables and set everything to unknown instead. This way we're forced to manually type check, when we use those values.
What do you think? Any ideas?
Maybe this is a non-issue: Maybe if we expect a stored value to be of a certain type, then that expectation is legit, and we should not handle this error manually , but instead we just let it drop to the console at runtime?