Add npm clean script to simplify resetting dependencies #372
Conversation
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
cliffhall
requested changes
May 1, 2025
package.json
Outdated
| "build-server": "cd server && npm run build", | ||
| "build-client": "cd client && npm run build", | ||
| "build-cli": "cd cli && npm run build", | ||
| "clean": "rimraf ./node_modules ./client/node_modules ./server/node_modules ./dist ./client/dist ./server/dist ./package-lock.json && npm install", |
Member
There was a problem hiding this comment.
./server/distshould be./server/build./cli/buildand./cli/node_modulesshould also be included.- It might be an opportune time to rename
./server/buildand./cli/buildto./server/distand./cli/distwhat do you think?
Member
Author
There was a problem hiding this comment.
Thanks, I made some changes to include the missed targets you called out. I think I'd prefer to keep the dist naming convention, unless you see a benefit to changing it? (Or is it just for consistency?)
Member
There was a problem hiding this comment.
Thanks, I made some changes to include the missed targets you called out. I think I'd prefer to keep the dist naming convention, unless you see a benefit to changing it? (Or is it just for consistency?)
Yeah, I like dist better, too. I was suggesting we could change the build folders to dist so that they'd all be the same.
Member
Author
There was a problem hiding this comment.
OK! I am in a 'if its not broken don't fix it' mood when it comes to these build files so how about we leave it as-is. I think another chance to revisit will come when I take a few things out of draft related to testing e2e (in a separate PR).
cliffhall
previously approved these changes
May 2, 2025
Member
cliffhall
left a comment
There was a problem hiding this comment.
Looks good. Fine to approve, unless you would like to do the build -> dist renaming and I'll review that if you do.
IgnacioC44
referenced
this pull request
in MCPJam/inspector
Jun 21, 2025
Add npm clean script to simplify resetting dependencies
richardkmichael
added a commit
to richardkmichael/inspector
that referenced
this pull request
Dec 5, 2025
The `--no-package-lock` workaround was added due to npm bug #4828, where npm < 11.3.0 generates incomplete lockfiles for packages with optional platform dependencies (esbuild, rollup). Optional cross-platform dependencies were restored to `package-lock.json` in 358f276, so npm will be able to install from the lock file in the GitHub Actions. Also, fixed in npm 11.3.0 (Apr 2025), but Node v22 ships npm v10 and will remain affected out-of-the-box. Investigation notes follow. What happened? -------------- 1. Switch from yarn to npm: `package-lock.json` added, `yarn.lock` removed - modelcontextprotocol@702f827 Presumably: - run `npm install` to generate a `package-lock.json` from the yarn-managed `node_modules`, on macos - bug #4828: npm omitted optional cross-platform dependencies from the lock file 2. Pull 47, tries `npm ci`, and reverts, on 11 Nov 2024 modelcontextprotocol@3789ef9 - "Try restoring npm ci" --> testing the new node release for the bug? - ran against `setup-node`, `node-version: 18`, likely: 18.20.5 (released nov 11, 2024; ~same day) - git show 3789ef9:.github/workflows/main.yml - Failed action, and logs have expired - https://github.com/modelcontextprotocol/inspector/actions/runs/11782443393/job/32817472448 - https://nodejs.org/en/download/archive/v18.20.5 - uses npm 10.8.2 - Re-tried in inspector fork - workflow run at 3789ef9 - change `node-version: 18` to `18.20.5` (exact node / npm on commit date) - Fails due to missing `linux-x64-gnu` platform dep (rollup, would similarly affect esbuild) 3. Cross-platform dependencies restored to lockfile on 1 May 2025 modelcontextprotocol#372 - modelcontextprotocol@358f276 - worked because `package-lock.json` and `node_modules` were both removed - i.e., not the bug conditions -> even npm < 11.3.0 generates correct lockfile - At that point, `--no-package-lock` could've been removed from CI, Dockerfile, etc. NPM --- npm (aborist) fixed #4828 npm/cli#8184 - npm/cli#4828 --> frequent http 500, due to many comments - npm/cli@a96d8f6 - will not be backported Released in 11.3.0 on 8 Apr npm/cli#8150 - https://github.com/npm/cli/releases/tag/v11.3.0 - arborist 9.0.2 - https://github.com/npm/cli/releases/tag/arborist-v9.0.2 - npm v11.3.0 ships with node v24.2.0, on 6 May 2025 - https://nodejs.org/en/download/archive/v24 Node v22 ships npm v10 - https://nodejs.org/en/download/archive/v22 - will always be affected, no backport coming
richardkmichael
added a commit
to richardkmichael/inspector
that referenced
this pull request
Dec 6, 2025
The `--no-package-lock` workaround was added due to npm bug #4828, where npm < 11.3.0 generates incomplete lockfiles for packages with optional platform dependencies (esbuild, rollup). Optional cross-platform dependencies were restored to `package-lock.json` in 358f276, so npm will be able to install from the lock file in the GitHub Actions. Also, fixed in npm 11.3.0 (Apr 2025), but Node v22 ships npm v10 and will remain affected out-of-the-box. Investigation notes follow. What happened? -------------- 1. Switch from yarn to npm: `package-lock.json` added, `yarn.lock` removed - modelcontextprotocol@702f827 Presumably: - run `npm install` to generate a `package-lock.json` from the yarn-managed `node_modules`, on macos - bug #4828: npm omitted optional cross-platform dependencies from the lock file npm can't easily fix this. `npm install foo@x.y.z` could write a resolved dependency, but at the exact version so `foo` would be pinned, and semver operator twiddling would be required to restore it as-was. The fix would be to "manually" re-add the missing metadata. 2. Pull 47, tries `npm ci`, and reverts, on 11 Nov 2024 modelcontextprotocol@3789ef9 - "Try restoring npm ci" --> testing the new node release for the bug? - ran against `setup-node`, `node-version: 18`, likely: 18.20.5 (released nov 11, 2024; ~same day) - git show 3789ef9:.github/workflows/main.yml - Failed action, and logs have expired - https://github.com/modelcontextprotocol/inspector/actions/runs/11782443393/job/32817472448 - https://nodejs.org/en/download/archive/v18.20.5 - uses npm 10.8.2 - Re-tried in inspector fork - workflow run at 3789ef9 - change `node-version: 18` to `18.20.5` (exact node / npm on commit date) - Fails due to missing `linux-x64-gnu` platform dep (rollup, would similarly affect esbuild) 3. Cross-platform dependencies restored to lockfile on 1 May 2025 modelcontextprotocol#372 - modelcontextprotocol@358f276 - worked because `package-lock.json` and `node_modules` were both removed - i.e., not the bug conditions -> even npm < 11.3.0 generates correct lockfile - At that point, `--no-package-lock` could've been removed from CI, Dockerfile, etc. NPM --- npm (aborist) fixed #4828 npm/cli#8184 - npm/cli#4828 --> frequent http 500, due to many comments - npm/cli@a96d8f6 - will not be backported Released in 11.3.0 on 8 Apr npm/cli#8150 - https://github.com/npm/cli/releases/tag/v11.3.0 - arborist 9.0.2 - https://github.com/npm/cli/releases/tag/arborist-v9.0.2 - npm v11.3.0 ships with node v24.2.0, on 6 May 2025 - https://nodejs.org/en/download/archive/v24 Node v22 ships npm v10 - https://nodejs.org/en/download/archive/v22 - will always be affected, no backport coming
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Motivation and Context
Sometimes I run into errors related to a dependency that only gets resolved by deleting the package-lock.json and node_modules before running npm install again.
For example:
How Has This Been Tested?
The following examples were run without the
npm installstep to demonstrate that these files actually get deleted:Tests on Windows:
Before:
After:
Breaking Changes
None
Types of changes
Checklist
Additional context
As a result of testing this script I ended up regenerating the
package-lock.json, so it has a lot of changes. Most of these are just adding "resolved" URLs and "integrity" hashes as well as minor/patch updates to packages. Shouldn't be any breaking changes.