rushstack icon indicating copy to clipboard operation
rushstack copied to clipboard

[rush] Doppelganger dependency created while using PNPM workspaces

Open matt-winfield opened this issue 2 years ago • 2 comments

Summary

I have two packages A and B, which both depend on react-redux@~8.0.2. Project B depends on project A. The dependency is listed identically in both projects package.json.

With PNPM workspaces disabled in rush.json, this works as expected and both projects symlinks point to the same folder. But with useWorkspaces enabled the projects point to different doppelgnager package folders.

Since they both use the exact same version, I would expect symlinks in both packages to point to the same package folder in /common/temp/node_modules/.pnpm. Instead two copies of the dependency folder are created with a random string (perhaps a hash?) appended to the end, and the packages point to different versions: image

According to the docs, doppelganger dependencies shouldn't happen when using pnpm: https://rushjs.io/pages/advanced/npm_doppelgangers/ (last sentence, scroll to the bottom)

As explained in the docs linked above, this causes issues with singleton instances (in the case of react-redux, it expects the Redux context to be a singleton. The packages referencing doppelgangers means that Package A methods relying on react-redux won't work instead a <Provider> created by Package B.)

Repro steps

  • Create 2 projects that depend on the same package version

  • Add the first project as a dependency of the first

  • rush update

    Expected result: Project A and Project B simlinks should point to the same package folder. No doppelganger package folders should exist

    Actual result: Two duplicate packages exist in /common/temp/node_modules/.pnpm, with random hashes appended. Package A and Package B symlinks point to different package folders.

Standard questions

Please answer these questions to help us investigate your issue more quickly:

Question Answer
@microsoft/rush globally installed version? 5.71.0
rushVersion from rush.json? 5.71.0
useWorkspaces from rush.json? true
Operating system? Windows
Would you consider contributing a PR? No, don't understand the codebase enough
Node.js version (node -v)? 16.15.0
PNPM Version pnpm 7.1.8

matt-winfield avatar Jun 05 '22 17:06 matt-winfield

Those aren't the same dependency; they have different peer dependency versions. Everything after the _ is the description of the peer dependency configuration. For a single peer dependency it is usually written out, but for multiple they get merged into a hash. You can inspect common/config/rush/pnpm-lock.yaml and look for those package names to see what the two different sets of peer dependency versions are.

dmichon-msft avatar Jun 08 '22 22:06 dmichon-msft

This is a huge PITA, I initially when migrating about 10 repos into a monorepo spent days digging through lock files and modifying package.jsons to find some set of dependencies which together result in singular dependencies in the tree.

For example, one time the resolution for deduplicating material-ui (which we noticed when styling went bonkers in the frontend) turned out to be adding a dev-dependency to @types/react in a library...

  • app
    • lib-frontend
    • lib-components (@types/react added here)

This to me is just bonkers. A @types/ dev dependency should have zero impact at runtime. Yes, @types/react should be included, but that's completely beside the point. The fact that I used @types/react in lib-frontend should NOT force me to include it in lib-components just to get correct styling on my page?!

Rants aside, this is a real problem.

To be pragmatic; how about this:

  1. Have rush validate that we have zero duplicate packages.
  2. Upon detecting a duplicate, report how to resolve this. It's far from trivial finding this yourself...

Thanks for an otherwise great tool! <3

majg0 avatar Sep 22 '22 10:09 majg0

I also "had" this problem too. Turned on workspaces, as I've been told Rush's linking will be deprecated, but had non-singleton singletons in my app. I'm working with NestJS, which has a plugin-module + DI system and requires that only one version of a module be installed, especially its own "core" module. It seems Rush is installing multiple versions of Nest's core module in my monorepo and thus, it breaks Nest's DI container badly.

Taking the tip above from David, I started sifting through the pnpm-lock.yaml file and found the core module being installed with two versions. I went and looked at what packages were "getting" them, found one strange one that shouldn't be there, found the package.json where it was being called for and deleted the dependency and low and behold, problem solved.

I agree with the suggestion above, if at all possible, Rush should be pointing out in a warning when it has to create multiple versions of the exact same versioned module, in fact creating the doppengänger Rush tries hard to avoid. That would be awesome. :)

Scott

smolinari avatar Oct 08 '22 10:10 smolinari

Are there any news about this? :( This issue breaks my NestJS apps.

duysolo avatar Oct 13 '22 04:10 duysolo

@duysolo - Try looking through the pnpm.lock.yaml as suggested above. You'll probably also find the doppergänger and it will point to doubled up or rather unnecessarily used packages. That's what happened with me and I'm also using NestJS (and now successfully).

Scott

smolinari avatar Oct 13 '22 07:10 smolinari

@smolinari how can you resolve the issue? Below is my application structure:

apps
    | mobile-api <-- This module use framework as dependency. It uses nestjs/core version 9.1.4
    | other-api...
libs
    | framework <-- This module will use different nestjs/core module, but the same version (9.1.4)

Every modules in apps use the same module nestjs/core, only the framework use different.

duysolo avatar Oct 13 '22 10:10 duysolo

I found the solution. In framework, I added one more dependency "@nestjs/platform-express" : "9.1.4" for express platform, or "@nestjs/platform-express" : "9.1.4" for fastify platform. Maybe the problem is rush cannot resolve the peer dependencies of @nestjs/core correctly.

duysolo avatar Oct 13 '22 10:10 duysolo

@duysolo - That was my problem. I had @nestjs/core being loaded twice and it was because one of the plugins I was using was also installing @nestjs/platform-express. I removed it from that libraries package.json and then all was working as it should.

Scott

smolinari avatar Oct 13 '22 11:10 smolinari

An @types/ devDependency may have zero impact at runtime, but it has a noticeable impact if you are compiling a TypeScript project against it, and pnpm has no way to know what your use case is. That is not to say that I don't see the utility of having a tool notify you that you have the same package with multiple peer configurations installed.

Turning on strictPeerDependencies: true at least blocks the state where you have a package installed in one project missing one of its peerDependencies. I've experimented in the past with tooling to inspect the lockfile and identify duplicate installations due to different peer configurations with an allow list, but it was done as a PR gate for an internal repository rather than being designed as a Rush feature.

You can use the common/config/rush/.pnpmfile.cjs afterAllResolved hook to check for this sort of thing and emit errors if you want to block it during rush update. It's a matter of scanning through the keys in the importers object and looking for ones that match up to the last _ in the name, then deciding based on a local allow-list if those are permissible or not.

dmichon-msft avatar Oct 13 '22 20:10 dmichon-msft

@dmichon-msft thank you for your great support 🍡

duysolo avatar Oct 14 '22 02:10 duysolo

I was just given this from @octogonz (thanks for that). Looks promising for resolving such doppelgänger errors.

https://github.com/microsoft/rushstack/issues/3695

Scott

smolinari avatar Oct 14 '22 06:10 smolinari