-
Notifications
You must be signed in to change notification settings - Fork 59
Refactor server start in examples #229
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
base: main
Are you sure you want to change the base?
Refactor server start in examples #229
Conversation
Split server startup logic into two functions in each `server-utils.ts`:
- `startStdioServer(createServer)` - handles stdio transport
- `startHttpServer(createServer)` - handles HTTP transport
And keep the transport selection visible in each `server.ts`:
```typescript
process.argv.includes("--stdio")
? await startStdioServer(createServer)
: await startHttpServer(createServer);
```
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
@modelcontextprotocol/ext-apps
@modelcontextprotocol/server-basic-react
@modelcontextprotocol/server-basic-vanillajs
@modelcontextprotocol/server-budget-allocator
@modelcontextprotocol/server-cohort-heatmap
@modelcontextprotocol/server-customer-segmentation
@modelcontextprotocol/server-scenario-modeler
@modelcontextprotocol/server-system-monitor
@modelcontextprotocol/server-threejs
@modelcontextprotocol/server-wiki-explorer
commit: |
| options: ServerOptions, | ||
| ): Promise<void> { | ||
| const { port, name = "MCP Server" } = options; | ||
| const port = parseInt(process.env.PORT ?? "3001", 10); |
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.
This introduces process.env again in server-utils, I don't like this direction tbh
And the startStdioServer method is a lot of lines to replace a one-liner.
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.
This introduces process.env again in server-utils, I don't like this direction tbh
The PORT env variable is very widely used, so reading from wherever in the code base is pretty typical for web servers.
And the startStdioServer method is a lot of lines to replace a one-liner.
The intent is instant clarity. Compare:
if (process.argv.includes("--stdio")) {
await createServer().connect(new StdioServerTransport());
} else {
const port = parseInt(process.env.PORT ?? "3001", 10);
await startServer(createServer, { port, name: "Basic MCP App Server (Preact)" });
}vs
process.argv.includes("--stdio")
? await startStdioServer(createServer)
: await startHttpServer(createServer);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.
Mmh fair enough won't die on this hill, thanks! ;-)
Split server startup logic into two functions in each
server-utils.ts:startStdioServer(createServer)- handles stdio transportstartHttpServer(createServer)- handles HTTP transportAnd keep the transport selection visible in each
server.ts: