Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

the v3 plan? #635

Open
tunnckoCore opened this issue May 27, 2020 · 42 comments
Open

the v3 plan? #635

tunnckoCore opened this issue May 27, 2020 · 42 comments
Labels
Status: In Progress This issue is being worked on, and has someone assigned. Type: Discussions Things that need discussion. Brainstorming. Type: Maintenance Updating phrasing or wording to make things clearer or removing ambiguity, without a functionality.

Comments

@tunnckoCore
Copy link
Member

tunnckoCore commented May 27, 2020

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(),
);
@node-formidable node-formidable deleted a comment from auto-comment bot May 27, 2020
@tunnckoCore
Copy link
Member Author

tunnckoCore commented May 27, 2020

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 ...');
});

@tunnckoCore tunnckoCore changed the title v3 v2 / v3 May 27, 2020
@tunnckoCore tunnckoCore changed the title v2 / v3 v3 May 27, 2020
@tunnckoCore tunnckoCore changed the title v3 v3 plan? May 27, 2020
@tunnckoCore tunnckoCore changed the title v3 plan? the v3 plan? May 27, 2020
@leonardovillela
Copy link
Contributor

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
Copy link
Member Author

tunnckoCore commented Dec 28, 2020

Yup, totally cool. I really like this API.

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

@tunnckoCore
Copy link
Member Author

tunnckoCore commented Dec 29, 2020

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
Copy link
Contributor

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

@tunnckoCore
Copy link
Member Author

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

@songkeys
Copy link

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
Copy link
Member Author

tunnckoCore commented Jan 29, 2021

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

@songkeys
Copy link

@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
Copy link
Contributor

After #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
Copy link

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
Copy link

Ah, I guess the current state of v2 is recorded over here, I'll ask over there: #718

@GrosSacASac
Copy link
Contributor

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

@GrosSacASac
Copy link
Contributor

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

@GrosSacASac
Copy link
Contributor

@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
Copy link
Contributor

#730

@jimmywarting

This comment has been minimized.

@GrosSacASac
Copy link
Contributor

3.x is already ESM only

@jimmywarting
Copy link
Collaborator

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 (<anonymous>)
      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.<anonymous> (formidable/src/Formidable.js:5838:23)
    at IncomingForm.emit (node:events:394:28)
    at IncomingForm._error (formidable/src/Formidable.js:6465:10)
    at ClientRequest.<anonymous> (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.<anonymous> (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.<anonymous> (formidable/src/Formidable.js:5838:23)
    at IncomingForm.emit (node:events:394:28)
    at IncomingForm._error (formidable/src/Formidable.js:6465:10)
    at ClientRequest.<anonymous> (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.<anonymous> (formidable/src/Formidable.js:5838:23)
    at IncomingForm.emit (node:events:394:28)
    at IncomingForm._error (formidable/src/Formidable.js:6465:10)
    at ClientRequest.<anonymous> (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.<anonymous> (formidable/src/Formidable.js:5838:23)
    at IncomingForm.emit (node:events:394:28)
    at IncomingForm._error (formidable/src/Formidable.js:6465:10)
    at ClientRequest.<anonymous> (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
Copy link
Contributor

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

@GrosSacASac
Copy link
Contributor

#763

@GrosSacASac GrosSacASac mentioned this issue Oct 9, 2021
Merged
@GrosSacASac
Copy link
Contributor

When do we publish v3 as latest ?

@tunnckoCore
Copy link
Member Author

tunnckoCore commented Oct 30, 2021

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
Copy link
Member Author

@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
Copy link
Contributor

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
Copy link
Member Author

Haha, totally... for both.

@jimmywarting
Copy link
Collaborator

the devDep seems a bit outdated, update them?

@GrosSacASac
Copy link
Contributor

Somebody should do it ...

@tunnckoCore
Copy link
Member Author

@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
Copy link
Contributor

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
Copy link
Member Author

tunnckoCore commented Feb 3, 2022

@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
Copy link
Contributor

I KNow it is old but it could happen again,

@tunnckoCore tunnckoCore added Type: Discussions Things that need discussion. Brainstorming. Status: In Progress This issue is being worked on, and has someone assigned. Type: Maintenance Updating phrasing or wording to make things clearer or removing ambiguity, without a functionality. and removed feature request labels Apr 2, 2022
@tunnckoCore
Copy link
Member Author

tunnckoCore commented Jun 11, 2022

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

parser.initWithBoundary(boundary);

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
Copy link
Member Author

@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
Copy link
Member Author

@jimmywarting 👆👆

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Status: In Progress This issue is being worked on, and has someone assigned. Type: Discussions Things that need discussion. Brainstorming. Type: Maintenance Updating phrasing or wording to make things clearer or removing ambiguity, without a functionality.
Projects
None yet
Development

No branches or pull requests

6 participants