Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
70 changes: 70 additions & 0 deletions src/core/query-string.t-test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
import { Equal, Expect } from "./type-test";
import {
HasExcessiveQuery,
HasMissingQuery,
IsValidQuery,
NonOptionalKeys,
ToQueryUnion,
} from "./query-string";

// eslint-disable-next-line @typescript-eslint/no-unused-vars
type ToQueryUnionCase = [
Expect<Equal<ToQueryUnion<"a=1">, "a">>,
Expect<Equal<ToQueryUnion<"a=1&b=2">, "a" | "b">>,
Expect<Equal<ToQueryUnion<"a=1&b=2&a=3">, "a" | "b">>,
];

// eslint-disable-next-line @typescript-eslint/no-unused-vars
type HasMissingQueryCase = [
Expect<Equal<HasMissingQuery<{ a: string }, "a">, false>>,
Expect<Equal<HasMissingQuery<{ a?: string }, "a">, false>>,
Expect<Equal<HasMissingQuery<{ a?: string }, "b">, false>>,
Expect<Equal<HasMissingQuery<{ a?: string }, never>, false>>,
Expect<Equal<HasMissingQuery<{ a: string }, "a" | "b">, false>>,
Expect<Equal<HasMissingQuery<{ a?: string }, "a" | "b">, false>>,
Expect<Equal<HasMissingQuery<{ a: string; b?: string }, "a">, false>>,
Expect<Equal<HasMissingQuery<{ a: string }, "b">, true>>,
Expect<Equal<HasMissingQuery<{ a: string; b: string }, "b">, true>>,
Expect<Equal<HasMissingQuery<{ a: string; b?: string }, "b">, true>>,
];

// eslint-disable-next-line @typescript-eslint/no-unused-vars
type HasExcessiveQueryCase = [
Expect<Equal<HasExcessiveQuery<{ a: string }, "a">, false>>,
Expect<Equal<HasExcessiveQuery<{ a: string }, "b">, true>>,
Expect<Equal<HasExcessiveQuery<{ a: string }, "a" | "b">, true>>,
Expect<Equal<HasExcessiveQuery<{ a: string; b: string }, "a" | "b">, false>>,
Expect<Equal<HasExcessiveQuery<{ a: string; b?: string }, "a" | "b">, false>>,
Expect<
Equal<HasExcessiveQuery<{ a: string; b: string }, "a" | "b" | "c">, true>
>,
];

// eslint-disable-next-line @typescript-eslint/no-unused-vars
type NonOptionalKeysCase = [
Expect<Equal<NonOptionalKeys<{ a: string }>, "a">>,
Expect<Equal<NonOptionalKeys<{ a?: string }>, never>>,
Expect<Equal<NonOptionalKeys<{ a: string; b?: string }>, "a">>,
Expect<Equal<NonOptionalKeys<{ a: string; b: string }>, "a" | "b">>,
];

// eslint-disable-next-line @typescript-eslint/no-unused-vars
type IsValidQueryCase = [
Expect<Equal<IsValidQuery<{ a: string }, "a">, true>>,
Expect<Equal<IsValidQuery<{ a: string }, "b">, "E: maybe missing query: a">>,
Expect<
Equal<
IsValidQuery<{ a: string }, "a" | "b">,
"E: maybe excessive query: a" | "E: maybe excessive query: b"
>
>,
Expect<Equal<IsValidQuery<{ a: string; b?: string }, "a">, true>>,
Expect<Equal<IsValidQuery<{ a: string; b?: string }, "a" | "b">, true>>,
Expect<
Equal<
IsValidQuery<{ a: string; b: string }, "a">,
"E: maybe missing query: a" | "E: maybe missing query: b"
>
>,
Expect<Equal<IsValidQuery<{ a: string; b: string }, "a" | "b">, true>>,
];
37 changes: 37 additions & 0 deletions src/core/query-string.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,3 +38,40 @@ export type SetProperty<T, K extends PropertyKey, V = true> = {
? T[P]
: never;
};

export type ExtractQuery<URL extends string> =
URL extends `${string}?${infer Query}` ? Query : undefined;

export type ToQueryUnion<Query extends string> =
Query extends `${infer Key}=${string}&${infer Rest}`
? Key | ToQueryUnion<Rest>
: Query extends `${infer Key}=${string}`
? Key
: `invalid query: ${Query}`;
Comment on lines +45 to +50
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Consider refining the error handling in ToQueryUnion

Currently, if the query string does not match the expected patterns, the ToQueryUnion type returns a string literal of the format invalid query: ${Query}. This may complicate type comparisons and error handling.

Consider returning never or a custom error type to improve type safety and clarity.

Suggested change:

export type ToQueryUnion<Query extends string> =
  Query extends `${infer Key}=${string}&${infer Rest}`
    ? Key | ToQueryUnion<Rest>
  : Query extends `${infer Key}=${string}`
    ? Key
- : `invalid query: ${Query}`;
+ : never;
📝 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
export type ToQueryUnion<Query extends string> =
Query extends `${infer Key}=${string}&${infer Rest}`
? Key | ToQueryUnion<Rest>
: Query extends `${infer Key}=${string}`
? Key
: `invalid query: ${Query}`;
export type ToQueryUnion<Query extends string> =
Query extends `${infer Key}=${string}&${infer Rest}`
? Key | ToQueryUnion<Rest>
: Query extends `${infer Key}=${string}`
? Key
: never;


export type HasMissingQuery<
// eslint-disable-next-line @typescript-eslint/no-explicit-any
QueryDef extends Record<string, any>,
QueryKeys extends string,
> = NonOptionalKeys<QueryDef> extends QueryKeys ? false : true;
Comment on lines +53 to +56
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Avoid using any in type definitions

Using any in type definitions reduces type safety. Consider replacing any with unknown or a more specific type to enhance type checking.

Suggested change:

- QueryDef extends Record<string, any>,
+ QueryDef extends Record<string, unknown>,

Alternatively, define a more precise type for QueryDef if possible.

📝 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
// eslint-disable-next-line @typescript-eslint/no-explicit-any
QueryDef extends Record<string, any>,
QueryKeys extends string,
> = NonOptionalKeys<QueryDef> extends QueryKeys ? false : true;
// eslint-disable-next-line @typescript-eslint/no-explicit-any
QueryDef extends Record<string, unknown>,
QueryKeys extends string,
> = NonOptionalKeys<QueryDef> extends QueryKeys ? false : true;


export type HasExcessiveQuery<
// eslint-disable-next-line @typescript-eslint/no-explicit-any
QueryDef extends Record<string, any>,
QueryKeys extends string,
// no union distribution
Comment on lines +59 to +62
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Avoid using any in type definitions

Using any in type definitions can reduce type safety. Consider replacing any with unknown or a more specific type.

Suggested change:

- QueryDef extends Record<string, any>,
+ QueryDef extends Record<string, unknown>,
📝 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
// eslint-disable-next-line @typescript-eslint/no-explicit-any
QueryDef extends Record<string, any>,
QueryKeys extends string,
// no union distribution
// eslint-disable-next-line @typescript-eslint/no-explicit-any
QueryDef extends Record<string, unknown>,
QueryKeys extends string,
// no union distribution

> = [QueryKeys] extends [keyof QueryDef] ? false : true;

export type NonOptionalKeys<T> = {
[K in keyof T]-?: undefined extends T[K] ? never : K;
}[keyof T];

export type IsValidQuery<
// eslint-disable-next-line @typescript-eslint/no-explicit-any
QueryDef extends Record<string, any>,
QueryKeys extends string,
> = [HasMissingQuery<QueryDef, QueryKeys>] extends [true]
? `E: maybe missing query: ${keyof QueryDef & string}`
: [HasExcessiveQuery<QueryDef, QueryKeys>] extends [true]
? `E: maybe excessive query: ${QueryKeys}`
: true;
Comment on lines +69 to +77
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Enhance error messages in IsValidQuery

The current error messages in IsValidQuery may not clearly indicate which query parameters are missing or excessive.

Consider modifying the implementation to specify the exact keys that are missing or excessive to improve developer experience.

Possible approach:

Implement utility types to compute the specific missing or excessive keys:

type MissingKeys<QueryDef, QueryKeys> = Exclude<NonOptionalKeys<QueryDef>, QueryKeys>;
type ExcessiveKeys<QueryDef, QueryKeys> = Exclude<QueryKeys, keyof QueryDef>;

Then adjust IsValidQuery to use these types in the error messages:

export type IsValidQuery<
  QueryDef extends Record<string, unknown>,
  QueryKeys extends string,
> = MissingKeys<QueryDef, QueryKeys> extends never
  ? ExcessiveKeys<QueryDef, QueryKeys> extends never
    ? true
    : `E: excessive query keys: ${ExcessiveKeys<QueryDef, QueryKeys>}`
  : `E: missing query keys: ${MissingKeys<QueryDef, QueryKeys>}`;

17 changes: 17 additions & 0 deletions src/fetch/index.t-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -220,6 +220,23 @@ const JSONT = JSON as JSONT;
headers: { Cookie: "a=b" },
});
}
{
// @ts-expect-error 定義されているパラメータを指定していない場合はエラー
f(`/api/projects/projectA/workflow/packages/list?a=b`, {
headers: { Cookie: "a=b" },
});
}
{
// @ts-expect-error 定義されていないパラメータを指定した場合は型エラー
f(`/api/projects/projectA/workflow/packages/list?state=true&a=b`, {
headers: { Cookie: "a=b" },
});

// @ts-expect-error 順序は関係ない
f(`/api/projects/projectA/workflow/packages/list?a=b&state=true`, {
headers: { Cookie: "a=b" },
});
}
})();
}

Expand Down
23 changes: 18 additions & 5 deletions src/fetch/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,21 +13,34 @@ import {
StatusCode,
IsAllOptional,
CaseInsensitive,
ExtractQuery,
IsValidQuery,
ToQueryUnion,
} from "../core";
import { UrlPrefixPattern, ToUrlParamPattern } from "../core";
import { TypedString } from "../json";

type IsValidUrl<
// eslint-disable-next-line @typescript-eslint/no-explicit-any
QueryDef extends Record<string, unknown> | undefined,
Url extends string,
Query extends string | undefined = ExtractQuery<Url>,
QueryKeys extends string = Query extends string ? ToQueryUnion<Query> : never,
> = IsValidQuery<
// eslint-disable-next-line @typescript-eslint/no-explicit-any, @typescript-eslint/ban-types
QueryDef extends Record<string, any> ? QueryDef : {},
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Use 'Record<string, never>' instead of '{}' as a type

Using {} as a type can lead to unintended behaviors since it represents any non-nullable value. For an empty object type, Record<string, never> provides better type safety and clarity.

Apply this diff to fix the issue:

-  QueryDef extends Record<string, any> ? QueryDef : {},
+  QueryDef extends Record<string, any> ? QueryDef : Record<string, never>,
📝 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
QueryDef extends Record<string, any> ? QueryDef : {},
QueryDef extends Record<string, any> ? QueryDef : Record<string, never>,
🧰 Tools
🪛 Biome

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

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

(lint/complexity/noBannedTypes)

QueryKeys
>;
Comment on lines +23 to +33
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Improve type safety and remove ESLint disable comments.

The IsValidUrl type is a good addition for enhancing URL validation. However, there are a few improvements to be made:

  1. Remove the ESLint disable comments and address the underlying issues.
  2. Replace {} with a more specific type.

Apply this diff to improve type safety:

 type IsValidUrl<
-  // eslint-disable-next-line @typescript-eslint/no-explicit-any
   QueryDef extends Record<string, unknown> | undefined,
   Url extends string,
   Query extends string | undefined = ExtractQuery<Url>,
   QueryKeys extends string = Query extends string ? ToQueryUnion<Query> : never,
 > = IsValidQuery<
-  // eslint-disable-next-line @typescript-eslint/no-explicit-any, @typescript-eslint/ban-types
-  QueryDef extends Record<string, any> ? QueryDef : {},
+  QueryDef extends Record<string, unknown> ? QueryDef : Record<string, never>,
   QueryKeys
 >;

This change removes the need for ESLint disable comments and improves type safety by using Record<string, never> instead of {} for an empty object type.

📝 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
type IsValidUrl<
// eslint-disable-next-line @typescript-eslint/no-explicit-any
QueryDef extends Record<string, unknown> | undefined,
Url extends string,
Query extends string | undefined = ExtractQuery<Url>,
QueryKeys extends string = Query extends string ? ToQueryUnion<Query> : never,
> = IsValidQuery<
// eslint-disable-next-line @typescript-eslint/no-explicit-any, @typescript-eslint/ban-types
QueryDef extends Record<string, any> ? QueryDef : {},
QueryKeys
>;
type IsValidUrl<
QueryDef extends Record<string, unknown> | undefined,
Url extends string,
Query extends string | undefined = ExtractQuery<Url>,
QueryKeys extends string = Query extends string ? ToQueryUnion<Query> : never,
> = IsValidQuery<
QueryDef extends Record<string, unknown> ? QueryDef : Record<string, never>,
QueryKeys
>;
🧰 Tools
🪛 Biome

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

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

(lint/complexity/noBannedTypes)


export type RequestInitT<
InputMethod extends CaseInsensitiveMethod,
// eslint-disable-next-line @typescript-eslint/no-explicit-any
Body extends Record<string, any> | string | undefined,
Body extends Record<string, unknown> | string | undefined,
HeadersObj extends string | Record<string, string> | undefined,
> = Omit<RequestInit, "method" | "body" | "headers"> &
(InputMethod extends "get" | "GET"
? { method?: InputMethod }
: { method: InputMethod }) &
// eslint-disable-next-line @typescript-eslint/no-explicit-any
(Body extends Record<string, any>
(Body extends Record<string, unknown>
? IsAllOptional<Body> extends true
? { body?: Body | TypedString<Body> }
: { body: TypedString<Body> }
Expand Down Expand Up @@ -86,7 +99,7 @@ type FetchT<UrlPrefix extends UrlPrefixPattern, E extends ApiEndpoints> = <
? MergeApiResponseBodies<ApiP<E, CandidatePaths, M, "responses">>
: Record<StatusCode, never>,
>(
input: Input,
input: IsValidUrl<Query, Input> extends true ? Input : never,
init: RequestInitT<
InputMethod,
ApiP<E, CandidatePaths, M, "body">,
Expand Down