fix(node): make hono an optional peer dependency#1896
fix(node): make hono an optional peer dependency#1896felixweinberger wants to merge 1 commit intomainfrom
Conversation
@modelcontextprotocol/node only uses getRequestListener from @hono/node-server, which does not require the hono framework at runtime. Consumers no longer need to install hono to use NodeStreamableHTTPServerTransport. @hono/node-server itself still hard-peers on hono, so a warning may remain (upstream).
@modelcontextprotocol/client
@modelcontextprotocol/server
@modelcontextprotocol/express
@modelcontextprotocol/fastify
@modelcontextprotocol/hono
@modelcontextprotocol/node
commit: |
|
@claude review |
There was a problem hiding this comment.
LGTM — straightforward packaging metadata change.
Extended reasoning...
Overview
This PR adds a peerDependenciesMeta entry to packages/middleware/node/package.json marking hono as optional, plus a changeset. No source code is touched.
Verification
Grepped packages/middleware/node/src and confirmed there are zero imports from 'hono' — the only hono-related import is getRequestListener from @hono/node-server, which is a separate package that does Node↔Fetch API conversion and does not require the hono framework to be loaded for that helper. The changeset prose accurately matches the implementation, including the honest caveat that @hono/node-server still hard-peers on hono upstream so a warning may persist (per the repo's "Documentation & Changesets" recurring catch, the claim is backed by the diff).
Security risks
None. This is package-manager metadata only; no runtime code, auth, or transport logic changes.
Level of scrutiny
Low. Mechanical, additive, self-contained packaging tweak. peerDependenciesMeta.optional is the standard idiom for this situation and cannot break existing consumers — those who already install hono are unaffected, and those who don't simply stop seeing an unmet-peer warning at this level.
Other factors
No outstanding reviewer comments. Bug-hunting system found nothing. CI green per PR description.
There was a problem hiding this comment.
LGTM — straightforward package.json metadata change.
Extended reasoning...
Overview
This PR adds a peerDependenciesMeta block to packages/middleware/node/package.json marking hono as optional, plus a changeset describing the change. No source code is modified.
Verification
Grepped packages/middleware/node/src and confirmed there are zero imports from hono itself — the only related import is getRequestListener from @hono/node-server (streamableHttp.ts:12), which is a separate package listed as a hard dependency. The changeset accurately describes this, including the honest caveat that @hono/node-server upstream still hard-peers on hono so a warning may persist. This satisfies the "Documentation & Changesets" recurring catch — the prose matches the implementation.
Security risks
None. This is package-manager metadata only; it loosens an install-time peer constraint and has no effect on runtime code paths, auth, or data handling.
Level of scrutiny
Low. A 5-line additive JSON change to dependency metadata with no behavioral impact on the SDK's published code. Worst-case failure mode is that a consumer who genuinely needs hono doesn't get it auto-installed — but this package never required it in the first place.
Other factors
No prior reviewer comments to address, no bugs flagged by the bug-hunting system, and the PR is already on main HEAD (b423b09). Tests/typecheck reported green by the author.
Part of the v2 backwards-compatibility series — see reviewer guide.
@modelcontextprotocol/nodepeers onhonobut doesn't import it directly (@hono/node-serverdoes). Making it optional removes the unmet-peer warning for the common case.Motivation and Context
@modelcontextprotocol/nodepeers onhonobut doesn't import it directly (@hono/node-serverdoes). Making it optional removes the unmet-peer warning for the common case.v1 vs v2 pattern & evidence
v1 pattern:
v2-native:
SameEvidence: Confirmed
getRequestListenerworks without hono installed.How Has This Been Tested?
/nodetests (74/74)v2-bc-integrationvalidation branchpnpm typecheck:all && pnpm lint:all && pnpm test:allgreenBreaking Changes
None — additive
@deprecatedshim. Removed in v3.Types of changes
Checklist
Additional context
Stacks on: none