Remove dev-only platform binaries from optionalDependencies#517
Closed
gh0stonio wants to merge 1 commit intomodelcontextprotocol:mainfrom
Closed
Remove dev-only platform binaries from optionalDependencies#517gh0stonio wants to merge 1 commit intomodelcontextprotocol:mainfrom
gh0stonio wants to merge 1 commit intomodelcontextprotocol:mainfrom
Conversation
…ostinstall Fixes modelcontextprotocol#222. Rollup resolves its own platform binaries transitively. Bun setup script already handles missing binaries gracefully. The postinstall script is redundant with prepare.
Contributor
Author
|
Alternative to #516 which takes a prepack/postpack approach to strip the entries at publish time instead of removing them entirely. |
9 tasks
Contributor
Author
|
Closing in favor of #516. This approach (removing optionalDependencies entirely) breaks contributor DX since bun setup relies on them being installed before |
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Summary
Fixes #222, removes
@oven/bun-*and@rollup/rollup-*fromoptionalDependenciesentirely, and removes the redundantpostinstallscript.Builds on the investigation from #451, thanks @ochafik! This is a simpler alternative to #516.
Context
At Datadog we're building on MCP Apps and this
optionalDependenciesbloat (~400MB of platform binaries) is blocking our CI pipelines, which motivated picking this up.Why #451's approach doesn't work
#451 moved the platform binaries from
optionalDependenciestodevDependencies. These packages declareos/cpuconstraints in their ownpackage.json, npm silently skips mismatches inoptionalDependenciesbut fails hard indevDependencies(e.g.npm cion macOS ARM errors on@oven/bun-linux-x64).Approach
Instead of moving deps around, just remove them — they're not needed:
@rollup/rollup-*: rollup already declares its own platform-specificoptionalDependencies, npm resolves them transitively, the top-level entries are redundant@oven/bun-*:setup-bun.mjs(run byprepare) already gracefully handles missing binaries with a "install bun separately" fallback. Contributors who need bun can install it globally via bun.shpostinstall: redundant withprepare(which already runssetup-bun.mjs), and the script isn't even shipped in the published package ("files": ["dist"])Test plan
npm cisucceedsnpm run buildsucceedsnpm pack— publishedpackage.jsoncontains nooptionalDependencies