-
-
Notifications
You must be signed in to change notification settings - Fork 409
Fix handling of input of type FormData for the Fetch client #1008
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
Conversation
29fc4dc
to
6d0ec6d
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Copilot reviewed 5 out of 25 changed files in this pull request and generated no comments.
Files not reviewed (20)
- templates/base/http-clients/fetch-http-client.ejs: Language not supported
- tests/spec/another-query-params/snapshots/basic.test.ts.snap: Language not supported
- tests/spec/custom-extensions/snapshots/basic.test.ts.snap: Language not supported
- tests/spec/defaultAsSuccess/snapshots/basic.test.ts.snap: Language not supported
- tests/spec/defaultResponse/snapshots/basic.test.ts.snap: Language not supported
- tests/spec/deprecated/snapshots/basic.test.ts.snap: Language not supported
- tests/spec/dot-path-params/snapshots/basic.test.ts.snap: Language not supported
- tests/spec/enumNamesAsValues/snapshots/basic.test.ts.snap: Language not supported
- tests/spec/extractRequestBody/snapshots/basic.test.ts.snap: Language not supported
- tests/spec/extractRequestParams/snapshots/basic.test.ts.snap: Language not supported
- tests/spec/extractResponseBody/snapshots/basic.test.ts.snap: Language not supported
- tests/spec/extractResponseError/snapshots/basic.test.ts.snap: Language not supported
- tests/spec/jsAxios/snapshots/basic.test.ts.snap: Language not supported
- tests/spec/moduleNameFirstTag/snapshots/basic.test.ts.snap: Language not supported
- tests/spec/moduleNameIndex/snapshots/basic.test.ts.snap: Language not supported
- tests/spec/on-insert-path-param/snapshots/basic.test.ts.snap: Language not supported
- tests/spec/patch/snapshots/basic.test.ts.snap: Language not supported
- tests/spec/responses/snapshots/basic.test.ts.snap: Language not supported
- tests/spec/singleHttpClient/snapshots/basic.test.ts.snap: Language not supported
- tests/spec/sortTypes-false/snapshots/basic.test.ts.snap: Language not supported
How long will this fix stay open? The problem is actual 😤 |
Hi everyone, resolving this issue can be helpfull for nodejs projects. |
+1, I encountered the same problem. So far, I have been adding this early return manually each time after generating a new build. Therefore, it would be very helpful if this could be checked and merged. |
ad65060
to
36a852f
Compare
🦋 Changeset detectedLatest commit: 25e12e3 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Signed-off-by: Sora Morimoto <[email protected]>
Several open issues are caused by the
multipart/form-data
formatter trying to provide a simple API based on an input of typeObject
. However,Object
can not express some of the typical use cases of a multipart request. On the other hand,FormData
could.One example (mentioned in #705 and #965) is sending a requested with multiple entries belonging to the same "key". Since a
FormData
object allows one key to be mapped to multiple values, one can simply do:This is clearly not possible with an
Object
, where the key-value relationship is 1:1. #1000 tries to achieve this by automatically convertingArray
values to multipleFormData
keys. However, I argue that it is not necessarily what the user wants, since they could either expect:FormData
entry, orFormData
entries.In principle, it is not so easy to infer which of the two is the one desired by the user.
Currently, however, inputs of type
FormData
are not correctly handled by themultipart/form-data
formatter for the Fetch client, which tries to enumerate the keys using theObject.keys(...)
construct:swagger-typescript-api/templates/base/http-clients/fetch-http-client.ejs
Lines 107 to 108 in 325e308
Unfortunately,
FormData
returns an empty list when used throughObject.keys()
This PR proposes a way to allow the user to directly supply a
FormData
, basically opting out of any smart formatting logic that might actually get in the way. In this way, the user takes responsibility for correctly managing the conversion.I argue that the breaking change is minimal since any use of
FormData
is broken under the current API. Additionally, this would align with the logic for the Axios client, which already short-circuits the formatting forFormData
inputs.swagger-typescript-api/templates/base/http-clients/axios-http-client.ejs
Lines 81 to 82 in 325e308