[rush] pnpm fails to properly find peer dependencies when in both local and remote packages

[joelrbrandt]

Is this a feature or a bug?

• Feature
• Bug

Please describe the actual behavior.

Repo which demonstrates the bug: https://github.com/joelrbrandt/rush-peer-dep-test

Steps to repro:

1. Create a new rush repo with rush init using rush 5.27.0. Set strictPeerDependencies to true.
2. Create a local package (e.g., lib) with a peerDependency (e.g., eslint@*)
3. Create another local package (e.g., client) which depends on BOTH the first local package (lib) and some remote package with the same peerDependency (e.g., @typescript-eslint/[email protected])
4. Add the peerDependency (eslint) as a dependency of the second local package (client)
5. Run rush update
6. Observe that update incorrectly fails with an "unmet" peer dependency for eslint in the remote package.

The above repo demonstrates this scenario. The main branch fails on rush update. However, the fixed branch, which moves eslint from being a peerDependency to a regular dependency in lib, succeeds.

Comparison of main and fixed: https://github.com/joelrbrandt/rush-peer-dep-test/compare/fixed

The rush configuration in this repo is the default generated by rush init from rush 5.27.0, with two exceptions:
"strictPeerDependencies" is set to true, and "resolutionStrategy" is explicitly set to "fewer-dependencies" (though I believe this is rush's default). I also .gitignored the pnpm-lock.yaml file for ease of testing.

I have verified that this happens with other combinations -- it first cropped up for me with ts-loader and one of my local libraries (a shared eslint config) both having peerDependencies on TypeScript. I thought it might be something wrong with the ts-loader package, so I tried another example, and found the one present in the above repo.

What is the expected behavior?

peerDependencies are correctly seen as being met.

Workarounds

Option 1: Don't use any peerDependencies in any local packages. This is not an ideal workaround because sometimes peerDependencies really are the right solution (e.g., when publishing shared eslint configs)

Option 2: Note: this does not work with rush deploy (see below). Write a readPackage hook which moves the peerDependency in the remote package to a real dependency in that same package. This is a bit sketchy because there's no longer anything enforcing that the dependency be listed somewhere downstream as a real dependency. So, a developer could accidentally publish a package from this repo which does not correctly list all its dependencies.

Update: The workaround option 2 seems to be incompatible with rush deploy. I couldn't find a good way to get detailed error messages from rush deploy. However, when I munge peerDependencies using the readPackage hook, my deploys fail with the error ERROR: Cannot convert undefined or null to object. If I switch to workaround 1 (and change nothing else about my config), deploys work fine.

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

• Tool: rush / pnpm
• Tool Version: [email protected], [email protected]
• Node Version: 12.18.1
  • Is this a LTS version? yes
  • Have you tested on a LTS version? yes
• OS: macOS 10.15.5 Catalina

[joelrbrandt]

Saw that rush 5.27.1 was released an hour ago. This bug reproduces in 5.27.1 as well.

It also reproduces with pnpm at 5.2.6

[dfee]

I'm curious if this is the same issue as my issue (which is the one before your issue): #1973

[D4N14L]

Can confirm that this bug is resolved using workspaces (PR here: #1938).

[joelrbrandt]

@D4N14L

I'm now using pnpm workspaces. Thank you!

Even still, I ran into another flavor of this bug with unmet peer dependencies in sub-sub dependencies of webpack. I can't vouch for the guaranteed causality of this statement: it seems to have happened when I added webpack as a devDependency in one of my packages that was itself a devDependency of another one of my packages (and which also had a devDependecy of webpack). To spell that out:

my-rush-managed-package:
  devDependencies:
    - my-other-rush-managed-package
    - webpack

my-other-rush-managed-package:
  devDependencies:
    - webpack # adding this caused bug to occur

Interestingly, switching from [email protected] to [email protected] resolved the issue!

So, the main question: are there known problems with using pnpm@5 with rush?

[D4N14L]

So, the main question: are there known problems with using pnpm@5 with rush?

No. The warnings that appear for deprecated commands are okay, Rush has just not been updated to avoid passing these arguments. You should be able to use PNPM 5. However, if you do encounter any issues with it, please let us know since we will want to fix those.

[joelrbrandt]

@D4N14L first off, thanks for the quick reply!

Second, feel free to close this bug if that fits with your workflow. I'm not sure what your team's conventions are around closing bugs, but at present, my issue is resolved. I'll reopen if I encounter similar issues.

Third, thanks for your hard work on Rush. Our team over at Adobe recently adopted it, and we love it.

[D4N14L]

@joelrbrandt great, glad to hear it! Since your issue is resolved, I'll close out this bug for now. If you run into any other issues/have any feature suggestions, feel free to open up another bug, or reach out over Gitter :)

[Measy]

Hi @D4N14L Looks like I have a similar issue
#2820