Allow to omit headers and body if they are all optional #100
Conversation
WalkthroughThe pull request introduces a new type utility, Changes
Possibly related PRs
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (4)
- src/common/type.t-test.ts (2 hunks)
- src/common/type.ts (1 hunks)
- src/fetch/index.t-test.ts (1 hunks)
- src/fetch/index.ts (2 hunks)
Additional context used
Biome
src/common/type.ts
[error] 147-147: Don't use '{}' as a type.
Prefer explicitly define the object shape. '{}' means "any non-nullable value".
(lint/complexity/noBannedTypes)
src/common/type.t-test.ts
[error] 116-116: Don't use '{}' as a type.
Prefer explicitly define the object shape. '{}' means "any non-nullable value".
(lint/complexity/noBannedTypes)
Additional comments not posted (6)
src/fetch/index.ts (2)
30-34: LGTM!The change to the
bodyproperty ofRequestInitTis logically correct and enhances flexibility by allowing it to be optional when all properties ofBodyare optional. This aligns with the PR objective of allowing to omitbodyif it is optional.
36-38: LGTM!The change to the
headersproperty ofRequestInitTis logically correct and enhances flexibility by allowing it to be optional when all properties ofHeadersObjare optional. This aligns with the PR objective of allowing to omitheadersif it is optional.src/common/type.ts (1)
145-147: LGTM!The
IsAllOptional<T>type is a useful addition to the type utilities. It correctly checks if all properties of a given typeTare optional using a conditional type.The static analysis hint about not using
{}as a type can be safely ignored in this case because{}is intentionally used to check if all properties are optional.Tools
Biome
[error] 147-147: Don't use '{}' as a type.
Prefer explicitly define the object shape. '{}' means "any non-nullable value".
(lint/complexity/noBannedTypes)
src/common/type.t-test.ts (2)
6-6: LGTM!The import statement for
IsAllOptionalis syntactically correct. It is likely used in the new code segment.
113-121: Comprehensive test cases forIsAllOptional.The test cases cover various scenarios and ensure that the
IsAllOptionaltype utility is working as expected. Great job on writing thorough tests!Tools
Biome
[error] 116-116: Don't use '{}' as a type.
Prefer explicitly define the object shape. '{}' means "any non-nullable value".
(lint/complexity/noBannedTypes)
src/fetch/index.t-test.ts (1)
146-161: LGTM!The changes introduce a new specification for the
/usersendpoint with a POST method, allowing for optional headers and body. The async function demonstrates the correct usage of the fetch function with this new specification, omitting the headers and body as they are optional.The changes enhance the API's functionality and provide flexibility in the usage of the endpoint.
| // eslint-disable-next-line @typescript-eslint/no-unused-vars | ||
| type IsAllOptionalTestCases = [ | ||
| // eslint-disable-next-line @typescript-eslint/ban-types | ||
| Expect<Equal<IsAllOptional<{}>, true>>, |
There was a problem hiding this comment.
Avoid using {} as a type.
The static analysis tool suggests not using {} as a type and explicitly defining the object shape instead. Using {} as a type could lead to unexpected behavior and make the code harder to understand and maintain.
Consider replacing {} with a more explicit type, such as Record<string, never>, to represent an empty object type.
Tools
Biome
[error] 116-116: Don't use '{}' as a type.
Prefer explicitly define the object shape. '{}' means "any non-nullable value".
(lint/complexity/noBannedTypes)
Summary by CodeRabbit
New Features
IsAllOptional, to evaluate if all properties of an object type are optional.Bug Fixes
Documentation