Improved accuracy of IconProps interface
#3848
Conversation
- Use the `IconName` and `IconPrefix` types provided by FontAwesome. - Define a `ResolvedIconProps` interface, for specific usages where we know `iconName` and `prefix` have been applied and will be present.
|
I started looking at this due to warnings being thrown around icons that turned out were actually caused by 9435122 But I had started tightening things up and still there is some utility in taking these changes. My only concern is that the use of the |
| */ | ||
| icon(opts?: IconProps): any { | ||
| icon(opts: IconProps & {iconName: IconName}): any { |
There was a problem hiding this comment.
why is the & {iconName:IconName} needed?
There was a problem hiding this comment.
I tried without it, and it seemed to work, but perhaps am missing something?
There was a problem hiding this comment.
I'm assuming this is because iconName is optional in the IconProps interface but required here? Could use the typefest SetRequired generic (i.e. SetRequired<IconProps, 'iconName'>)
|
I am for getting this merged -- seems like a nice pickups to get a list of all of the available names. Very easy to cast |
IconNameandIconPrefixtypes provided by FontAwesome.ResolvedIconPropsinterface, for specific usages where we knowiconNameandprefixhave been applied and will be present.Hoist P/R Checklist
Pull request authors: Review and check off the below. Items that do not apply can also be
checked off to indicate they have been considered. If unclear if a step is relevant, please leave
unchecked and note in comments.
developbranch as of last change.breaking-changelabel + CHANGELOG if so.If your change is still a WIP, please use the "Create draft pull request" option in the split
button below to indicate it is not ready yet for a final review.