node-formidable / formidable

The most used, flexible, fast and streaming parser for multipart form data. Supports uploading to serverless environments, AWS S3, Azure, GCP or the filesystem. Used in production.
MIT License
7.01k stars 680 forks source link

Include typings for Typescript, for V2/V3 of Formidable #774

Open gboer opened 2 years ago

gboer commented 2 years ago

Support plan

Context

What problem are you trying to solve?

I am trying to upgrade to Formidable V2 or V3, but since there are no typings available for this version, it is incredibly time-consuming to figure out how the API looks and what I need to place where. Since this project is downloaded millions of times per week, I can only assume that a lot of other people are running into the same issue. This makes adopting the new version unlikely, any time soon.

Do you have a new or modified API suggestion to solve the problem?

Include the typings with the new version.

GrosSacASac commented 2 years ago

@karlhorky I saw you helped with types in the past, what should we do ?

karlhorky commented 2 years ago

Thanks for the ping! I also ran into this problem when upgrading (since the internals of v2 are incompatible with v1).

I fixed the problems that I encountered just as a patch (using patch-package in my local repository):

diff --git a/node_modules/@types/formidable/index.d.ts b/node_modules/@types/formidable/index.d.ts
index 249da05..e4b4e50 100755
--- a/node_modules/@types/formidable/index.d.ts
+++ b/node_modules/@types/formidable/index.d.ts
@@ -208,12 +208,12 @@ declare namespace formidable {
          * The path this file is being written to. You can modify this in the `'fileBegin'` event in case
          * you are unhappy with the way formidable generates a temporary path for your files.
          */
-        path: string;
+        filepath: string;

         /**
          * The name this file had according to the uploading client.
          */
-        name: string | null;
+        originalFilename: string | null;

         /**
          * The mime type of this file, according to the uploading client.

But of course this is not a solution for everyone... so continuing in the next comment 👇

karlhorky commented 2 years ago

So I would ask you, @GrosSacASac and @tunnckoCore, would the maintainers of formidable be open to either of the following:

  1. Converting the project to TypeScript (or JSDoc + .d.ts files)
  2. Maintaining types as part of the formidable package

A less desirable alternative would be:

  1. Supporting the DefinitelyTyped community (the people who maintain @types/formidable) in PRs like https://github.com/DefinitelyTyped/DefinitelyTyped/pull/52697

Oh and for people who are looking for a solution, @gboer has opened a PR to DefinitelyTyped to add the new types 👍 Thanks!

https://github.com/DefinitelyTyped/DefinitelyTyped/pull/56925

gboer commented 2 years ago

hehe yeah, I was about to add that :) But there are no types for v3 yet and in the long run, it would still be nice if the typings would be delivered with formidable itself. Makes things a lot simpler for everyone.

karlhorky commented 2 years ago

Yeah easiest long term and most beneficial would be if formidable@3 would be written in TypeScript.

GrosSacASac commented 2 years ago

Yes, as I said here https://github.com/node-formidable/formidable/issues/500 I would welcome if someone makes a PR to add types in formidable. If one of you wants I can also give github formidable access so you can make a branch instead of a fork to make a PR.

tunnckoCore commented 2 years ago

yes, totally. vould be rewritten to typescript, I'm open for that.

gboer commented 2 years ago

small update, the types for Formidable V2 are now available in @types/formidable@2.0.0 :)

tunnckoCore commented 2 years ago

@gboer great! :)

I'm for rewriting it to TypeScript with both hands, also moving to a monorepo #569. I didn't write a lot of code last year or more, so.. starting a rewrite in typescript could be incredibly tricky for me :rofl: me and typescript ... ah, it's hard relationship through the years hahaha

Akxe commented 2 years ago

I will happily convert v3 to a typesafe version, but I could not find the branch 😁

PS: Found it, began the work 😊

tunnckoCore commented 2 years ago

@Akxe v3 is master already, you can start from there. I'll soon bump npm latest to v3 too.

Akxe commented 2 years ago

The typescript rewrite is quite hard without major changes to the codebase. There is a lot of Object.assign(this, ...) and a lot of sharing of the this "variable".

I would usually do either:

Parsers/plugins as standalone classes

that do not share anything with the Formidable/IncommingForm class

They would expose API that the Formidable would call and then either get a one-time result, a generator function, or even EventEmmiter back.

Parsers as an extension of Formidable

This is an option too, I did not finish looking through the plugin code in-depth yet.


Please before v3 is marked as latest edit README.MD with info, that you need to install v2 in order for webpack and probably similar to work.

tunnckoCore commented 2 years ago

@Akxe they already are extensions/plugins.

There is a lot of Object.assign(this, ...) and a lot of sharing of the this "variable".

No problemo. The this use was intentional, but they work without it too.

Please before v3 is marked

Good point.

wbt commented 2 years ago

You may want to merge this before treating anything from the types repo as a starting point, FYI.