openapi-fetch improvements #1169
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
🦋 Changeset detectedLatest commit: 3a61261 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 |
Deploying with
|
| Latest commit: |
3a61261
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://9b9a3004.openapi-ts.pages.dev |
| Branch Preview URL: | https://improvements.openapi-ts.pages.dev |
1 task
drwpow
commented
Jun 16, 2023
| @@ -54,6 +56,31 @@ export type FetchResponse<T> = | |||
| | { data: T extends { responses: any } ? NonNullable<FilterKeys<Success<T["responses"]>, JSONLike>> : unknown; error?: never; response: Response } | |||
| | { data?: never; error: T extends { responses: any } ? NonNullable<FilterKeys<Error<T["responses"]>, JSONLike>> : unknown; response: Response }; | |||
|
|
|||
| /** Call URLSearchParams() on the object, but remove `undefined` and `null` params */ | |||
There was a problem hiding this comment.
Minor QoL improvement: yes, the library did use new URLSearchParams() as the default. But handling undefined / null params is annoying to have to implement one’s own querySerializer for.
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
|
|
||
| /** Construct URL string from baseUrl and handle path and query params */ | ||
| export function createFinalURL<O>(url: string, options: { baseUrl?: string; params: { query?: Record<string, unknown>; path?: Record<string, unknown> }; querySerializer: QuerySerializer<O> }): string { | ||
| let finalURL = `${options.baseUrl ? options.baseUrl.replace(TRAILING_SLASH_RE, "") : ""}${url as string}`; |
Check failure
Code scanning / CodeQL
Polynomial regular expression used on uncontrolled data
drwpow
commented
Jun 16, 2023
| } | ||
|
|
||
| /** Construct URL string from baseUrl and handle path and query params */ | ||
| export function createFinalURL<O>(url: string, options: { baseUrl?: string; params: { query?: Record<string, unknown>; path?: Record<string, unknown> }; querySerializer: QuerySerializer<O> }): string { |
There was a problem hiding this comment.
createFinalURL() is now exposed for testing (#1161)
drwpow
force-pushed
the
improvements
branch
2 times, most recently
from
da2d8a5 to
9ddc881
Compare
Merged
1 task
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Changes
Fixes:
finalUrl#1161baseUrlandpathmay result in invalid URLs #1150✨ new
parseAsoptionTo address the handling of non-JSON responses (#1123, #1159), you can now parse the response as something other than
.json()parseAstakes any of the following options:jsontextarrayBufferblobstream(this skips parsing, and returns the Response.body as-is)The reason for the manual detection is:
Content-Typeheader isn’t possible because it could be wrong or ambiguous (e.g. it’s not clear fromContent-Typealone when someone may want a.blob()vs.arrayBuffer()).arrayBuffer()in certain instances)How to Review
Tests should pass
Checklist
examples/directory updated (only applicable for openapi-typescript)