Skip to content

Commit

Permalink
[node-modules] Support reducing trees requiring multiple hoisting pas…
Browse files Browse the repository at this point in the history
…ses to a terminal result (#2394)

* Improves NM_DEBUG_LEVEL=2 output

* More hoistedFrom details

* Do not hide reason when ident is unhoistable

* Don't clone hoistedFrom

* Hopefully finally fix reasons, remove self-refs form the output and shorten locators

* Shorrten hoistedFrom

* Implements multi-pass hoisting (its much slower atm)

* Speedup used dependencies computation

* Dump number of rounds

* Cosmetics

* Adds multi-pass unit test

* Uses special version of used dependencies computation for zero round (the same as in master)

* Limit the number of rounds to a minimum thanks to shadowed nodes hoisting detection

* Properly track shadowed nodes

* Check for shadowed nodes always

* Add changelog entry

* Enable fast lookup
  • Loading branch information
larixer committed Feb 5, 2021
1 parent e1d5ec6 commit fbbb7be
Show file tree
Hide file tree
Showing 5 changed files with 239 additions and 65 deletions.
24 changes: 24 additions & 0 deletions .yarn/versions/21795af1.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
releases:
"@yarnpkg/cli": patch
"@yarnpkg/plugin-node-modules": 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-stage"
- "@yarnpkg/plugin-typescript"
- "@yarnpkg/plugin-version"
- "@yarnpkg/plugin-workspace-tools"
- vscode-zipfs
- "@yarnpkg/builder"
- "@yarnpkg/core"
- "@yarnpkg/doctor"
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
### Bugfixes

- The patched fs now supports file URLs.
- The node-modules linker now ensures that hoisting result is terminal, by doing several hoisting rounds when needed

### Settings

Expand Down
19 changes: 11 additions & 8 deletions packages/yarnpkg-pnpify/sources/buildNodeModulesTree.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ export type NodeModulesPackageNode = {
linkType: LinkType,
// Contains ["node_modules"] if there's nested n_m entries
dirList?: undefined,
nodePath: string,
aliases: Array<string>,
};

Expand Down Expand Up @@ -352,11 +353,12 @@ function getTargetLocatorPath(locator: PhysicalPackageLocator, pnp: PnpApi, opti
const populateNodeModulesTree = (pnp: PnpApi, hoistedTree: HoisterResult, options: NodeModulesTreeOptions): NodeModulesTree => {
const tree: NodeModulesTree = new Map();

const makeLeafNode = (locator: PhysicalPackageLocator, aliases: Array<string>): {locator: LocatorKey, target: PortablePath, linkType: LinkType, aliases: Array<string>} => {
const makeLeafNode = (locator: PhysicalPackageLocator, nodePath: string, aliases: Array<string>): {locator: LocatorKey, nodePath: string, target: PortablePath, linkType: LinkType, aliases: Array<string>} => {
const {linkType, target} = getTargetLocatorPath(locator, pnp, options);

return {
locator: stringifyLocator(locator),
nodePath,
target,
linkType,
aliases,
Expand All @@ -376,7 +378,7 @@ const populateNodeModulesTree = (pnp: PnpApi, hoistedTree: HoisterResult, option
};

const seenNodes = new Set<HoisterResult>();
const buildTree = (pkg: HoisterResult, locationPrefix: PortablePath) => {
const buildTree = (pkg: HoisterResult, locationPrefix: PortablePath, parentNodePath: string) => {
if (seenNodes.has(pkg))
return;

Expand All @@ -397,7 +399,8 @@ const populateNodeModulesTree = (pnp: PnpApi, hoistedTree: HoisterResult, option
const nodeModulesDirPath = ppath.join(locationPrefix, NODE_MODULES);
const nodeModulesLocation = ppath.join(nodeModulesDirPath, ...packageNameParts);

const leafNode = makeLeafNode(locator, references.slice(1));
const nodePath = `${parentNodePath}/${locator.name}`;
const leafNode = makeLeafNode(locator, parentNodePath, references.slice(1));
if (!dep.name.endsWith(WORKSPACE_NAME_SUFFIX)) {
const prevNode = tree.get(nodeModulesLocation);
if (prevNode) {
Expand All @@ -408,9 +411,9 @@ const populateNodeModulesTree = (pnp: PnpApi, hoistedTree: HoisterResult, option
const locator2 = structUtils.parseLocator(leafNode.locator);

if (prevNode.linkType !== leafNode.linkType)
throw new Error(`Assertion failed: ${nodeModulesLocation} cannot merge nodes with different link types`);
throw new Error(`Assertion failed: ${nodeModulesLocation} cannot merge nodes with different link types ${prevNode.nodePath}/${structUtils.stringifyLocator(locator1)} and ${parentNodePath}/${structUtils.stringifyLocator(locator2)}`);
else if (locator1.identHash !== locator2.identHash)
throw new Error(`Assertion failed: ${nodeModulesLocation} cannot merge nodes with different idents ${structUtils.stringifyLocator(locator1)} and ${structUtils.stringifyLocator(locator2)}`);
throw new Error(`Assertion failed: ${nodeModulesLocation} cannot merge nodes with different idents ${prevNode.nodePath}/${structUtils.stringifyLocator(locator1)} and ${parentNodePath}/s${structUtils.stringifyLocator(locator2)}`);

leafNode.aliases = [...leafNode.aliases, ...prevNode.aliases, structUtils.parseLocator(prevNode.locator).reference];
}
Expand Down Expand Up @@ -441,14 +444,14 @@ const populateNodeModulesTree = (pnp: PnpApi, hoistedTree: HoisterResult, option
}
}

buildTree(dep, leafNode.linkType === LinkType.SOFT ? leafNode.target : nodeModulesLocation);
buildTree(dep, leafNode.linkType === LinkType.SOFT ? leafNode.target : nodeModulesLocation, nodePath);
}
};

const rootNode = makeLeafNode({name: hoistedTree.name, reference: Array.from(hoistedTree.references)[0] as string}, []);
const rootNode = makeLeafNode({name: hoistedTree.name, reference: Array.from(hoistedTree.references)[0] as string}, ``, []);
const rootPath = rootNode.target;
tree.set(rootPath, rootNode);
buildTree(hoistedTree, rootPath);
buildTree(hoistedTree, rootPath, ``);

return tree;
};
Expand Down

0 comments on commit fbbb7be

Please sign in to comment.