Pinning bundled npm does not work

[Sh4rK]

Hi,

If I pin a node version in a project (let's say 15.11.0), this is added to package.json:

  "volta": {
    "node": "15.11.0"
  }

So far so good.

However, if I want to make sure that the bundled npm is used, and run volta pin npm@bundled, nothing happens, package.json is not updated with anything.

This is problematic, because there's no distinction between two cases:

• I want to use the bundled npm.
• I want to use the global default npm.

And in fact, if I set a different npm version as global default, that's gonna be the one that's executed in the project directory, not the bundled one.

To remedy this, I think there could be two solutions:

• If node is pinned in the project, but npm is not, use bundled npm.
  This would intuitively make sense to me, since there supposedly wouldn't be much value in fixing node, but allowing whatever npm, since the point is to standardize the environment (and also, for example, the package-lock.json format can depend on npm version, so you want to keep it aligned between developers).
• Introduce bundled as a valid version for npm in package.json, like on the command line.
  This is more explicit than the above, but for reasons explained there, I prefer that option to this one.

What do you think?

[charlespierce]

Hi @Sh4rK, thanks for reporting! That is a gap, and I think I agree with your first solution: If Node is pinned but npm isn't, then we use the bundled version. That fits with the intent of npm@bundled and with how we currently write it without any settings.

[Sh4rK]

Great! I'll take a shot at the implementation. Can you give some pointers on where to start looking?

[charlespierce]

@Sh4rK That would be great, I'd be happy to review and help. The merging of project and default platforms happens in the Platform::current associated function here: https://github.com/volta-cli/volta/blob/main/crates/volta-core/src/platform/mod.rs#L249

What it currently does is: If either yarn or npm isn't set on the project, then load the default and merge the two together.

Looking at the structure of that function, I think the change is relatively straightforward. Since the Project platform won't be set at all if Node isn't pinned (that's the baseline requirement), if we're in that branch at all, then we know for sure that we have a Node version. So I believe the only change necessary is to remove the check against npm and the merging logic for npm. If we have Node but no custom npm, that's assumed to be "bundled", so we shouldn't try to merge. And if we don't have Node at all, then we're using the default platform all the way through.