-
Notifications
You must be signed in to change notification settings - Fork 2k
feat(packaging): ship CommonJS builds alongside ESM for v2 packages #2405
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| 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 | ||
| --- | ||
|
|
||
| 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. | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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 . | ||
|
|
@@ -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 | ||
|
|
@@ -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 | ||
|
|
||
|
|
@@ -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
|
||
|
Comment on lines
+258
to
+259
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟡 The new Jest section calls the removed 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
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 | ||
|
|
||
|
|
||
There was a problem hiding this comment.
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 afeatthat adds new backwards-compatible public surface (arequireexport condition,.cjs/.d.ctsartifacts, and amainfield on every package), which conventionally warrants aminorbump. 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 tominor.Extended reasoning...
What the issue is.
.changeset/cjs-support-v2-packages.mdmarks all nine packages (server,client,core,server-legacy,codemod,express,hono,fastify,node) aspatch. The PR itself is titledfeat(packaging)and adds new backwards-compatible functionality: every package gains arequirecondition on each export leaf, new.cjs/.d.ctsbuild artifacts, and a newmainfield for bare-requirefallback. 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 tominorfor feature changesets even during the prerelease cycle: thetoWebRequestexport (#2390),SdkHttpError, and the auth-isschangesets were all recorded asminor, andpackages/server/CHANGELOG.mdalready has "Minor Changes" sections from the 2.0.0-alpha/beta cycle (e.g. #2286, #1689).patchin this repo is used for docs and fixes.Concrete walk-through of the effect. The workspace is in changesets pre mode (
.changeset/pre.json, tagbeta), and all nine packages are at2.0.0-beta.1. When the release PR is generated: (1) changesets reads this file and seespatchfor each package; (2) because semver-inc of a prerelease version yields the next prerelease regardless of bump type, the published version is2.0.0-beta.2whether the entry sayspatchorminor— 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 newrequire()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.mdfrompatchtominor(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.
There was a problem hiding this comment.
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