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

the v3 plan? #635

Open tunnckoCore opened 4 years ago

tunnckoCore commented 4 years ago

Okay, some thoughts.

I think we can just wrap the Node's stream.pipeline() and provide it to our users, but they still will be able to do whatever they want, using the Node's pipeline(). Something like this

const { pipeline, MultipartParser, formidable } = require('formidable');

// accepts a callback, or returns a promise if no cb
const { fields, files } = await pipeline(req, formidableOptions);

// if they want more transform or whatever,
// they can add more things, just like in the Node's pipeline()
const { fields, files } = await pipeline(
  req,
  ...streams,
  ...iterators,
);

// or directly using the native pipeline()

const parser = new MultipartParser(); // it's Transform Stream

await promisify(stream.pipeline)(
  req,
  parser,
  formidable(),
);
tunnckoCore commented 4 years ago
const server = http.createServer((req, res) => {
  if (req.url === '/api/upload' && req.method.toLowerCase() === 'post') {
    stream.pipeline(
      req,
      new MultipartParser(),
      async function* consume(source) {
        console.log(source.headers);
        for await (const chunk of source) {
          console.log('chunk:', chunk.toString('utf-8'), '=====');
          yield chunk;
        }
      },
      console.error
    );
    return;
  }

  // show a file upload form
  res.writeHead(200, { 'content-type': 'text/html' });
  res.end(`
    <h2>With Node.js <code>"http"</code> module</h2>
    <form action="/api/upload" enctype="multipart/form-data" method="post">
      <div>Text field title: <input type="text" name="title" /></div>
      <div>first name: <input type="text" name="first_name" /></div>
      <div>File: <input type="file" name="multipleFiles" multiple="multiple" /></div>
      <input type="submit" value="Upload" />
    </form>
  `);
});

server.listen(3000, () => {
  console.log('Server listening on http://localhost:3000 ...');
});
leonardovillela commented 3 years ago

It would be nice to have some kind of plugins that handle common use cases when uploading files. Like for example have a plugin to upload a file directly to s3, other to azures cloud blob storage and etc...

tunnckoCore commented 3 years ago

Yup, totally cool. I really like this API.

The new year will come with a new design :tada: I cannot believe that a whole year passed and there's still no v2. ;/

tunnckoCore commented 3 years ago

We can land something cool in v2 as preparation for v3.

Provide additional exports like formidable/with-aws, formidable/with-azure abstractions like

import formidable from 'formidable';

export default (req, res, options = {}) => {
  // the code from AWS example

  const form = formidable({
    ...options,
    fileWriteStreamHandler: uploadStream,
  });

  form.parse(req, () => {
    res.writeHead(200);
    res.end();
  });

  return form;
} 

usage in serverless env (bbut should fix #594 for this to be so clean)

import formidableWithAWS from 'formidable/with-aws';

export default (req, res) => {
  formidableWithAWS(req, res, { credentials })
}
GrosSacASac commented 3 years ago

As long as it is not included in the default formidable export , I am in favour about adding extensions

tunnckoCore commented 3 years ago

Exactly. Built-in "Extensions" (examples, solutions to common things) is good word.

Songkeys commented 3 years ago

The new year will come with a new design 🎉 I cannot believe that a whole year passed and there's still no v2. ;/

@tunnckoCore Is it going to be switched to v3 without a stable v2?

tunnckoCore commented 3 years ago

@Songkeys we will have v2 stable in the latest dist-tag soon.

Songkeys commented 3 years ago

@tunnckoCore What prevents us having a stable v2 so far? I'm willing to contribute. But we need a roadmap and a todo-list for stable v2 first, I think. Currently this repo is lack of active maintenance. And many users are still stuck in the 1.X version. Plans and Ideas?

GrosSacASac commented 3 years ago

After https://github.com/node-formidable/formidable/pull/689 is merged I will open another PR for the filter branch.

Ideally we fix: the few failing tests (may require some deep untangling of code)

karlhorky commented 3 years ago

I guess #689 was merged and then the filter PR #716 was also merged, right? I guess that means a v2 release sometime in the next few weeks?

karlhorky commented 3 years ago

Ah, I guess the current state of v2 is recorded over here, I'll ask over there: https://github.com/node-formidable/formidable/issues/718

GrosSacASac commented 3 years ago

One one the first thing I want to do is https://github.com/nodejs/node/issues/38231, this will get rid of all the issues around multiples: true

GrosSacASac commented 3 years ago

I created a 3.x branch and opened its first PR for ES Modules

GrosSacASac commented 3 years ago

@tunnckoCore about your first comment first example, I think it is confusing that the pipeline function returns a promise and not a stream. Also it would have the same name as stream.pipeline.

In the second example something is out of order: formidable reads request content type to chose the correct parser: json multipart etc. but in your example the parser is chosen before.

What we could do is make the formidable function return a promise if no callback is provided, or make it promisable (is that even a word 🥇 ?) with https://nodejs.org/api/util.html#util_custom_promisified_functions .

GrosSacASac commented 3 years ago

https://github.com/node-formidable/formidable/pull/730

tunnckoCore commented 3 years ago

What we could do is make the formidable function return a promise if no callback is provided, or make it promisable (is that even a word ?) with https://nodejs.org/api/util.html#util_custom_promisified_functions .

What I was thinking for v3 is to focus just on the parser(s) and make helper tools and/or integrate them well with the Nodejs/Deno/Web built-ins, not that big monolith. That's basically what the examples are, at least at my point of view :D

it is confusing that the pipeline function

intentionally named it pipeline to be similar to the stream.pipeline, which is not confusing but quite the opposite to me - feels familiar and will expect to work similarly ;d

GrosSacASac commented 2 years ago

I published v2 and v3 on my npm account https://www.npmjs.com/package/@grossacasacs/formidable

jimmywarting commented 2 years ago

I have some ideas also...

it's most likely that nodejs will ship the fetch api into core, so they have added support for things like Blobs into core to prep for the fetch api. now Blob is accessible from require('node:buffer').Blob They are also likely to ship File and FormData. (and some form of fetch-blob's fileFrom(path) to fs)

So how about turning things into more standardized & spec'ed api's? eg: make use of FormData and Files (or Blob)

Eventually we could just use node's built in fetch api and do:
formData = await new Response(stream, { headers }).formData() without any dependencies

eventually you would just hand the developer a FormData instance of which they can do things like:

const file = formData.get('avatar')
const rawImageData = await file.arrayBuffer()

// and re-upload files or even the hole formData instance to AWS or some
// bucket using existing fetch api and not have to worry about having to include
// things like a multipart serialization package. 
GrosSacASac commented 2 years ago

Well yes, I am still waiting for things to be native in Node.js, and I keep an eye open for those things. Blob for example is just experimental and formidable package should work on Node.js 14 still. Node fetch still not in Node.js core sadly

FormData package is for sending formdata, and formidable is for receiving. I am not sure if we can reuse some code. Or are you proposing to use FormData package to make it easy to forward a received form ? In that case feel free to add an example inside ./examples first.

GrosSacASac commented 2 years ago

Things I am thinking about

jimmywarting commented 2 years ago

FormData package is for sending formdata, and formidable is for receiving. I am not sure if we can reuse some code. Or are you proposing to use FormData package to make it easy to forward a received form ? In that case feel free to add an example inside ./examples first.

FormData could be used for both sending and receiving, We wouldn't have Response.formData() or Request.formData() otherwise. They exist so you can intercept the payload from Service Worker. FormData did not have any method for reading/manipulating a FormData earlier but now we have .has(), .delete(), .get(), .entries(), for..of and so on

So yes, I'm proposing to use a FormData package to make it easy to forward a received form. But also for parsing and consuming a received form

I think it would be cool if form.formData() could utilize a FormData package and resolve the stream with a FormData instance. Here is an example from the Readme that i have rewritten:

import { createServer } from 'node:http'
import formidable from 'formidable'

const server = createServer(async (req, res) => {
  if (req.url === '/api/upload' && req.method.toLowerCase() === 'post') {
    // parse a file upload
    const form = formidable({ uploadDir: '/tmp' });

    // Similar to whatwg `request.fromData()`
    const formData = await form.formData(req)
    console.log(...formData.entries())

    // file is a web File object
    const file = formData.get('file') // ( The file could be backed up by the filesystem - in /tmp dir )
    console.log(file.name)

    res.writeHead(200, { 
      'Content-Type': file.type,
      'Content-Length': file.size
    })

    // file.stream() is a whatwg:stream
    for await (const uint8 of file.stream()) {
      res.send(uint8)
    }

    return res.end()
  }

  // show a file upload form
  res.writeHead(200, { 'Content-Type': 'text/html' });
  res.end(`
    <h2>With Node.js <code>"http"</code> module</h2>
    <form action="/api/upload" enctype="multipart/form-data" method="post">
      <div>Text field title: <input type="text" name="title" /></div>
      <div>File: <input type="file" name="file" /></div>
      <input type="submit" value="Upload" />
    </form>
  `)
})

server.listen(8080, () => {
  console.log('Server listening on http://localhost:8080/ ...')
});

(calling form.formData() would work for both urlencoded and multipart payloads)


On a side note: you said you wanted to handle a (web)stream... that would be 👍, it would be great if formidable was not so bound to HttpIncomingRequest so you can just hand it some iterable object (wheater it be stream, req, webstream or any asyncIterator) - so something like: const formData = await form.formData(iterable, contentType) maybe? idk

GrosSacASac commented 2 years ago

I see it as breaking changes (fields and files output changen new dependency) so we can start with version 4.x I add you to formdiable github so you can create new branches and make commits if you want to try to implement your ideas in a new branch, (use 3.x as a starting point)

Overall I like the ideas to move towards web apis when possible.

jimmywarting commented 2 years ago

Hmm, okey - i can give it a go. One thing doe... my FormData package is ESM-only, so is fetch-blob which it also depends on.

  ├─┬ formdata-polyfill (esm-only)
  │ └─┬ fetch-blob (esm-only)
  │   └── web-streams-polyfill

b/c .formData() would be async then it would be possible to lazy load ESM-only packages using the async import('formdata-polyfill') from cjs. So it would be no biggie if you would like to keep it as commonjs. But i was just wondering if you wish to make the switch to ESM-only as well...? since it is going to be a major change anyway...

(Also formdata-polyfill + fetch-blob both depend on node v12.20+ stuff) including them would mean you would have to target same node versions

GrosSacASac commented 2 years ago

3.x is already ESM only

jimmywarting commented 2 years ago

Think there are som bugs in 3.x that needs to be addressed first... the test didn't run so well...

Dump ``` PASS test/integration/file-write-stream-handler-option.test.js ✓ file write stream handler (43 ms) PASS test/integration/json.test.js ✓ json (50 ms) FAIL test/integration/fixtures.test.js ✕ fixtures (56 ms) ● fixtures ReferenceError: require is not defined 27 | const group = basename(fp, '.js'); 28 | const filepath = join(FIXTURES_PATH, fp); > 29 | const mod = require(filepath); | ^ 30 | 31 | Object.keys(mod).forEach((k) => { 32 | Object.keys(mod[k]).forEach((_fixture) => { at reduce (test/integration/fixtures.test.js:29:21) at Array.reduce () at Server.findFixtures (test/integration/fixtures.test.js:26:8) PASS test/unit/querystring-parser.test.js ✓ on constructor (2 ms) PASS test/integration/store-files-option.test.js ✓ store files option (62 ms) PASS test/integration/octet-stream.test.js ✓ octet stream (72 ms) PASS test/unit/multipart-parser.test.js ✓ on constructor (3 ms) ✓ initWithBoundary (3 ms) ✓ initWithBoundary failing (1 ms) ✓ on .end() throwing (38 ms) ✓ on .end() successful (1 ms) PASS test/standalone/content-transfer-encoding.test.js ✓ content transfer encoding (48 ms) PASS test/unit/volatile-file.test.js VolatileFile ✓ open() (4 ms) ✓ toJSON() ✓ toString() (1 ms) ✓ write() ✓ end() (1 ms) ✓ destroy() (1 ms) PASS test/standalone/issue-46.test.js ✓ issue 46 (28 ms) FAIL test/standalone/keep-alive-error.test.js ✕ keep alive error (393 ms) ● keep alive error assert.strictEqual(received, expected) Expected value to strictly be equal to: 1 Received: 0 Message: should "errors" === 1, has: 0 49 | 50 | setTimeout(() => { > 51 | strictEqual(errors, 1, `should "errors" === 1, has: ${errors}`); | ^ 52 | 53 | const clientTwo = createConnection(choosenPort); 54 | at test/standalone/keep-alive-error.test.js:51:9 at Timeout.task [as _onTimeout] (node_modules/jsdom/lib/jsdom/browser/Window.js:516:19) FAIL test/unit/persistent-file.test.js ● Test suite failed to run ReferenceError: formidable/test/unit/persistent-file.test.js: The module factory of `jest.mock()` is not allowed to reference any out-of-scope variables. Invalid variable access: jest Allowed objects: AbortController, AbortSignal, AggregateError, Array, ArrayBuffer, Atomics, BigInt, BigInt64Array, BigUint64Array, Boolean, Buffer, DataView, Date, Error, EvalError, Event, EventTarget, FinalizationRegistry, Float32Array, Float64Array, Function, Generator, GeneratorFunction, Infinity, Int16Array, Int32Array, Int8Array, InternalError, Intl, JSON, Map, Math, MessageChannel, MessageEvent, MessagePort, NaN, Number, Object, Promise, Proxy, RangeError, ReferenceError, Reflect, RegExp, Set, SharedArrayBuffer, String, Symbol, SyntaxError, TextDecoder, TextEncoder, TypeError, URIError, URL, URLSearchParams, Uint16Array, Uint32Array, Uint8Array, Uint8ClampedArray, WeakMap, WeakRef, WeakSet, WebAssembly, arguments, atob, btoa, clearImmediate, clearInterval, clearTimeout, console, decodeURI, decodeURIComponent, encodeURI, encodeURIComponent, escape, eval, expect, global, globalThis, isFinite, isNaN, jest, parseFloat, parseInt, performance, process, queueMicrotask, require, setImmediate, setInterval, setTimeout, undefined, unescape. Note: This is a precaution to guard against uninitialized mock variables. If it is ensured that the mock is required lazily, variable names prefixed with `mock` (case insensitive) are permitted. 15 | 16 | jest.mock('fs', () => { > 17 | const fs = jest.requireActual('fs'); | ^^^^ 18 | return { 19 | ...fs, 20 | unlink: jest.fn(), at File.buildCodeFrameError (node_modules/@babel/core/lib/transformation/file/file.js:240:12) at NodePath.buildCodeFrameError (node_modules/@babel/traverse/lib/path/index.js:138:21) at newFn (node_modules/@babel/traverse/lib/visitors.js:175:21) at NodePath._call (node_modules/@babel/traverse/lib/path/context.js:55:20) at NodePath.call (node_modules/@babel/traverse/lib/path/context.js:42:17) at NodePath.visit (node_modules/@babel/traverse/lib/path/context.js:92:31) at TraversalContext.visitQueue (node_modules/@babel/traverse/lib/context.js:116:16) formidable/src/Formidable.js:5838 callback(err, fields, files); ^ ReferenceError: fields is not defined at IncomingForm. (formidable/src/Formidable.js:5838:23) at IncomingForm.emit (node:events:394:28) at IncomingForm._error (formidable/src/Formidable.js:6465:10) at ClientRequest. (formidable/src/Formidable.js:5866:12) at ClientRequest.emit (node:events:394:28) at Socket.socketErrorListener (node:_http_client:447:9) at Socket.emit (node:events:394:28) at emitErrorNT (node:internal/streams/destroy:157:8) at emitErrorCloseNT (node:internal/streams/destroy:122:3) at processTicksAndRejections (node:internal/process/task_queues:83:21) console.error Error: Uncaught [AssertionError: should "errors" === 1, has: 0] at reportException (formidable/node_modules/jsdom/lib/jsdom/living/helpers/runtime-script-errors.js:62:24) at Timeout.task [as _onTimeout] (formidable/node_modules/jsdom/lib/jsdom/browser/Window.js:521:9) at listOnTimeout (node:internal/timers:557:17) at processTimers (node:internal/timers:500:7) AssertionError [ERR_ASSERTION]: should "errors" === 1, has: 0 at formidable/test/standalone/keep-alive-error.test.js:51:9 at Timeout.task [as _onTimeout] (formidable/node_modules/jsdom/lib/jsdom/browser/Window.js:516:19) at listOnTimeout (node:internal/timers:557:17) at processTimers (node:internal/timers:500:7) { generatedMessage: false, code: 'ERR_ASSERTION', actual: 0, expected: 1, operator: 'strictEqual' } at VirtualConsole. (node_modules/jsdom/lib/jsdom/virtual-console.js:29:45) at reportException (node_modules/jsdom/lib/jsdom/living/helpers/runtime-script-errors.js:66:28) at Timeout.task [as _onTimeout] (node_modules/jsdom/lib/jsdom/browser/Window.js:521:9) PASS test/unit/custom-plugins.test.js ✓ should call 3 custom and 1 builtin plugins, when .parse() is called (31 ms) ✓ .parse throw error when some plugin fail (19 ms) ✓ multipart plugin fire `error` event when malformed boundary (63 ms) ✓ formidable() throw if not at least 1 built-in plugin in options.enabledPlugins (7 ms) console.error undefined at Application.onerror (node_modules/koa/lib/application.js:204:13) at Object.onerror (node_modules/koa/lib/context.js:121:14) at onerror (node_modules/koa/lib/application.js:163:32) console.error Error: plugin on index 1 failed with: fields is not defined at IncomingForm._parseContentType (formidable/src/Formidable.js:423:23) at IncomingForm.writeHeaders (formidable/src/Formidable.js:218:10) at IncomingForm.parse (formidable/src/Formidable.js:184:10) at handler (formidable/test/unit/custom-plugins.test.js:182:12) at formidable/test/unit/custom-plugins.test.js:15:11 at dispatch (formidable/node_modules/koa-compose/index.js:42:32) at formidable/node_modules/koa-compose/index.js:34:12 at Application.handleRequest (formidable/node_modules/koa/lib/application.js:166:12) at Server.handleRequest (formidable/node_modules/koa/lib/application.js:148:19) at Server.emit (node:events:394:28) at parserOnIncoming (node:_http_server:927:12) at HTTPParser.parserOnHeadersComplete (node:_http_common:128:17) at Application.onerror (node_modules/koa/lib/application.js:205:13) at Object.onerror (node_modules/koa/lib/context.js:121:14) at onerror (node_modules/koa/lib/application.js:163:32) console.error undefined at Application.onerror (node_modules/koa/lib/application.js:206:13) at Object.onerror (node_modules/koa/lib/context.js:121:14) at onerror (node_modules/koa/lib/application.js:163:32) (node:27924) ExperimentalWarning: VM Modules is an experimental feature. This feature could change at any time (Use `node --trace-warnings ...` to show where the warning was created) formidable/src/Formidable.js:5838 callback(err, fields, files); ^ ReferenceError: fields is not defined at IncomingForm. (formidable/src/Formidable.js:5838:23) at IncomingForm.emit (node:events:394:28) at IncomingForm._error (formidable/src/Formidable.js:6465:10) at ClientRequest. (formidable/src/Formidable.js:5866:12) at ClientRequest.emit (node:events:394:28) at Socket.socketErrorListener (node:_http_client:447:9) at Socket.emit (node:events:394:28) at emitErrorNT (node:internal/streams/destroy:157:8) at emitErrorCloseNT (node:internal/streams/destroy:122:3) at processTicksAndRejections (node:internal/process/task_queues:83:21) (node:27934) ExperimentalWarning: VM Modules is an experimental feature. This feature could change at any time (Use `node --trace-warnings ...` to show where the warning was created) formidable/src/Formidable.js:5838 callback(err, fields, files); ^ ReferenceError: fields is not defined at IncomingForm. (formidable/src/Formidable.js:5838:23) at IncomingForm.emit (node:events:394:28) at IncomingForm._error (formidable/src/Formidable.js:6465:10) at ClientRequest. (formidable/src/Formidable.js:5866:12) at ClientRequest.emit (node:events:394:28) at Socket.socketErrorListener (node:_http_client:447:9) at Socket.emit (node:events:394:28) at emitErrorNT (node:internal/streams/destroy:157:8) at emitErrorCloseNT (node:internal/streams/destroy:122:3) at processTicksAndRejections (node:internal/process/task_queues:83:21) (node:27936) ExperimentalWarning: VM Modules is an experimental feature. This feature could change at any time (Use `node --trace-warnings ...` to show where the warning was created) formidable/src/Formidable.js:5838 callback(err, fields, files); ^ ReferenceError: fields is not defined at IncomingForm. (formidable/src/Formidable.js:5838:23) at IncomingForm.emit (node:events:394:28) at IncomingForm._error (formidable/src/Formidable.js:6465:10) at ClientRequest. (formidable/src/Formidable.js:5866:12) at ClientRequest.emit (node:events:394:28) at Socket.socketErrorListener (node:_http_client:447:9) at Socket.emit (node:events:394:28) at emitErrorNT (node:internal/streams/destroy:157:8) at emitErrorCloseNT (node:internal/streams/destroy:122:3) at processTicksAndRejections (node:internal/process/task_queues:83:21) FAIL test/unit/formidable.test.js ● Test suite failed to run Call retries were exceeded at ChildProcessWorker.initialize (node_modules/jest-worker/build/workers/ChildProcessWorker.js:193:21) FAIL test/standalone/connection-aborted.test.js (6.177 s) ✕ connection aborted (5010 ms) ● connection aborted : Timeout - Async callback was not invoked within the 5000 ms timeout specified by jest.setTimeout.Timeout - Async callback was not invoked within the 5000 ms timeout specified by jest.setTimeout.Error: 6 | const PORT = 13539; 7 | > 8 | test('connection aborted', (done) => { | ^ 9 | const server = createServer((req) => { 10 | const form = formidable(); 11 | at new Spec (node_modules/jest-jasmine2/build/jasmine/Spec.js:116:22) at test/standalone/connection-aborted.test.js:8:1 A worker process has failed to exit gracefully and has been force exited. This is likely caused by tests leaking due to improper teardown. Try running with --detectOpenHandles to find leaks. ---------------------|---------|----------|---------|---------|---------------------------------------------------------------------------------------------------------------------------------------------- File | % Stmts | % Branch | % Funcs | % Lines | Uncovered Line #s ---------------------|---------|----------|---------|---------|---------------------------------------------------------------------------------------------------------------------------------------------- All files | 78.87 | 66.1 | 82.46 | 78.71 | src | 74.92 | 55.97 | 82.86 | 74.68 | Formidable.js | 73.33 | 57.94 | 82.98 | 73.09 | ...8,232,238,241-244,257-262,272-278,300-307,319,333,343-350,353-360,363,373-380,394-395,399-406,475,484,517,524-537,547,556-566,573-574,588 FormidableError.js | 100 | 100 | 100 | 100 | PersistentFile.js | 66.67 | 33.33 | 72.73 | 66.67 | 18,27-48,53,57-58,71 VolatileFile.js | 77.42 | 57.14 | 90 | 77.42 | 17,26,43,54,58-59,71 index.js | 100 | 100 | 100 | 100 | src/parsers | 83.48 | 75.93 | 85.71 | 83.33 | Dummy.js | 50 | 0 | 50 | 50 | 14-16 JSON.js | 83.33 | 0 | 100 | 83.33 | 23-24 Multipart.js | 87.5 | 77.88 | 100 | 87.37 | 69-71,97,129-130,141,148,156-158,164,195,204,223,229,269,289-292,303-309,316 OctetStream.js | 100 | 0 | 100 | 100 | 4 Querystring.js | 33.33 | 100 | 33.33 | 33.33 | 17-31 index.js | 0 | 0 | 0 | 0 | src/plugins | 80.29 | 71.7 | 78.26 | 80.29 | index.js | 0 | 0 | 0 | 0 | json.js | 100 | 100 | 100 | 100 | multipart.js | 80.52 | 72.09 | 75 | 80.52 | 120-153 octetstream.js | 94.44 | 66.67 | 100 | 94.44 | 62,81 querystring.js | 23.08 | 50 | 25 | 23.08 | 16,26-41 ---------------------|---------|----------|---------|---------|---------------------------------------------------------------------------------------------------------------------------------------------- Test Suites: 5 failed, 10 passed, 15 total Tests: 3 failed, 22 passed, 25 total Snapshots: 0 total Time: 8.721 s, estimated 9 s ```
GrosSacASac commented 2 years ago

One day I tried to make all the test pass but I didn't finish, I 'll have a look this evening again

GrosSacASac commented 2 years ago

https://github.com/node-formidable/formidable/pull/763

GrosSacASac commented 2 years ago

When do we publish v3 as latest ?

tunnckoCore commented 2 years ago

I suggest somewhere in December or January. Lets give some time to see how it goes.

(i will edit the things now as to #769, what to finish it as quick as possible cuz a lot of people we see PRs so it should be clear)

tunnckoCore commented 2 years ago

@Songkeys @jimmywarting check #769 and the comments from today in #718.

I'm back, i guess. Interested in crypto more and more so i need dev environment and will be on github more. I seen they launched Codespaces publicly, I should try it tho.

GrosSacASac commented 2 years ago

Crypto Hodl gang ! Codespace is visual studio in the cloud, pretty cool I used something similar hacked together in 2015, neat for multi user collaboration

tunnckoCore commented 2 years ago

Haha, totally... for both.

jimmywarting commented 2 years ago

the devDep seems a bit outdated, update them?

GrosSacASac commented 2 years ago

Somebody should do it ...

tunnckoCore commented 2 years ago

@jimmywarting meh, they are okay, especially as I didn't look on latest updates on Prettier and Airbnb, just check ESLint 7 before some weeks/months. Jest is usually not that breaking too so.

GrosSacASac commented 2 years ago

Read this https://www.theregister.com/2018/11/26/npm_repo_bitcoin_stealer/

For me the biggest question is not if it is compatible, but if it looks good (including sub dependencies) I try to read the npm diff with npmfs but it takes a lot of time

tunnckoCore commented 2 years ago

@GrosSacASac that's old, is there some recent things like that? But yea, got it, we should stay updated, true.

There's dependabot PRs, but the automerge isn't working for some reason or isn't enabled. That would be the best.

GrosSacASac commented 2 years ago

I KNow it is old but it could happen again,

tunnckoCore commented 2 years ago

I like the FormData and Blob ideas.

But whatever we do, we really need to make benchmarks first. Everything boils down to that, otherwise there's no sense in Formidable. Why it exists in the first place. It used to be one of the fastest streaming multipart parsers, and that is at its core.

With all the recent Nodejs versions, updates, new standards and all, the question comes more and more. And is it fast at all, with the new Streams APIs. Working in blind isn't good.

From what I understand, and from @jimmywarting's examples, I think we should make the following:

Initialize FormData above that, in the Multipart plugin https://github.com/node-formidable/formidable/blob/81dd350835e14dccca667fc46bc5c35f16f1b5ec/src/plugins/multipart.js#L51

and on parser.on('data') just fd.append the incoming file. Getting the things from part and put them on append('file', new File(part.name, { type: part.mimetype, ... }))

Right? Then just wrap that and expose formData() method to the user so it will get the fd. From there our job ends: we parsed the request and give you standard FormData. And can just provide extensions for further common things.

or

fd.append('file', {
  size: part.size,
  type: part.mimetype,
  name: part.name,
  stream() { return part },
  [Symbol.toStringTag]: 'File'
})

something like this


const form = formidable({ uploadDir: "./uploads" });
const formData = await form.parse(req);

// extension/utility that write files to disk,
for (const [, file] of formData.entries()) {
  file.stream().pipe(fs.createWriteStream(file.name));
}

quite not right to me tho, but that's userland

tunnckoCore commented 2 years ago

@jimmywarting ha, didn't knew that node-fetch is using formidable's multipart parser, hehe.

The only difference is that it's not Transform stream and uses uint8array. Seems like it also expose toFormData which is exactly what I thought in my previous comment LOL - thin layer on top of the parser.

Do you mind to extract it as @formidable/multipart and we will both use it? Not sure if the namespace is free.

tunnckoCore commented 3 months ago

@jimmywarting 👆👆