eBay / nice-modal-react

A modal state manager for React.
https://ebay.github.io/nice-modal-react
MIT License
1.96k stars 110 forks source link

Fix: now NiceModal.show args TS type follows React Props type #61

Closed spaguette closed 2 years ago

spaguette commented 2 years ago

This PR partially addresses another PR and type safety of show, but only in the global object call (NiceModal.show), not when using a useModal hook return value.

I created it because it should be a simple fix (although might be a breaking change for someone not following the type safety here), and it's a missing puzzle item for me keeping from using the library.

Usage:

const MyModalWithoutProps = NiceModal.create(() => {
  const modal = useModal();
  return (
    <Modal /*other niceModal props*/>
      Greetings!
    </Modal>
  );
});

NiceModal.show(MyModalWithoutProps, { name: 'Nate' }) // does not work
NiceModal.show(MyModalWithoutProps) // works
NiceModal.show(MyModalWithoutProps, { id: 1 }) // does not work

const MyModal = NiceModal.create(({ name }: { name: string }) => {
  const modal = useModal();
  return (
    <Modal /*other niceModal props*/>
      Greetings: {name}!
    </Modal>
  );
});

NiceModal.show(MyModal, { name: 'Nate' }) // works
NiceModal.show(MyModal) // does not work
NiceModal.show(MyModal, { id: 1 }) // does not work
NiceModal.show(MyModal, { name: 'Nate', id: 1 }) // does not work
codecov-commenter commented 2 years ago

Codecov Report

Merging #61 (71fe1d4) into main (004dca3) will not change coverage. The diff coverage is n/a.

@@            Coverage Diff            @@
##              main       #61   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files            1         1           
  Lines          187       187           
  Branches        29        29           
=========================================
  Hits           187       187           
Impacted Files Coverage Δ
src/index.tsx 100.00% <ø> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 004dca3...71fe1d4. Read the comment docs.

spaguette commented 2 years ago

On the second thought, if the props were passed before through the global register, it would be too strict 🤔 I don't have a solution for that case unfortunately, therefore closing the PR.