[rush] with pnpm installs patch versions newer than 'locked' versions

[matteralus]

rush: 5.5.4
pnpm: 2.25.7

Rush docs claim: "Rush's unique installation strategy produces a single shrinkwrap/lock file for all your projects that installs extremely fast. "

The top of my shrinkwrap.yaml looks like this:

dependencies:
'@babel/core': 7.4.4
'@babel/polyfill': 7.4.4
'@babel/preset-env': 7.4.4
'@babel/preset-typescript': 7.3.3
'@types/chai': 4.1.7
'@types/lodash': 4.14.123

But on a clean install of my app I got this:

dependencies:
+ @babel/core 7.4.4
+ @babel/polyfill 7.4.4
+ @babel/preset-env 7.4.4
+ @babel/preset-typescript 7.3.3
+ @types/chai 4.1.7
+ @types/lodash 4.14.125

"rush install" from scratch installed a newer version of @types/lodash that broke my build. Luckily not any actual functional code, but this is dangerous, deploying the same app to multiple nodes is like rolling the dice as far as what patch versions each instance gets.

The 'specifier' section of the yaml does have an @types/lodashs with a ^, but if this is what really is considered the 'locked' version there should be some documentation that explains this.

If this is working as designed it should be documented somewhere you have to start out with pinned versions on all your package.jsons

[octogonz]

This sounds like incorrect behavior, but it's difficult to tell without more details. It sounds like you're saying that when rush install invoked pnpm install in the common/temp folder, the installed packages there did not match the plan described in shrinkwrap.yaml. (Or, perhaps it did get installed correctly, but then the symlinks for a particular project got wired up wrong?)

I haven't heard of anyone else reporting this problem.

Could you first try running rush check and make sure it doesn't print out any warnings?

If it's still occurring, maybe you could share a branch that repros the issue?

[HarryGifford]

@octogonz I'm having this issue too. Similar to #1219. After running rush install Rush generates a new shrinkwrap in /common/temp/shrinkwrap.yaml which is different to the one in /common/config/rush/shrinkwrap.yaml.

[octogonz]

The intended design is that common/temp/shinkwrap.yaml will be (1) a correct solution of the version ranges, and it will be (2) a deterministic outcome (i.e. unaffected by new versions being published to the registry). It is /not/ intended to be (3) exactly the same as common/config/rush/shrinkwrap.yaml. This relaxation avoids unnecessary modifications to the file tracked by Git, which can cause merge conflicts, which are a significant concern in very active monorepos.

So the question here is whether (1) or (2) is being violated, not (3).

[HarryGifford]

@octogonz In our case we have a Component Governance tool which scans the build directory and reads any package.json, package-lock.json, shrinkwrap.yaml files etc. and gives a warning whenever new packages are added. Because the temp shrinkwrap changes, we're seeing these warnings even when the config shrinkwrap is unchanged.

When running rush install I'd like it to use the config shrinkwrap and not generate a new one, as it means that different people will get different versions of packages when they install them. Are we using the shrinkwrap in the wrong way?

[octogonz]

When running rush install I'd like it to use the config shrinkwrap and not generate a new one, as it means that different people will get different versions of packages when they install them.

@HarryGifford Rush used to work this way, but it meant every time you edit package.json it would create a diff for the shrinkwrap file. This led to lots of unnecessary merge conflicts.

In our case we have a Component Governance tool which scans the build directory and reads any package.json, package-lock.json, shrinkwrap.yaml files etc. and gives a warning whenever new packages are added. Because the temp shrinkwrap changes, we're seeing these warnings even when the config shrinkwrap is unchanged.

What's the problem with that? The temp shrinkwrap file is supposed to be a deterministic function of the package.json files, so it should not change unless the commit includes a change to a package.json file somewhere. (BTW we're on Gitter if you want to chat about this in realtime.)

[matteralus]

Repro repo: https://github.com/matteralus/rush-newer-dep-repro

@octogonz Sorry for taking so long on this...I saw there was a repro for transitive dependency but I wanted to make sure there was one for a direct dependency.

Note that I had to install @types/lodash at a specific version and then 'hack' the package.json and specifiers section of pnpm-lock.yaml with the '^' to get things in the right state. When you do 'rush install' you will see the latest version of @types/lodash get installed instead of 4.14.123

Some other information: I was not able to reproduce this with the default version of pnpm, it looks like slightly different options are passed when going from the default to 3.x.x

pnpm-2.15.1\node_modules.bin\pnpm install --store C:\proj\rush-newer-dep-repro\common\temp\pnpm-store --no-lock --no-prefer-frozen-shrinkwrap --reporter ndjson

pnpm-3.4.1\node_modules.bin\pnpm install --store C:\proj\rush-newer-dep-repro\common\temp\pnpm-store --no-lock --no-prefer-frozen-shrinkwrap --reporter ndjson --resolution-strategy fewer-dependencies

[octogonz]

@zkochan this seems like a PNPM bug.

Isolated repro steps:

1. Run npm install -g @microsoft/rush
2. Clone https://github.com/matteralus/rush-newer-dep-repro from above
3. Run rush install in that folder, to generate the tarballs
4. cd common/temp
5. In the common/temp folder, rimraf the node_modules folder.
6. In that folder, overwrite pnpm-lock.yaml with the contents of pnpm-lock-preinstall.yaml
7. Run a plain old pnpm install using the latest version e.g. PNPM 3.5.0

Expected: Lodash 4.14.123 gets installed, because the lockfile specifies this installation plan (and it is not inconsistent with any package.json file).

Actual: The package-lock.json file gets upgraded to 4.14.134.

@iclanton FYI

[sachinjoseph]

@octogonz Is this the same issue as the one here? pnpm/pnpm#1876

[octogonz]

Yes, that is what Zoltan suspected. Seems that downgrading PNPM might be a possible workaround.

[sachinjoseph]

@octogonz I see some inconsistency in pnpm install behavior which I think is related. I have opened an issue here pnpm/pnpm#1878

[matteralus]

I saw some fixes went in for #1142..I retested this with rush 5.10.3 and pnpm 3.6.0 and the the version of @lodash/types is being pinned as I would expect by the pnpm-lock.yaml