mholt / PapaParse

Fast and powerful CSV (delimited text) parser that gracefully handles large files and malformed input
http://PapaParse.com
MIT License
12.54k stars 1.15k forks source link

Maintain our own ts definitions? #545

Open dboskovic opened 6 years ago

dboskovic commented 6 years ago

I think it might be a good idea to move our TS definition file into the core repo and maintain our own type file / updating it as part of PRs. Right now we rely on the DefinitelyTyped contributions which are already out of date.

Any thoughts @pokoli @mholt?

Current TS definitions maintained here: https://github.com/DefinitelyTyped/DefinitelyTyped/tree/master/types/papaparse

pokoli commented 6 years ago

@dboskovic sorry if the question is quiet obious but I will like to know why the TS Definition file is required and for what it is issued. Sincerly I did not know that this exists.

dboskovic commented 6 years ago

It's not required unless you're a Typescript user. Then you need type definitions. So the DefinitelyTyped project allows people to contribute the types for libraries that don't provide them as part of the project. TS is becoming very popular so it makes sense to me that we would maintain the type file with our library rather than leaving it separate for others to maintain.

pokoli commented 6 years ago

I do not think we should force all contributors to update them as it can be people that use PapaParse with plain Javascript (I'm one of them).

dboskovic commented 6 years ago

Hmmm, I guess we could just conduct a review of the DefinitelyTyped definitions from time to time and make sure they're up to date.

pokoli commented 6 years ago

Probably it's enought to update it for each new PapaParse version

wartab commented 5 years ago

A type definition file also has advantages for plain JavaScript users as IDEs like WebStorm will still make use of them to help autocompleting when not just doing TypeScript. I think that you should really add this here, even if some contributors will not bother filling it in. It's easier to remember editing it directly in the concerned package, too.

pokoli commented 5 years ago

It is possible to generate/update type definitions based on a script? If so we may problably include this on our release process

bforbis commented 5 years ago

The only easy way to generate type definitions is to convert your project to Typescript. It just pulls the definitions directly from your codebase. If that is too big of a lift or undesirable, you could just have a separate type declaration file in your repo that you maintain on the side, but you would need to write some tests in typescript to make sure they work.

crhistianramirez commented 1 year ago

+1 for a type definition file that lives with the project.

Pros:

Cons:

dboskovic commented 1 year ago

I'd be anxious about rocking the boat here too much in a typescript refactor of the existing code. It's not just typing, but different generated code and possibly some things to optimize.

I think this library is due for an overall rewrite though - and if we did that, I'd be in full support of typescript.

pokoli commented 1 year ago

Just to be honest, I'm not fan of rebuilding the library from scratch and also I'm not a TypeScript developer because javascript is not my primary language (I mainly work on Python). So don't count for me for start the TypeScript migration.

Having said that, if somebody wants to start the port in a fork, I will be happy to review them and if this really improves the current codebase merge it on a new major version.

dboskovic commented 1 year ago

I'll talk to our team and see if we want to invest in a refactor. But I'm guessing that the original proposal of just bringing the type definitions home to this repo is still the most reasonable scope of work for the community to do.

pokoli commented 1 year ago

@dboskovic We can bring the definitions where if you prefer but I can not guarantee to keep them updated unless there is some script that I can include in the release process to create new types. And I will prefer to have nothing that having something not updated.

I do not think there will be such much new types added to public functions. Probably I can ask on MR to include the TS definitions also. It will be great to get some way of CI process to fail if there are missing definitions. Do you know if there is some way to check pragmaticly that all TS definitions are updated?