Improved typings for getMenuProps #534
Conversation
Fixes the typing part of #490
|
@kentcdodds Thanks! Who would you recommend for this review? |
Codecov Report
@@ Coverage Diff @@
## master #534 +/- ##
=====================================
Coverage 100% 100%
=====================================
Files 3 3
Lines 349 349
Branches 84 84
=====================================
Hits 349 349Continue to review full report at Codecov.
|
|
Hopefully they're watching the repo and will get to it when they have an opportunity... We'll give them a day or two. |
There was a problem hiding this comment.
Thanks for the PR, @franklixuefei. I suggested a few changes.
typings/index.d.ts
Outdated
| interface OptionalExtraGetItemPropsOptions { | ||
| [key: string]: any | ||
| export interface GetMenuPropsOptions<Item> | ||
| extends Record<string, any> { |
There was a problem hiding this comment.
This doesn't seem to be quite right. I also don't understand what the <Item> generic type is doing here as it doesn't appear to be used.
From looking over the docs, this is what I would expect this type to look like:
export interface GetMenuPropsOptions {
refKey?: string
aria-label?: string
}
There was a problem hiding this comment.
You are completely right. I kind of copied the interface from GetMenuItemOptions which has the generic type parameter and I forgot to remove it.
typings/index.d.ts
Outdated
| refKey?: string; | ||
| } | ||
|
|
||
| export interface GetMenuPropsOtherOptions<Item> { |
There was a problem hiding this comment.
Let's remove the <Item> part of this as it isn't adding any value.
typings/index.d.ts
Outdated
| getLabelProps: (options?: GetLabelPropsOptions) => any | ||
| getMenuProps: (options?: {}) => any | ||
| getMenuProps: (options?: GetMenuPropsOptions<Item>, otherOptions?: GetMenuPropsOtherOptions<Item>) => any |
There was a problem hiding this comment.
Let's remove the <Item> occurrences on this line.
| @@ -105,24 +105,29 @@ export interface GetInputPropsOptions | |||
| export interface GetLabelPropsOptions | |||
| extends React.HTMLProps<HTMLLabelElement> {} | |||
|
|
|||
| export interface getToggleButtonPropsOptions | |||
| export interface GetToggleButtonPropsOptions | |||
There was a problem hiding this comment.
Thanks for reviewing this PR! I've made changes as necessary and please take a look 😄
|
I haven't figured out why it's failing. Could someone help me with this? |
|
Ping 😄 |
|
🎉 This PR is included in version 2.1.3 🎉 The release is available on:
Your semantic-release bot 📦🚀 |
* Improved typings for getMenuProps Fixes the typing part of downshift-js#490 * Update index.d.ts * Fixed build error * Update index.d.ts
Fixes the typing part of #490
Why: The typings for
getMenuPropswas too broad.How:
Created interfaces for the types of parameters of
getMenuPropsChecklist: