Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[node-modules] Support reducing trees requiring multiple hoisting passes to a terminal result #2394

Merged
merged 17 commits into from
Feb 5, 2021
Merged

[node-modules] Support reducing trees requiring multiple hoisting passes to a terminal result #2394

Changes from 1 commit
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
ac1f725
Improves NM_DEBUG_LEVEL=2 output
larixer Jan 20, 2021
ba9600e
More hoistedFrom details
larixer Jan 20, 2021
e37edef
Do not hide reason when ident is unhoistable
larixer Jan 21, 2021
7407c3f
Don't clone hoistedFrom
larixer Jan 21, 2021
d38fef7
Hopefully finally fix reasons, remove self-refs form the output and s…
larixer Jan 21, 2021
d078946
Shorrten hoistedFrom
larixer Jan 21, 2021
d629c22
Implements multi-pass hoisting (its much slower atm)
larixer Jan 27, 2021
bcf10b3
Speedup used dependencies computation
larixer Jan 27, 2021
03058a9
Dump number of rounds
larixer Jan 27, 2021
bdea387
Cosmetics
larixer Jan 28, 2021
0e672e4
Adds multi-pass unit test
larixer Jan 28, 2021
aae413d
Uses special version of used dependencies computation for zero round …
larixer Jan 29, 2021
d14b15e
Limit the number of rounds to a minimum thanks to shadowed nodes hois…
larixer Jan 30, 2021
ce866cc
Properly track shadowed nodes
larixer Feb 1, 2021
35f45bf
Check for shadowed nodes always
larixer Feb 1, 2021
d1619bc
Add changelog entry
larixer Feb 1, 2021
4576b49
Enable fast lookup
larixer Feb 3, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Hopefully finally fix reasons, remove self-refs form the output and s… 
…horten locators
  • Loading branch information
@larixer
larixer committed Jan 21, 2021
commit d38fef78d1c42507d3bd261642bd53d4324bbfcf
41 changes: 24 additions & 17 deletions packages/yarnpkg-pnpify/sources/hoist.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -360,7 +360,7 @@ const hoistTo = (tree: HoisterWorkTree, rootNodePath: Array<HoisterWorkTree>, ro
}
};

const getNodeHoistInfo = (rootNodePathLocators: Set<Locator>, nodePath: Array<HoisterWorkTree>, node: HoisterWorkTree, hoistedDependencies: Map<PackageName, HoisterWorkTree>, {outputReason}: {outputReason: boolean}): HoistInfo => {
const getNodeHoistInfo = (rootNodePathLocators: Set<Locator>, nodePath: Array<HoisterWorkTree>, node: HoisterWorkTree, hoistedDependencies: Map<PackageName, HoisterWorkTree>, hoistIdents: Map<PackageName, Ident>, hoistIdentMap: Map<Ident, Array<Ident>>, {outputReason}: {outputReason: boolean}): HoistInfo => {
let reasonRoot;
let reason: string | null = null;
let dependsOn: Set<HoisterWorkTree> | null = new Set();
Expand All @@ -374,6 +374,14 @@ const getNodeHoistInfo = (rootNodePathLocators: Set<Locator>, nodePath: Array<Ho
if (outputReason && !isHoistable)
reason = `- self-reference`;

if (isHoistable) {
const hoistedIdent = hoistIdents.get(node.name);
isHoistable = hoistedIdent === node.ident;
if (outputReason && !isHoistable) {
reason = `- filled by: ${prettyPrintLocator(hoistIdentMap.get(node.name)![0])} at ${reasonRoot}`;
}
}

if (isHoistable) {
let isNameAvailable = false;
const hoistedDep = hoistedDependencies.get(node.name);
Expand Down Expand Up @@ -455,13 +463,7 @@ const hoistGraph = (tree: HoisterWorkTree, rootNodePath: Array<HoisterWorkTree>,
const dependantTree = new Map<PackageName, Set<PackageName>>();
const hoistInfos = new Map<HoisterWorkTree, HoistInfo>();
for (const subDependency of getSortedRegularDependencies(parentNode)) {
let hoistInfo: HoistInfo | null = null;
const hoistableIdent = hoistIdents.get(subDependency.name);
if (hoistableIdent !== subDependency.ident)
hoistInfo = {isHoistable: Hoistable.NO, reason: null};

if (!hoistInfo)
hoistInfo = getNodeHoistInfo(rootNodePathLocators, [rootNode, ...nodePath, parentNode], subDependency, hoistedDependencies, {outputReason: options.debugLevel >= DebugLevel.REASONS});
const hoistInfo = getNodeHoistInfo(rootNodePathLocators, [rootNode, ...nodePath, parentNode], subDependency, hoistedDependencies, hoistIdents, hoistIdentMap, {outputReason: options.debugLevel >= DebugLevel.REASONS});

hoistInfos.set(subDependency, hoistInfo);
if (hoistInfo.isHoistable === Hoistable.DEPENDS) {
Expand Down Expand Up @@ -526,7 +528,7 @@ const hoistGraph = (tree: HoisterWorkTree, rootNodePath: Array<HoisterWorkTree>,
if (unhoistableNodes.has(node)) {
const hoistInfo = hoistInfos.get(node)!;
const hoistableIdent = hoistIdents.get(node.name);
if (hoistableIdent === node.ident && hoistInfo.isHoistable !== Hoistable.YES)
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) {
Expand Down Expand Up @@ -789,19 +791,24 @@ const buildPreferenceMap = (rootNode: HoisterWorkTree): PreferenceMap => {

const prettyPrintLocator = (locator: Locator) => {
const idx = locator.indexOf(`@`, 1);
const name = locator.substring(0, idx);
let name = locator.substring(0, idx);
if (name.endsWith(`$wsroot$`))
name = `wh:${name.replace(`$wsroot$`, ``)}`;
const reference = locator.substring(idx + 1);
if (reference === `workspace:.`) {
return `.`;
} else if (!reference) {
return `${name}`;
} else {
const version = (reference.indexOf(`#`) > 0 ? reference.split(`#`)[1] : reference).replace(`npm:`, ``);
if (reference.startsWith(`virtual`)) {
return `v:${name}@${version}`;
} else {
return `${name}@${version}`;
let version = (reference.indexOf(`#`) > 0 ? reference.split(`#`)[1] : reference).replace(`npm:`, ``);
if (reference.startsWith(`virtual`))
name = `v:${name}`;
if (version.startsWith(`workspace`)) {
name = `w:${name}`;
version = ``;
}

return `${name}${version ? `@${version}` : ``}`;
}
};

Expand Down Expand Up @@ -830,10 +837,10 @@ const dumpDepTree = (tree: HoisterWorkTree) => {
parents.add(pkg);
for (let idx = 0; idx < dependencies.length; idx++) {
const dep = dependencies[idx];
if (!pkg.peerNames.has(dep.name)) {
if (!pkg.peerNames.has(dep.name) && dep !== pkg) {
const reason = pkg.reasons.get(dep.name);
const identName = getIdentName(dep.locator);
str += `${prefix}${idx < dependencies.length - 1 ? `├─` : `└─`}${(parents.has(dep) ? `>` : ``) + (identName !== dep.name ? `a:${dep.name}:` : ``) + prettyPrintLocator(dep.locator) + (reason ? ` ${reason}` : ``) + (dep !== pkg && dep.hoistedFrom.length > 0 ? `, hoisted from: ${dep.hoistedFrom.join(`, `)}` : ``)}\n`;
str += `${prefix}${idx < dependencies.length - 1 ? `├─` : `└─`}${(parents.has(dep) ? `>` : ``) + (identName !== dep.name ? `a:${dep.name}:` : ``) + prettyPrintLocator(dep.locator) + (reason ? ` ${reason}` : ``) + (dep !== pkg && dep.hoistedFrom.length > 0 ? `, hoisted from: ${dep.hoistedFrom.join(`, `)}` : ``)}\n`;
str += dumpPackage(dep, parents, `${prefix}${idx < dependencies.length - 1 ? `│ ` : ` `}`);
}
}
Expand Down