[core] refactor(InputGroup): improve interface

[ejose19]

Checklist

• Includes tests
• Update documentation

Changes proposed in this pull request:

This PR reduces the friction between InputGroup and HTMLInputElement by only replacing the properties that aren't supported { defaultValue: string[], value: string[] }

This has 2 benefits:

• Users can now make reusable components by declaring the prop as just IInputGroupProps instead of IInputGroupProps & HTMLInputProps (just like the rest of Blueprint components)
• No need to declare an explicit (and redundant) typing for onChange when the function is declared in the prop

<InputGroup onChange={(e) => e.currentTarget.value} /> // Previously e would be of type "any" triggering "noImplicitAny" rule

[adidahiya]

This is a good change but in its current state, it is a breaking change for anyone importing IInputGroupProps (yes, that type is used in many places by consumers). I suggest you mark the current interface as /** @deprecated */ and introduce a new one called IInputGroupProps2 which we can migrate to in the next major version.

[adidahiya]

@ejose19 after #4475, do you think you could do the same kind of thing for Button and AnchorButton as you did for InputGroup in this PR?

[ejose19]

@adidahiya Done with all the review issues, I also replaced IInputGroupProps and IInputGroupProps & HTMLInputProps in other places that weren't update.

Can you tell me what's the issue with Button and AnchorButton exactly?

[adidahiya]

In the same way you've refactored IInputGroupProps & HTMLInputProps -> IInputGroupProps2 here, I think we could do something like IButtonProps & React.HTMLButtonAttributes<HTMLButtonElement> -> IButtonProps2

[ejose19]

@adidahiya I've checked the code and what you want can't be done with an interface, as ts will error with: An interface can only extend an object type or intersection of object types with statically known members.. However with a type there's not an issue, does that works for you? and if it does should the type name be named with an I as well?

[adidahiya]

sorry to revisit this -- why are you making this change to remove onChange from the interface? you're Omiting those fields anyway in IInputGroupProps2 below..?

[ejose19]

The point of IControlledProps (as far I understand it) is to define the correct types we support, which is only value: string on value and defaultValue props, however onChange is compatible without any other change, so we're defining unnecessary properties (and not following the original typing which is React.ChangeEvent not React.FormEventHandler [even though React.ChangeEventHandler could be used]).

So unless there's an specific reason so not use the native type, I don't see why it should be redefined.

[adidahiya]

@ejose19 yeah I think a type is fine. we can discuss further about that in a separate issue or PR. I would keep the I prefix for now, and I am considering removing that prefix in v4.0 across the whole repo