-
Notifications
You must be signed in to change notification settings - Fork 527
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Some noImplicitAny TS fixes #2385
Conversation
params: NavigationParams, | ||
): Entity | null | undefined { | ||
): Entity | undefined { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder what kinda state we're in if this doesn't have an Entity?
There's a bunch of code in the frontend that assumes this exists.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A very bad state. I think it's unreachable unless the state or the store get horribly corrupted somehow.
Really this might be taken as a sign that we should deeply refactor the state handling to reflect this. Not sure if it's worth the trouble, though.
>, | ||
history: ReturnType<typeof historySelector>, | ||
entity: ReturnType<typeof entities.selectors.getSelectedEntity>, | ||
) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is the return type intentionally dropped here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, TS is good about deducing it. And leaving out null
and undefined
should help when we transition to strictNull
Co-authored-by: Eemeli Aro <eemeli@gmail.com>
This is mostly about fixing selectors to work on the
RootState
.This reduces the TS errors for
--noImplicitAny
, but increases--strictNullChecks
. The culprit there isgetSelectedEntity
, which used to beany
and isEntity | undefined
now.I wonder what state we're in when that's undefined.
I also made things just
undefined
instead ofnull | undefined