Fix empty object required 2nd param #1366
Conversation
🦋 Changeset detectedLatest commit: 5310b1b The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 packages
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: |
5310b1b
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://99f038b7.openapi-ts.pages.dev |
| Branch Preview URL: | https://fix-optional-param.openapi-ts.pages.dev |
| @@ -48,7 +48,7 @@ describe("client", () => { | |||
|
|
|||
| // data | |||
| mockFetchOnce({ status: 200, body: JSON.stringify(["one", "two", "three"]) }); | |||
| const dataRes = await client.GET("/string-array", {}); | |||
| const dataRes = await client.GET("/string-array"); | |||
There was a problem hiding this comment.
Note: the removal of all , {}) in the test suite is a good sign. The test suite has quite a few // @ts-expect-error comments which are vital to testing the type assertions.
But in addition, the test suite also has runtime tests to ensure the function is behaving as-expected. The fact that no tests were modified, while still passing, is a great sign.
drwpow
force-pushed
the
fix-optional-param
branch
from
4223436 to
5310b1b
Compare
| async GET<P extends PathsWithMethod<Paths, "get">>(url: P, init: FetchOptions<FilterKeys<Paths[P], "get">>) { | ||
| return coreFetch<P, "get">(url, { ...init, method: "GET" } as any); | ||
| async GET<P extends GetPaths>(url: P, ...init: HasRequiredKeys<GetFetchOptions<P>> extends never ? [GetFetchOptions<P>?] : [GetFetchOptions<P>]) { | ||
| return coreFetch<P, "get">(url, { ...init[0], method: "GET" } as any); |
There was a problem hiding this comment.
The fix was weird, and not straightforward. Essentially, it involved multiple complicated steps:
- First, convert the function parameters into an array (
...init) (though for simplicity, we can do this starting at the 2nd param, rather than having to do it for the full spread) - Next, we’ll need to take each method’s individual FetchOptions and introspect whether it has a single required key. (we needed to create a helper for this)
- Lastly, expand the parameter array into a tuple type, marking it either optional (
[T?]) or not ([T]) based on the result of the previous step
There was a problem hiding this comment.
Again, all of this is very annoying, very strange TypeScript introspection. But that’s the name of the game here, so it’s all worth it… I guess.
| @@ -145,8 +166,8 @@ export function defaultBodySerializer<T>(body: T): string { | |||
| } | |||
|
|
|||
| /** 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}`; | |||
There was a problem hiding this comment.
Small micro-optimization: we don’t need to be running a dynamic RegEx to remove a trailing slash. It’s much more performant to just remove a single character if needed (done above on createClient(), also to save cycles on running a RegEx every request
Changes
Fixes the elusive/annoying #1127. Now, rather than
.GET('/url', {})being required, you can just write.GET('/url'). Minor QoL improvement, but very important fix!How to Review
Tests should pass.
Checklist
docs/updated (if necessary)pnpm run update:examplesrun (only applicable for openapi-typescript)