feat - new promise based api

[StarpTech]

This is a preview of the new interface. I appreciate any feedback. Tests suite is incomplete but the features are working already.

Checkout the examples to get a better impression or play with the form.html

Related: #109, #139

Checklist

• run npm run test and npm run benchmark
• tests and/or benchmarks are included
• documentation is changed or added
• commit message and code follows the Developer's Certification of Origin
  and the Code of conduct

[jsumners]

I rather prefer the original interface. This strikes me as too inflexible.

[StarpTech]

Could you be more specific? What do you miss?

[mcollina]

I like this API quite a lot, I think it'd be more usable for the majority of use cases. It also looks a lot like a new module to me.

[jsumners]

Could you be more specific? What do you miss?

It looks to me like this implementation assumes "file" uploads and always writes the data out to a file on the filesystem. Multipart isn't always about uploading files. In particular, I have a case where I need to look for a part named "json" and read the data attached to it as an object; I have no need of writing this to the filesystem.

[StarpTech]

It looks to me like this implementation assumes "file" uploads and always writes the data out to a file on the filesystem. Multipart isn't always about uploading files. In particular, I have a case where I need to look for a part named "json" and read the data attached to it as an object; I have no need of writing this to the filesystem.

Oh no, please check out all examples. You can work with just streams or files in disk mode.

[jsumners]

It also looks a lot like a new module to me.

At a minimum, I agree with this statement. One reason I think a major framework has recently been deprecated is due to the way it was routinely using semver major to release what amounted to new frameworks under the same name. This PR completely changes the API for this module, and will be a significant hinderance to people using the module. I think that is a worse developer experience than the current API.

[StarpTech]

Wow, why so negative? Let focus at first on the PR. We can discuss afterward how we can incorporate that in the fastify ecosystem.

[jsumners]

I'm not seeing any negativity in my statement. I only stated that I agree this module should remain as-is.

[StarpTech]

I'm not seeing any negativity in my statement. I only stated that I agree this module should remain as-is.

You made it already clear with your first comment.

[StarpTech]

@mcollina on windows the tests npm test will run in a timeout. When I run the tests via node they will fail only occasionally. (On Linux all test running successfully)

E.g it will be stuck.

$ node test/multipart-stream.test.js 
TAP version 13
# Subtest: should throw fileSize limitation error on small payload
    1..4
    ok 1 - expect truthy value
    ok 2 - should be equal

It looks like it's not related to a specific test. On my machine 'should throw fileSize limitation error on small payload' and 'should not allow __proto__ as file name' fail regularly. Perhaps you find the bug in the code.

[StarpTech]

@mcollina I could fix it 😄

[mcollina]

go for a fix! I don't have a clue - the first is likely due to difference in the network stack.

[StarpTech]

On windows ci npm install does not even start 🤯 On MAC the tests failing. On Linux everything works fine. I need some help.

[mcollina]

Really good work! I've left some comments and a few nits.

[mcollina]

lgtm, let's wait for the others anyway.

[StarpTech]

lgtm, let's wait for the others anyway.

@mcollina I will wait for one more. The activity here is extremely low. I'd like to use this module.

[mcollina]

Can you change the title of this PR?

[delvedor]

LGTM