Allow non-enum Input in components #2093
Conversation
| @@ -21,7 +21,7 @@ export class AvatarComponent { | |||
| @Input() shadow: boolean; | |||
| @Input() text: string; | |||
| @Input() overlay: boolean; | |||
| @Input() size: AvatarSize = AvatarSize.SM; | |||
| @Input() size: AvatarSize | keyof AvatarSize = AvatarSize.SM; | |||
There was a problem hiding this comment.
Unfortunately keyof AvatarSize returns all of the keys for a string object (i think it is a string object - not 100% sure).
I've created an example here: link to typescript playground.
Notice the type of TestType
I think we need something like valueof AvatarSize (unfortunately i do not think valueof exists...).
This comment goes for all of the places where keyof has been used.
There was a problem hiding this comment.
It is only the case where the case in the enum maps to a string. I already fixed it for ButtonSize, but didn't see it for AvatarSize. I found 2 others.
There was a problem hiding this comment.
I've marked some values which resolves wrongly.
But in general i do not think creating a union type by explicitly writing out the string values of the enum is a good solution as it introduces duplication.
I've however discovered that we might be able to use template literal types to solve this!
I've taken the liberty of using your branch as the base branch for a new PR resolving your issue using those.
Unfortunately our version of Prettier does not support template literal types as it is quite old (1.19.1)! I will discuss with the rest of the team tomorrow if we should look into upgrading prettier.
| @@ -38,7 +38,7 @@ export class InputComponent implements OnChanges { | |||
|
|
|||
| @HostBinding('class') | |||
| @Input() | |||
| size: InputSize = InputSize.large; | |||
| size: InputSize | keyof InputSize = InputSize.large; | |||
There was a problem hiding this comment.
Resolves to:
(property) InputComponent.size: number | InputSize | "toString" | "charAt" | "charCodeAt" | "concat" | "indexOf" | "lastIndexOf" | "localeCompare" | "match" | "replace" | "search" | "slice" | "split" | ... 29 more ... | "padEnd"| @@ -42,7 +42,7 @@ export class PopoverComponent implements AfterViewInit, OnDestroy { | |||
| wrapperElement: ElementRef<HTMLDivElement>; | |||
|
|
|||
| @Input() | |||
| popout: HorizontalDirection = HorizontalDirection.right; | |||
| popout: HorizontalDirection | keyof HorizontalDirection = HorizontalDirection.right; | |||
There was a problem hiding this comment.
Resolves to:
(property) PopoverComponent.popout: number | "toString" | "charAt" | "charCodeAt" | "concat" | "indexOf" | "lastIndexOf" | "localeCompare" | "match" | "replace" | "search" | "slice" | "split" | "substring" | "toLowerCase" | ... 28 more ... | HorizontalDirection
| @@ -28,7 +28,7 @@ export class SegmentedControlComponent { | |||
| } | |||
| } | |||
|
|
|||
| @Input() mode: SegmentedControlMode = SegmentedControlMode.default; | |||
| @Input() mode: SegmentedControlMode | keyof SegmentedControlMode = SegmentedControlMode.default; | |||
There was a problem hiding this comment.
Resolves to:
(property) SegmentedControlComponent.mode: number | "toString" | "charAt" | "charCodeAt" | "concat" | "indexOf" | "lastIndexOf" | "localeCompare" | "match" | "replace" | "search" | "slice" | "split" | "substring" | "toLowerCase" | ... 28 more ... | SegmentedControlMode
Which issue does this PR close?
This PR closes #1985
What is the new behavior?
Everywhere I could find a component with an enum-based
Input, the type declaration is changed so it now also takes one of the cases of the enum as string. One exception isButtonSize, where the name and value of the enum cases aren't the identical. In that case, the enum cases as strings are added to the union directly.This change should not be a breaking change, as it just adds to the list of types/values that can be used on
Inputproperties.Does this PR introduce a breaking change?
Checklist:
The following tasks should be carried out in sequence in order to follow the process of contributing correctly.
Reminders
Review
When the pull request has been approved it will be automatically merged to master via automerge.