formidable
formidable copied to clipboard
node-formidable
Promise support for form.parse()
I am thinking that it could be nice to make form.parse() promisified: if no callback is provided, it will return a promise.
This could allow us to rewrite the following code:
form.parse(req, (err, fields, files) => {
if (err) {
// ... handle error
return
}
// do something with fields and files
})
Into something like this:
try {
const [fields, files] = await form.parse(req)
// ... do something with fields and files
} catch (e) {
// ... handle error
}
Is this something that has been considered already?
I had a quick look at the code and it seems that parse currently returns this (i suppose to allow method chaining). This seems to be a potential blocker for this feature (as it will require a breaking change), but as an alternative, it could be possible to create a new method called parsePromise or something similar, which can act as a wrapper for the original parse method...
The main advantages I see in supporting this syntax are the following:
- Unified error handling (one catch to rule them all)
- Integrates well with async handlers where there might be other async logic before or after the form parsing
- Fewer callbacks nesting
Thoughts?
If this idea resonates with the maintainers and the community, I'd be happy to find some time to submit a PR...
Wait for v2 to be finalized. We will try to make no breaking change for v2 but for v3 this could be good.
If you want to help review this PR https://github.com/node-formidable/formidable/pull/686
Yep, it was considered. Maybe we can introduce it in v2 as parseAsync, later in v 3 just merge them.
Should form.parse()
- awlays return a promise (vote with 👍 )
- or be callback based if a function is passed to it and else return a promise (vote with ❤️ )
- other 😕 (explain in comment)
@GrosSacASac
or be callback based if a function is passed to it and else return a promise
for v2 and above. Then we should consider the Streaming things and APIs, but lets that be v4 haha.
Just a thought... instead of supporting both callbacks and promises in the same function you could mimic the way fs does it. Something like:
// For Callback
import { formidable } from 'formidable';
const formidable = require('formidable');
// For Promise
import { formidable } from 'formidable/promise';
const formidable = require('formidable').promises;
@jwerre yep, i'm leaning more towards that, or just a separate exports.
@GrosSacASac haha :rofl: on v3 or for v4? I think I said we will drop v2 (as main) around march or april? Uh, let me see where that thing was haha.
How wrapping function in promise would take a year?
const [fields, files] = (await new Promise(async (r, re) => {
await form.parse(req, async (err, fields, files) => {
if (err) return re(err)
return [fields, files]
})
})) as [formidable.Fields, formidable.Files]
but please do coverage test on a code above
It's not hard. It's just that there are several other things and at least 2 parallel versions used.
Plus, it's that hard to just use util.promisify, or whatever it was called, on form.parse.
It's a standard callback function anyway.
here is a "one line" solution for creating the promise
const parseForm = async (req) => new Promise((resolve, reject) => new formidable.IncomingForm().parse(req, (err, fields, files) => err ? reject(err) : resolve([fields, files])))
usage:
const [fields, files] = await parseForm(req)
@ds2345 right. The beauty of callback APIs, you can easily turn them into promises.. haha.
Hi @ds2345 , I believe your code works, but it seems my code never enters into callback (err, fields, files) => err ? reject(err) : resolve([fields, files])), no error or exception either, the last log output is START form.parse(). The file is uploaded by axios.postForm(), it's just an 82bytes markdown file. Did I miss anything to make this code work? Thanks in advance!
BTW, the version of formidable I currently use is 2.1.1
async #parseForm(request) {
return new Promise((resolve, reject) => {
// const form = formidable({ multiples: true, api: { bodyParser: false } });
console.log('START form.parse()');
new formidable.IncomingForm().parse(request, (error, fields, files) => {
console.log(
'form.parse() error:',
error,
', fields:',
fields,
', files:',
files
);
error ? reject(error) : resolve([fields, files]);
});
});
}
async handle(request) {
console.time('TaskAttachmentSaveHandler.handle() parseForm');
const [fields, files] = await this.#parseForm(request);
console.timeEnd('TaskAttachmentSaveHandler.handle() parseForm');
}
It's very disappointing to see that a library in 2023 isn't capable of using promises.
I would happily submit a PR, as this fix would be trivial to make ... but from the comments here it sounds like the maintainers are against promises?
It is not delayed because it is hard, but because I was busy and I try to fix all the test, see the ongoing PR for commit details
So based on votes I decided to make it both callback and promise as a transition and then later make it promise only (breaking change)
Awesome job! Thanks. Also, I should mention, can you update npm so the types for the library will be updated too? TY
Oh nice, v3 is now latest on npm 👀 🔥 Thanks for finally publishing v3!!
However... would be good to get some documentation / types!
I've documented this a bit:
Migration guide?
Unfortunately, in upgrading from [email protected] to [email protected] just now, it appears that our code is breaking.
I was looking for a "Migrating from v2 to v3" guide, I only found this Version History file which seems more like (partly outdated) historical information:
But maybe this is not documentation that the Formidable team will provide? 😬
Release notes?
There are also release notes for v3.0.0, which seem to potentially have breaking changes, although none of them are explicitly called out as such:
3.0.0
- feat: remove options.multiples (https://github.com/node-formidable/formidable/pull/730)
- use modern URLSearchParams https://developer.mozilla.org/en-US/docs/Web/API/URLSearchParams internally
- files and fields values are always arrays
- fields with [] in the name do not receive special treatment
- remove unused qs and querystring dependency
- feat: Use ES modules (https://github.com/node-formidable/formidable/pull/727)
- options.enabledPlugins must contain the plugin themselves instead of the plugins names
Broken types?
Maybe the DefinitelyTyped types at @types/formidable are also outdated now, because of the data structure changes described in the 3.0.0 release notes.
cc @martin-badin @gboer @peterblazejewicz as "code owners" of @types/formidable
Soo... looks like it's time for a bit of trial and error to see how to get this working... 😬
The problems in our project was associated with this change from the v3 changelog:
- files and fields values are always arrays
Our code expected a single file as an object instead of an array with a single item inside.
It appears @types/formidable is already compatible with this change.