formidable icon indicating copy to clipboard operation
formidable copied to clipboard

Include typings for Typescript, for V2/V3 of Formidable

Open gboer opened this issue 4 years ago • 14 comments

Support plan

  • which support plan is this issue covered by?: Community
  • is this issue currently blocking your project?: Yes
  • is this issue affecting a production system?: Yes

Context

  • node version: v14.18.1
  • module (formidable) version: Any V2/V3
  • environment (e.g. node, browser, native, OS): NodeJS
  • used with (i.e. popular names of modules): Typescript 4.X
  • any other relevant information: N/A

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.

gboer avatar Nov 02 '21 09:11 gboer

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

GrosSacASac avatar Nov 02 '21 15:11 GrosSacASac

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 avatar Nov 02 '21 16:11 karlhorky

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

karlhorky avatar Nov 02 '21 16:11 karlhorky

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.

gboer avatar Nov 02 '21 16:11 gboer

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

karlhorky avatar Nov 02 '21 16:11 karlhorky

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.

GrosSacASac avatar Nov 02 '21 16:11 GrosSacASac

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

tunnckoCore avatar Nov 02 '21 16:11 tunnckoCore

small update, the types for Formidable V2 are now available in @types/[email protected] :)

gboer avatar Nov 05 '21 20:11 gboer

@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

tunnckoCore avatar Nov 06 '21 21:11 tunnckoCore

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

PS: Found it, began the work 😊

Akxe avatar Apr 28 '22 14:04 Akxe

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

tunnckoCore avatar Apr 29 '22 20:04 tunnckoCore

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.

Akxe avatar Apr 29 '22 20:04 Akxe

@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.

tunnckoCore avatar Apr 30 '22 02:04 tunnckoCore

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

wbt avatar Apr 30 '22 02:04 wbt