[rush] Don't install peerDependencies if the autoInstallPeers setting isn't true

[davemyersworld]

Summary

Fixes #4843

When a rush monorepo is configured to use pnpm with useWorkspaces:false, and autoInstallPeers:false (the default), peerDependencies of monorepo packages are incorrectly installed.

Details

I fixed the problem by testing for this exact case.

I'm not sure if other package managers should be considered as well (thus far, I have not).

How it was tested

By running testrush update such that the code in this PR executed against my repro-repository and confirming no react-router in the node_modules directory

Impacted documentation

none

[iclanton]

What about optional peer dependencies? Is that case missing here?

[davemyersworld]

What about optional peer dependencies? Is that case missing here?

@iclanton Good question. I looked at how PackageJsonEditor organizes dependencies, and it looks like it doesn't distinguish between normal peerDependencies and ones with a peerDependenciesMeta entry.

According to the PNPM docs, auto-install-peers: true is not supposed to install the optional peerDependencies.

So, I think an appropriate fix would be to add a further conditional to ensure that optionalPeers never get added to the tempDependencies map.

I'll take a look at that tomorrow, thanks.

[davemyersworld]

What about optional peer dependencies? Is that case missing here?

@iclanton Good question. I looked at how PackageJsonEditor organizes dependencies, and it looks like it doesn't distinguish between normal peerDependencies and ones with a peerDependenciesMeta entry.

According to the PNPM docs, auto-install-peers: true is not supposed to install the optional peerDependencies.

So, I think an appropriate fix would be to add a further conditional to ensure that optionalPeers never get added to the tempDependencies map.

I'll take a look at that tomorrow, thanks.

Update: Having looked at it, it appears that PackageJsonEditor.ts doesn't account for peerDependenciesMeta (though it does have a field for dependenciesMeta. So, this will take a bit more to incorporate than I expected, it'll have to wait for next week as I'm traveling for a bit.

[iclanton]

@davemyersworld - did you get a chance to address the optional peers issue?

[davemyersworld]

@iclanton Thanks for checking in.

For reasons unrelated to this PR, the project I was working became blocked, so my effort on this PR did as well.