fix(packaging): inline json-schema-typed in dist .d.mts; move validator-provider types to subpath-only; drop dead typesVersions.sse#2362
Conversation
🦋 Changeset detectedLatest commit: 7e8ad10 The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 packages
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 |
@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 — dts-bundling/packaging-only fix with no runtime or public-API impact; consider adding a patch changeset so the packaging fix actually ships.
Extended reasoning...
Overview
This PR is a packaging/types-bundling fix: it adds json-schema-typed to dts.resolve in the server and client tsdown.config.ts, maps fast-uri to a new dts-only shim (packages/core/src/validators/fastUriShim.d.ts) via dts.compilerOptions.paths, removes a dead typesVersions.*.sse block from @modelcontextprotocol/node (which pointed at a nonexistent dist/sse.d.mts and had no matching ./sse export), and adds a short README note about TS ≥6.0 requiring "types": ["node"].
Security risks
None. The changes only affect generated .d.mts declaration output, a dead package.json field, and README prose. The paths mapping lives under dts.compilerOptions, so runtime bundling and the published JS are unchanged. The new .d.ts shim is a plain interface with no runtime code, and @modelcontextprotocol/core is private so the shim itself is never published standalone.
Level of scrutiny
Moderate-low. This is build-configuration territory rather than protocol or transport logic, and the failure mode (broken declaration output) is caught by pnpm build:all and the author's fresh-install tsc --noEmit smoke with skipLibCheck: false, both reported green. The dead typesVersions removal is verifiably safe — the node middleware package only exports "." and never built dist/sse.d.mts.
Other factors
The bug-hunting pass found no issues. The only open item is the missing changeset (already flagged by changeset-bot): since this is a fix to what gets published, it presumably needs a patch changeset for client/server/node to actually reach npm — left to the author/maintainers to decide given the alpha release cadence.
felixweinberger
force-pushed
the
fweinberger/packaging-dts-resolve
branch
from
1868cb0 to
2c87d9e
Compare
felixweinberger
force-pushed
the
fweinberger/packaging-dts-resolve
branch
from
2c87d9e to
f489a78
Compare
felixweinberger
changed the title
felixweinberger
force-pushed
the
fweinberger/packaging-dts-resolve
branch
from
f489a78 to
6211034
Compare
…or provider types to subpath-only
The bundled server/client dist .d.mts files leaked two type references that
break downstream consumers with skipLibCheck:false (the TS default):
- types-*.d.mts imported 'json-schema-typed', which is not a dependency of
server/client (only of core, which is bundled). Fixed by adding
'json-schema-typed' to dts.resolve so the JSONSchema type is inlined.
- ajvProvider-*.d.mts referenced URIComponent without a declaration.
ajv@8.18.0's .d.ts does `import { URIComponent } from "fast-uri"`, but
fast-uri ships its types as `export = namespace`, which the dts bundler
cannot destructure — it drops the import and leaves the bare reference.
Fixed by removing the type-only re-exports of AjvJsonSchemaValidator /
CfWorkerJsonSchemaValidator / CfWorkerSchemaDraft from the public root
barrel: those classes/types are still exported from the dedicated
/validators/ajv and /validators/cf-worker subpaths (and the runtime
_shims conditional), so the inlined ajv type chunk is no longer reachable
from index.d.mts. The codemod already routes v1 imports to the subpaths.
Also:
- @modelcontextprotocol/node package.json: remove typesVersions.sse entry
pointing at nonexistent dist/sse.d.mts.
- server/client READMEs: note that TS >=6.0 needs "types": ["node"] in
tsconfig (TS 6 dropped implicit @types inclusion; the published .d.mts
references Buffer).
Verified: pnpm build:all + typecheck:all + docs:check green; fresh npm
install of packed tarballs + `npx tsc --noEmit` (NodeNext + bundler,
strict, skipLibCheck:false) is clean.
felixweinberger
force-pushed
the
fweinberger/packaging-dts-resolve
branch
from
6211034 to
7e8ad10
Compare
There was a problem hiding this comment.
LGTM — prior feedback addressed (fastUriShim + paths mapping landed, changeset now covers CfWorkerSchemaDraft, breaking export move documented); only nit left is the PR description's "add fast-uri as a runtime dependency" bullet, which isn't in the diff and appears stale given the shim approach.
Extended reasoning...
Overview
Packaging-only PR touching the server/client tsdown dts configs (add json-schema-typed to dts.resolve, map fast-uri to a new 24-line fastUriShim.d.ts), removal of the validator-provider type re-exports from the curated public root barrel, removal of a dead typesVersions.sse entry in @modelcontextprotocol/node, README notes about TS ≥6.0 types: ["node"], and a changeset documenting the export-surface move. No runtime logic changes.
Security risks
None. Changes are confined to type-declaration bundling configuration, a .d.ts shim (no runtime code), package metadata, and documentation. No auth, crypto, network, or input-handling code is touched.
Level of scrutiny
Moderate. The diff affects what ships in the published packages (dist .d.mts content and the public type surface), but the only API change is removing three type-only re-exports from the root in an alpha line, which is documented in the changeset and covered by the codemod. The hand-written URIComponent shim is dts-only and matches fast-uri's published shape; worst case is a type mismatch with no runtime impact. The author reports a green pnpm build:all plus a fresh-install tsc --noEmit smoke with skipLibCheck: false on packed tarballs, which is the right end-to-end verification for this class of fix.
Other factors
All concerns from my previous review rounds were addressed in this revision: the fastUriShim.d.ts and fast-uri paths mapping that the description promised are now actually in the diff, the changeset names CfWorkerSchemaDraft alongside the two validator classes, and the PR description now carries an explicit Breaking Changes section. The bug-hunting system found no issues in this revision. The one residual nit is that the description still claims fast-uri was added as a runtime dependency of server/client, which the diff does not contain (and the shim approach makes unnecessary, since undeclared deps are bundled) — a stale prose detail the author may want to tidy, but not something that affects the shipped artifacts.
1 participant
Fixes the four findings from the Tier 0.5 fresh-install smoke (fresh
npm installof pkg.pr.new tarballs +tsc --noEmitwithskipLibCheck: false).Motivation and Context
The bundled
dist/*.d.mtsfor@modelcontextprotocol/serverand@modelcontextprotocol/clientleak two type references that break any consumer withskipLibCheck: false(the TS default):json-schema-typed—core/src/validators/types.tsdoesimport type { JSONSchema } from 'json-schema-typed'; core is bundled into server/client butdts.resolveonly listed['ajv', 'ajv-formats'], so the import was left external. server/client don't declare it as a dep → TS2307 on the root entrypoint.URIComponent— ajv's published.d.tsreferencesURIComponentfromfast-uri, whoseexport =types the dts bundler cannot inline → dangling bare reference (TS2304) at the/validators/ajvsubpath.Fixes:
'json-schema-typed'todts.resolveinpackages/{server,client}/tsdown.config.ts.AjvJsonSchemaValidator/CfWorkerJsonSchemaValidator/CfWorkerSchemaDrafttypes from the package root (core/public). They remain available from the/validators/{ajv,cf-worker}subpaths. The root entrypoint no longer transitively imports ajv types at all.packages/core/src/validators/fastUriShim.d.ts— a 24-line build-time-onlyURIComponentinterface mapped viadts.compilerOptions.paths['fast-uri']. ajv's types referenceURIComponentfrom fast-uri'sexport = namespacetypes, which the dts bundler cannot inline; the shim provides a named export the bundler can resolve. Type-only; no runtime impact, no added dependency.packages/middleware/node/package.json: droptypesVersions.*.sse— points at a nonexistentdist/sse.d.mts.packages/{server,client}/README.md: note that TS ≥6.0 needs"types": ["node"]intsconfig.json.How Has This Been Tested?
pnpm build:all— green; the pre-existing[MISSING_EXPORT] URIComponentbuild warning is gone.pnpm packed tarballs:npx tsc --noEmit(NodeNext, strict,skipLibCheck: false) → clean (exit 0) for both the root entrypoint andimport { AjvJsonSchemaValidator } from '@modelcontextprotocol/server/validators/ajv'. Same withmoduleResolution: bundler.test/integration cloudflareWorkers.test.ts(nonodejs_compat) — passes;node:moduleis not in any dist chunk.node_moduleshas nojson-schema-typed/ajv/fast-uri— types are fully self-contained.Breaking Changes
AjvJsonSchemaValidator,CfWorkerJsonSchemaValidatorandCfWorkerSchemaDraftare no longer re-exported from the@modelcontextprotocol/server/@modelcontextprotocol/clientroot entrypoints; import them from the/validators/{ajv,cf-worker}subpath instead. These are v2-only additions (not present in v1), and the codemod already routes v1 deep-path imports to the subpath, so this affects only within-alpha consumers.Types of changes
Checklist