-
Notifications
You must be signed in to change notification settings - Fork 0
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
Improve function types and bump libraries #429
Conversation
rkuykendall
commented
Jan 11, 2022
•
edited
Loading
edited
if (isString(value)) { | ||
return value.trim(); | ||
if (isNumber(value)) { | ||
return String(value); |
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.
This is the only functional change here, and it's reflected in the test suite.
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.
was this change necessary for the ant design upgrade?
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.
Nope. Just seemed like a good change. getOrDefault
should probably return a string, right? If not, I can change it to string | number
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 would leave it as string | number
, since I see this function as defaulting only if the value is empty. but don't feel strongly either way. doesn't look like it's being used for numbers currently.
if (isString(value)) { | ||
return value.trim(); | ||
if (isNumber(value)) { | ||
return String(value); |
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.
was this change necessary for the ant design upgrade?