chore: npm -> pnpm VSCODE-714 #1191

nirinchev · 2025-12-02T15:18:28Z

Description

This PR migrates the vscode project from npm to pnpm. npm has been a source of frustration on various levels and with pnpm we're getting a more predictable and significantly faster experience. There are a handful of code changes related to the migration as well as a bump of the mcp server package.

Copilot

Pull request overview

This PR migrates the project's package manager from npm to pnpm, as indicated by the ticket number VSCODE-714. The changes update build scripts, configuration files, CI/CD workflows, and documentation to use pnpm instead of npm.

Key changes include:

Updated all npm commands to pnpm equivalents across scripts, workflows, and documentation
Added pnpm configuration files and updated package.json with pnpm-specific settings
Modified dependency versions and removed unused dependencies
Updated test assertions to reflect changes in connection string formatting

Reviewed changes

Copilot reviewed 19 out of 22 changed files in this pull request and generated no comments.

Show a summary per file

File	Description
package.json	Added pnpm package manager specification, updated scripts to use pnpm, modified dependency versions, and added pnpm overrides configuration
README.md	Updated installation instructions to use pnpm instead of npm
CONTRIBUTING.md	Updated development setup and testing commands to use pnpm
.npmrc	Added pnpm hoisting configuration for specific packages
.prettierignore	Added pnpm-lock.yaml to ignored files
.depcheckrc	Updated ignore list formatting and added rimraf and snyk entries
.eslintrc.js	Added TypeScript ts-ignore comment configuration
scripts/snyk-test.js	Replaced npm-specific workarounds with pnpm commands, removed obsolete package-lock manipulation
.github/workflows/*.yaml	Updated all workflow files to use pnpm with action-setup and frozen lockfile installations
src/test/suite/connectionController.test.ts	Simplified connection string assertions by removing anonymous and connection ID checks
src/telemetry/connectionTelemetry.ts	Changed property name from dbType to serverName
src/mcp/*.ts	Updated MCP configuration handling to use UserConfigSchema and removed defaultUserConfig imports
syntaxes/mongodbInjection.tmLanguage.json	Added syntax highlighting for new MongoDB similarity operators and scoreFusion

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

gagik · 2025-12-03T15:29:00Z

src/test/suite/connectionController.test.ts

-
      expect(mongoClientConnectionOptions).to.deep.equal({
-        url: `mongodb://localhost:27088/?appName=mongodb-vscode+${version}--${anonymousId}--${latestConnectionId}`,
+        url: `mongodb://localhost:27088/?appName=mongodb-vscode+${version}`,


how was this green before?

This changed after we bumped the connection form dependency. Previously, npm had it pinned on an old version.

src/mcp/mcpConfig.ts

gagik

Minor note about the set + question

himanshusinghs · 2025-12-03T15:31:28Z

.github/workflows/actions/test-and-build/action.yaml

      shell: bash
      run: |
-        npm ci --omit=optional
+        pnpm install --frozen-lockfile


Does pnpm by default omit optional? I faintly recall there were some transitive optional dependencies that were problematic for the install here.

If we omit the optional dependencies when installing, we're getting build errors: https://github.com/mongodb-js/vscode/actions/runs/19892320307/job/57013930209?pr=1191 - i.e. the atlas-local optional dependencies are missing when we try to webpack.

Interesting - then I'd be worried about snyk output as well. We might need to double check if snyk is even evaluating the correct packages or not.

Double checked it locally and it works as expected.

himanshusinghs · 2025-12-03T15:33:47Z

scripts/snyk-test.js

-/**
- * "node_modules/@vscode/vsce-sign" package which is a dev dependency used for
- * publishing extension declares platform specific optionalDependencies, namely
- * the following:
- * - "@vscode/vsce-sign-alpine-arm64"
- * - "@vscode/vsce-sign-alpine-x64"
- * - "@vscode/vsce-sign-darwin-arm64"
- * - "@vscode/vsce-sign-darwin-x64"
- * - "@vscode/vsce-sign-linux-arm"
- * - "@vscode/vsce-sign-linux-arm64"
- * - "@vscode/vsce-sign-linux-x64"
- * - "@vscode/vsce-sign-win32-arm64"
- * - "@vscode/vsce-sign-win32-x64"


Yea these were the ones! How does pnpm address these?

this seems only relevant in the build step.
I think we were omitting optional dependencies just so we could create a workaround around us doing this, which is weird. I think this new approach should be fine unless I'm overlooking something

It was an unfortunate workaround because Snyk decided not to ignore the dev dependencies for analysis but still a valid one because without it Snyk would fail and never get to analyse production dependencies.

Without much understanding of how Snyk plays with pnpm, I would assume that Snyk setup is broken again but I will check.

I checked it locally and it seems to work just fine. Snyk was indeed detecting errors from our installed packages. I am curious now how snyk interacts with pnpm lock file? 😄

I re-enabled snyk vuln report generation and also enabled uploading the report in the gha run so that we can reference it. There are a few recommendations, but will take a look at those in another PR.

src/mcp/mcpController.ts

himanshusinghs · 2025-12-03T15:36:34Z

src/telemetry/connectionTelemetry.ts

      is_enterprise: build.isEnterprise,
      is_genuine: genuineMongoDB.isGenuine,
-      non_genuine_server_name: genuineMongoDB.dbType,
+      non_genuine_server_name: genuineMongoDB.serverName,


himanshusinghs · 2025-12-03T15:37:55Z

src/test/suite/connectionController.test.ts

      );
      // But the active connection should
      expect(testConnectionController.getActiveConnectionString()).equals(
-        `${TEST_DATABASE_URI}/?appName=mongodb-vscode+${version}--${anonymousId}--${connectionId}`,


This seems wrong - it should append the anonymous id though? Or does that happen somewhere else?

This only gets appended when we connect to atlas: https://github.com/mongodb-js/compass/blob/305f949daaa869fd40105cf67bb41cd6940e67b5/packages/connection-form/src/utils/set-app-name-if-missing.ts#L25

Should we have a test to ensure the id is appended for Atlas as well? If that is not already there?

I don't think that's there already, but I'd rather leave that to a separate PR - it's a bit annoying to write.

Copilot

Pull request overview

Copilot reviewed 19 out of 22 changed files in this pull request and generated 2 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

.github/workflows/actions/test-and-build/action.yaml

himanshusinghs

pnpm pnpm pnpm 🚀

nirinchev added 7 commits December 2, 2025 17:14

chore: npm -> pnpm

1fc3f5a

Merge branch 'main' into ni/pnpm

73ef717

update lockfile

1cea411

fix lint

75f72c9

install optionals

9b24d54

use globs for rimraf

daebb92

update mocha

a068094

nirinchev changed the title ~~chore: npm -> pnpm~~ chore: npm -> pnpm VSCODE-714 Dec 3, 2025

nirinchev added 2 commits December 3, 2025 16:57

fix tests

5eb7abe

fix lint

04a7906

nirinchev marked this pull request as ready for review December 3, 2025 15:25

nirinchev requested a review from a team as a code owner December 3, 2025 15:25

Copilot AI review requested due to automatic review settings December 3, 2025 15:25

Copilot AI reviewed Dec 3, 2025

View reviewed changes

gagik reviewed Dec 3, 2025

View reviewed changes

src/mcp/mcpConfig.ts Outdated Show resolved Hide resolved

gagik approved these changes Dec 3, 2025

View reviewed changes

himanshusinghs requested changes Dec 3, 2025

View reviewed changes

nirinchev added 2 commits December 3, 2025 19:05

upload .sbom artifacts

f9a3950

enable snyk vuln ticket generation

0283ac0

nirinchev requested a review from Copilot December 3, 2025 17:47

Copilot AI reviewed Dec 3, 2025

View reviewed changes

.github/workflows/actions/test-and-build/action.yaml Outdated Show resolved Hide resolved

.github/workflows/actions/test-and-build/action.yaml Outdated Show resolved Hide resolved

use pnpm consistently

fe63628

himanshusinghs approved these changes Dec 3, 2025

View reviewed changes

nirinchev merged commit cb73d67 into main Dec 3, 2025
12 checks passed

nirinchev deleted the ni/pnpm branch December 3, 2025 20:46

chore: npm -> pnpm VSCODE-714 #1191

chore: npm -> pnpm VSCODE-714 #1191

Uh oh!

Conversation

nirinchev commented Dec 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

gagik left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

himanshusinghs Dec 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

himanshusinghs left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

nirinchev commented Dec 2, 2025 •

edited

Loading

himanshusinghs Dec 3, 2025 •

edited

Loading