[rush] rush update corrupts the npm-shrinkwrap.json file.

[scsewall]

I am using Rush 5.1.0.

I edited the package.json of one of the projects in my monorepo to add a new dependency.

I then ran "rush install" and it correctly told me that I needed to run "rush update".

After running "rush update", the diff on npm-shrinkwrap.json shows the following empty object for each project in the monorepo:

"@rush-temp/my-test-app": {},

Instead of:

"@rush-temp/my-test-app": {
"version": "file:projects/my-test-app.tgz",
"integrity": "sha1-t0BfXTi6wyKD/G+nbngFUTuNeaI=",
"requires": {
list of packages
}
},

After this, all rush install/update commands fail because the empty object trips it up. The only way out is to delete npm-shrinkwrap.json and re-run rush update.

Am I using an incorrect workflow?

[octogonz]

Does it repro with NPM version 4.5.0?

[scsewall]

I currently have Rush configured to use NPM 5.5.1. I tried to move back to NPM 4.5.0 but ran into strange npm install errors during rush install. I also ran into different errors when I tried to move up to NPM 6.4.1. I then tried to switch to PNPM but am running into "missing dependencies". That is, some projects are not enumerating all of their dependencies but are getting away with it when using NPM. Would your recommendation be to move to PNPM + fix the missing dependencies?

[octogonz]

Would your recommendation be to move to PNPM + fix the missing dependencies?

Generally people don't have too much trouble figuring out how to handle the "bad" packages with PNPM. Their Gitter channel is a great resource for help. But if you can't get it working, I'd suggest to try the Yarn option. We're very interested to have people try out the new Yarn support, and get some feedback about how well it works. Whereas with NPM many of the issues are beyond our control unless NPM itself can fix the regressions.

We like PNPM since it's definitely the most technically correct solution, and also it seems to have the least bugs in our experience (particular with regards to the file:/package.tgz references that Rush relies on). But we're a little biased because we support very large monorepos that encounter every last unlikely edge case. :-) For smaller repos these tradeoffs are less significant.

There's some more details in the docs: https://rushjs.io/pages/maintainer/package_managers/

[kenotron]

I hit this as well with officedev/office-ui-fabric-react

[octogonz]

@kenotron What version of NPM are you using?

[kenotron]

5.4.0 which is the version we had been using since Fabric 5.x. Mind you, we're doing something different here that we didn't do before. We used to always do rush generate which blew away everything and starts to get all the new versions of deps afresh. That's the equivalent to rush update --full. I'm just trying to get the smallest update possible via rush update. I'm going to dig into rush code a little bit to see if I can help...

[octogonz]

@kenotron I just encountered this issue today also while working in the office-ui-fabric-react repo. It's really strange: The node_modules\@rush-temp\styling folder contains a perfectly valid package.json file, but for some reason npm shrinkwrap ignores all the dependencies specified there. I used rush update --purge so it's a deterministic repro.

If I get some time I'll see if I can debug this a little more.

[octogonz]

Okay after 2 hours of staring at npm install progress bars, I figured out that the npm shrinkwrap command is what corrupts the file. Rush runs this after npm install in order to rename package-lock.json to npm-shrinkwrap.json (so we don't have to manage different filenames depending on the NPM version).

I'll do a little more digging and see if I can figure out why this is happening. I was totally expecting our _fixupNpm5Regression() function to be the culprit, but the issue repros with this code completely disabled. It may simply be an NPM bug.

[octogonz]

It's just an NPM bug. If I run npm shrinkwrap using the latest NPM 6, it does not blank out the entries.

I suppose we could get this working by having Rush itself rename the package-lock.json file rather than invoking npm shrinkwrap. I wouldn't want to stop calling the file npm-shrinkwrap.json however unless we drop support for NPM 4...

...which is blocked because NPM 5 seems to have similar bugs as NPM 6 in its handling of file: dependencies (see #708). So if we're going to get this working, I think we might as well focus on NPM 6 and forget about NPM 5.

The quick fix would be to delete the entire NPM cache every time we do an incremental install. This sucks for install times, but it would at least give people a reliable working solution and let them use a modern version of NPM. Of course the better options would be (1) delete the specific cache entry based on its SHA key, as suggested in the issue I linked, or (2) get NPM to finally fix their regression, but those improvements can be made later.

@kenotron would that plan work for you? Assuming it doesn't reveal a bunch of other lurking NPM issues, this shouldn't be a whole lot of work. IIRC the issue I got stuck on last time was read-package-tree incompatibilities, but presumably someone fixed that by now.

[kenotron]

I don't mind moving ahead with npm 6 since even the latest LTS node auto installs that for you - so it's more ubiquitous now. How much work would it be to do (1), in your opinion? Is it something rush can provide as a temporary solution until npm can fix their bug (do they have a bug on them on this?)

[scsewall]

Adding my comments since I logged the original issue...

We are still plagued by this problem, so having a working solution (even if it is slower) would be better than the status quo. Moving back to NPM 4 (from NPM 5.5.1) was never going to be an acceptable workaround for me, but moving ahead seems reasonable.

I made progress tracking down the "phantom dependencies" in my monorepo, so I may address this by switching Rush to use PNPM.