-
Notifications
You must be signed in to change notification settings - Fork 56
Description
The Problem
Describe the bug
https://pkg-size.dev/@modelcontextprotocol%2Fext-apps
Depending on which platform you're on, Installing @modelcontextprotocol/ext-apps adds 403 MB (linux, 60 MB on my MacOS) to the installed Node modules. That is a lot for a package like this, that could probably be less than 5 MB, and potentially even less than 500 kB if stretched. Not only does this hurt Developers building servers with this pacakge, as their development, production and CI and environments will be slowed down, but it also hurts anyone building packages on top of this package like we do in Storybook.
So why is this happening?
In #145 @ochafik added the Bun-specific binaries to fix a Powershell-specific issue. These contributes the majority of MBs, 57 MB on MacOS, 401 MB on Linux (based on pkg-size.dev output).
In #73 @ochafik added Rollup-specific binaries to fix an npm CLI issue that affects installing the examples. The size added here is smaller, around 1.7 MB, but that's still a lot given that it is not used at all in the package.
To Reproduce
Steps to reproduce the behavior:
mkdir ext-apps-depscd ext-apps-depsnpm init --yesnpm install -S @modelcontextprotocol/ext-appsdu -sh node_modules
Expected behavior
Bun and rollup not being installed, depending the package not adding more than ~5 MBs.
Proposed Solution
It seems to me, that with the examples, this repo is actually a monorepo with multiple packages, which is also evident by the workspace configuration in the root package.json. However the main @modelcontextprotocol/ext-apps package is in fact also the whole repository, because it is also the root package. I'd heavily recommend separating the main package out more clearly, so it lives in its own directory within the repo. That way, example applications and packages cannot taint each other. You can still use the root (now private) package.json to keep dependencies in sync and to fix the npm CLI bug with Rollup. Additionally you can have a clear separation between scripts used for developing and building the package and scripts that consumers of the package should run on installation (which is likely none).
This setup is how monorepos are usually structured and most teams have successs with this.
Based on my limited understanding of this repository, it seems that all the scripts in the root package.json are meant for development of the package, in this repo, and are not actually meant to run when consumers of the package installs the package. However they are still included in the main package's package.json (because it is the root), which is what's causing these troubles and is invalid according to npm spec, because the scripts points to files that are not distributed in the final package.
Looking specifically at the Powershell bug, it's caused by the package consumer's environment running the build script of this package (for whatever reason?), running npm run generate:schemas && bun build.bun.ts which doesn't seem like any consumers should ever do?
Additionally I'd recommend switching over to pnpm as it has greater monorepo package management, but it's a more opinionated discussion with more trade offs, so feel free to ignore that.
If you agree with my proposal, I'd be happy to contribute it. However it would result in significant changes due to the files moving around, so it needs to be coordinated with other work to cause minimal friction in terms of merge conflicts, etc.