-
-
Notifications
You must be signed in to change notification settings - Fork 518
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
Encode the request body if Content-Type
header is x-www-form-urlencoded
#2096
base: main
Are you sure you want to change the base?
Conversation
👷 Deploy request for openapi-ts pending review.Visit the deploys page to approve it
|
|
@@ -72,7 +72,7 @@ export default function createClient(clientOptions) { | |||
}); | |||
} | |||
|
|||
const serializedBody = body === undefined ? undefined : bodySerializer(body); | |||
const serializedBody = body === undefined ? undefined : bodySerializer(body, headers); |
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.
So these are the headers unique to this request (probably could be named better). Below on line 92
, we merge all the headers together. We should probably move that up, adn pass that in here to bodySerializer
. That way it takes into account headers that users have set in their createClient()
call, and it handles the normalization for you so it’s always a Header
type (and you aren’t dealing with what the user passed in)
This approach looks good to me, and good test! But before we merge, we’ll want to:
|
@obulat Still willing to accept this PR with the changes mentioned! |
Changes
Fixes #2069
This PR updates the
defaultBodySerializer
to correctly serialize the body ifContent-Type
header is set tox-www-form-urlencoded
. This is useful for OAuth2 token requests.How to Review
I'm not sure about the headers: what type are they, and are they normalized?
I added handling for both a plain JS object (
headers['Accept-Content']
) and aHeaders
object (headers.get['Accept-Content']
), as well as both capitalized and lower-case versions.Checklist
docs/
updated (if necessary)pnpm run update:examples
run (only applicable for openapi-typescript)