Skip to content

Commit

Permalink
fix(core): ensure to touch all nodes in package-graph
Browse files Browse the repository at this point in the history
  • Loading branch information
ghiscoding committed Aug 6, 2022
1 parent 29a76bb commit f4f7bbc
Show file tree
Hide file tree
Showing 5 changed files with 21 additions and 21 deletions.
1 change: 1 addition & 0 deletions package.json
Expand Up @@ -15,6 +15,7 @@
"dist-list-cmd": "node ./packages/cli/dist/cli.js list --all",
"dist-roll-version-dry-run": "node ./packages/cli/dist/cli.js version --git-dry-run",
"dist-roll-publish-dry-run": "node ./packages/cli/dist/cli.js publish from-package --git-dry-run",
"dist-roll-publish-alpha-dry-run": "node ./packages/cli/dist/cli.js publish --git-dry-run --exact --include-merged-tags --preid alpha --dist-tag next prerelease",
"dist-exec-win": "node ./packages/cli/dist/cli.js exec --scope {@lerna-lite/cli,@lerna-lite/core} -- echo hello from package: %LERNA_PACKAGE_NAME%",
"dist-exec-unix": "node ./packages/cli/dist/cli.js exec -- echo hello from package: ${LERNA_PACKAGE_NAME}",
"dist-pack-tarball": "node ./packages/cli/dist/cli.js run pack-tarball",
Expand Down
27 changes: 13 additions & 14 deletions packages/core/src/package-graph/package-graph.ts
Expand Up @@ -10,9 +10,9 @@ import { NpaResolveResult } from '../models';
*
* @extends {Map<string, PackageGraphNode>}
*/
export class PackageGraph extends Map {
export class PackageGraph extends Map<string, PackageGraphNode> {
/**
* @param {import("@lerna/package").Package[]} packages An array of Packages to build the graph out of.
* @param {Package[]} packages - An array of Packages to build the graph out of.
* @param {'allDependencies'|'dependencies'} [graphType]
* Pass "dependencies" to create a graph of only dependencies,
* excluding the devDependencies that would normally be included.
Expand All @@ -28,7 +28,6 @@ export class PackageGraph extends Map {
localDependencies = 'force'; // eslint-disable-line
}

// @ts-ignore
super(packages.map((pkg: Package) => [pkg?.name ?? '', new PackageGraphNode(pkg)]));

if (packages.length !== this.size) {
Expand Down Expand Up @@ -127,7 +126,7 @@ export class PackageGraph extends Map {
* they depend on. i.e if packageA depended on packageB `graph.addDependencies([packageA])`
* would return [packageA, packageB].
*
* @param {import("@lerna/package").Package[]} filteredPackages The packages to include dependencies for.
* @param {Package[]} filteredPackages - The packages to include dependencies for.
*/
addDependencies(filteredPackages: Package[]) {
return this.extendList(filteredPackages, 'localDependencies');
Expand All @@ -138,7 +137,7 @@ export class PackageGraph extends Map {
* that depend on them. i.e if packageC depended on packageD `graph.addDependents([packageD])`
* would return [packageD, packageC].
*
* @param {import("@lerna/package").Package[]} filteredPackages The packages to include dependents for.
* @param {Package[]} filteredPackages - The packages to include dependents for.
*/
addDependents(filteredPackages: Package[]) {
return this.extendList(filteredPackages, 'localDependents');
Expand All @@ -149,16 +148,15 @@ export class PackageGraph extends Map {
* `PackageGraphNode` property that is a collection of `PackageGraphNode`s.
* Returns input packages with any additional packages found by traversing `nodeProp`.
*
* @param {import("@lerna/package").Package[]} packageList The list of packages to extend
* @param {'localDependencies'|'localDependents'} nodeProp The property on `PackageGraphNode` used to traverse
* @param {Package[]} packageList - The list of packages to extend
* @param {'localDependencies'|'localDependents'} nodeProp - The property on `PackageGraphNode` used to traverse
*/
extendList(packageList: Package[], nodeProp: 'localDependencies' | 'localDependents') {
// the current list of packages we are expanding using breadth-first-search
const search = new Set<PackageGraphNode>(packageList.map(({ name }) => this.get(name)));
const search = new Set<PackageGraphNode>(packageList.map(({ name }) => this.get(name) as PackageGraphNode));

// an intermediate list of matched PackageGraphNodes
/** @type {PackageGraphNode[]} */
const result: PackageGraphNode[] = [];
const result: Array<PackageGraphNode> = [];

search.forEach((currentNode) => {
// anything searched for is always a result
Expand All @@ -167,7 +165,7 @@ export class PackageGraph extends Map {
currentNode[nodeProp].forEach((meta, depName) => {
const depNode = this.get(depName);

if (depNode !== currentNode && !search.has(depNode)) {
if (depNode && depNode !== currentNode && !search.has(depNode)) {
search.add(depNode);
}
});
Expand Down Expand Up @@ -245,7 +243,7 @@ export class PackageGraph extends Map {
const cyclePaths: string[] = [];
const nodeToCycle = new Map<PackageGraphNode, CyclicPackageGraphNode>();
const cycles = new Set<CyclicPackageGraphNode>();
const alreadyVisited = new Set<PackageGraphNode>();
const alreadyVisited = new Set<string>();
const walkStack: Array<PackageGraphNode | CyclicPackageGraphNode> = [];

function visits(baseNode, dependentNode) {
Expand All @@ -259,10 +257,11 @@ export class PackageGraph extends Map {
}

// Otherwise the same node is checked multiple times which is very wasteful in a large repository
if (alreadyVisited.has(topLevelDependent)) {
const identifier = `${baseNode.name}:${topLevelDependent.name}`;
if (alreadyVisited.has(identifier)) {
return;
}
alreadyVisited.add(topLevelDependent);
alreadyVisited.add(identifier);

if (topLevelDependent === baseNode || (topLevelDependent.isCycle && topLevelDependent.has(baseNode.name))) {
const cycle: any = new CyclicPackageGraphNode();
Expand Down
4 changes: 2 additions & 2 deletions packages/diff/src/diff-command.ts
@@ -1,4 +1,4 @@
import { Command, CommandType, DiffCommandOption, Package, spawn, ValidationError } from '@lerna-lite/core';
import { Command, CommandType, DiffCommandOption, PackageGraphNode, spawn, ValidationError } from '@lerna-lite/core';

import { getLastCommit } from './lib/get-last-commit';
import { hasCommit } from './lib/has-commit';
Expand All @@ -17,7 +17,7 @@ export class DiffCommand extends Command<DiffCommandOption> {
}

initialize() {
let targetPackage: Package | undefined = undefined;
let targetPackage: PackageGraphNode | undefined = undefined;
const packageName = this.options.pkgName;

if (packageName) {
Expand Down
6 changes: 3 additions & 3 deletions packages/publish/src/publish-command.ts
Expand Up @@ -563,7 +563,7 @@ export class PublishCommand extends Command<PublishCommandOption> {

for (const [depName, resolved] of node.localDependencies) {
// other canary versions need to be updated, non-canary is a no-op
const depVersion = this.updatesVersions?.get(depName) || this.packageGraph?.get(depName).pkg.version;
const depVersion = this.updatesVersions?.get(depName) || this.packageGraph?.get(depName)!.pkg.version;

// it no longer matters if we mutate the shared Package instance
node.pkg.updateLocalDependency(
Expand All @@ -588,7 +588,7 @@ export class PublishCommand extends Command<PublishCommandOption> {
return pMap(updatesWithLocalLinks, (node: PackageGraphNode) => {
for (const [depName, resolved] of node.localDependencies) {
// regardless of where the version comes from, we can't publish 'file:../sibling-pkg' specs
const depVersion = this.updatesVersions?.get(depName) || this.packageGraph?.get(depName).pkg.version;
const depVersion = this.updatesVersions?.get(depName) || this.packageGraph?.get(depName)!.pkg.version;

// it no longer matters if we mutate the shared Package instance
node.pkg.updateLocalDependency(
Expand Down Expand Up @@ -616,7 +616,7 @@ export class PublishCommand extends Command<PublishCommandOption> {

// 1. update & bump version of local dependencies
for (const [depName, resolved] of node.localDependencies) {
const depVersion = this.updatesVersions?.get(depName) || this.packageGraph?.get(depName).pkg.version;
const depVersion = this.updatesVersions?.get(depName) || this.packageGraph?.get(depName)!.pkg.version;

// it no longer matters if we mutate the shared Package instance
node.pkg.updateLocalDependency(
Expand Down
4 changes: 2 additions & 2 deletions packages/version/src/version-command.ts
Expand Up @@ -451,7 +451,7 @@ export class VersionCommand extends Command<VersionCommandOption> {
let hasBreakingChange = false;

for (const [name, bump] of versions) {
hasBreakingChange = hasBreakingChange || isBreakingChange(this.packageGraph?.get(name).version, bump);
hasBreakingChange = hasBreakingChange || isBreakingChange(this.packageGraph?.get(name)!.version, bump);
}

if (hasBreakingChange) {
Expand Down Expand Up @@ -610,7 +610,7 @@ export class VersionCommand extends Command<VersionCommandOption> {
pkg.set('version', this.updatesVersions?.get(pkg?.name ?? ''));

// update pkg dependencies
for (const [depName, resolved] of this.packageGraph?.get(pkg.name).localDependencies) {
for (const [depName, resolved] of this.packageGraph?.get(pkg.name)!.localDependencies) {
const depVersion = this.updatesVersions?.get(depName);

if (depVersion && resolved.type !== 'directory') {
Expand Down

0 comments on commit f4f7bbc

Please sign in to comment.