Skip to content

Relax default type for response schemas #733

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

Open
kibertoad wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Relax default type for response schemas #733

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
36 changes: 18 additions & 18 deletions packages/app/api-contracts/src/apiContracts.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,9 +65,9 @@ export type PayloadRouteDefinition<
RequestHeaderSchema extends z.Schema | undefined = undefined,
IsNonJSONResponseExpected extends boolean = false,
IsEmptyResponseExpected extends boolean = false,
ResponseSchemasByStatusCode extends
| Partial<Record<HttpStatusCode, z.Schema>>
| undefined = undefined,
ResponseSchemasByStatusCode extends Partial<Record<HttpStatusCode, z.Schema>> | undefined =
| Record<HttpStatusCode, z.Schema>
| undefined,
> = CommonRouteDefinition<
Comment on lines +68 to 71
Copy link

@coderabbitai coderabbitai bot Sep 5, 2025

Choose a reason for hiding this comment

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

⚠️ Potential issue

Default now requires a full mapping; defeats “relax default” goal

Setting the default to Record<HttpStatusCode, z.Schema> | undefined narrows usage when generics are omitted—partials won’t type-check. Default to Partial<...> | undefined; Record<...> remains assignable to Partial<...> anyway.

-  ResponseSchemasByStatusCode extends Partial<Record<HttpStatusCode, z.Schema>> | undefined =
-    | Record<HttpStatusCode, z.Schema>
-    | undefined,
+  ResponseSchemasByStatusCode extends Partial<Record<HttpStatusCode, z.Schema>> | undefined =
+    Partial<Record<HttpStatusCode, z.Schema>> | undefined,
📝 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
ResponseSchemasByStatusCode extends Partial<Record<HttpStatusCode, z.Schema>> | undefined =
| Record<HttpStatusCode, z.Schema>
| undefined,
> = CommonRouteDefinition<
// … other generic parameters …
ResponseSchemasByStatusCode extends Partial<Record<HttpStatusCode, z.Schema>> | undefined =
Partial<Record<HttpStatusCode, z.Schema>> | undefined,
) = CommonRouteDefinition<
// … remaining parameters …
🤖 Prompt for AI Agents 
packages/app/api-contracts/src/apiContracts.ts around lines 68 to 71: the
generic default for ResponseSchemasByStatusCode was changed to
Record<HttpStatusCode, z.Schema> | undefined which forces a full mapping when
generics are omitted; revert the default to Partial<Record<HttpStatusCode,
z.Schema>> | undefined so callers can omit keys (a full Record still satisfies
Partial), i.e. update the generic default back to the Partial form.

SuccessResponseBodySchema,
PathParamsSchema,
Expand All @@ -88,9 +88,9 @@ export type GetRouteDefinition<
RequestHeaderSchema extends z.Schema | undefined = undefined,
IsNonJSONResponseExpected extends boolean = false,
IsEmptyResponseExpected extends boolean = false,
ResponseSchemasByStatusCode extends
| Partial<Record<HttpStatusCode, z.Schema>>
| undefined = undefined,
ResponseSchemasByStatusCode extends Partial<Record<HttpStatusCode, z.Schema>> | undefined =
| Record<HttpStatusCode, z.Schema>
| undefined,
> = CommonRouteDefinition<
Comment on lines +91 to 94
Copy link

@coderabbitai coderabbitai bot Sep 5, 2025

Choose a reason for hiding this comment

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

⚠️ Potential issue

Same issue in GetRouteDefinition default

Defaulting to Record | undefined rejects partial maps by default. Use Partial | undefined.

-  ResponseSchemasByStatusCode extends Partial<Record<HttpStatusCode, z.Schema>> | undefined =
-    | Record<HttpStatusCode, z.Schema>
-    | undefined,
+  ResponseSchemasByStatusCode extends Partial<Record<HttpStatusCode, z.Schema>> | undefined =
+    Partial<Record<HttpStatusCode, z.Schema>> | undefined,
📝 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
ResponseSchemasByStatusCode extends Partial<Record<HttpStatusCode, z.Schema>> | undefined =
| Record<HttpStatusCode, z.Schema>
| undefined,
> = CommonRouteDefinition<
ResponseSchemasByStatusCode extends Partial<Record<HttpStatusCode, z.Schema>> | undefined =
Partial<Record<HttpStatusCode, z.Schema>> | undefined,
🤖 Prompt for AI Agents 
In packages/app/api-contracts/src/apiContracts.ts around lines 91 to 94, the
generic default for ResponseSchemasByStatusCode uses Record<HttpStatusCode,
z.Schema> | undefined which disallows partial maps; change the default to
Partial<Record<HttpStatusCode, z.Schema>> | undefined so callers can omit or
provide partial status-code maps, and apply the same change to the
GetRouteDefinition default mentioned in the comment.

SuccessResponseBodySchema,
PathParamsSchema,
Expand All @@ -110,9 +110,9 @@ export type DeleteRouteDefinition<
RequestHeaderSchema extends z.Schema | undefined = undefined,
IsNonJSONResponseExpected extends boolean = false,
IsEmptyResponseExpected extends boolean = true,
ResponseSchemasByStatusCode extends
| Partial<Record<HttpStatusCode, z.Schema>>
| undefined = undefined,
ResponseSchemasByStatusCode extends Partial<Record<HttpStatusCode, z.Schema>> | undefined =
| Record<HttpStatusCode, z.Schema>
| undefined,
> = CommonRouteDefinition<
Comment on lines +113 to 116
Copy link

@coderabbitai coderabbitai bot Sep 5, 2025

Choose a reason for hiding this comment

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

⚠️ Potential issue

Same issue in DeleteRouteDefinition default

Align default with the relaxed intent so partials work out of the box.

-  ResponseSchemasByStatusCode extends Partial<Record<HttpStatusCode, z.Schema>> | undefined =
-    | Record<HttpStatusCode, z.Schema>
-    | undefined,
+  ResponseSchemasByStatusCode extends Partial<Record<HttpStatusCode, z.Schema>> | undefined =
+    Partial<Record<HttpStatusCode, z.Schema>> | undefined,
📝 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
ResponseSchemasByStatusCode extends Partial<Record<HttpStatusCode, z.Schema>> | undefined =
| Record<HttpStatusCode, z.Schema>
| undefined,
> = CommonRouteDefinition<
ResponseSchemasByStatusCode extends Partial<Record<HttpStatusCode, z.Schema>> | undefined =
Partial<Record<HttpStatusCode, z.Schema>> | undefined,
🤖 Prompt for AI Agents 
In packages/app/api-contracts/src/apiContracts.ts around lines 113 to 116, the
generic default for DeleteRouteDefinition uses a strict Record<HttpStatusCode,
z.Schema> | undefined which prevents partial response schema maps; change the
default generic to Partial<Record<HttpStatusCode, z.Schema>> | undefined so
partial response schema objects are accepted by default and align with the
relaxed intent.

SuccessResponseBodySchema,
PathParamsSchema,
Expand All @@ -133,9 +133,9 @@ export function buildPayloadRoute<
RequestHeaderSchema extends z.Schema | undefined = undefined,
IsNonJSONResponseExpected extends boolean = false,
IsEmptyResponseExpected extends boolean = false,
ResponseSchemasByStatusCode extends
| Partial<Record<HttpStatusCode, z.Schema>>
| undefined = undefined,
ResponseSchemasByStatusCode extends Partial<Record<HttpStatusCode, z.Schema>> | undefined =
| Record<HttpStatusCode, z.Schema>
| undefined,
>(
Comment on lines +136 to 139
Copy link

@coderabbitai coderabbitai bot Sep 5, 2025

Choose a reason for hiding this comment

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

⚠️ Potential issue

Builder: buildPayloadRoute default also too strict

Mirror the fix here so callers can pass partial maps without specifying generics.

-  ResponseSchemasByStatusCode extends Partial<Record<HttpStatusCode, z.Schema>> | undefined =
-    | Record<HttpStatusCode, z.Schema>
-    | undefined,
+  ResponseSchemasByStatusCode extends Partial<Record<HttpStatusCode, z.Schema>> | undefined =
+    Partial<Record<HttpStatusCode, z.Schema>> | undefined,
📝 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
ResponseSchemasByStatusCode extends Partial<Record<HttpStatusCode, z.Schema>> | undefined =
| Record<HttpStatusCode, z.Schema>
| undefined,
>(
ResponseSchemasByStatusCode extends Partial<Record<HttpStatusCode, z.Schema>> | undefined =
Partial<Record<HttpStatusCode, z.Schema>> | undefined,
🤖 Prompt for AI Agents 
In packages/app/api-contracts/src/apiContracts.ts around lines 136 to 139, the
generic default for ResponseSchemasByStatusCode is too strict (requires a full
Record<HttpStatusCode, z.Schema>), preventing callers from passing partial maps
without specifying generics; change the default type to
Partial<Record<HttpStatusCode, z.Schema>> | undefined (i.e., use Partial instead
of Record) so callers can provide partial status-code-to-schema maps implicitly,
and ensure any related usages or overloads accept the updated union type.

params: PayloadRouteDefinition<
RequestBodySchema,
Expand Down Expand Up @@ -183,9 +183,9 @@ export function buildGetRoute<
RequestHeaderSchema extends z.Schema | undefined = undefined,
IsNonJSONResponseExpected extends boolean = false,
IsEmptyResponseExpected extends boolean = false,
ResponseSchemasByStatusCode extends
| Partial<Record<HttpStatusCode, z.Schema>>
| undefined = undefined,
ResponseSchemasByStatusCode extends Partial<Record<HttpStatusCode, z.Schema>> | undefined =
| Record<HttpStatusCode, z.Schema>
| undefined,
>(
Comment on lines +186 to 189
Copy link

@coderabbitai coderabbitai bot Sep 5, 2025

Choose a reason for hiding this comment

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

⚠️ Potential issue

Builder: buildGetRoute default too strict

Default should be Partial | undefined for the same reason.

-  ResponseSchemasByStatusCode extends Partial<Record<HttpStatusCode, z.Schema>> | undefined =
-    | Record<HttpStatusCode, z.Schema>
-    | undefined,
+  ResponseSchemasByStatusCode extends Partial<Record<HttpStatusCode, z.Schema>> | undefined =
+    Partial<Record<HttpStatusCode, z.Schema>> | undefined,
📝 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
ResponseSchemasByStatusCode extends Partial<Record<HttpStatusCode, z.Schema>> | undefined =
| Record<HttpStatusCode, z.Schema>
| undefined,
>(
ResponseSchemasByStatusCode extends Partial<Record<HttpStatusCode, z.Schema>> | undefined =
Partial<Record<HttpStatusCode, z.Schema>> | undefined,
>(
🤖 Prompt for AI Agents 
In packages/app/api-contracts/src/apiContracts.ts around lines 186 to 189, the
generic default for ResponseSchemasByStatusCode in buildGetRoute is too strict
(currently uses Record<HttpStatusCode, z.Schema> | undefined); change the
default to Partial<Record<HttpStatusCode, z.Schema>> | undefined so callers can
omit some status codes; update the generic parameter default accordingly and
ensure any related type usages still accept the looser Partial | undefined form.

params: Omit<
GetRouteDefinition<
Expand Down Expand Up @@ -233,9 +233,9 @@ export function buildDeleteRoute<
RequestHeaderSchema extends z.Schema | undefined = undefined,
IsNonJSONResponseExpected extends boolean = false,
IsEmptyResponseExpected extends boolean = true,
ResponseSchemasByStatusCode extends
| Partial<Record<HttpStatusCode, z.Schema>>
| undefined = undefined,
ResponseSchemasByStatusCode extends Partial<Record<HttpStatusCode, z.Schema>> | undefined =
| Record<HttpStatusCode, z.Schema>
| undefined,
>(
Comment on lines +236 to 239
Copy link

@coderabbitai coderabbitai bot Sep 5, 2025

Choose a reason for hiding this comment

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

⚠️ Potential issue

Builder: buildDeleteRoute default too strict

Apply the same correction here.

-  ResponseSchemasByStatusCode extends Partial<Record<HttpStatusCode, z.Schema>> | undefined =
-    | Record<HttpStatusCode, z.Schema>
-    | undefined,
+  ResponseSchemasByStatusCode extends Partial<Record<HttpStatusCode, z.Schema>> | undefined =
+    Partial<Record<HttpStatusCode, z.Schema>> | undefined,
📝 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
ResponseSchemasByStatusCode extends Partial<Record<HttpStatusCode, z.Schema>> | undefined =
| Record<HttpStatusCode, z.Schema>
| undefined,
>(
ResponseSchemasByStatusCode extends Partial<Record<HttpStatusCode, z.Schema>> | undefined =
Partial<Record<HttpStatusCode, z.Schema>> | undefined,
>(
🤖 Prompt for AI Agents 
In packages/app/api-contracts/src/apiContracts.ts around lines 236 to 239, the
generic parameter ResponseSchemasByStatusCode currently constrains to
Partial<Record<HttpStatusCode, z.Schema>> but its default is
Record<HttpStatusCode, z.Schema> | undefined which is too strict; change the
default to match the constraint (Partial<Record<HttpStatusCode, z.Schema>> |
undefined) so callers won’t be forced to supply every status code.

params: Omit<
DeleteRouteDefinition<
Expand Down
Loading