Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 18 additions & 0 deletions .changeset/cjs-support-v2-packages.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
---
'@modelcontextprotocol/server': patch
'@modelcontextprotocol/client': patch
'@modelcontextprotocol/core': patch
'@modelcontextprotocol/server-legacy': patch
'@modelcontextprotocol/codemod': patch
'@modelcontextprotocol/express': patch
'@modelcontextprotocol/hono': patch
'@modelcontextprotocol/fastify': patch
'@modelcontextprotocol/node': patch
---
Comment on lines +1 to +11

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 The changeset records all nine packages as patch, but this PR is a feat that adds new backwards-compatible public surface (a require export condition, .cjs/.d.cts artifacts, and a main field on every package), which conventionally warrants a minor bump. Since the packages are in beta pre mode the published version is the same either way, but the generated CHANGELOG will list this feature under "Patch Changes" — consider changing the changeset entries to minor.

Extended reasoning...

What the issue is. .changeset/cjs-support-v2-packages.md marks all nine packages (server, client, core, server-legacy, codemod, express, hono, fastify, node) as patch. The PR itself is titled feat(packaging) and adds new backwards-compatible functionality: every package gains a require condition on each export leaf, new .cjs/.d.cts build artifacts, and a new main field for bare-require fallback. Under semver conventions, a new backwards-compatible feature is a minor bump, not a patch.

Why this is a surface addition, not a fix. The repo's own topology pin test (packages/core-internal/test/packageTopologyPins.test.ts) describes a CJS build as "a new public surface" — i.e. the project already treats dual-format output as an addition to the public API contract, not an internal detail. Repo precedent also points to minor for feature changesets even during the prerelease cycle: the toWebRequest export (#2390), SdkHttpError, and the auth-iss changesets were all recorded as minor, and packages/server/CHANGELOG.md already has "Minor Changes" sections from the 2.0.0-alpha/beta cycle (e.g. #2286, #1689). patch in this repo is used for docs and fixes.

Concrete walk-through of the effect. The workspace is in changesets pre mode (.changeset/pre.json, tag beta), and all nine packages are at 2.0.0-beta.1. When the release PR is generated: (1) changesets reads this file and sees patch for each package; (2) because semver-inc of a prerelease version yields the next prerelease regardless of bump type, the published version is 2.0.0-beta.2 whether the entry says patch or minor — so no version-number harm; (3) however, the changelog generator groups entries by bump type, so the "Ship CommonJS builds alongside ESM…" entry lands under a "Patch Changes" heading in each package's CHANGELOG.md. Consumers scanning the changelog for new capabilities would not expect a new require() entry point to be listed as a patch, and the release notes would misrepresent the nature of the change after the prerelease exits.

Why nothing else catches it. changeset-bot only verifies that a changeset exists and reports the declared bump types (its comment on this PR faithfully lists all nine as "Patch"); nothing validates that the bump type matches the semantic nature of the change.

How to fix. Change each frontmatter entry in .changeset/cjs-support-v2-packages.md from patch to minor (nine one-word edits). No other change is needed; the changeset body text is accurate.

Impact assessment. This is purely a changelog-categorization/convention issue — no build, runtime, or version-resolution behavior is affected, and in pre mode the published version is identical either way. It should not block merge; it's just worth a quick edit so the release notes categorize the CJS support as the feature it is.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

meh doesn't matter imo


Ship CommonJS builds alongside ESM. Each package now emits both `.mjs`/`.d.mts`
and `.cjs`/`.d.cts` (via tsdown `format: ['esm', 'cjs']`), and its `exports` map
adds a `require` condition so `require('@modelcontextprotocol/…')` works from
CommonJS consumers. Output extensions are normalized across all packages
(`@modelcontextprotocol/core` moves from `.js`/`.d.ts` to `.mjs`/`.d.mts`); the
public import paths are unchanged.
14 changes: 7 additions & 7 deletions docs/behavior-surface-pins.md
Original file line number Diff line number Diff line change
Expand Up @@ -23,13 +23,13 @@ CI pass — that reopens the silent-drift hole the pin exists to close.

## Where pins live

| Surface | File |
| --- | --- |
| Wire error-code tables, error classes, version constants | `packages/core-internal/test/types/errorSurfacePins.test.ts` |
| Schema strict/strip/loose boundaries, key existence | `packages/core-internal/test/types/schemaBoundaryPins.test.ts` |
| Published package set, export maps, ESM-only topology | `packages/core-internal/test/packageTopologyPins.test.ts` |
| stdio environment-inheritance safelist | `packages/client/test/client/stdioEnvPins.test.ts` |
| 2025-11-25 wire method-registry membership, schema identity | `packages/core-internal/test/types/registryPins.test.ts` |
| Surface | File |
| ----------------------------------------------------------- | -------------------------------------------------------------- |
| Wire error-code tables, error classes, version constants | `packages/core-internal/test/types/errorSurfacePins.test.ts` |
| Schema strict/strip/loose boundaries, key existence | `packages/core-internal/test/types/schemaBoundaryPins.test.ts` |
| Published package set, export maps, dual ESM/CJS topology | `packages/core-internal/test/packageTopologyPins.test.ts` |
| stdio environment-inheritance safelist | `packages/client/test/client/stdioEnvPins.test.ts` |
| 2025-11-25 wire method-registry membership, schema identity | `packages/core-internal/test/types/registryPins.test.ts` |

## Writing a new pin

Expand Down
57 changes: 15 additions & 42 deletions docs/migration/upgrade-to-v2.md
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,8 @@

## TL;DR — quick path

1. **Prerequisites.** Node.js 20+ and ESM (`"type": "module"` or `.mts`). v2 ships ESM
only; CommonJS callers must use dynamic `import()`.
1. **Prerequisites.** Node.js 20+. v2 is ESM-first but ships a CommonJS build too, so
both `import` and `require('@modelcontextprotocol/…')` resolve natively.
2. **Run the codemod.**
```bash
npx @modelcontextprotocol/codemod@beta v1-to-v2 .
Expand Down Expand Up @@ -212,8 +212,9 @@
does **not** require the `hono` framework — your package manager may emit a harmless
unmet-peer warning for `hono` (upstream `@hono/node-server` declares it).

v2 requires **Node.js 20+** and ships **ESM only**. If your project uses CommonJS
(`require()`), either migrate to ESM or use dynamic `import()`.
v2 requires **Node.js 20+**. It is ESM-first but ships a **CommonJS build alongside
ESM**, so CommonJS projects can `require('@modelcontextprotocol/…')` directly — no
dynamic `import()` shim required.

Repo-local tooling that encodes the literal v1 package name — dependency-pin lints,
version allowlists, CI checks, scripts — fails after the manifest swap and is invisible
Expand All @@ -225,13 +226,13 @@
catch real errors.

Tooling that pins SDK **dist text** (reading a constant out of a built file with
`require.resolve` + a regex) breaks in three stacked ways: the v2 exports maps offer
nothing a CJS `require.resolve` can find; the literal usually lives in a
`require.resolve` + a regex) breaks in two stacked ways: the literal usually lives in a
content-hashed sibling chunk (`dist/sse-<hash>.mjs`), not the subpath's entry module,
so fixed-path reads do not survive a rebuild — scan the package's `dist/` directory
for the literal instead; and the emitted quote style differs from v1, so a
quote-anchored pattern misses silently — match either quote. v2 also ships ESM only:
`/dist/cjs/` ↔ `/dist/esm/` flavor-pair path swaps have no equivalent.
quote-anchored pattern misses silently — match either quote. The build layout also
changed: v2 emits `.mjs`/`.cjs` siblings in a flat `dist/`, so v1's `/dist/cjs/` ↔
`/dist/esm/` flavor-pair path swaps have no equivalent.

#### Registry availability during the beta

Expand All @@ -250,41 +251,13 @@
(`pnpm install && pnpm build`, then `pnpm pack` in the package directory) and
reference it with a committed `file:` dependency.

#### CommonJS test runners (Jest) cannot resolve the v2 packages

Every leaf of the v2 packages' `exports` maps carries only the `types` and `import`
conditions — there is no `require` or `default` leaf — so the packages cannot be
resolved by CJS resolvers at all. Jest under its default CommonJS resolution (including
`next/jest` setups) fails with `Cannot find module '@modelcontextprotocol/client'` even
when a transform that handles ESM is configured: resolution fails before any transform
runs. Vitest and native Node ESM are unaffected.

The interim recipe — interim because the packaging shape is still under discussion and
a later alpha may make it unnecessary — maps the bare specifiers straight to the dist
ESM files and lets the transform convert them (the dists contain no `import.meta`, so
an ESM→CJS transform such as SWC or Babel handles them cleanly):

```js
// jest.config.js
transformIgnorePatterns: [], // or a pattern that still transforms @modelcontextprotocol/*
moduleNameMapper: {
'^@modelcontextprotocol/client$': '<rootDir>/node_modules/@modelcontextprotocol/client/dist/index.mjs',
'^@modelcontextprotocol/server$': '<rootDir>/node_modules/@modelcontextprotocol/server/dist/index.mjs',
// `_shims` is the packages' internal runtime-selection self-reference;
// pin it to the Node build under jest.
'^@modelcontextprotocol/client/_shims$': '<rootDir>/node_modules/@modelcontextprotocol/client/dist/shimsNode.mjs',
'^@modelcontextprotocol/server/_shims$': '<rootDir>/node_modules/@modelcontextprotocol/server/dist/shimsNode.mjs',
},
```
#### CommonJS test runners (Jest)

The entries are exact-anchored — add one per subpath you import (`/stdio` →
`dist/stdio.mjs`, `/validators/cf-worker` → `dist/validators/cfWorker.mjs`) and one for
`@modelcontextprotocol/core` (`dist/index.js`) if you import the raw schemas. The
`_shims` mappings are required whenever the matching root mapping is present: the dist
entry files import `@modelcontextprotocol/client/_shims` (a package self-reference)
internally, and that specifier fails CJS resolution the same way. In a hoisted
monorepo, point the paths at the `node_modules` directory your package manager actually
installs into.
v2 ships a CommonJS build, so CJS test runners resolve the packages natively through the
`require` export condition — Jest (including `next/jest` setups) no longer needs a
`moduleNameMapper` workaround to import `@modelcontextprotocol/*`. If you carried a
v1-era mapping that pinned these packages to their `dist/*.mjs` files, remove it. Vitest

Check warning on line 259 in docs/migration/upgrade-to-v2.md

View check run for this annotation

Claude / Claude Code Review

Jest section mislabels the removed v2-beta moduleNameMapper recipe as a v1-era mapping

The new Jest section calls the removed `moduleNameMapper` recipe "a v1-era mapping", but that mapping was this guide's own v2-alpha/beta interim workaround (v1 shipped `dist/cjs/`/`dist/esm/` `.js` builds and never needed one, and the mapped specifiers/`dist/*.mjs` paths only exist in v2). Consider rewording to "the earlier v2-beta `moduleNameMapper` workaround" so readers who added it during the beta recognize it and remove the now-obsolete pin.
Comment on lines +258 to +259

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 The new Jest section calls the removed moduleNameMapper recipe "a v1-era mapping", but that mapping was this guide's own v2-alpha/beta interim workaround (v1 shipped dist/cjs//dist/esm/ .js builds and never needed one, and the mapped specifiers/dist/*.mjs paths only exist in v2). Consider rewording to "the earlier v2-beta moduleNameMapper workaround" so readers who added it during the beta recognize it and remove the now-obsolete pin.

Extended reasoning...

What the issue is. The rewritten "CommonJS test runners (Jest)" section (docs/migration/upgrade-to-v2.md:258-259) now reads: "If you carried a v1-era mapping that pinned these packages to their dist/*.mjs files, remove it." The mapping being described is unambiguously the moduleNameMapper recipe that this same guide prescribed for the v2 alpha/beta ESM-only packaging — the section this PR deletes explicitly called it "the interim recipe — interim because the packaging shape is still under discussion and a later alpha may make it unnecessary." Users who added it did so while already on v2 prereleases, not on v1.\n\nWhy "v1-era" cannot be right. A v1-era mapping to dist/*.mjs could not have existed:\n\n1. The bare specifiers the recipe maps — @modelcontextprotocol/client, @modelcontextprotocol/server, and the /_shims self-reference — only exist in v2. v1 was the single @modelcontextprotocol/sdk package.\n2. v1 shipped dist/cjs/ and dist/esm/ .js flavor pairs (as this same guide notes a few lines above, ~233-235) and resolved fine under Jest's default CommonJS resolution, so no moduleNameMapper was ever needed for v1, and no dist/index.mjs / dist/shimsNode.mjs paths existed to pin.\n\nStep-by-step reader scenario showing the impact. (1) A user upgraded to 2.0.0-beta.1 a few weeks ago and, following this guide's then-current Jest section, added moduleNameMapper entries pointing @modelcontextprotocol/client at dist/index.mjs and /_shims at dist/shimsNode.mjs. (2) They upgrade to the release that ships this PR and re-read the Jest section. (3) The guide tells them to remove a "v1-era mapping" — they never migrated a Jest config from v1, or they grep for something v1-flavored, don't recognize the mapping they added during the beta as the thing being referred to, and leave it in place. (4) The stale pin keeps routing resolution straight to the .mjs dist files through the transform-based path, silently bypassing the new require condition this PR ships — exactly the workaround the section says is no longer needed.\n\nWhy nothing else catches it. This is prose in a migration guide; no test or lint validates the historical attribution, and the sentence was introduced by this PR's own rewrite (commit c3241d8), so it is in scope for this review.\n\nHow to fix. One-line reword, e.g.: "If you carried the earlier v2-beta moduleNameMapper workaround that pinned these packages to their dist/*.mjs files, remove it" (or "the interim mapping this guide previously prescribed"). Everything else in the rewritten section is accurate.\n\nSeverity. Documentation prose accuracy only — nothing breaks at merge time and the fix is a few words, so this is a nit, not a blocker.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

given v2 is so new, I don't think we need to do within v2 documentation tbh

and native Node ESM are unaffected.

#### Bundlers: nested `zod` copies in zod@3-pinned monorepos

Expand Down
81 changes: 65 additions & 16 deletions packages/client/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -21,40 +21,89 @@
],
"exports": {
".": {
"types": "./dist/index.d.mts",
"import": "./dist/index.mjs"
"import": {
"types": "./dist/index.d.mts",
"default": "./dist/index.mjs"
},
"require": {
"types": "./dist/index.d.cts",
"default": "./dist/index.cjs"
}
Comment thread
claude[bot] marked this conversation as resolved.
},
"./stdio": {
"types": "./dist/stdio.d.mts",
"import": "./dist/stdio.mjs"
"import": {
"types": "./dist/stdio.d.mts",
"default": "./dist/stdio.mjs"
},
"require": {
"types": "./dist/stdio.d.cts",
"default": "./dist/stdio.cjs"
}
},
"./validators/ajv": {
"types": "./dist/validators/ajv.d.mts",
"import": "./dist/validators/ajv.mjs"
"import": {
"types": "./dist/validators/ajv.d.mts",
"default": "./dist/validators/ajv.mjs"
},
"require": {
"types": "./dist/validators/ajv.d.cts",
"default": "./dist/validators/ajv.cjs"
}
},
"./validators/cf-worker": {
"types": "./dist/validators/cfWorker.d.mts",
"import": "./dist/validators/cfWorker.mjs"
"import": {
"types": "./dist/validators/cfWorker.d.mts",
"default": "./dist/validators/cfWorker.mjs"
},
"require": {
"types": "./dist/validators/cfWorker.d.cts",
"default": "./dist/validators/cfWorker.cjs"
}
},
"./_shims": {
"workerd": {
"types": "./dist/shimsWorkerd.d.mts",
"import": "./dist/shimsWorkerd.mjs"
"import": {
"types": "./dist/shimsWorkerd.d.mts",
"default": "./dist/shimsWorkerd.mjs"
},
"require": {
"types": "./dist/shimsWorkerd.d.cts",
"default": "./dist/shimsWorkerd.cjs"
}
},
"browser": {
"types": "./dist/shimsBrowser.d.mts",
"import": "./dist/shimsBrowser.mjs"
"import": {
"types": "./dist/shimsBrowser.d.mts",
"default": "./dist/shimsBrowser.mjs"
},
"require": {
"types": "./dist/shimsBrowser.d.cts",
"default": "./dist/shimsBrowser.cjs"
}
},
"node": {
"types": "./dist/shimsNode.d.mts",
"import": "./dist/shimsNode.mjs"
"import": {
"types": "./dist/shimsNode.d.mts",
"default": "./dist/shimsNode.mjs"
},
"require": {
"types": "./dist/shimsNode.d.cts",
"default": "./dist/shimsNode.cjs"
}
},
"default": {
"types": "./dist/shimsNode.d.mts",
"import": "./dist/shimsNode.mjs"
"import": {
"types": "./dist/shimsNode.d.mts",
"default": "./dist/shimsNode.mjs"
},
"require": {
"types": "./dist/shimsNode.d.cts",
"default": "./dist/shimsNode.cjs"
}
}
}
},
"main": "./dist/index.cjs",
"types": "./dist/index.d.mts",
"typesVersions": {
"*": {
Expand Down
3 changes: 2 additions & 1 deletion packages/client/tsdown.config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,8 @@ export default defineConfig({
'src/validators/ajv.ts',
'src/validators/cfWorker.ts'
],
format: ['esm'],
format: ['esm', 'cjs'],
fixedExtension: true,
outDir: 'dist',
clean: true,
sourcemap: true,
Expand Down
11 changes: 9 additions & 2 deletions packages/codemod/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -25,10 +25,17 @@
},
"exports": {
".": {
"types": "./dist/index.d.mts",
"import": "./dist/index.mjs"
"import": {
"types": "./dist/index.d.mts",
"default": "./dist/index.mjs"
},
"require": {
"types": "./dist/index.d.cts",
"default": "./dist/index.cjs"
}
}
},
"main": "./dist/index.cjs",
"files": [
"dist"
],
Expand Down
3 changes: 2 additions & 1 deletion packages/codemod/tsdown.config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,8 @@ import { defineConfig } from 'tsdown';
export default defineConfig({
failOnWarn: 'ci-only',
entry: ['src/cli.ts', 'src/index.ts'],
format: ['esm'],
format: ['esm', 'cjs'],
fixedExtension: true,
outDir: 'dist',
clean: true,
sourcemap: true,
Expand Down
33 changes: 25 additions & 8 deletions packages/core-internal/test/packageTopologyPins.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ interface PackageManifest {
name: string;
private?: boolean;
type?: string;
main?: string;
files?: string[];
bin?: Record<string, string>;
exports?: Record<string, unknown>;
Expand Down Expand Up @@ -75,16 +76,32 @@ describe('public package topology', () => {
expect(Object.keys(manifest.exports ?? {})).toEqual(expected.exportKeys);
});

test('ships ESM only', () => {
test('ships dual ESM + CJS', () => {
// The v2 packages are ESM-first but ship a CommonJS build too, so
// require('@modelcontextprotocol/…') resolves natively. Pin that
// deliberate dual surface: type stays 'module', a `.cjs` main is
// present for bare-require fallback, and every export condition
// that resolves a module format offers BOTH an `import` (ESM) and a
// `require` (CJS) branch — recursively, so nested runtime conditions
// (e.g. ./_shims → workerd/browser/node/default) are covered.
expect(manifest.type).toBe('module');
// No entry may grow a 'require' condition: the v2 packages are
// ESM-only by design (a CJS build would be a new public surface).
const conditionsOf = (entry: unknown): string[] =>
entry !== null && typeof entry === 'object'
? Object.entries(entry).flatMap(([key, value]) => [key, ...conditionsOf(value)])
: [];
expect(manifest.main).toMatch(/\.cjs$/);

const assertDual = (node: unknown): void => {
if (node === null || typeof node !== 'object') {
return;
}
const keys = Object.keys(node);
if (keys.includes('import') || keys.includes('require')) {
expect(keys).toContain('import');
expect(keys).toContain('require');
}
for (const value of Object.values(node)) {
assertDual(value);
}
};
for (const entry of Object.values(manifest.exports ?? {})) {
expect(conditionsOf(entry)).not.toContain('require');
assertDual(entry);
}
});

Expand Down
13 changes: 10 additions & 3 deletions packages/core/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -22,11 +22,18 @@
],
"exports": {
".": {
"types": "./dist/index.d.ts",
"import": "./dist/index.js"
"import": {
"types": "./dist/index.d.mts",
"default": "./dist/index.mjs"
},
"require": {
"types": "./dist/index.d.cts",
"default": "./dist/index.cjs"
}
}
},
"types": "./dist/index.d.ts",
"main": "./dist/index.cjs",
"types": "./dist/index.d.mts",
"files": [
"dist"
],
Expand Down
3 changes: 2 additions & 1 deletion packages/core/tsdown.config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,8 @@ import { defineConfig } from 'tsdown';
export default defineConfig({
failOnWarn: 'ci-only',
entry: ['src/index.ts'],
format: ['esm'],
format: ['esm', 'cjs'],
fixedExtension: true,
outDir: 'dist',
clean: true,
sourcemap: true,
Expand Down
11 changes: 9 additions & 2 deletions packages/middleware/express/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -23,10 +23,17 @@
],
"exports": {
".": {
"types": "./dist/index.d.mts",
"import": "./dist/index.mjs"
"import": {
"types": "./dist/index.d.mts",
"default": "./dist/index.mjs"
},
"require": {
"types": "./dist/index.d.cts",
"default": "./dist/index.cjs"
}
}
},
"main": "./dist/index.cjs",
"types": "./dist/index.d.mts",
"files": [
"dist"
Expand Down
3 changes: 2 additions & 1 deletion packages/middleware/express/tsdown.config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,8 @@ import { defineConfig } from 'tsdown';
export default defineConfig({
failOnWarn: 'ci-only',
entry: ['src/index.ts'],
format: ['esm'],
format: ['esm', 'cjs'],
fixedExtension: true,
outDir: 'dist',
clean: true,
sourcemap: true,
Expand Down
Loading
Loading