fastify-multipart
fastify-multipart copied to clipboard
`saveRequestFiles` is consuming value fields and give no way to retreive them if there were no files
Prerequisites
- [X] I have written a descriptive issue title
- [X] I have searched existing issues to ensure the bug has not already been reported
Fastify version
3.20.x
Plugin version
6.0.1
Node.js version
21.7.1
Operating system
Linux
Operating system version (i.e. 20.04, 11.3, 10)
Alpine 3.15 I think
Description
I have a endpoint, where I process optional files. I really like the convenience of saveRequestFiles, but there is one problem - if there were no files - there is no way to access fields (I don't want to use attachFieldsToBody btw) - they are already consumed.
// this prints "start" and "end" only
const files = await req.saveRequestFiles();
// `files` is an empty array
console.log("start");
for await (const part of parts) {
console.log(part);
}
console.log("end");
I know I can still do stuff manually, but maybe there is a way to improve saveRequestFiles somehow? Even "dumb" stuff like attaching extra fields property directly to an array returned by saveRequestFiles() would be nice (that would be non-breaking change)
Link to code that reproduces the bug
No response
Expected Behavior
No response
Thanks for reporting! Would you like to send a Pull Request to address this issue? Remember to add unit tests.
@mcollina I may implement it, but do you agree with proposed solution (adding fields property to an array, which is not the best solution - not future proof in case fields would be added into array prototype) or do you have any better idea?
Another non-breaking solution would be to add another function like saveRequestedFiles, that would return the list of saved files and value fields separately. But there are many functions already (parts(), files(), file(), etc), which is already confusing so i would not do that.
Yet another non breaking solution is to add an param to saveRequestedFiles that would change the return data, but that's not the most beautiful API (but fortunately can be typed with TypeScript).
Do you have any idea maybe?
I think we can change this to:
const { files, values } = await req.saveRequestFiles();
Are you fine with breaking changes?
Yes, it seems we forgot about values in the design of that API.
hmm... that means no backport for me, as i'm using a version 6.x so it's compatible with fastify 3.x
@dzek69 well, that's well out of support anyway, there would never be a backport for fastify 3.x.
{ attachFieldsToBody: true } is definitely the way to go.
I don't want to use attachFieldsToBody btw
I'm missing a good reason against using this?