Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support optional component props generic in NiceModalArgs? #80

Open
oroszi1mark opened this issue Oct 6, 2022 · 1 comment
Open

Support optional component props generic in NiceModalArgs? #80

oroszi1mark opened this issue Oct 6, 2022 · 1 comment

Comments

@oroszi1mark
Copy link

Hello, thanks for this great tool, it's a joy to work with!

When passing optional props to the registered modal component (and the component type passes a condition in type NiceModalArgs<T>), the component props are typed as Partial<Omit<React.ComponentProps<T>, 'id'>>.

Using Partial<> means that otherwise required component props are incorrectly loosened up. The related comment specifically mentions using Partial<>, so I guess there was a good reason to do so. But to achieve type safety, now a wrapper function must be used that enforces the required component props:

function registerModalWithProps<C extends React.FC<any>>(id: string, component: C, props: React.ComponentProps<C>) {
  register(id, component, props)
}

Could this situation be avoided if an optional component props generic were to be introduced?

type ReactJSXOrConstructor<T> = T extends keyof JSX.IntrinsicElements | React.JSXElementConstructor<any> ? T : never

type PartialComponentProps<T> = Partial<React.ComponentProps<ReactJSXOrConstructor<T>>>

declare type NiceModalArgs<T, P = PartialComponentProps<T>> = T extends ReactJSXOrConstructor<T> ? Omit<P, 'id'> : Record<string, unknown>;

I understand the optional generic would creep into the signatures of useModal(), register(), and possibly even show(), so an on-demand wrapper function might be simpler overall.

@HasanMothaffar
Copy link

I faced some hard to find bugs because of using <Partial>, I'll go back to using the wrapper function mentioned above.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants