-
Notifications
You must be signed in to change notification settings - Fork 2k
chore: add missing changesets for CommonJS builds and codemod iterations #2412
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
Merged
+18
−0
Merged
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,13 @@ | ||
| --- | ||
| '@modelcontextprotocol/server': minor | ||
| '@modelcontextprotocol/client': minor | ||
| '@modelcontextprotocol/core': minor | ||
| '@modelcontextprotocol/server-legacy': minor | ||
| '@modelcontextprotocol/codemod': minor | ||
| '@modelcontextprotocol/express': minor | ||
| '@modelcontextprotocol/hono': minor | ||
| '@modelcontextprotocol/fastify': minor | ||
| '@modelcontextprotocol/node': minor | ||
| --- | ||
|
|
||
| Ship CommonJS builds alongside ESM for all v2 packages, so `require()` consumers and CJS-only toolchains can use the SDK without a bundler shim. | ||
|
Check failure on line 13 in .changeset/cjs-dual-builds.md
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| --- | ||
| '@modelcontextprotocol/codemod': patch | ||
| --- | ||
|
|
||
| v1-to-v2 migration fixes from continued real-world migrations (codemod iterations 5). |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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/cjs-dual-builds.mdfile duplicates the existing, still-unconsumed.changeset/cjs-support-v2-packages.mdthat PR #2405 already added for the same CommonJS dual-build change across the same 9 packages, and the two disagree on bump level (existing: patch, new: minor). Drop this new file (or, if a minor bump is actually intended, edit/remove the original instead) — otherwise the next Version Packages run produces two near-identical CHANGELOG entries per package and a silently escalated bump. The codemod-iterations-5.md changeset is genuinely missing from #2398 and is fine to add.Extended reasoning...
What the bug is. The PR description states that "The CommonJS dual-build change ... merged without changesets", but that premise is incorrect. PR #2405 (commit f172626,
feat(packaging): ship CommonJS builds alongside ESM for v2 packages) already included.changeset/cjs-support-v2-packages.md, and that file is still present at HEAD on this branch. This PR adds a second changeset,.changeset/cjs-dual-builds.md, describing the exact same change ("Ship CommonJS builds alongside ESM") for the exact same 9 packages.Why the existing state doesn't prevent it. A changeset is only removed once it is consumed by a
changeset versionrun (or listed in.changeset/pre.json'schangesetsarray during pre-release mode). At HEAD,pre.json's consumed list contains only"beta-release", socjs-support-v2-packages.mdhas not been consumed yet. Both changesets are therefore pending and will both be picked up by the next Version Packages run.Concrete impact — step through the next
changeset versionrun:beta-release(already consumed, skipped),cjs-support-v2-packages.md(patch × 9 packages),cjs-dual-builds.md(minor × 9 packages),codemod-iterations-5.md(patch for codemod).server,client,core,server-legacy,codemod,express,hono,fastify,node), the CHANGELOG gains two entries describing the same CJS dual-build change, one worded per each file — redundant and confusing release notes.minordeclaration silently overrides thepatchlevel the original author chose in feat(packaging): ship CommonJS builds alongside ESM for v2 packages #2405 (visible in the changeset-bot comment on this PR: everything jumps to Minor, and the pre-1.0 middleware packages to Major). Whichever level is correct, having two conflicting declarations for one change means the escalation happens implicitly rather than as a deliberate decision.Why this matters for this PR specifically. Half of the PR is this duplicate file, and its stated purpose ("the release automation has nothing to version" for the CJS change) is not true. The other half —
.changeset/codemod-iterations-5.md— is legitimate: commit 1772473 (#2398) merged with no changeset, so that one is genuinely missing.How to fix. Either:
.changeset/cjs-dual-builds.mdfrom this PR (if the originalpatchlevel is correct), orminorbump is actually intended for the CJS change, edit or replace the originalcjs-support-v2-packages.mdinstead of adding a second changeset alongside it, so exactly one changeset describes the change at the intended level.