Uh oh!
There was an error while loading. Please reload this page.
fix(node): make hono an optional peer dependency - #1896
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/nodecommit: |
felixweinberger
commented
Apr 16, 2026
@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.
bhosmer-ant
left a comment
There was a problem hiding this comment.
Reviewed at b423b09f. LGTM.
Verified: zero from 'hono' imports anywhere in packages/middleware/node/src/ — only @hono/node-server is consumed (streamableHttp.ts:12, for getRequestListener). pnpm install --lockfile-only produces no diff. Matches the optional-peer pattern in packages/core/package.json.
One observation for accuracy:@hono/node-server@1.19 itself still declares a required hono: ^4 peer, so npm 7+/pnpm with auto-install-peers will still pull hono transitively, and Yarn will still warn on the @hono/node-server → hono edge. The changeset already notes this, so no action — just flagging that the practical win is narrower than "removes the unmet-peer warning for the common case" until upstream relaxes.
This refines #1504 (the entry stays in peerDependencies so Yarn's peer-forwarding requirement is still satisfied) rather than reverting it.
Process: deep-analysis subagent + direct verification. Bughunter unavailable this run (env auth issue).
🦋 Changeset detectedLatest commit: 450c97a The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Uh oh!
There was an error while loading. Please reload this page.
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