Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[rush] NPM 5.x or newer ignores changes for Rush's "file:" version specifiers #708

Closed
jbcpollak opened this issue Jun 20, 2018 · 3 comments
Labels
external issue The root cause is with an external component that needs a fix or workaround

Comments

@jbcpollak
Copy link

Using the following versions:

rush 5.0.0-dev.23 (the global rush command was installed via npm i -g @microsoft/rush)
node 10.4.1
npm 5.6.0 (as configured in rush.json)

I have a repo where one project (app1) already depends on another project (repo-scripts) and the chance library. I then add these two dependencies to a third project (lib1) and run rush update, but the dependencies never appear for lib1.

For example, see this repo: https://github.com/6RiverSystems/monorepo-example

I was working with @pgonzal on this tonight, he said it was ok to name-drop him here. ;)

Since common/temp is never committed, you might not be able to reproduce this problem out of the box. To reproduce this situation do the following:

  • remove "chance" as a dev dependency from the lib1 project.
  • rm -rf common/temp
  • rush update
  • add "chance" back to lib1
  • rush update

Expected result: the removed library should now be in lib1/node_modules/chance

Actual result: the library is missing

@octogonz
Copy link
Collaborator

Thanks very much for providing this repro. I'll take a look asap.

@octogonz
Copy link
Collaborator

octogonz commented Jun 21, 2018

I was able to repro your issue (after running rush update --full to solve the integrity error from your issue #709).

Interestingly, it did NOT repro when I tried rolling back to NPM 4.5.0, which was the last "golden version number" we had settled on before switching to PNPM. For some context, last year we made several attempts to upgrade our biggest monorepo to NPM 5.x, but we kept encountering NPM bugs and eventually gave up. Some other smaller repos did seem to get 5.x working though, for example it looks like today OUIFR is using 5.4.0. So downgrading to 4.5.0 is a quick workaround, although the installs are noticeably slower.

Returning to your issue: The important thing to understand about Rush's installation strategy is that it does not try to solve the node_modules equation itself; instead when rush link is making the symlinked node_modules folder for each project, Rush stays faithful to WWND ("what would NPM do?") by analyzing what it finds in common/temp/node_modules. And if there is a mistake there, then Rush will replicate the mistake in your lib1/node_modules symlinks.

Before you add chance to lib1, the @rush-temp/lib1 project looks like this:

@rush-temp/lib1 file:

{
  "name": "@rush-temp/lib1",
  "version": "0.0.0",
  "private": true,
  "dependencies": {
    "@types/chai": "^4.0.8",
    "@types/mocha": "^2.2.44",
    "@types/node": "^8.0.52",
    "@types/uuid": "^3.4.3",
    "chai": "^4.1.2",
    "eslint": "^4.19.1",
    "eslint-config-6river": "^1.3.0",
    "eslint-plugin-mocha": "^5.0.0",
    "eslint-plugin-typescript": "^0.9.0",
    "gulp": "^3.9.1",
    "gulp-sourcemaps": "^2.6.4",
    "gulp-typescript": "^4.0.2",
    "merge2": "^1.2.2",
    "mocha": "^3.1.2",
    "mocha-junit-reporter": "^1.15.0",
    "npm-run-all": "^4.1.3",
    "nsp": "^3.2.1",
    "shx": "^0.3.0",
    "typescript": "^2.9.2",
    "typescript-eslint-parser": "^11.0.0"
  },
  "rushDependencies": {
    "repo-tools": "0.0.0-development"
  }
}

After you add "chance": "^1.0.13" to the real project package.json, rush update will correctly regenerate the above file to have the chance line in it.

Rush knows that npm install has a limitation with the file: references you see in the common project common/temp/package.json:

common/temp/package.json file:

{
  "dependencies": {
    "@types/bunyan": "^1.8.3",
    "@types/chai": "^4.0.8",
    "@types/express": "^4.0.39",
    "@types/lodash": "^4.14.110",
    "@types/loopback": "^3.2.2",
    "@types/loopback-boot": "^2.23.4",
    "@types/mocha": "^2.2.44",
    "@types/node": "^8.0.52",
    "@types/ramda": "^0.25.9",
    "@types/uuid": "^3.4.3",
    "chai": "^4.1.2",
    "chance": "^1.0.13",
    "compression": "^1.7.1",
    "cors": "^2.8.4",
    "coveralls": "^3.0.0",
    "csv": "^2.0.0",
    "db-migrate": "^0.11.1",
    "db-migrate-pg": "0.1.10",
    "eslint": "^4.19.1",
    "express": "^4.16.2",
    "express-csv-middleware": "0.1.0",
    "gulp": "^3.9.1",
    "gulp-sourcemaps": "^2.6.4",
    "handlebars": "^4.0.11",
    "helmet": "2.1.1",
    "istanbul": "0.4.5",
    "lodash": "^4.17.10",
    "loopback": "2.29.0",
    "loopback-boot": "^2.27.0",
    "loopback-component-bunyan": "1.3.0",
    "loopback-component-explorer": "2.5.0",
    "loopback-connector-postgresql": "2.5.0",
    "loopback-connector-remote": "1.3.1",
    "loopback-datasource-juggler": "2.46.1",
    "mocha": "^3.1.2",
    "mocha-junit-reporter": "^1.15.0",
    "moment": "^2.19.2",
    "moment-timezone": "0.5.11",
    "nsp": "^3.2.1",
    "nyc": "^11.3.0",
    "ramda": "^0.25.0",
    "request-promise": "^3.0.0",
    "rxjs": "^5.5.2",
    "serve-favicon": "^2.4.5",
    "sinon": "1.17.6",
    "source-map-support": "^0.5.0",
    "ts-node": "^3.3.0",
    "typescript-eslint-parser": "^11.0.0",
    "uuid": "^3.1.0",
    "@rush-temp/app1": "file:./projects/app1.tgz",
    "@rush-temp/lib1": "file:./projects/lib1.tgz",
    "@rush-temp/repo-tools": "file:./projects/repo-tools.tgz"
  },
  "description": "Temporary file generated by the Rush tool",
  "name": "rush-common",
  "private": true,
  "version": "0.0.0"
}

Whereas PNPM detects changes to "file:./projects/lib1.tgz" based on an integrity hash, NPM will never recheck it once it's been added to the common/temp/node_modules folder. Rush knows this, so in NPM-mode we do the following (which you'll see in your Rush logs):

  1. Run npm prune
  2. Delete everything under node_modules/@rush-temp/*
  3. Run npm install

In the past that was pretty reliable way to get NPM to reflect the update. However with Rush 5.6.0 I see the following:

  • Rush correctly put chance in this file: common/temp/projects/lib1/package.json
  • But NPM still has the old file contents here: common/temp/node_modules/@rush-temp/lib1/package.json
  • ...presumably because the old tarball is being cached here: common/temp/npm-cache/_cacache/content-v2/sha512/ac/c9 (which I figured out from the SHA in the shrinkwrap file)

I'm open to ideas for how to solve this. I would not be comfortable messing with NPM's cache folder.
Step 2 above is fairly expensive, so but maybe there's another way to make NPM recheck the tarball. As I mentioned, PNPM's solution is to calculate and compare an integrity hash for the tarball.

@octogonz octogonz changed the title [rush] update does not adding new dependencies to a project if they are already used by another project [rush] NPM 5.x or newer seems to be incompatible with "rush update" Aug 29, 2018
@octogonz octogonz added the external issue The root cause is with an external component that needs a fix or workaround label Sep 14, 2018
@octogonz octogonz changed the title [rush] NPM 5.x or newer seems to be incompatible with "rush update" [rush] NPM 5.x or newer ignores changes for Rush's "file:" version specifiers Oct 17, 2018
@halfnibble
Copy link
Contributor

NPM 5 is no longer supported by npm and will not receive the necessary bugfix. I'm closing this issue in favor of issue #886.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
external issue The root cause is with an external component that needs a fix or workaround
Projects
None yet
Development

No branches or pull requests

3 participants