Add type safety to modal.show()

[avivm]

First of all thanks for a great library!

I think it will be great if you can add type safety to the modal arguments when calling the show method. Could be using generics:

modal.show<Props>(props: Props)

Of course it will be amazing if somehow it will infer it from the modal component itself but I'm not sure it's possible and the generics solution at least gives basic type safety.

Thanks!

[supnate]

Thanks! This looks like a great suggestion. However because it support using id to create a modal so I'm afraid it is impossible to infer the props type since it's runtime behavior.

However, if use a component for the useModal(MyDialog) hook maybe it can be type safe. I will take a look.

[LoicUV]

Did you have the time to look into this by any chance ?

[xxleyi]

https://github.com/eBay/nice-modal-react/pull/20

Make a pull request, try to give better TS type.

[supnate]

Thanks @xxleyi for the contribution! It's already published in the version v1.2.0.

[lukasz-karolewski]

It seems the goal was to strongly type modal props, but this changes does not seem to accomplish that. it stronly types the modal itself... and props are still Record<string, unknown>

while at it would be great to strongly promises since right now those are Promise

Maybe you can you examples of usage guys had in mind?

[xxleyi]

@lukasz-karolewski The improvement is, when you use React component with useModal or show, the props will be Partial<Props of React component> | undefined.

You can give it a try in here: https://codesandbox.io/s/nice-modal-react-ts-type-0gql5?file=/src/ModalTypeExample.tsx

[laukaichung]

@lukasz-karolewski The improvement is, when you use React component with useModal or show, the props will be Partial<Props of React component> | undefined.

You can give it a try in here: https://codesandbox.io/s/nice-modal-react-ts-type-0gql5?file=/src/ModalTypeExample.tsx

How can I fix

No overload matches this call.
  Overload 1 of 2, '(modal: ({ a, b }: { a: number; b: string; }) => Element, args?: Partial<Omit<{ a: number; b: string; }, "id">> | undefined): Promise<unknown>', gave the following error.
    Type 'string' is not assignable to type 'number | undefined'.
  Overload 2 of 2, '(modal: string, args?: Record<string, unknown> | undefined): Promise<unknown>', gave the following error.
    Argument of type '({ a, b }: { a: number; b: string; }) => Element' is not assignable to parameter of type 'string'.ts(2769)

as shown in the codesandbox example?

[xxleyi]

@lukasz-karolewski The improvement is, when you use React component with useModal or show, the props will be Partial<Props of React component> | undefined. You can give it a try in here: https://codesandbox.io/s/nice-modal-react-ts-type-0gql5?file=/src/ModalTypeExample.tsx

How can I fix

No overload matches this call.
  Overload 1 of 2, '(modal: ({ a, b }: { a: number; b: string; }) => Element, args?: Partial<Omit<{ a: number; b: string; }, "id">> | undefined): Promise<unknown>', gave the following error.
    Type 'string' is not assignable to type 'number | undefined'.
  Overload 2 of 2, '(modal: string, args?: Record<string, unknown> | undefined): Promise<unknown>', gave the following error.
    Argument of type '({ a, b }: { a: number; b: string; }) => Element' is not assignable to parameter of type 'string'.ts(2769)

as shown in the codesandbox example?

As the message say: Type 'string' is not assignable to type 'number | undefined', where number part is right.

The purpose of code in codesandbox is to show typescript checker can catch the type issue.