[Request] Improve typing of modal props

[gprst]

It seems not to be possible to call NiceModal.show with a strong typing of the called modal props.

Let's say that MyModal has the following props:

type Props = {
  onSubmit: () => void;
  name: string;
};

Then:

import NiceModal from '@ebay/nice-modal-react';
import MyModal from './components/MyModal';

const App = () => {
  const onClick = () => NiceModal.show(MyModal, {
    something: '123', // No error, even though `MyModal` doesn't have a `something` prop
    name: '456',
  }); // No error, even though prop `onSubmit` hasn't been provided
  
  return <button onClick={onClick}>Show modal</button>;
};

It would be nice to have errors when the provided object doesn't match the type of the modal props 🙂

[jludwiggreenaction]

If it isn't to automatically generate the type, we should probably be able to pass it as a simple generic parameter right?

[baxxos]

If it isn't to automatically generate the type, we should probably be able to pass it as a simple generic parameter right?

I wonder if we could get around this by simply overriding the NiceModal.show() function. I haven't used the library yet (I'm just considering it), however I imagine we should be able to do something like this?

const originalShow = NiceModal.show
NiceModal.show = <T>(modal: T, props: React.ComponentProps<T>, ...args) => originalShow(modal, props, ...args)

[xenostar]

We're able to sort of get around this for now by utilizing the satisfies operator like:

NiceModal.show('demo-modal-name', {
  name: "example name prop",
} satisfies DemoModalProps);

However, a better API would be something like:

NiceModal.show<DemoModalProps>('demo-modal-name', {
  name: "example name prop",
});

[HasanMothaffar]

I'm using this workaround in the codebase instead of NiceModal.show.

import NiceModal, { NiceModalHocProps } from "@ebay/nice-modal-react";
import { ComponentProps, FC } from "react";

type Props<C extends FC<any>> = Omit<ComponentProps<C>, keyof NiceModalHocProps> & Partial<NiceModalHocProps>;

export function showModal<C extends FC<any>>(component: C, props: Props<C>) {
    return NiceModal.show(component, props);
}

The Props just makes NiceModalHocProps optional in the component props type. Without it, the ComponentProps<C> type will produce props of component C and NiceModalHocProps, so calling showModal without id or other required props from NiceModalHocProps results in an error.

I don't know if there's a cleaner way to make NiceModalHocProps partial without omitting them first.

[caspergreen]

I thought it made more sense to type out what is required of the modal and what the modal resolves to, when you actually create the modal, so I have a wrapper like this,

export const ActualNiceModal = {
  ...NiceModal,
  create: <Props extends {}, ResolvedValue = void>(fc: FC<Props>) => {
    const storedModal = NiceModal.create(fc);

    return {
      show: (props: Omit<Props, 'id'>): Promise<ResolvedValue> => {
        return NiceModal.show(storedModal, props);
      },
      hide: () => {
        return NiceModal.hide(storedModal);
      },
    };
  },
};

When you create the modal, you define the types rather than when you use the modal. Above changes the API a bit, so you would call .show on the modal.

If you want something more similar to the current API, maybe something like this can work:

type SmartModal<Props extends {}, ResolvedValue = void> = FC<Props>;

export const ActualNiceModal = {
  ...NiceModal,
  show<Props extends {}, ResolvedValue = void>(
    fc: SmartModal<Props, ResolvedValue>,
    props: Omit<Props, 'id'>
  ) {
    return NiceModal.show<ResolvedValue>(fc, props);
  },
  create<Props extends {}, ResolvedValue = void>(
    fc: FC<Props>
  ): SmartModal<Props & NiceModalHocProps, ResolvedValue> {
    return NiceModal.create<Props>(fc);
  },
};

[marcusthelin]

To make the modal component's props optional doesn't make sense to me. Why lose the type safety?

[HasanMothaffar]

To make the modal component's props optional doesn't make sense to me. Why lose the type safety?

I too didn't understand why the modal's component props were made optional. But today I came across this section in the docs https://github.com/eBay/nice-modal-react?tab=readme-ov-file#declare-your-modal-instead-of-register

I guess they made the props optional to allow showing a modal only by its ID. But I think maybe another API could be used for this?