[RFC] refactor: remove vestigial generic type parameters from Protocol, Server, and Client #1451
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@modelcontextprotocol/client
@modelcontextprotocol/server
@modelcontextprotocol/express
@modelcontextprotocol/hono
@modelcontextprotocol/node
commit: |
…ver, and Client After the string-based setRequestHandler API (PR #1446), the generic type parameters <RequestT, NotificationT, ResultT> on Server/Client and <SendRequestT, SendNotificationT, SendResultT> on Protocol no longer serve any purpose since custom methods cannot be registered. This removes them entirely, simplifying: - Protocol class declaration and all methods - RequestHandlerExtra type (no longer generic) - Server and Client class declarations - ExperimentalServerTasks and ExperimentalClientTasks - 4 @ts-expect-error comments that were needed to work around the generics Net -103 lines of type complexity.
…ity to RequestMethod/NotificationMethod
felixweinberger
force-pushed
the
fweinberger/remove-custom-method-generics
branch
from
cf4db2b to
b504ef6
Compare
felixweinberger
changed the title
…etRequestHandler Adds a ResultTypeMap that maps each method string to its specific result type, matching the existing RequestTypeMap pattern. This gives handlers correct return type inference per method, eliminating the need for explicit return type annotations and 'as const' assertions on literal values.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Removes the generic type parameters
<RequestT, NotificationT, ResultT>fromServer,Client, andProtocol(as<SendRequestT, SendNotificationT, SendResultT>).Motivation and Context
After #1446 switched
setRequestHandlerto string method names from a closed set, these generics no longer serve any purpose. They were originally needed to support custom/arbitrary method schemas, which is no longer possible.No other official SDK (Python, Go, C#) supports custom method generics, and the protocol spec defines a closed set of methods.
Changes
<SendRequestT, SendNotificationT, SendResultT>fromProtocolclass<RequestT, NotificationT, ResultT>fromServerandClientclassesRequestHandlerExtratype (hardcode to baseRequest/Notification)ExperimentalServerTasksandExperimentalClientTasks@ts-expect-errorcomments that existed solely to work around the generic constraintsas unknown as SendResultT/as SendNotificationTcastsServerRequest,ServerNotification,Request,Notification,Result)Net -103 lines.
How Has This Been Tested?
pnpm typecheck:all— zero errorspnpm test:all— all tests pass (1 pre-existing failure in authExtensions unrelated)pnpm lint:all— cleanBreaking Changes
Server<CustomRequest, CustomNotification, CustomResult>pattern is no longer supported.Serveris already deprecated in favor ofMcpServer.Client<CustomRequest, CustomNotification, CustomResult>— same.RequestHandlerExtrais no longer generic. Code usingRequestHandlerExtra<ServerRequest, ServerNotification>should use plainRequestHandlerExtra.Types of changes
Checklist
Additional context
This stacks on top of #1446. The
RequestTypeMap[M]from that PR actually provides better per-method type safety than the old generics, which defaulted to base types and were routinely bypassed with@ts-expect-error.