[rush] rush install fails with npm@5

[ryym]

Hi,

Does not Rush support npm@5 yet?
Using npm@5, rush install results in the error when common/temp directory does not exist.

ERROR: Failed to parse package.json for foo: Invalid version: "file:projects/foo.tgz"

(The same error as #354)

This becomes a problem when I run rush install for a cloned repository.
Usually common/temp does not be committed so rush install for a cloned repository always fail.

Reproduction steps:

1. set up package directories and rush.json
2. run rush generate
   • (rush install succeeds at this time)
3. remove common/temp
4. run rush install
   • ERROR: Failed to parse package.json for ...

I confirmed this error does not occur with [email protected].

environment:

• rush: 3.0.19
• npm: 5.3.0
• node: 6.9.5
• OS: macOS Sierra 10.12.6

[mtfranchetto]

Got the same error today.

EDIT: just found this, maybe it will help:

September 5, 2017 - Changes requiring action in Rush 3.0.15

Rush 3.0.15 changes some behavior in the temp modules folder to be compatible with NPM 5 and PNPM. You will likely see the following error during installation:

ERROR: Failed to parse package.json for : Invalid version: "file:projects/.tgz"
This is due to your shrinkwrap file needing a small update for Rush 3.0.15. To fix the issue, run rush generate and checkin the changes to the shrinkwrap file. (Alternately, you could change all the "file:projects/{PROJECT_NAME}" references in the shrinkwrap to "file:/{PROJECT_NAME}.tgz".)

[HipsterZipster]

@nickpape-msft This issue was raised 2 weeks ago and earlier in #354, can you please lend us a hand? Considering we have a scripted workaround, I'm happy to contribute the code change back to Rush if you could just point me in the right direction.

On Node 8.7.0 with NPM 5.4.2 (and all other versions of npm 5.x that we've used), the problem has been deterministic and repeatable for every member of my team. This was a total blocker for us so I figured out a hacky solution that lets us move forward.
@mtfranchetto @ryym try this out and let me know.

Before committing the [root]/common/config/rush/npm-shrinkwrap.json, run a node.js script (or just do a regex replace in your friendly text-editor) that changes:

• /file:projects\/([\w\-]+)+.tgz/ (flags: img)
  to
• file:projects\\\\$1

Can someone from the Rush team pls point me to where this code change should go in the underlying Rush lib?

[mtfranchetto]

@HipsterZipster Yes, it's working!

[ryym]

@HipsterZipster I confirmed too. It works!

[octogonz]

Hey guys, sorry about the slow response. I will investigate this morning.

FYI we tried upgrading to NPM 5 several times for our internal repos, but each time we had to rollback because of regressions in the NPM tool. We got really frustrated by this and have been experimenting with migrating to PNPM instead of NPM. It is a drop-in replacement that introduces a fundamentally different node_modules directory structure that makes the Rush symlinking algorithm much simpler, and should make installs a lot faster. The PR is here if you're interested. (We also looked at Yarn, but it's the same old crazy design as NPM, just with a much more solid implementation.)

Anyway if NPM 5 finally has a usable release, we should make sure it works.

[octogonz]

Update: This seems to be a bug in read-package-tree which is how "rush link" scans the node_modules folder. That library is part of NPM itself, but even when I upgrade to the version used by NPM 5, it still cannot read a folder containing tgz references. (Even though "npm install" and "npm shrinkwrap" are totally cool with these references.)

It seems to be due to some difference in the package.json files emitted by NPM. I'm trying to pin down what has changed exactly. But I suspect the fix will be in read-package-tree not Rush.

[octogonz]

Actually it's looking like this is yet another NPM 5 regression:

Rush generates a common/temp/package.json that does this:

  "dependencies": {
    "yargs": "~4.6.0",
    "z-schema": "~3.18.3",
    "@rush-temp/api-documenter": "file:./projects/api-documenter.tgz",

...and then our common/temp/projects/api-documenter.tgz contains a package.json like this:

{
  "name": "@rush-temp/api-documenter",
  "version": "0.0.0",
  "private": true,
  "dependencies": {

...but then NPM 5 installs a common/temp/node_modules/@ rush-temp/api-documenter/package.json that wrongly has this:

  "version": "file:projects/api-documenter.tgz"

...whereas under NPM 4 it looked like this:

  "version": "0.0.0"

[redacted for diplomatic reasons heheh]

[octogonz]

@nickpape-msft @qz2017 @iclanton FYI

[octogonz]

I started a conversation with NPM to see if they can fix this.

[HipsterZipster]

Thank you @pgonzal for your attention to this and for uncovering the underlying issue within NPM.
Did you create an issue with NPM? If so, can you provide link?

I assume this regression from the Rush perspective is an effect of the new file: symlink 'feature' (npm/npm#15900) in which case rush may need to run an after generate task like my regex replace above in order to support it. Wdyt?

I agree it'd be amazing if NPM5 upped its game and removed the regressions, especially for big firms that block Yarn registry and invest in their own npm registries and license filtering solutions.

pnpm looks like an interesting option, but may not be an option for for many corporate users since it uses hard links, which require admin permissions on Windows. grrrr 👎

EDIT:
Update: @pgonzal Seems like the pnpm approach to hard links does actually work in a non admin-enabled environment so I can't wait to try out Rush pnpm support whenever you think it's even remotely close to being ready! I'll add we're not using the Gulp build so really just core Rush support would be perfect.

[octogonz]

I was able to isolate a simple repro. I've opened NPM issue 19006.

In the meantime we'll see if we can come up with a better workaround.

@nickpape-msft FYI

[octogonz]

Rush 4 has been published. :-)

[ryym]

@pgonzal Thank you!