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

fix(core): use smarter nesting of npm packages #15502

Merged
merged 1 commit into from
Mar 7, 2023
Merged

fix(core): use smarter nesting of npm packages #15502

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
93 changes: 67 additions & 26 deletions packages/nx/src/lock-file/npm-parser.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -413,7 +413,7 @@ function mapSnapshots(
const visitedNodes = new Map<ProjectGraphExternalNode, Set<string>>();
const visitedPaths = new Set<string>();

const remappedPackages: MappedPackage[] = [];
const remappedPackages: Map<string, MappedPackage> = new Map();

// add first level children
Object.values(graph.externalNodes).forEach((node) => {
Expand All @@ -423,14 +423,15 @@ function mapSnapshots(
node.data.packageName,
node.data.version
);
remappedPackages.push(mappedPackage);
remappedPackages.set(mappedPackage.path, mappedPackage);
visitedNodes.set(node, new Set([mappedPackage.path]));
visitedPaths.add(mappedPackage.path);
} else {
nestedNodes.add(node);
}
});

let remappedPackagesArray: MappedPackage[];
if (nestedNodes.size) {
const invertedGraph = reverse(graph);
nestMappedPackages(
Expand All @@ -441,9 +442,13 @@ function mapSnapshots(
visitedPaths,
rootLockFile
);
// initially we naively map package paths to topParent/../parent/child
// but some of those should be nested higher up the tree
remappedPackagesArray = elevateNestedPaths(remappedPackages);
} else {
remappedPackagesArray = Array.from(remappedPackages.values());
}

return remappedPackages.sort((a, b) => a.path.localeCompare(b.path));
return remappedPackagesArray.sort((a, b) => a.path.localeCompare(b.path));
}

function mapPackage(
Expand All @@ -452,8 +457,8 @@ function mapPackage(
version: string,
parentPath = ''
): MappedPackage {
const path = parentPath + `node_modules/${packageName}`;
const lockfileVersion = rootLockFile.lockfileVersion;

let valueV3, valueV1;
if (lockfileVersion < 3) {
valueV1 = findMatchingPackageV1(
Expand All @@ -471,7 +476,7 @@ function mapPackage(
}

return {
path,
path: parentPath + `node_modules/${packageName}`,
name: packageName,
valueV1,
valueV3,
Expand All @@ -480,7 +485,7 @@ function mapPackage(

function nestMappedPackages(
invertedGraph: ProjectGraph,
result: MappedPackage[],
result: Map<string, MappedPackage>,
nestedNodes: Set<ProjectGraphExternalNode>,
visitedNodes: Map<ProjectGraphExternalNode, Set<string>>,
visitedPaths: Set<string>,
Expand All @@ -499,15 +504,13 @@ function nestMappedPackages(

if (visitedNodes.has(targetNode)) {
visitedNodes.get(targetNode).forEach((path) => {
const parentPath =
findParentPath(path, node.data.packageName, visitedPaths) + '/';
const mappedPackage = mapPackage(
rootLockFile,
node.data.packageName,
node.data.version,
parentPath
path + '/'
);
result.push(mappedPackage);
result.set(mappedPackage.path, mappedPackage);
if (visitedNodes.has(node)) {
visitedNodes.get(node).add(mappedPackage.path);
} else {
Expand Down Expand Up @@ -537,21 +540,59 @@ function nestMappedPackages(
}
}

function findParentPath(
path: string,
packageName: string,
visitedPaths: Set<string>
): string {
const segments = path.split('/node_modules/');
let parentPath = path;
while (
segments.length > 1 &&
!visitedPaths.has(`${parentPath}/node_modules/${packageName}`)
) {
segments.pop();
parentPath = segments.join('/node_modules/');
}
return parentPath;
// sort paths by number of segments and then alphabetically
function sortMappedPackagesPaths(mappedPackages: Map<string, MappedPackage>) {
return Array.from(mappedPackages.keys()).sort((a, b) => {
const aLength = a.split('/node_modules/').length;
const bLength = b.split('/node_modules/').length;
if (aLength > bLength) {
return 1;
}
if (aLength < bLength) {
return -1;
}
return a.localeCompare(b);
});
}

function elevateNestedPaths(
remappedPackages: Map<string, MappedPackage>
): MappedPackage[] {
const result = new Map<string, MappedPackage>();
const sortedPaths = sortMappedPackagesPaths(remappedPackages);

sortedPaths.forEach((path) => {
const segments = path.split('/node_modules/');

// we keep hoisted packages intact
if (segments.length === 1) {
result.set(path, remappedPackages.get(path));
return;
}

const packageName = segments.pop();
const getNewPath = (segs) =>
`${segs.join('/node_modules/')}/node_modules/${packageName}`;

// check if grandparent has the same package
while (
segments.length > 1 &&
!result.has(getNewPath(segments.slice(0, -1)))
) {
segments.pop();
}
const newPath = getNewPath(segments);
if (path !== newPath) {
result.set(newPath, {
...remappedPackages.get(path),
path: newPath,
});
} else {
result.set(path, remappedPackages.get(path));
}
});

return Array.from(result.values());
}

function findMatchingPackageV3(
Expand Down