trustpilot/node-trustpilot

TypeScript rewrite

Closed this issue · 11 comments

martinbuberl commented

Starting this issue because of @miklosaubert open PR: TypeScript rewrite PR.

I'd like to have a conversation if we'd like to use Vanilla Node, TypeScript, Flow or something else for this library and explore the whys, pros and cons without this becoming a flame war between strong and weakly typed 🔥 personal preferences.

thomasthiebaud commented

I'd love to use typescript for this library:

  • because the library may grow in size soon. (I might start to move API call from B2bCoreFramework from this library).
  • because this library is public, and types will provide auto-completion
RckMrkr commented

We don't necessarily need it to be written in TypeScript, but if we do not, I think it would be nice to provide types for the project. That way the people who do use TypeScript have types in their projects.

anjmao commented

Fully agree with @thomasthiebaud .
Pros:

  1. Out of the box support for ES6, including async, await.
  2. Much better tooling support for refactoring, code navigation ...
  3. Catch common mistakes and bugs faster.
  4. Autogenerated typings out of the box, so it can be used by other clients who also prefer Typescript.
  5. More readably and understandable code because types better describes shape of the model.

Cons:

  1. 404
  • 👍4
spewu commented

I think it is a great idea - to get all of the benefits of strong typing. It makes it much easier to use the library. I think @anjmao provided an excellent summary.

KrisZane commented

I believe it should be taken into consideration that only a very small percentage of the JS community uses TypeScript, so it will limit the use of any exposed functionality, as well as community collaboration.

The only numbers I can get on this though is from Google Trends (~3%) and NPM (~23%), which seems to be rather conflicting.

mufaza commented

A library should minimize the dependencies it requires, and this will add dependencies without adding much value. Stronger typing makes a lot of sense in bigger projects, but this is a small project.

We want as many people as possible to use this library, and this might hurt this mission.

madsgodvinjensen commented

I guess there are two different uses here: Maintaining the library and using the library.

For using the library I completely agree that typings would help. Both in reducing the issues with wrongly typed properties and as a sort of documentation with auto-complete.

For maintaining it my preferences are not as strong. I am honestly fine with vanilla es6...

b-dur commented

I would just like to add my 5 cents into this conversation.

I think we can get the best of both worlds. The library can be maintained with typescript to leverage the benefits of strong typing. When it is published it can be published both as is (typescript) and as vanilla JS. Or maybe even compatible version for ES2015, ES2016, ES2017, and/or ES5.

martinbuberl commented

I generally agree with @b-dur that this would be best practice.

However, I do fear that this extra work will scare some historic maintainers. The level of contribution to this OSS project is already very low.

@miklosaubert TypeScript rewrite was done in 20%, him basically learning TypeScript. So first his PR would need a proper code review from somebody with more TypeScript experience. PS: I've discussed this with him.

Maybe we can start there?

b-dur commented

I think we should at minimum provide definition files with this library.

And then, why not just build it in typescript, instead of maintaining definition files manually? And I really don't think we need to fear anything 😃

miklosaubert commented

The rewrite was recently done by @thefesty . Do you mind closing the issue @martinbuberl ?