-
-
Notifications
You must be signed in to change notification settings - Fork 54
refactor(types): update types to be more concise #314
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
Conversation
ac39f74 to
631cf6e
Compare
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.
FYI: optional is not the same as undefined, by removing this, passing an undefined value is not allowed anymore.
FYI: optional is not the same as undefined, by removing this, passing an undefined value is not allowed anymore.
I'm almost certain that's not right: https://www.typescriptlang.org/play/?#code/JYOwLgpgTgZghgYwgAgCoQM5mQbwFDKHIAOUA9sdGAJ4D8AXMllKAOYDceAvnngmSCzJIWRuiEBeXCXKUoNRgFcQAEwgxQEFci6cgA
This doesn't give a syntax error
631cf6e to
9cc75bf
Compare
It was off the back of this comment that I made the type just string[].
Basically the types reflect the ideal typings and not an enumeration of every single type that could be accepted. By limiting the type this way, we also make the upgrade path for those following the types easier if/when we remove the string support. It's also not a breaking change as it's the current typing, so we aren't introducing any new issues by leaving the typing like this.
From my end it's ok but I'm only using a few of them for my IDE intellisense.
| undefinedfor props that are already declared optional (undefinedis an implied value)Recordbuilt-in type over object maps