amrlabib / react-timer-hook

React timer hook
https://www.npmjs.com/package/react-timer-hook
MIT License
515 stars 112 forks source link

TypeScript? #11

Closed dartess closed 4 years ago

dartess commented 4 years ago

Hello! What do you think about rewriting code from JavaScript to TypeScript? I could probably help with PR.

amrlabib commented 4 years ago

Hi, honestly don't see the need for it in a small library, the library is composed of 3 very small hooks, i think keeping it simple as is without Typescript or Flow is better for now

dartess commented 4 years ago

The library itself, quite possibly, does not need a TS. But when I use it in projects written on the TS, I get a lack of types. All that any hook returns is type "any". This is inconvenient, and sometimes impossible (with strict checks enabled, such as --noImplicitAny).

I see several options:

  1. Writing typings to me myself and copy them from project to project (other users will have to do the same). In this case, if the version is updated, everything may be broken.

  2. Writing typings for the DefinitelyTyped repository. Then anyone can download them manually. But at the same time, in this case of an update, everything may also be broken.

  3. Writing typings in your repository (d.ts files near JS files). In this case, the typings will be available immediately after installing this package, without additional actions and individual packages from DefinitelyTyped. There still remains the option that the library will be updated, and the typings will not.

  4. And the best option is to rewrite the project on the TS and get the same javascript at the output, but with automatically generated d.ts with typings.

Maybe I’m wrong somewhere, I’m not an expert in the development of packages on the TS, but I write my projects on the TS and so far have to use the first option.

amrlabib commented 4 years ago

I am not an expert in Typescript didn't work much with it till now, but feel a bit weird about the fact that working with Typescript is forcing you or causing issues for you to work with another external library, also forcing you to have libraries to be written in TS, the question now, for the same reason why we don't rewrite the library with Flow ? for those using Flow ?

Based on my understanding you should be able to use any external library that is not written in Typescript or Flow inside any project written in anyone of them. maybe i am missing something

I will also start learning more about Typescript to try to understand how far this can cause issues.

dartess commented 4 years ago

why we don't rewrite the library with Flow

I have no experience with flow. It seems that I saw some libraries that had typings for both TS and flow at the same time. But now I have no idea where I saw it.

However, I can definitely say that TS is trite more popular than flow among developers for 2018.

Based on my understanding you should be able to use any external library that is not written in Typescript or Flow inside any project written in anyone of them.

We can use them, but without types we cannot use them fully. If the authors of library cannot or do not want to use TS (option 4) or distribute the library with typings (option 3), the community itself writes typings for everyone (option 2) or for themselves (option 1).

amrlabib commented 4 years ago

Hi @dartess sorry for my late response about this issue, as i read a bit more about TypeScript still didn't use it in any of my current projects, but i had a glance of where you are right now if you use strict mode and got better understanding of the options you previously mentioned.

So coming back to these options, we all agree that option 1 and 2 are not ideal.

I am open to option 3 where we add type definition files and we distribute it with the library, i prefer this option because we still keep the source code of the library written in vanilla javascript, this way the library can be used in any project not only Typescript projects, also this will give us the option to add flow support later using the same way in flow "library definition" files

Also don't worry about type definition file not being updated with new versions i will make sure to update it with any library update.

Let me know if you can help in adding type definition file for TypeScript, otherwise i will try to add it soon, also will try to add one for flow at some point

Thanks

dartess commented 4 years ago

this way the library can be used in any project not only Typescript projects

The library can be used in any project in any case. TS compiles to vanilla JS+d.ts with typings. Developers of javascript projects will not see any changes in any case.

also this will give us the option to add flow support later using the same way in flow "library definition" files

  1. Flow is so rarely used that it was removed from the survey state of javascript 2019 (https://2019.stateofjs.com/javascript-flavors/). The results of the survey for the previous year I have already sent above.

  2. Even if we will want to add files for flow, I can try using https://github.com/joarwilk/flowgen for automatic generation files for flow and if the result is satisfactory I would suggest adopting the fourth option.

Based on the information above, are you sure that option 3 is the best?

Let me know if you can help in adding type definition file for TypeScript, otherwise i will try to add it soon, also will try to add one for flow at some point

I can help with options 3 or 4 until January 9th.

amrlabib commented 4 years ago

Yes TS will compile to vanilla JS but this means, either react-timer-hook will handle that compiling configuration, or the other library that use the code will need to handle that and what i wanted to do is an out of the box solution.

I like solution 3 because it requires adding a single file for TypeScript or Flow without adding extra compiling configuration to the library, this does make the library simple, generic and future proof

From your perspective Flow is not used enough or does not require the effort to support it, but i believe that still many projects are using Flow for typing i am currently working on some of them and i know that other people are using them as well, being removed from a survey is not enough to not support it at all (in my opinion), even if TypeScript is used more than Flow

Finally as i tried to explain i am trying to find the best solution and avoid attaching a specific technology to the library, also i am not a big fan of languages that is superset of javascript like (Just my own opinion), Typing is nice to have and that is why i like the idea of option 3 where we have a separate d.ts file without writing the whole library with another superset language to add Typing for a simple library like react-timer-hook

Anyways since option 3 will make the library usable by TypeScript without any down sides for anyone to use the library it will really help if you could help with that option.

Thanks

dartess commented 4 years ago

I understand you, thanks. I’ll prepare typings as soon as I can.

amrlabib commented 4 years ago

TypeScript type definition file is available from v1.1.9, Thanks @dartess for your effort on this