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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor react-modal to use TypeScript #1037

Open
doeg opened this issue Feb 9, 2024 · 1 comment
Open

Refactor react-modal to use TypeScript #1037

doeg opened this issue Feb 9, 2024 · 1 comment

Comments

@doeg
Copy link

doeg commented Feb 9, 2024

I wanted to open a ticket to discuss the possibility of refactoring this library to use TypeScript. 馃樃

In addition to all of the benefits typed code has with respect to safety, productivity, and maintainability, built-in types would also remove the need for third-party type definitions. (See https://github.com/DefinitelyTyped/DefinitelyTyped/tree/master/types/react-modal.)

If we adopt this proposal, we could (and arguably should) do this as a progressive enhancement to the library. Some related guides on adding TypeScript to existing projects:

This dovetails with #1036, which proposes a new build system to replace bespoke webpack. If the plan is indeed to move away from webpack, starting with #1036 and then integrating TypeScript after would be the most reasonable sequence of events, I think. :)

#960 is a related issue that flags a type error ('ReactModal' cannot be used as a JSX component) arising from using react-modal with React 18.

@diasbruno would love to know your thoughts on this. 馃檱 I did not see any previous discussions around adopting TypeScript in the issues/pull requests for this repo.

@diasbruno
Copy link
Collaborator

I think that's fine. We just need to deprecate the ones defined on DefinatelyTyped.

I believe this can be done in 2 steps:

  • Move to a build system with typescript enabled
  • Provide the types, or, type the library

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