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
7k stars 680 forks source link

WIP: TypeScript #899

Open tommyhtran opened 1 year ago

tommyhtran commented 1 year ago

This PR aims to convert the source code into TypeScript without major refactoring. Benchmarks, examples, and tests are excluded from the scope of this PR.

I'm aware of #854 already open, but development on it appears to have stalled. This is simply my attempt at converting this project.

This PR addresses #774 and #830.

My test results are at parity with commit 493ec88520c960d0d848d55a929630fde62f2aae but with one exception due to two tests using the same port.

 FAIL  test/standalone/connection-aborted.test.js (6.828 s)
  × connection aborted (5010 ms)

  ● connection aborted

    listen EADDRINUSE: address already in use 127.0.0.1:13539
Test Suites: 6 failed, 10 passed, 16 total
Tests:       17 failed, 3 skipped, 57 passed, 77 total
Snapshots:   0 total
Time:        8.456 s
Ran all test suites.

strictNullChecks has been disabled and is pending future refactoring.

The following errors are currently reported by the TypeScript compiler:

open/close ``` src/Formidable.ts:9:20 - error TS2792: Cannot find module 'hexoid'. Did you mean to set the 'moduleResolution' option to 'node', or to add aliases to the 'paths' option? 9 import hexoid from 'hexoid'; ~~~~~~~~ src/Formidable.ts:11:21 - error TS2792: Cannot find module 'dezalgo'. Did you mean to set the 'moduleResolution' option to 'node', or to add aliases to the 'paths' option? 11 import dezalgo from 'dezalgo'; ~~~~~~~~~ src/Formidable.ts:229:24 - error TS2339: Property 'end' does not exist on type 'OctetStreamParser | JSONParser | DummyParser | (typeof MultipartParser & { STATES: {}; }) | QuerystringParser'. Property 'end' does not exist on type 'typeof MultipartParser & { STATES: {}; }'. 229 this._parser.end(); ~~~ src/Formidable.ts:252:18 - error TS2349: This expression is not callable. Each member of the union type '{ (event: "close", listener: () => void): OctetStreamParser; (event: "data", listener: (chunk: any) => void): OctetStreamParser; (event: "end", listener: () => void): OctetStreamParser; (event: "error", listener: (err: Error) => void): OctetStreamParser; (event: "pause", listener: () => void): OctetStreamParser; (ev...' has signatures, but none of those signatures are compatible with each other. 252 this._parser.once('error', (error: Error) => { ~~~~ src/Formidable.ts:271:18 - error TS2339: Property 'write' does not exist on type 'OctetStreamParser | JSONParser | DummyParser | (typeof MultipartParser & { STATES: {}; }) | QuerystringParser'. Property 'write' does not exist on type 'typeof MultipartParser & { STATES: {}; }'. 271 this._parser.write(buffer); ~~~~~ src/Formidable.ts:305:9 - error TS2345: Argument of type 'BufferEncoding | "7bit" | "8bit"' is not assignable to parameter of type 'BufferEncoding'. Type '"7bit"' is not assignable to type 'BufferEncoding'. 305 part.transferEncoding || this.options.encoding, ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ src/helpers/firstValues.ts:5:40 - error TS7006: Parameter 'fields' implicitly has an 'any' type. 5 const firstValues = (form: Formidable, fields, exceptions: string[] = []) => { ~~~~~~ src/helpers/firstValues.ts:15:20 - error TS7053: Element implicitly has an 'any' type because expression of type '0' can't be used to index type '{}'. Property '0' does not exist on type '{}'. 15 return [key, value[0]]; ~~~~~~~~ src/helpers/readBooleans.ts:1:23 - error TS7006: Parameter 'fields' implicitly has an 'any' type. 1 const readBooleans = (fields, listOfBooleans: string[]) => { ~~~~~~ src/index.ts:9:21 - error TS7019: Rest parameter 'args' implicitly has an 'any[]' type. 9 const formidable = (...args) => new Formidable(...args); ~~~~~~~ src/parsers/JSON.ts:26:16 - error TS2345: Argument of type 'unknown' is not assignable to parameter of type 'Error'. Type '{}' is missing the following properties from type 'Error': name, message 26 callback(e); ~ src/parsers/Multipart.ts:39:16 - error TS7006: Parameter 'c' implicitly has an 'any' type. 39 function lower(c) { ~ src/parsers/Multipart.ts:46:3 - error TS7053: Element implicitly has an 'any' type because expression of type 'string' can't be used to index type '{}'. No index signature with a parameter of type 'string' was found on type '{}'. 46 STATES[stateName] = STATE[stateName]; ~~~~~~~~~~~~~~~~~ src/parsers/Multipart.ts:46:23 - error TS7053: Element implicitly has an 'any' type because expression of type 'string' can't be used to index type '{ PARSER_UNINITIALIZED: number; START: number; START_BOUNDARY: number; HEADER_FIELD_START: number; HEADER_FIELD: number; HEADER_VALUE_START: number; HEADER_VALUE: number; HEADER_VALUE_ALMOST_DONE: number; ... 4 more ...; END: number; }'. No index signature with a parameter of type 'string' was found on type '{ PARSER_UNINITIALIZED: number; START: number; START_BOUNDARY: number; HEADER_FIELD_START: number; HEADER_FIELD: number; HEADER_VALUE_START: number; HEADER_VALUE: number; HEADER_VALUE_ALMOST_DONE: number; ... 4 more ...; END: number; }'. 46 STATES[stateName] = STATE[stateName]; ~~~~~~~~~~~~~~~~ src/parsers/Multipart.ts:123:7 - error TS7053: Element implicitly has an 'any' type because expression of type '`${string}Mark`' can't be used to index type 'MultipartParser'. 123 this[`${name}Mark`] = typeof idx === 'number' ? idx : i; ~~~~~~~~~~~~~~~~~~~ src/parsers/Multipart.ts:127:14 - error TS7053: Element implicitly has an 'any' type because expression of type '`${string}Mark`' can't be used to index type 'MultipartParser'. 127 delete this[`${name}Mark`]; ~~~~~~~~~~~~~~~~~~~ src/parsers/Multipart.ts:137:44 - error TS7053: Element implicitly has an 'any' type because expression of type 'string' can't be used to index type 'MultipartParser'. No index signature with a parameter of type 'string' was found on type 'MultipartParser'. 137 this._handleCallback(name, buffer, this[markSymbol], buffer.length); ~~~~~~~~~~~~~~~~ src/parsers/Multipart.ts:140:44 - error TS7053: Element implicitly has an 'any' type because expression of type 'string' can't be used to index type 'MultipartParser'. No index signature with a parameter of type 'string' was found on type 'MultipartParser'. 140 this._handleCallback(name, buffer, this[markSymbol], i); ~~~~~~~~~~~~~~~~ src/parsers/Multipart.ts:341:39 - error TS2339: Property 'stateToString' does not exist on type 'typeof MultipartParser'. 341 return `state = ${MultipartParser.stateToString(this.state)}`; ~~~~~~~~~~~~~ src/parsers/Multipart.ts:346:17 - error TS2339: Property 'stateToString' does not exist on type 'typeof MultipartParser'. 346 MultipartParser.stateToString = (stateNumber: number) => { ~~~~~~~~~~~~~ src/parsers/Multipart.ts:348:20 - error TS7053: Element implicitly has an 'any' type because expression of type 'string' can't be used to index type '{ PARSER_UNINITIALIZED: number; START: number; START_BOUNDARY: number; HEADER_FIELD_START: number; HEADER_FIELD: number; HEADER_VALUE_START: number; HEADER_VALUE: number; HEADER_VALUE_ALMOST_DONE: number; ... 4 more ...; END: number; }'. No index signature with a parameter of type 'string' was found on type '{ PARSER_UNINITIALIZED: number; START: number; START_BOUNDARY: number; HEADER_FIELD_START: number; HEADER_FIELD: number; HEADER_VALUE_START: number; HEADER_VALUE: number; HEADER_VALUE_ALMOST_DONE: number; ... 4 more ...; END: number; }'. 348 const number = STATE[stateName]; ~~~~~~~~~~~~~~~~ src/PersistentFile.ts:73:27 - error TS2339: Property 'closed' does not exist on type 'Writable'. 73 if (this._writeStream.closed) { ~~~~~~ src/plugins/multipart.ts:25:29 - error TS2684: The 'this' context of type 'void' is not assignable to method's 'this' of type 'IncomingForm'. 25 const initMultipart = createInitMultipart(m[1] || m[2]); ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ src/plugins/multipart.ts:80:9 - error TS2322: Type 'string' is not assignable to type 'string[]'. 80 part.headers[headerField] = headerValue; ~~~~~~~~~~~~~~~~~~~~~~~~~ src/plugins/multipart.ts:160:41 - error TS2345: Argument of type 'number' is not assignable to parameter of type 'string'. 160 const offset = parseInt(part.transferBuffer.length / 4, 10) * 4; ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ src/plugins/octetstream.ts:61:22 - error TS2339: Property 'emit' does not exist on type 'OctetStreamParser | JSONParser | DummyParser | (typeof MultipartParser & { STATES: {}; }) | QuerystringParser'. Property 'emit' does not exist on type 'typeof MultipartParser & { STATES: {}; }'. 61 this._parser.emit('doneWritingFile'); ~~~~ src/plugins/octetstream.ts:80:20 - error TS2349: This expression is not callable. Each member of the union type '{ (event: "close", listener: () => void): OctetStreamParser; (event: "data", listener: (chunk: any) => void): OctetStreamParser; (event: "end", listener: () => void): OctetStreamParser; (event: "error", listener: (err: Error) => void): OctetStreamParser; (event: "pause", listener: () => void): OctetStreamParser; (ev...' has signatures, but none of those signatures are compatible with each other. 80 this._parser.once('doneWritingFile', done); ~~~~ src/VolatileFile.ts:76:27 - error TS2339: Property 'closed' does not exist on type 'Writable'. 76 if (this._writeStream.closed || this._writeStream.destroyed) { ~~~~~~ Found 28 errors. ```

I would appreciate any help with fixing these errors.

GrosSacASac commented 1 year ago

Thanks , I cannot look right now

tommyhtran commented 1 year ago

I've gotten TypeScript errors down to the following:

src/Formidable.ts:305:9 - error TS2345: Argument of type 'BufferEncoding | "7bit" | "8bit"' is not assignable to parameter of type 'BufferEncoding'.
  Type '"7bit"' is not assignable to type 'BufferEncoding'.

305         part.transferEncoding || this.options.encoding,
            ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

src/plugins/multipart.ts:24:29 - error TS2684: The 'this' context of type 'void' is not assignable to method's 'this' of type 'IncomingForm'.

24       const initMultipart = createInitMultipart(m[1] || m[2]);
                               ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

src/plugins/multipart.ts:109:11 - error TS2322: Type 'string' is not assignable to type 'BufferEncoding | "7bit" | "8bit"'.

109           part.transferEncoding = headerValue.toLowerCase();
              ~~~~~~~~~~~~~~~~~~~~~

src/plugins/multipart.ts:149:41 - error TS2345: Argument of type 'number' is not assignable to parameter of type 'string'.

149                 const offset = parseInt(part.transferBuffer.length / 4, 10) * 4;
                                            ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Found 4 errors.

There are two things that should probably be addressed in a separate PR.

  1. https://github.com/node-formidable/formidable/blob/493ec88520c960d0d848d55a929630fde62f2aae/src/plugins/multipart.js#L130 A number value is passed to parseInt() when it should be a string value. I believe the proper fix is to call Math.floor() instead of parseInt().
  2. https://github.com/node-formidable/formidable/blob/493ec88520c960d0d848d55a929630fde62f2aae/src/plugins/multipart.js#L98-L99 Formidable accepts 7bit and 8bit values in the HTTP Content-Transfer-Encoding header. These values eventually get passed into a StringDecoder (https://github.com/node-formidable/formidable/blob/493ec88520c960d0d848d55a929630fde62f2aae/src/Formidable.js#L287-L289), but Node does not recognize these two values(see https://nodejs.org/api/buffer.html#buffers-and-character-encodings). Should 7bit be mapped to ascii and 8bit be mapped to latin1?
tunnckoCore commented 1 year ago

@tommyhtran hey! Thanks for your work on this front. I'm not in friendly relationships with typescript :laughing: But yea.

I can't help much except the few things. I really start hating and rage-quit the editor and coding for some time when I got to some more deep TS codebase. :rofl: :rofl: Not sure how I'll feel here too :laughing:

As about 1), we can convert it to parseInt(String(part.transferBuffer.length / 4), 10) or yea probably Math.floor.

As about the second, I really am not familiar with 7-8bits thing. It's there just because legacy reasons :D Maybe better would be to just drop it and see if someone is even using it. From the docs there seems like that yes 7bit is ascii. Maybe if you look at the v0.8 - 0.10 docs... ahhahaha.

Jokes aside, what you're saying looks about right at first glance.

But also... the fact that it was there for so many years, and no one even tried (because we wouldn't be able) to use ascii or latin1 shows that it isn't even needed. I think?

I'll review it at a later point, but I don't think a review is much needed. Maybe the only thing that I could ask is to try with separate file approach - e.g. just a typings file that we will distribute with the package. Just to see how it looks like.

hyperupcall commented 8 months ago

Another approach is to not change the files to TypeScript and just add typings via JSDoc. And as said before, use a separate typings file for the more complicated types.

hichemfantar commented 5 months ago

Is this still WIP or is it abandoned?

GrosSacASac commented 5 months ago

@hichemfantar see comment to vote https://github.com/node-formidable/formidable/issues/830#issuecomment-1594544883