-
Notifications
You must be signed in to change notification settings - Fork 1.6k
[v2] Decouple server from express and hono - http framework-agnostic MCP server
#1326
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
[v2] Decouple server from express and hono - http framework-agnostic MCP server
#1326
Conversation
…port, add server-express, server-hono to pkg.pr.new
|
@modelcontextprotocol/client
@modelcontextprotocol/server
@modelcontextprotocol/express
@modelcontextprotocol/hono
@modelcontextprotocol/node
commit: |
…examples, update faq
|
how are you calculating those sizes? they look too big to be the production install size, is it the size of your local it might be worth including the production install size since that'll be what affects most people a quick glance at it shows me the server is roughly ~9MB in main, and 6.5MB in your branch. huge saving! |
… middleware folder, move express and hono to middleware folder
|
I noticed the node middleware declares the server as a peer, but the other two declare it as a dependency. should the three be consistent? also - might it be a good idea to set hono and express as peers rather than dependencies? since these middlewares are meant to be used with them |
pcarleton
left a comment
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.
Auth pieces LGTM!
Nice catch, yes, all middlewares should have these in peers, including |
pcarleton
left a comment
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.
Sorry missed this earlier, but the PRM path needs to have the pathname after .well-known rather than before
…com:KKonstantinov/typescript-sdk into feature/v2-decouple-web-servers-from-server
felixweinberger
left a comment
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.
Nice refactor! Looked through and this LGTM except for @pcarleton's point above which would be great to address.
|
Done!
|
Dismissing @pcarleton's review as I believe his change request has been addressed
v1includedexpress(andhonoin recent version), forcing the dependency down to users. (expressbeing >1MB along with middleware deps such asexpress-rate-limitetc.).Achieving final & true implementation of #1299
This PR decouples
@modelcontextprotocol/serverfrom HTTP frameworks completely, and introduces three middleware packages:@modelcontextprotocol/node,@modelcontextprotocol/express,@modelcontextprotocol/hono.The middleware packages are optional, and users could choose to use them or to map to the MCP SDK themselves and not using any of these plugins.
However, the
@modelcontextprotocol/serveris completely HTTP framework dependency-free.Some additional changes:
StreamableHTTPServerTransporttoNodeStreamableHTTPServerTransportimport { A, type B}). Preferimport type { B }andimport { A }on separate lines.expressdependency from@modelcontextprotocol/servercompletely@modelcontextprotocol/server-expressand@modelcontextprotocol/server-hono- each having its own minimum dependenciesssetransport for server (deprecated)serverauthbetter-auth)better-authserver auth demo examples with@modelcontextprotocol/inspectorexpressto5.2.1to get rid of CVE reported on5.0.1on theqslibrary (https://security.snyk.io/vuln/SNYK-JS-QS-14724253)Motivation and Context
v2
How Has This Been Tested?
Unit tests
Breaking Changes
v2
Types of changes
Checklist