LegendApp / legend-state

Legend-State is a super fast and powerful state library that enables fine-grained reactivity and easy automatic persistence
https://legendapp.com/open-source/state/
MIT License
2.75k stars 81 forks source link

typescript infers optional observable object props wrong? #353

Open psiho opened 3 weeks ago

psiho commented 3 weeks ago

Using 2.1.14

I have observavble object:

myObservableObject: {
  something: true,
  start?: {
    cityName: string
  } | null
}

And I try to use cityName prop in my React component like: const city = myObservableObject.start.cityName.get() ... I was expecting TS error here, that start might be null or undefined, but instead ther's no error and city infers type string After ton of code written, I've started getting type errors because of this in several places (my Observable object is quite large and complex)

Is this bug? Or my TS knowledge is lacking? How to deal with this and keep using TS to help locate errors in types? I know I can do: const city = myObservableObject.start.get()?.cityName but in this case I have to search for all problem points myself, without using TS help. That beats the point of TS as I'm surely going to miss many spots.

jmeistrich commented 6 days ago

Can you try the latest v3 @alpha version? One of the changes was rewriting the types, especially around nullability.

But I do think this is actually correct in this case:

const city = myObservableObject.start.cityName.get()

An observable can never be null or undefined so there can't be any TS or runtime errors there. Then the type of cityName is string so that is correct too. Making all child properties nullable because a parent is nullable would likely cause more problems I think... I'm not sure how to do this better - what would you suggest?

psiho commented 6 days ago

tbh, I don't know how to do it better either. I understand observable cannot be null and I don't see an alternative. I agree making child properties nullable might just cause more trouble. It was more a rant and ask for help. I guess I bacame very dependant on typescript and LegendState also is very simple to use, to the point it appears like regular variables typed in the same way as other ones. So, for now, I'm happy to be aware of this and my only suggestion can be to include something about this case in docs somewhere. Like best practices, because I don't think it's that obvious at first. For the rest, my TS knowledge is not on the level to suggest anything better.