-
Notifications
You must be signed in to change notification settings - Fork 1.8k
feat: zod v4 #869
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.
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
feat: zod v4 #869
Changes from 17 commits
6db0e68
08513d3
3b28aac
1cc8c7b
9b178b7
9298723
aca892b
e2d351c
2010203
8531d8e
1461fa2
9cf8b2e
5a761b5
30fb023
3cd0546
5a64ed1
ab26df8
aa5e35b
374a673
79f6c04
a55f3aa
467c0c2
3440675
e8e91ed
b6b7d96
5813fc3
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2,7 +2,7 @@ | |
| /* eslint-disable no-constant-binary-expression */ | ||
| /* eslint-disable @typescript-eslint/no-unused-expressions */ | ||
| import { Client } from "./index.js"; | ||
| import { z } from "zod"; | ||
| import { z } from "zod/v4"; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I've found conflicting guidance on this import. https://zod.dev/library-authors?id=how-to-support-zod-4 suggests this import should never be used - it should be On the other hand I'm not sure this repo counts as a "Library" as intended by these docs; currently Ultimately because we have a server-client model in MCP and the two sides communicate via Maybe that's something @colinhacks could help answer or suggest how we gain confidence on this? In general JSON generated by v3 should normally be parseable by v4 and vice versa I assume, I'm not sure what if anything would break over the wire. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. FYI, I've run some tests on this as I was trying to migrate a big project that uses Not sure of the impact of peerDependencies vs devDependencies in the context of this project, but not specifying the import (zod/v3, zod/v4 or zod/v4/core) seems to be able to have some impacts in some contexts.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks @mmorainville-fountain that's great input - in that case I don't see a downside to using the After writing this review + speaking with @ochafik offline I also realized there's a bit more to worry about than just JSONRPC transport layer between clients and server. The quick start shows how SDK users would define their input and output schema in terms of I'll see if I can figure out a way to do some testing to find out - given how highly requested this is I wouldn't want this to depend on v2 ideally, so we'd need something fully backwards compatible. Maybe moving to a peerDependency is the right move.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Just spotted this comment from @dclark27 #869 (comment) exploring this with a potential regression test, will take a look at this. |
||
| import { | ||
| RequestSchema, | ||
| NotificationSchema, | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,13 +1,4 @@ | ||
| import { | ||
| ZodTypeAny, | ||
| ZodTypeDef, | ||
| ZodType, | ||
| ParseInput, | ||
| ParseReturnType, | ||
| RawCreateParams, | ||
| ZodErrorMap, | ||
| ProcessedCreateParams, | ||
| } from "zod"; | ||
| import { ZodTypeAny } from "zod/v4"; | ||
|
|
||
| export enum McpZodTypeKind { | ||
| Completable = "McpCompletable", | ||
|
|
@@ -17,82 +8,38 @@ export type CompleteCallback<T extends ZodTypeAny = ZodTypeAny> = ( | |
| value: T["_input"], | ||
| context?: { | ||
| arguments?: Record<string, string>; | ||
| }, | ||
| } | ||
| ) => T["_input"][] | Promise<T["_input"][]>; | ||
|
|
||
| export interface CompletableDef<T extends ZodTypeAny = ZodTypeAny> | ||
| extends ZodTypeDef { | ||
| export interface CompletableDef<T extends ZodTypeAny = ZodTypeAny> { | ||
| type: T; | ||
| complete: CompleteCallback<T>; | ||
| typeName: McpZodTypeKind.Completable; | ||
| } | ||
|
|
||
| export class Completable<T extends ZodTypeAny> extends ZodType< | ||
| T["_output"], | ||
| CompletableDef<T>, | ||
| T["_input"] | ||
| > { | ||
| _parse(input: ParseInput): ParseReturnType<this["_output"]> { | ||
| const { ctx } = this._processInputParams(input); | ||
| const data = ctx.data; | ||
| return this._def.type._parse({ | ||
| data, | ||
| path: ctx.path, | ||
| parent: ctx, | ||
| }); | ||
| } | ||
|
|
||
| unwrap() { | ||
| return this._def.type; | ||
| } | ||
|
|
||
| static create = <T extends ZodTypeAny>( | ||
| type: T, | ||
| params: RawCreateParams & { | ||
| complete: CompleteCallback<T>; | ||
| }, | ||
| ): Completable<T> => { | ||
| return new Completable({ | ||
| type, | ||
| typeName: McpZodTypeKind.Completable, | ||
| complete: params.complete, | ||
| ...processCreateParams(params), | ||
| }); | ||
| }; | ||
| } | ||
|
|
||
| /** | ||
| * Wraps a Zod type to provide autocompletion capabilities. Useful for, e.g., prompt arguments in MCP. | ||
| */ | ||
| export function completable<T extends ZodTypeAny>( | ||
| schema: T, | ||
| complete: CompleteCallback<T>, | ||
| ): Completable<T> { | ||
| return Completable.create(schema, { ...schema._def, complete }); | ||
| } | ||
|
|
||
| // Not sure why this isn't exported from Zod: | ||
| // https://github.com/colinhacks/zod/blob/f7ad26147ba291cb3fb257545972a8e00e767470/src/types.ts#L130 | ||
| function processCreateParams(params: RawCreateParams): ProcessedCreateParams { | ||
| if (!params) return {}; | ||
| const { errorMap, invalid_type_error, required_error, description } = params; | ||
| if (errorMap && (invalid_type_error || required_error)) { | ||
| throw new Error( | ||
| `Can't use "invalid_type_error" or "required_error" in conjunction with custom error map.`, | ||
| ); | ||
| complete: CompleteCallback<T> | ||
| ): T & { | ||
| _def: (T extends { _def: infer D } ? D : unknown) & CompletableDef<T>; | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. first of all I want to claim that I am not a part of the mcp organization so I hope I am not a blocking phase for this feature. I really am anticipating for the SDK to support zod v4 and I want to thank you for your time and effort! I took a little more time to further learn about zod and the SDK so I can provide some meaningful insights if possible according to https://zod.dev/library-authors?id=how-to-support-zod-3-and-zod-4-simultaneously you can support both z3 and z4 which can make this feature (in case that the maintainers of this repo are willing) a non-breaking change so it can be part of sdk v1 rather than v2. I suggest asking the maintainers before re-implementing the whole PR of course! besides backward compatibility, it looks like one thing that I am not sure about is your choice to There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I also am not sure about the |
||
| } { | ||
| const target = schema as unknown as { _def?: Record<string, unknown> }; | ||
| const originalDef = (target._def ?? {}) as Record<string, unknown>; | ||
| // Only mutate the existing _def object to respect read-only property semantics | ||
| if ( | ||
| (originalDef as { typeName?: unknown }).typeName !== | ||
| McpZodTypeKind.Completable | ||
| ) { | ||
| (originalDef as { typeName?: McpZodTypeKind; type?: ZodTypeAny }).typeName = | ||
| McpZodTypeKind.Completable; | ||
| (originalDef as { typeName?: McpZodTypeKind; type?: ZodTypeAny }).type = | ||
| schema; | ||
| } | ||
| if (errorMap) return { errorMap: errorMap, description }; | ||
| const customMap: ZodErrorMap = (iss, ctx) => { | ||
| const { message } = params; | ||
|
|
||
| if (iss.code === "invalid_enum_value") { | ||
| return { message: message ?? ctx.defaultError }; | ||
| } | ||
| if (typeof ctx.data === "undefined") { | ||
| return { message: message ?? required_error ?? ctx.defaultError }; | ||
| } | ||
| if (iss.code !== "invalid_type") return { message: ctx.defaultError }; | ||
| return { message: message ?? invalid_type_error ?? ctx.defaultError }; | ||
| (originalDef as { complete?: CompleteCallback<T> }).complete = complete; | ||
| return schema as unknown as T & { | ||
| _def: (T extends { _def: infer D } ? D : unknown) & CompletableDef<T>; | ||
| }; | ||
| return { errorMap: customMap, description }; | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need this? This seems like an unintentional change.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I left this in as I was seeing issues running the auth test suite. Adding this prevent the mocks from duplicating and was able to get my tests back running again. We can remove if perhaps it's just my local setup.