Skip to content

feat(monorepo-tools): add pnpm mode support#621

Closed
gagik wants to merge 4 commits intomainfrom
gagik/pnpm
Closed

feat(monorepo-tools): add pnpm mode support#621
gagik wants to merge 4 commits intomainfrom
gagik/pnpm

Conversation

@gagik
Copy link
Collaborator

@gagik gagik commented Feb 26, 2026

We have hardcoded npm function calls here, this adjusts it to support PNPM mode with MONOREPO_TOOLS_USE_PNPM=true.

We have hardcoded npm function calls here, this adjusts it to support PNPM mode with MONOREPO_TOOLS_USE_PNPM
if (packageLockOnly) {
// npm uses --package-lock-only, pnpm uses --lockfile-only
args.push(
packageManager === 'pnpm' ? '--lockfile-only' : '--package-lock-only',
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need a pnpm-lock.yaml to align behavior locally and on CI? And if we do add pnpm-lock do we have a way of keeping it in sync with package-lock.json?

Copy link
Collaborator Author

@gagik gagik Feb 26, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if a repo uses pnpm, then we will only have pnpm-lock, we should not have package-lock.
One can also set pnpm as default package manager in package.json so then even if someone runs npm i it'll redirect to pnpm

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These tools could try to look for pnpm-lock to determine what mode to run in maybe but pnpm lock might be at a higher level folder etc so env just seems more flexible

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Understood but this PR is for adding optional use of pnpm, so there is no pnpm-lock file, as a result is anyone who uses pnpm going to suffer from a broken experience at least locally when they go to install and they get a very different set of transient dependencies than are used on CI? (as a comparable example, I often accidentally work in compass on the wrong npm version and even that deviation causes a change in the result of npm i and tests fail)

Main concern here, sure we exec pnpm, but is it useful? (will it be useful? because it may work today but when it breaks what do we do about it to fix it, what will be the urgency to fix it, etc.)

@gagik
Copy link
Collaborator Author

gagik commented Feb 26, 2026

Seems like Ubuntu runner was blocked by coveralls... Tests succeeded otherwise.

@kmruiz
Copy link
Collaborator

kmruiz commented Feb 26, 2026

Before implementing pnpm everywhere, which is a pretty important change, if what we want is better performance we should first try npm with the "linked" install strategy, which uses the same package-lock.json but it is faster, as mimicks how pnpm works internally.

@gagik
Copy link
Collaborator Author

gagik commented Feb 26, 2026

@kmruiz I don't disagree, it's worth trying; I was originally misled and thought we had a consensus on interest in pnpm.

That said this PR doesn't adopt it on its own, just makes it possible to at least run experiments in the repos which use our monorepo tools. We can try npm as well but it'd still be worth having something to compare to, so unblocking that effort to be able to experiment.

On a more general level though, pnpm goes beyond pure performance:

  • for local development, pnpm's global store means faster installations across repos which share packages (somewhat common with our products). Also means less redundant storage of node_modules.
  • we can pin pnpm versions to ensure consistency across people working with the repository, most notably avoiding the mess with package-lock being different depending on npm version.
  • in my opinion better monorepo tooling and support, helping us avoid the some patch scripts we have to deal with some of npm's limitations.

@nbbeeken
Copy link
Collaborator

Gaining consensus on something like this is usually through a scope and design so we can account for the impact and we can plan time to dedicate to the effort it takes to migrate. You make great points about pnpm's design and Kevin's suggestion for trying npm's install-strategy should be accounted for in the design of either a migration or dual package manager support.

Sorry if you were misled! I think the situation is that folks have generally been somewhat unprompted about this issue. The vscode and mcp-server adoption of pnpm didn't impact me so I had no stake in the matter one way or the other. I think we're finally crossing the boundary into needing more stakeholders to share their input on this matter and we should open that forum, we can start simply with a slack thread but I think there will be value in planning this out in a document.

If this PR enables optional use of pnpm and that use won't lead to difficulties with the lockfile or other parts of the tooling then I think it may be fine to keep this train rolling. 🙂

@kmruiz
Copy link
Collaborator

kmruiz commented Feb 26, 2026

It's a matter of consistency, this repository is used across all our tooling and every single inconsistency across npm and pnpm will just spread around our repositories. pnpm handles dependency resolution differently than npm, and if at some point we need overrides, we need to maintain them for both npm and pnpm.

For libraries, we don't publish the package-lock.json as it's the ending product which should be able to override and define it's own dependencies.

for local development, pnpm's global store means faster installations across repos which share packages (somewhat common with our products). Also means less redundant storage of node_modules.

If that's the point, have you tried the linked installation policy of npm? Officially it's called isolated mode and solved most of the problems (if not all, to be fair) that pnpm tries to fix itself.

we can pin pnpm versions to ensure consistency across people working with the repository, most notably avoiding the mess with package-lock being different depending on npm version.

The problem is not across pnpm versions, is about npm and pnpm. We are not going to change mongosh for example in the short term, and we mainly use npm everywhere. And we in general don't commit the package-lock.json on libraries.

in my opinion better monorepo tooling and support, helping us avoid the some patch scripts we have to deal with some of npm's limitations.

We already use lerna and workspaces across many products successfully, and while I'm sure it can be improved in several ways, the consequences of adopting pnpm in this repository might be far more detrimental to the stability of our overall portfolio than the possible benefits of simplified workspace resolution.

I think we're finally crossing the boundary into needing more stakeholders to share their input on this matter and we should open that forum, we can start simply with a slack thread but I think there will be value in planning this out in a document.

I do agree! But we should start with a problem statement. What are we trying to solve? We've rushed to adopt pnpm in some of our products without consensus, and while they are end products so the impact is low (like the mcp server) we've already had problems with vscode because we have a test suite in mongosh for testing the integration. While the issue with vscode+mongosh is solved, we've had the CI broken for months because at the end, maintaining two dependency management systems has a cost.

We should be more careful adopting technologies, because while pnpm might look good, there has been many forks and alternatives to npm in the past that are now deprecated or abandoned, and while npm might not be the best (and I can assure that I don't precisely love it), it's the standard and is pretty well suited to what we do.

@gagik
Copy link
Collaborator Author

gagik commented Feb 27, 2026

If this PR enables optional use of pnpm and that use won't lead to difficulties with the lockfile or other parts of the tooling then I think it may be fine to keep this train rolling. 🙂

Yeah this requires explicit opt-in through an environment variable to even get to PNPM mode, so it's not like this library will suddenly start enforcing PNPM or a different lockfile.

In the current reality, monorepo tools cannot be used in vscode or MongoDB MCP Server. We aren't using them so it doesn't matter, but as far as it goes it doesn't make the status quo of mongosh / Compass usage of npm any different.

while they are end products so the impact is low (like the mcp server) we've already had problems with vscode because we have a test suite in mongosh for testing the integration

I have fixed this test_vscode many times, it breaks often and from many subtle changes to the vscode and its CI, so pnpm is anything that different there. I had many more issues with npm package-lock resolutions. Granted, the projects with pnpm I have used have been simpler, so it's not necessarily better.

What are we trying to solve?

It's a great question but I believe sometimes an experiment doesn't need an answer to this.
My motivation was to investigate how much effort and impact a potential migration could do. To me, if it takes <1 hour to get a POC going, it's a way more meaningful starting point to get feedback and afterwards write a scope/TD from rather than spending hours on discussion and technical writing working with pure hypotheticals about i.e. work required or performance impact.

Since pnpm is also adopted by the cloud org (and half of our repos, but that's more arguable), it was also my impression that we're way past more core discussions about whether pnpm carries a deprecation risk. On that front, I can't say I'm more sold on stability of a NPM mode whose best documentation is an RFC prooposal.

That all said, this was mainly driven by curiosity and desire to do something interesting on the side while I'm doing some more routine tasks rather than any strong desire to push this, especially since I did not realize people were not that sold on adopting it. This was meant to isolated in a experiment PR mongodb-js/mongosh#2682 but I admit this stopped being an experiment with this PR which was actually looking to be merged.

So I will pause my effort in that direction for now, it was reaching the limit I was going to bother testing it out with anyways. We can revisit this PR and other changes afterwards 🙂

@gagik gagik closed this Feb 27, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants