Skip to content

Accepts string to body #109

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.

Sign up for GitHub

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

Merged
merged 2 commits into from
Sep 24, 2024
Merged

Accepts string to body #109

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
7925633
Accepts string to body
mpppk Sep 23, 2024
ade2e7a
Fix
mpppk Sep 24, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 3 additions & 4 deletions src/common/spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,10 +39,8 @@ export type AnyApiEndpoints = { [Path in string]: AnyApiEndpoint };

export interface BaseApiSpec<
Params,
// eslint-disable-next-line @typescript-eslint/no-explicit-any
Query,
Body,
// eslint-disable-next-line @typescript-eslint/no-explicit-any
RequestHeaders,
Responses extends AnyApiResponses,
> {
Expand All @@ -60,7 +58,8 @@ export type ApiSpec<
>,
// eslint-disable-next-line @typescript-eslint/no-explicit-any
Query extends Record<string, string> = Record<string, any>,
Body extends object = object,
// eslint-disable-next-line @typescript-eslint/no-explicit-any
Body extends Record<string, any> | string = Record<string, any> | string,
RequestHeaders extends Record<string, string> = Record<string, string>,
Responses extends AnyApiResponses = AnyApiResponses,
> = BaseApiSpec<Params, Query, Body, RequestHeaders, Responses>;
Expand Down Expand Up @@ -90,7 +89,7 @@ export type ApiP<
> = E[Path] extends ApiEndpoint
? E[Path][M] extends ApiSpec<ParseUrlParams<Path>>
? // eslint-disable-next-line @typescript-eslint/no-explicit-any
E[Path][M][P] extends Record<string, any>
E[Path][M][P] extends Record<string, any> | string
? E[Path][M][P]
: undefined
: undefined
Expand Down
15 changes: 15 additions & 0 deletions src/fetch/index.t-test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -173,6 +173,21 @@ const JSONT = JSON as JSONT;
})();
}

{
type Spec = DefineApiEndpoints<{
"/users": {
post: {
body: string;
responses: { 200: { body: string } };
};
};
}>;
(async () => {
const f = fetch as FetchT<"", Spec>;
await f(`/users`, { method: "post", body: "some string" });
})();
}

{
type Spec = DefineApiEndpoints<{
"/packages/list": {
Expand Down
10 changes: 6 additions & 4 deletions src/fetch/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,8 @@ import { TypedString } from "../json";
export type RequestInitT<
InputMethod extends CaseInsensitiveMethod,
// eslint-disable-next-line @typescript-eslint/no-explicit-any
Body extends Record<string, any> | undefined,
HeadersObj extends Record<string, string> | undefined,
Body extends Record<string, any> | string | undefined,
HeadersObj extends string | Record<string, string> | undefined,
> = Omit<RequestInit, "method" | "body" | "headers"> &
(InputMethod extends "get" | "GET"
? { method?: InputMethod }
Expand All @@ -31,8 +31,10 @@ export type RequestInitT<
? IsAllOptional<Body> extends true
? { body?: Body | TypedString<Body> }
: { body: TypedString<Body> }
: // eslint-disable-next-line @typescript-eslint/ban-types
{}) &
: Body extends string
? { body: string }
: // eslint-disable-next-line @typescript-eslint/ban-types
{}) &
Comment on lines +34 to +37
Copy link

@coderabbitai coderabbitai bot Sep 24, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approve new condition, but address remaining type issue

The new condition for handling string body types is a good addition and aligns well with the changes made to the Body type parameter. However, the use of {} as a type is still present and should be addressed.

Please apply the following change to address the remaining type issue:

    : Body extends string
      ? { body: string }
    : // eslint-disable-next-line @typescript-eslint/ban-types
-     {})
+     Record<string, never>)

This change replaces {} with Record<string, never>, providing a more precise empty object type and complying with linter rules.

Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
: Body extends string
? { body: string }
: // eslint-disable-next-line @typescript-eslint/ban-types
{}) &
: Body extends string
? { body: string }
: // eslint-disable-next-line @typescript-eslint/ban-types
Record<string, never>) &
Tools
Biome

[error] 37-37: Don't use '{}' as a type.

Prefer explicitly define the object shape. '{}' means "any non-nullable value".

(lint/complexity/noBannedTypes)

(HeadersObj extends Record<string, string>
? IsAllOptional<HeadersObj> extends true
? { headers?: HeadersObj | Headers }
Expand Down