fix(nm): apply hoisting algorithm on aliased dependencies (#4237)

* Apply hoisting algorithm on aliased dependencies

* Adds a unit test to check hoisting of the aliased packages

* Updates conditions to detect hoisting loops to account for aliases

Author: larixer

.yarn/versions/6983a0de.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

@@ -8,6 +8,7 @@ Yarn now accepts sponsorships! Please give a look at our [OpenCollective](https:
 
 ### Installs
 - The pnpm linker no longer tries to remove `node_modules` directory, when `node-modules` linker is active
+- The nm linker applies hoisting algorithm on aliased dependencies
 
 **Note:** features in `master` can be tried out by running `yarn set version from sources` in your project (existing contrib plugins are updated automatically, while new contrib plugins can be added by running `yarn plugin import from sources <name>`).
 

packages/yarnpkg-nm/sources/hoist.ts

@@ -52,6 +52,7 @@ export enum HoisterDependencyKind {
 export type HoisterTree = {name: PackageName, identName: PackageName, reference: string, dependencies: Set<HoisterTree>, peerNames: Set<PackageName>, hoistPriority?: number, dependencyKind?: HoisterDependencyKind};
 export type HoisterResult = {name: PackageName, identName: PackageName, references: Set<string>, dependencies: Set<HoisterResult>};
 type Locator = string;
+type AliasedLocator = string & { __aliasedLocator: true };
 type Ident = string;
 type HoisterWorkTree = {name: PackageName, references: Set<string>, ident: Ident, locator: Locator, dependencies: Map<PackageName, HoisterWorkTree>, originalDependencies: Map<PackageName, HoisterWorkTree>, hoistedDependencies: Map<PackageName, HoisterWorkTree>, peerNames: ReadonlySet<PackageName>, decoupled: boolean, reasons: Map<PackageName, string>, isHoistBorder: boolean, hoistedFrom: Map<PackageName, Array<string>>, hoistedTo: Map<PackageName, string>, hoistPriority: number, dependencyKind: HoisterDependencyKind};
 
@@ -576,6 +577,8 @@ const getNodeHoistInfo = (rootNode: HoisterWorkTree, rootNodePathLocators: Set<L
   }
 };
 
+const getAliasedLocator = (node: HoisterWorkTree): AliasedLocator => `${node.name}@${node.locator}` as AliasedLocator;
+
 /**
  * Performs actual graph transformation, by hoisting packages to the root node.
  *
@@ -591,10 +594,11 @@ const hoistGraph = (tree: HoisterWorkTree, rootNodePath: Array<HoisterWorkTree>,
   let anotherRoundNeeded = false;
   let isGraphChanged = false;
 
-  const hoistNodeDependencies = (nodePath: Array<HoisterWorkTree>, locatorPath: Array<Locator>, parentNode: HoisterWorkTree, newNodes: Set<HoisterWorkTree>) => {
+  const hoistNodeDependencies = (nodePath: Array<HoisterWorkTree>, locatorPath: Array<Locator>, aliasedLocatorPath: Array<AliasedLocator>, parentNode: HoisterWorkTree, newNodes: Set<HoisterWorkTree>) => {
     if (seenNodes.has(parentNode))
       return;
-    const nextLocatorPath = [...locatorPath, parentNode.locator];
+    const nextLocatorPath = [...locatorPath, getAliasedLocator(parentNode)];
+    const nextAliasedLocatorPath = [...aliasedLocatorPath, getAliasedLocator(parentNode)];
 
     const dependantTree = new Map<PackageName, Set<PackageName>>();
     const hoistInfos = new Map<HoisterWorkTree, HoistInfo>();
@@ -679,11 +683,11 @@ const hoistGraph = (tree: HoisterWorkTree, rootNodePath: Array<HoisterWorkTree>,
         if ((hoistableIdent === node.ident || !parentNode.reasons.has(node.name)) && hoistInfo.isHoistable !== Hoistable.YES)
           parentNode.reasons.set(node.name, hoistInfo.reason!);
 
-        if (!node.isHoistBorder && nextLocatorPath.indexOf(node.locator) < 0) {
+        if (!node.isHoistBorder && nextAliasedLocatorPath.indexOf(getAliasedLocator(node)) < 0) {
           seenNodes.add(parentNode);
           const decoupledNode = decoupleGraphNode(parentNode, node);
 
-          hoistNodeDependencies([...nodePath, parentNode], [...locatorPath, parentNode.locator], decoupledNode, nextNewNodes);
+          hoistNodeDependencies([...nodePath, parentNode], nextLocatorPath, nextAliasedLocatorPath, decoupledNode, nextNewNodes);
 
           seenNodes.delete(parentNode);
         }
@@ -693,6 +697,7 @@ const hoistGraph = (tree: HoisterWorkTree, rootNodePath: Array<HoisterWorkTree>,
 
   let newNodes;
   let nextNewNodes = new Set(getSortedRegularDependencies(rootNode));
+  const aliasedRootNodePathLocators = Array.from(rootNodePath).map(x => getAliasedLocator(x));
   do {
     newNodes = nextNewNodes;
     nextNewNodes = new Set();
@@ -701,7 +706,7 @@ const hoistGraph = (tree: HoisterWorkTree, rootNodePath: Array<HoisterWorkTree>,
         continue;
       const decoupledDependency = decoupleGraphNode(rootNode, dep);
 
-      hoistNodeDependencies([], Array.from(rootNodePathLocators), decoupledDependency, nextNewNodes);
+      hoistNodeDependencies([], Array.from(rootNodePathLocators), aliasedRootNodePathLocators, decoupledDependency, nextNewNodes);
     }
   } while (nextNewNodes.size > 0);
 
@@ -1011,8 +1016,7 @@ const dumpDepTree = (tree: HoisterWorkTree) => {
       if (!pkg.peerNames.has(dep.name) && dep !== pkg) {
         const reason = pkg.reasons.get(dep.name);
         const identName = getIdentName(dep.locator);
-        const hoistedFrom = pkg.hoistedFrom.get(dep.name) || [];
-        str += `${prefix}${idx < dependencies.length - 1 ? `├─` : `└─`}${(parents.has(dep) ? `>` : ``) + (identName !== dep.name ? `a:${dep.name}:` : ``) +  prettyPrintLocator(dep.locator) + (reason ? ` ${reason}` : ``) + (dep !== pkg && hoistedFrom.length > 0 ? `, hoisted from: ${hoistedFrom.join(`, `)}` : ``)}\n`;
+        str += `${prefix}${idx < dependencies.length - 1 ? `├─` : `└─`}${(parents.has(dep) ? `>` : ``) + (identName !== dep.name ? `a:${dep.name}:` : ``) +  prettyPrintLocator(dep.locator) + (reason ? ` ${reason}` : ``)}\n`;
         str += dumpPackage(dep, parents, `${prefix}${idx < dependencies.length - 1 ? `│ ` : `  `}`);
       }
     }

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

@@ -2,11 +2,11 @@ import {hoist, HoisterTree, HoisterResult, HoisterDependencyKind} from '../sourc
 
 const toTree = (obj: any, key: string = `.`, nodes = new Map()): HoisterTree => {
   let node = nodes.get(key);
-  const identName = key.match(/@?[^@]+/)![0];
+  const name = key.match(/@?[^@]+/)![0];
   if (!node) {
     node = {
-      name: identName,
-      identName,
+      name,
+      identName: (obj[key] || {}).identName || name,
       reference: key.match(/@?[^@]+@?(.+)?/)![1] || ``,
       dependencies: new Set<HoisterTree>(),
       peerNames: new Set<string>((obj[key] || {}).peerNames || []),
@@ -551,4 +551,13 @@ describe(`hoist`, () => {
 
     expect(getTreeHeight(hoist(toTree(tree), {check: true}))).toEqual(5);
   });
+
+  it(`should hoist aliased packages`, () => {
+    const tree = {
+      '.': {dependencies: [`Aalias`]},
+      Aalias: {identName: `A`, dependencies: [`A`]},
+      A: {dependencies: [`B`]},
+    };
+    expect(getTreeHeight(hoist(toTree(tree), {check: true}))).toEqual(3);
+  });
 });