fix(middleware): stop bundling workspace type graphs into .d.ts (avoids dts-gen OOM)#2339
Conversation
…ds dts-gen OOM) The four middleware packages were inlining the entire @modelcontextprotocol/server (and transitively core) type graph into their bundled .d.mts instead of emitting an external `import` for the peerDependency. On main this is ~14s and a 540 KB sourcemap for a 200-line wrapper; once core's public surface grows past a fairly low threshold the tsdown dts step OOMs at the default Node heap. Root cause: rolldown-plugin-dts decides external-vs-bundle by checking the resolved path for `node_modules`. With pnpm workspace symlinks the realpath is `packages/server/...` (no `node_modules`), and the dev tsconfig `paths` map the specifier to source, so the peer dep is treated as local and inlined. Fix: clear `paths` for dts emit and set `preserveSymlinks: true` so the resolved path keeps `node_modules` and the plugin externalises it. The emitted .d.mts now references `@modelcontextprotocol/server` directly (correct, since it's a peer dep). Also switch the @modelcontextprotocol/node source import from the private `core` package to `server`, which re-exports the same types. Note: middleware dts emit now reads server's built `dist/*.d.mts` rather than its source. `pnpm -r build` orders this correctly; a standalone middleware build needs server built first. Claude-Session: https://claude.ai/code/session_01PHVERaA4Sw7Ui3zk88xSWC
…ide TODO rolldown-plugin-dts@0.21.0 (03998d41) honours rolldown's `external` resolution before the node_modules path test, which makes this override unnecessary once tsdown is bumped past ^0.18. The bump also removes `dts.resolve`, so it needs to land together with a server/client config rework — tracked separately. Claude-Session: https://claude.ai/code/session_01PHVERaA4Sw7Ui3zk88xSWC
|
@modelcontextprotocol/client
@modelcontextprotocol/codemod
@modelcontextprotocol/server
@modelcontextprotocol/server-legacy
@modelcontextprotocol/express
@modelcontextprotocol/fastify
@modelcontextprotocol/hono
@modelcontextprotocol/node
commit: |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
LGTM — build-config-only change scoped to dts emit, with the type-only import correctly retargeted to the declared peer dep (all six types are re-exported from @modelcontextprotocol/server).
Extended reasoning...
Overview
This PR only touches the four packages/middleware/* tsdown configs (replacing dead baseUrl/paths overrides with paths: {} + preserveSymlinks: true under dts.compilerOptions) and retargets a type-only import in packages/middleware/node/src/streamableHttp.ts from the private @modelcontextprotocol/core package to @modelcontextprotocol/server. There is no runtime logic change — the JS bundles are unaffected, and the only behavioral difference is that the bundled .d.mts files now emit external imports for the workspace peer dependency instead of inlining its type graph.
Verification
I confirmed that all of AuthInfo, JSONRPCMessage, MessageExtraInfo, RequestId, and Transport are exported from @modelcontextprotocol/server (via its export * from '@modelcontextprotocol/core/public', which re-exports types/types.ts and shared/transport.ts), and WebStandardStreamableHTTPServerTransportOptions is exported directly — so the consolidated import in streamableHttp.ts resolves. @modelcontextprotocol/server is already a required peerDependency of every middleware package, so the externalized type import resolves for existing consumers.
Security risks
None — this is build/declaration-emit configuration plus an import-source swap for type-only imports. No auth, protocol, or transport runtime code paths change.
Level of scrutiny
Low-to-moderate: it does change the shape of published .d.mts artifacts for four packages, but in the direction of correctness (peer dep types referenced externally rather than duplicated/inlined), and the rationale plus a TODO for the upstream rolldown-plugin-dts fix are well documented in the config comment. The packages are at 2.0.0-alpha, limiting downstream blast radius.
Other factors
The bug-hunting pass found no issues, the repository is already at this PR's head commit with typecheck/build passing, and the author's PR description documents byte-level comparisons of dist output. The only open item is the changeset-bot notice (no changeset), which the maintainers can decide on; it doesn't affect correctness.
3 participants
The four
packages/middleware/*tsdown builds were inlining the entire@modelcontextprotocol/server(and transitivelycore) type graph into their bundled.d.mtsinstead of emitting an externalimportfor the peer dependency. Onmainthis is ~14s and a 540 KB sourcemap for@modelcontextprotocol/node(a ~200-line wrapper); oncecore's public type surface grows past a fairly low threshold the dts step OOMs at the default Node heap.Motivation and Context
rolldown-plugin-dts(the dts bundler tsdown uses) decides external-vs-bundle by testing the resolved path fornode_modules. With pnpm workspace symlinks the realpath ispackages/server/...— nonode_modulesin it — so the peer dep is treated as local source and inlined. The devtsconfig.jsonpaths(which map@modelcontextprotocol/*to workspace source for IDE/typecheck) compound this by routing the resolver straight at source files. The explicitdts.compilerOptions.pathsoverrides that were already in these configs were dead — wrong relative depth — so they never did anything.This is fixed upstream in
rolldown-plugin-dts@0.21.0(sxzz/rolldown-plugin-dts@03998d41), which honours rolldown'sexternalresolution before thenode_modulespath test. We're pinned one minor behind viatsdown: ^0.18.0. Bumping tsdown is the right long-term fix but the same upstream release also removed thedts.resolveoption thatpackages/{server,client}/tsdown.config.tsrely on, so the bump is a separate, larger change — TODO left inpackages/middleware/node/tsdown.config.ts.How Has This Been Tested?
@modelcontextprotocol/nodebuild: 14s → 0.8s;dist/index.d.mts22.77 KB → 4.90 KB (sourcemap 539 KB → 0.37 KB). Output now starts withimport { AuthInfo, JSONRPCMessage, MessageExtraInfo, RequestId, Transport, WebStandardStreamableHTTPServerTransportOptions } from "@modelcontextprotocol/server".hono/fastifydist/index.d.mtsare byte-identical tomain(they only import from their framework peer);expressdiffers only by replacing inlined server types with the external import.dist/index.mjs) is byte-identical for all four — the change is scoped todts.compilerOptions.NodeStreamableHTTPServerTransportis assignable toTransportfrom@modelcontextprotocol/serverand accepted byMcpServer.connect().coretype surface that previously crashed withnode::OOMErrorHandler/ exit 134.pnpm typecheck:all,pnpm build:all, middleware test suites (126 tests) all pass.Breaking Changes
None.
@modelcontextprotocol/serveris already a requiredpeerDependencyof every middleware package, so the external type import resolves for all existing consumers.Types of changes
Checklist
Additional context
packages/middleware/node/src/streamableHttp.ts's type-only import from the private@modelcontextprotocol/corepackage to@modelcontextprotocol/server, which re-exports the same five types viacore/public. Middleware packages should only depend on their declared peer dep.packages/server/distto exist — output is byte-identical with it absent, so there is no new build-order constraint.tsdown(pullsrolldown-plugin-dts >=0.21.0) and drop this override; rework thedts.resolve: ['ajv', 'ajv-formats']usage in server/client at the same time.