[rush] Running rush update when adding a new package to the rush.json installs extra packages in the pnpm-lock file

[rochaudh]

Is this a feature or a bug?

• [ ] Feature
• [X] Bug

Please describe the actual behavior. When creating a new package, adding the reference to the rush.json, and then running rush update, there seem to be extra packages installed in the pnpm-lock.yaml file.

In addition to the package that was added, the pnpm-lock file adds references to packages with different versions and there are new dependencies added to existing packages. This occurs even if the new package.json is an exact copy of the other package.json files.

Reason this is a problem: We scan all our packages and their dependencies as a part of OSS security and compliance from the pnpm-lock file. If there are unnecessary package versions added, we have to manually remove these references before we can get an accurate representation of the packages and dependencies we pull in.

This occurs when all the package.json files in the repo have the same dependencies and versions for that package.

If the issue is a bug, how can we reproduce it? Please provide detailed steps and include a GitHub branch if applicable. Your issue will get resolved faster if you can make it easy to investigate.

Please see this branch and the latest commit to see the extra packages that were added: Branch name: users/rochaudh/test_rush Commit history: https://github.com/rochaudh/RushWithReact/commits/users/rochaudh/test_rush Commit with pnpm-lock file: https://github.com/rochaudh/RushWithReact/commit/ecd2f2676a15e03a4a6f29e264daab1d37230edd

1. Create a new package to a repo with existing packages (preferably a repo with only one package currently). I copied the first package and just changed the name. As a result, the package.json is exactly the same.
2. Run rush update and notice that the pnpm-lock file was updated with extra dependency packages and different versions of the packages (along with the reference to the package that was added).

What is the expected behavior? The pnpm-lock file should only include the reference to the new package added.

If this is a bug, please provide the tool version, Node.js version, and OS.

• Tool: Rush
• Tool Version: 5.16.1
• Node Version: v10.16.0
  • Is this a LTS version? yes
  • Have you tested on a LTS version? yes
• OS: Windows 10

[iclanton]

This most likely has happened because a new version of a dependency was published causing a version selector in a package.json (either in a project in your repo, or in a dependency) to match a newer version. rush update both re-analyzes the repo's dependency graph and tries to pull in new versions from package feeds. @apostolisms - maybe we can consider modifying that behavior?

The way many large organizations mitigate this is by using a private feed that proxies the public npmjs.org feed, but only allows approved packages and versions to be proxied and cached. If you point your Rush repo at a feed with that behavior, as far as Rush knows, the only packages that exist are the ones that you've already approved.

[octogonz]

This most likely has happened because a new version of a dependency was published causing a version selector in a package.json (either in a project in your repo, or in a dependency) to match a newer version. rush update both re-analyzes the repo's dependency graph and tries to pull in new versions from package feeds.

This is really only true for rush update --full. The rush update preserves the old package-lock.yaml, so that PNPM will try to perform a minimal incremental set of changes to satisfy the new requirements. The rules for how this works aren't very formalized to my knowledge, but if we want to make it conservative at the level of rush update, it seems like this would need to be an enhancement for PNPM, not Rush.

@zkochan FYI

[octogonz]

@zkochan We had a different issue today where the rushstack repo's master branch started failing to build, even though the shrinkwrap file had not changed in Git. It seems that when running pnpm install, PNPM is upgrading portions of the shrinkwrap file that are not affected by any changes in the package.json files.

This essentially means that we've lost determinism for our repo history. If we go back in time and try to install historical branches, they will now fail because handlebars gets upgraded.

Repro

1. Unzip this archive (which is based on running rush install against bef2124bcd6dde8c78fdc2f931e4fa684141b848 ):

   pnpm-lock-issue-1670b.zip

2. Delete pnpm-lock.yaml

3. Copy pnpm-lock-preinstall.yaml to be pnpm-lock.yaml

4. Run pnpm install using PNPM 3.1.1 (which created this lockfile)

Expected: PNPM may update some entries if they are out of sync with package.json, but it should not be upgrading handlebars.

Actual:

If you diff the before/after files, you will see that this:

pnpm-lock-preinstall.yaml

  /istanbul/0.3.22:
    dependencies:
      abbrev: 1.0.9
      async: 1.5.2
      escodegen: 1.7.1
      esprima: 2.5.0
      fileset: 0.2.1
      handlebars: 4.5.3
      js-yaml: 3.13.1
      mkdirp: 0.5.1
      nopt: 3.0.6
      once: 1.4.0
      resolve: 1.1.7
      supports-color: 3.2.3
      which: 1.3.1
      wordwrap: 1.0.0
    deprecated: |-
      This module is no longer maintained, try this instead:
        npm i nyc
      Visit https://istanbul.js.org/integrations for other alternatives.
    dev: false
    hasBin: true
    resolution:
      integrity: sha1-PhZNhQIf4ZyYXR8OfvDD4i0BLrY=

...gets changed to this:

pnpm-lock.yaml

  /istanbul/0.3.22:
    dependencies:
      abbrev: 1.0.9
      async: 1.5.2
      escodegen: 1.7.1
      esprima: 2.5.0
      fileset: 0.2.1
      handlebars: 4.7.0  <====== WHY WAS THIS UPGRADED?
      js-yaml: 3.13.1
      mkdirp: 0.5.1
      nopt: 3.0.6
      once: 1.4.0
      resolve: 1.1.7
      supports-color: 3.2.3
      which: 1.3.1
      wordwrap: 1.0.0
    deprecated: |-
      This module is no longer maintained, try this instead:
        npm i nyc
      Visit https://istanbul.js.org/integrations for other alternatives.
    dev: false
    hasBin: true
    resolution:
      integrity: sha1-PhZNhQIf4ZyYXR8OfvDD4i0BLrY=

[octogonz]

Seems that the issue does not repro with PNPM 4.7.1 (the latest release).

[zkochan]

This seems like the same as https://github.com/pnpm/pnpm/issues/2226 which was fixed in pnpm v4.6.0

[octogonz]

Thanks @zkochan ! This issue is serious enough that we probably need a solution for repos that cannot upgrade to PNPM 4 immediately.

Do you know which version ranges are affected? What's the best way to workaround it?

I saw the thread mentioned setting hoist=false - will that work for PNPM 3.1.1?

[zkochan]

hoist was added in v4. In v3 this is probably happening when shamefully-flatten is true.

[octogonz]

Hmm... we're not using shamefully-flatten in this repro.