fix(nm): optimize hoisting by treating peer deps same as other deps

.yarn/versions/a6283714.yml

@@ -0,0 +1,25 @@
+releases:
+  "@yarnpkg/cli": patch
+  "@yarnpkg/nm": patch
+  "@yarnpkg/plugin-nm": patch
+  "@yarnpkg/pnpify": patch
+
+declined:
+  - "@yarnpkg/plugin-compat"
+  - "@yarnpkg/plugin-constraints"
+  - "@yarnpkg/plugin-dlx"
+  - "@yarnpkg/plugin-essentials"
+  - "@yarnpkg/plugin-init"
+  - "@yarnpkg/plugin-interactive-tools"
+  - "@yarnpkg/plugin-npm-cli"
+  - "@yarnpkg/plugin-pack"
+  - "@yarnpkg/plugin-patch"
+  - "@yarnpkg/plugin-pnp"
+  - "@yarnpkg/plugin-pnpm"
+  - "@yarnpkg/plugin-stage"
+  - "@yarnpkg/plugin-typescript"
+  - "@yarnpkg/plugin-version"
+  - "@yarnpkg/plugin-workspace-tools"
+  - "@yarnpkg/builder"
+  - "@yarnpkg/core"
+  - "@yarnpkg/doctor"

CHANGELOG.md

@@ -12,6 +12,7 @@ Features in `master` can be tried out by running `yarn set version from sources`
 - `node-modules` linker now honors user-defined symlinks for `<workspace>/node_modules` directories
 - `node-modules` linker supports hoisting into inner workspaces that are parents of other workspaces
 - `node-modules` linker attemps to hoist tree more exhaustivel until nothing can be hoisted
+- `node-modules` linker uses aggregated count of peer and regular usages to decide hoisting priority, instead of preferring peer usages over regular as before, which should result in fewer duplicates
 
 ## 4.1.0
 

packages/yarnpkg-nm/sources/hoist.ts

@@ -298,10 +298,10 @@ const getHoistIdentMap = (rootNode: HoisterWorkTree, preferenceMap: PreferenceMa
     const entry2 = preferenceMap.get(key2)!;
     if (entry2.hoistPriority !== entry1.hoistPriority) {
       return entry2.hoistPriority - entry1.hoistPriority;
-    } else if (entry2.peerDependents.size !== entry1.peerDependents.size) {
-      return entry2.peerDependents.size - entry1.peerDependents.size;
     } else {
-      return entry2.dependents.size - entry1.dependents.size;
+      const entry1Usages = entry1.dependents.size + entry1.peerDependents.size;
+      const entry2Usages = entry2.dependents.size + entry2.peerDependents.size;
+      return entry2Usages - entry1Usages;
     }
   });
 

packages/yarnpkg-nm/tests/hoist.test.ts

@@ -265,6 +265,43 @@ describe(`hoist`, () => {
     expect(getTreeHeight(hoist(toTree(tree), {check: true}))).toEqual(3);
   });
 
+  it(`should honor package popularity considering number of all references over number of references by peers`, () => {
+    // . -> A -> Z@X
+    //   -> B -> Z@X
+    //   -> C -> Z@X
+    //   -> D -> Z@Y
+    //        -> U -> Z@Y
+    // should be hoisted to:
+    // . -> A
+    //   -> B
+    //   -> C
+    //   -> D -> U
+    //        -> Z@Y
+    //   -> Z@X
+    const tree = toTree({
+      '.': {dependencies: [`A`, `B`, `C`, `D`]},
+      A: {dependencies: [`Z@X`]},
+      B: {dependencies: [`Z@X`]},
+      C: {dependencies: [`Z@X`]},
+      D: {dependencies: [`Z@Y`, `U`]},
+      U: {dependencies: [`Z@Y`], peerNames: [`Z`]},
+    });
+    const result = hoist(tree, {check: true});
+    expect(getTreeHeight(result)).toEqual(3);
+
+    const topLevelDeps = [...result.dependencies];
+    const hoistedZ = topLevelDeps.find(x => x.name === `Z`)!;
+    expect(hoistedZ.references).toContain(`X`);
+    expect(hoistedZ.references).not.toContain(`Y`);
+
+    const D = topLevelDeps.find(x => x.name === `D`)!;
+    const dDeps = [...D.dependencies];
+    expect(dDeps.length).toEqual(2);
+    const nestedZ = dDeps.find(x => x.name === `Z`)!;
+    expect(nestedZ.references).not.toContain(`X`);
+    expect(nestedZ.references).toContain(`Y`);
+  });
+
   it(`should hoist dependencies after hoisting peer dep`, () => {
     // . -> A -> B --> D@X
     //      -> D@X