Skip to content
This repository was archived by the owner on Mar 1, 2026. It is now read-only.

Commit 354157b

Browse files
mwillbanksmwillbanks
committed
fix: coderabbit review comments
1 parent e8c726e commit 354157b

5 files changed

Lines changed: 100 additions & 68 deletions

File tree

‎packages/clients/tanstack-query/src/svelte/index.svelte.ts‎

Lines changed: 36 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -95,6 +95,14 @@ function useQuerySettings() {
9595
return { endpoint: endpoint ?? DEFAULT_QUERY_ENDPOINT, ...rest };
9696
}
9797

98+
function merge(rootOpt: unknown, opt: unknown): Accessor<any> {
99+
return () => {
100+
const rootOptVal = typeof rootOpt === 'function' ? (rootOpt as any)() : rootOpt;
101+
const optVal = typeof opt === 'function' ? (opt as any)() : opt;
102+
return { ...rootOptVal, ...optVal };
103+
};
104+
}
105+
98106
export type ModelQueryOptions<T> = Omit<CreateQueryOptions<T, DefaultError>, 'queryKey'> & ExtraQueryOptions;
99107

100108
export type ModelQueryResult<T> = CreateQueryResult<WithOptimistic<T>, DefaultError> & { queryKey: QueryKey };
@@ -133,37 +141,37 @@ export type ClientHooks<Schema extends SchemaDef, Options extends QueryOptions<S
133141

134142
type ProcedureHookGroup<Schema extends SchemaDef> = {
135143
[Name in keyof ExtractProcedures<Schema>]: ExtractProcedures<Schema>[Name] extends { mutation: true }
136-
? {
137-
useMutation(
138-
options?: Omit<
139-
CreateMutationOptions<ProcedureReturn<Schema, Name>, DefaultError, ProcedurePayload<Schema, Name>>,
140-
'mutationFn'
141-
> &
142-
QueryContext,
143-
): CreateMutationResult<ProcedureReturn<Schema, Name>, DefaultError, ProcedurePayload<Schema, Name>>;
144-
}
145-
: {
146-
useQuery: ProcedureHookFn<
147-
ProcedurePayload<Schema, Name>,
148-
Omit<ModelQueryOptions<ProcedureReturn<Schema, Name>>, 'optimisticUpdate'>,
149-
CreateQueryResult<ProcedureReturn<Schema, Name>, DefaultError> & { queryKey: QueryKey }
150-
>;
151-
152-
useInfiniteQuery: ProcedureHookFn<
153-
ProcedurePayload<Schema, Name>,
154-
ModelInfiniteQueryOptions<ProcedureReturn<Schema, Name>>,
155-
ModelInfiniteQueryResult<InfiniteData<ProcedureReturn<Schema, Name>>>
156-
>;
157-
};
144+
? {
145+
useMutation(
146+
options?: Omit<
147+
CreateMutationOptions<ProcedureReturn<Schema, Name>, DefaultError, ProcedurePayload<Schema, Name>>,
148+
'mutationFn'
149+
> &
150+
QueryContext,
151+
): CreateMutationResult<ProcedureReturn<Schema, Name>, DefaultError, ProcedurePayload<Schema, Name>>;
152+
}
153+
: {
154+
useQuery: ProcedureHookFn<
155+
ProcedurePayload<Schema, Name>,
156+
Omit<ModelQueryOptions<ProcedureReturn<Schema, Name>>, 'optimisticUpdate'>,
157+
CreateQueryResult<ProcedureReturn<Schema, Name>, DefaultError> & { queryKey: QueryKey }
158+
>;
159+
160+
useInfiniteQuery: ProcedureHookFn<
161+
ProcedurePayload<Schema, Name>,
162+
ModelInfiniteQueryOptions<ProcedureReturn<Schema, Name>>,
163+
ModelInfiniteQueryResult<InfiniteData<ProcedureReturn<Schema, Name>>>
164+
>;
165+
};
158166
};
159167

160168
export type ProcedureHooks<Schema extends SchemaDef> = Schema extends { procedures: Record<string, any> }
161169
? {
162-
/**
163-
* Preferred procedures API.
164-
*/
165-
$procs: ProcedureHookGroup<Schema>;
166-
}
170+
/**
171+
* Preferred procedures API.
172+
*/
173+
$procs: ProcedureHookGroup<Schema>;
174+
}
167175
: {};
168176

169177
// Note that we can potentially use TypeScript's mapped type to directly map from ORM contract, but that seems
@@ -254,14 +262,6 @@ export function useClientQueries<Schema extends SchemaDef, Options extends Query
254262
schema: Schema,
255263
options?: Accessor<QueryContext>,
256264
): ClientHooks<Schema, Options> {
257-
const merge = (rootOpt: unknown, opt: unknown): Accessor<any> => {
258-
return () => {
259-
const rootOptVal = typeof rootOpt === 'function' ? rootOpt() : rootOpt;
260-
const optVal = typeof opt === 'function' ? opt() : opt;
261-
return { ...rootOptVal, ...optVal };
262-
};
263-
};
264-
265265
const result = Object.keys(schema.models).reduce(
266266
(acc, model) => {
267267
(acc as any)[lowerCaseFirst(model)] = useModelQueries<Schema, GetModels<Schema>, Options>(
@@ -317,14 +317,6 @@ export function useModelQueries<
317317

318318
const modelName = modelDef.name;
319319

320-
const merge = (rootOpt: unknown, opt: unknown): Accessor<any> => {
321-
return () => {
322-
const rootOptVal = typeof rootOpt === 'function' ? rootOpt() : rootOpt;
323-
const optVal = typeof opt === 'function' ? opt() : opt;
324-
return { ...rootOptVal, ...optVal };
325-
};
326-
};
327-
328320
return {
329321
useFindUnique: (args: any, options?: any) => {
330322
return useInternalQuery(schema, modelName, 'findUnique', args, merge(rootOptions, options));
@@ -434,7 +426,7 @@ export function useInternalInfiniteQuery<TQueryFnData, TData>(
434426
CreateInfiniteQueryOptions<TQueryFnData, DefaultError, InfiniteData<TData>>,
435427
'queryKey' | 'initialPageParam'
436428
> &
437-
QueryContext
429+
QueryContext
438430
>,
439431
) {
440432
const { endpoint, fetch } = useFetchOptions(options);

‎packages/server/src/api/common/procedures.ts‎

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,53 @@ export function getProcedureDef(schema: SchemaDef, proc: string): ProcedureDef |
1111
return procs[proc];
1212
}
1313

14+
/**
15+
* Maps and validates the incoming procedure payload for server-side routing.
16+
*
17+
* Supported payload formats:
18+
* - **Envelope (preferred)**: `{ args: { ... } }`
19+
* - **Direct object**: `{ ... }` (allowed only when *every* parameter is optional)
20+
*
21+
* The function returns the original `payload` unchanged; it only enforces payload
22+
* *shape* and argument presence/keys so downstream code can safely assume a
23+
* consistent contract.
24+
*
25+
* Validation / branching behavior (mirrors the code below):
26+
* - **Zero-parameter procedures** (`params.length === 0`)
27+
* - `undefined` payload is accepted.
28+
* - Object payloads without an `args` key are treated as “no args” and accepted.
29+
* - Envelope payloads with `args: {}` are accepted.
30+
* - Any other payload (including `args` with keys) is rejected.
31+
* - **All-optional parameter procedures**
32+
* - Payload may be omitted (`undefined`).
33+
* - If payload is an object and has no `args` key, it is treated as the direct
34+
* object form.
35+
* - **Missing payload** (required parameters exist)
36+
* - `undefined` is rejected.
37+
* - **Non-object or array payload**
38+
* - Rejected.
39+
* - **Undefined/invalid `args` (envelope form)**
40+
* - If `args` is missing and not all params are optional: rejected.
41+
* - If `args` exists but is not a non-array object: rejected.
42+
* - **Unknown keys**
43+
* - Any key in the `args` object that is not declared by the procedure is
44+
* rejected (prevents silently ignoring typos).
45+
* - **Missing required params**
46+
* - Any declared non-optional param missing from `args` is rejected.
47+
*
48+
* Rationale for rejecting null/falsey payloads:
49+
* - The checks `!payload` and `!argsPayload` intentionally reject values like
50+
* `null`, `false`, `0`, or `''` instead of treating them as “no args”. This
51+
* keeps the API strictly object-based and yields deterministic, descriptive
52+
* errors rather than surprising coercion.
53+
*
54+
* @throws {Error} "procedure does not accept arguments"
55+
* @throws {Error} "missing procedure arguments"
56+
* @throws {Error} "procedure payload must be an object"
57+
* @throws {Error} "procedure `args` must be an object"
58+
* @throws {Error} "unknown procedure argument: <key>"
59+
* @throws {Error} "missing procedure argument: <name>"
60+
*/
1461
export function mapProcedureArgs(
1562
procDef: { params: ReadonlyArray<{ name: string; optional?: boolean; array?: boolean }> },
1663
payload: unknown,

‎packages/server/src/api/common/utils.ts‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4,17 +4,17 @@ import SuperJSON from 'superjson';
44
* Supports the SuperJSON request payload format used by api handlers
55
* `{ meta: { serialization }, ...json }`.
66
*/
7-
export async function processSuperJsonRequestPayload(payload: unknown) {
7+
export async function processSuperJsonRequestPayload(payload: unknown) : Promise<{ result: unknown; error: string | undefined; }> {
88
if (!payload || typeof payload !== 'object' || Array.isArray(payload) || !('meta' in (payload as any))) {
9-
return { result: payload, error: undefined as string | undefined };
9+
return { result: payload, error: undefined };
1010
}
1111

1212
const { meta, ...rest } = payload as any;
1313
if (meta?.serialization) {
1414
try {
1515
return {
1616
result: SuperJSON.deserialize({ json: rest, meta: meta.serialization }),
17-
error: undefined as string | undefined,
17+
error: undefined,
1818
};
1919
} catch (err) {
2020
return {
@@ -25,7 +25,7 @@ export async function processSuperJsonRequestPayload(payload: unknown) {
2525
}
2626

2727
// drop meta when no serialization info is present
28-
return { result: rest, error: undefined as string | undefined };
28+
return { result: rest, error: undefined };
2929
}
3030

3131
/**

‎packages/server/src/api/rest/index.ts‎

Lines changed: 10 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ import type { ApiHandler, LogConfig, RequestContext, Response } from '../../type
1111
import { getZodErrorMessage, log, registerCustomSerializers } from '../utils';
1212
import {
1313
getProcedureDef,
14-
mapProcedureArgs as mapProcedureArgsCommon,
14+
mapProcedureArgs,
1515
} from '../common/procedures';
1616
import {
1717
processSuperJsonRequestPayload,
@@ -528,7 +528,7 @@ export class RestApiHandler<Schema extends SchemaDef = SchemaDef> implements Api
528528

529529
let procInput: unknown;
530530
try {
531-
procInput = this.mapProcedureArgs(procDef, processedArgsPayload);
531+
procInput = mapProcedureArgs(procDef, processedArgsPayload);
532532
} catch (err) {
533533
return this.makeProcBadInputErrorResponse(err instanceof Error ? err.message : 'invalid procedure arguments');
534534
}
@@ -555,13 +555,6 @@ export class RestApiHandler<Schema extends SchemaDef = SchemaDef> implements Api
555555
}
556556
}
557557

558-
private mapProcedureArgs(
559-
procDef: { params: ReadonlyArray<{ name: string; optional?: boolean; array?: boolean }> },
560-
payload: unknown,
561-
): unknown {
562-
return mapProcedureArgsCommon(procDef, payload);
563-
}
564-
565558
private makeProcBadInputErrorResponse(message: string): Response {
566559
const resp = this.makeError('invalidPayload', message, 400);
567560
log(this.log, 'debug', () => `sending error response: ${JSON.stringify(resp)}`);
@@ -933,16 +926,16 @@ export class RestApiHandler<Schema extends SchemaDef = SchemaDef> implements Api
933926
prev:
934927
offset - limit >= 0 && offset - limit <= total - 1
935928
? this.replaceURLSearchParams(baseUrl, {
936-
'page[offset]': offset - limit,
937-
'page[limit]': limit,
938-
})
929+
'page[offset]': offset - limit,
930+
'page[limit]': limit,
931+
})
939932
: null,
940933
next:
941934
offset + limit <= total - 1
942935
? this.replaceURLSearchParams(baseUrl, {
943-
'page[offset]': offset + limit,
944-
'page[limit]': limit,
945-
})
936+
'page[offset]': offset + limit,
937+
'page[limit]': limit,
938+
})
946939
: null,
947940
}));
948941
}
@@ -2008,8 +2001,8 @@ export class RestApiHandler<Schema extends SchemaDef = SchemaDef> implements Api
20082001
} else {
20092002
currPayload[relation] = select
20102003
? {
2011-
select: { ...select },
2012-
}
2004+
select: { ...select },
2005+
}
20132006
: true;
20142007
}
20152008
}

‎packages/server/src/api/rpc/index.ts‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ export type RPCApiHandlerOptions<Schema extends SchemaDef = SchemaDef> = {
3535
* RPC style API request handler that mirrors the ZenStackClient API
3636
*/
3737
export class RPCApiHandler<Schema extends SchemaDef = SchemaDef> implements ApiHandler<Schema> {
38-
constructor(private readonly options: RPCApiHandlerOptions<Schema>) {}
38+
constructor(private readonly options: RPCApiHandlerOptions<Schema>) { }
3939

4040
get schema(): Schema {
4141
return this.options.schema;
@@ -154,7 +154,7 @@ export class RPCApiHandler<Schema extends SchemaDef = SchemaDef> implements ApiH
154154

155155
const clientResult = await (client as any)[model][op](processedArgs);
156156
let responseBody: any = { data: clientResult };
157-
157+
158158
// superjson serialize response
159159
if (clientResult) {
160160
const { json, meta } = SuperJSON.serialize(clientResult);
@@ -318,7 +318,7 @@ export class RPCApiHandler<Schema extends SchemaDef = SchemaDef> implements ApiH
318318
status = 400;
319319
error.dbErrorCode = err.dbErrorCode;
320320
})
321-
.otherwise(() => {});
321+
.otherwise(() => { });
322322

323323
const resp = { status, body: { error } };
324324
log(this.options.log, 'debug', () => `sending error response: ${safeJSONStringify(resp)}`);

0 commit comments

Comments
 (0)