feat(package-graph): Improve JSDoc-inferred types, encapsulation

core/package-graph/index.js

@@ -6,16 +6,21 @@ const { CyclicPackageGraphNode } = require("./lib/cyclic-package-graph-node");
 const { PackageGraphNode } = require("./lib/package-graph-node");
 const { reportCycles } = require("./lib/report-cycles");
 
+/** @typedef {import("./lib/package-graph-node").PackageGraphNode} PackageGraphNode */
+
 /**
- * A PackageGraph.
- * @constructor
- * @param {!Array.<Package>} packages An array of Packages to build the graph out of.
- * @param {String} graphType ("allDependencies" or "dependencies")
- *    Pass "dependencies" to create a graph of only dependencies,
- *    excluding the devDependencies that would normally be included.
- * @param {Boolean} forceLocal Force all local dependencies to be linked.
+ * A graph of packages in the current project.
+ *
+ * @extends {Map<string, PackageGraphNode>}
  */
 class PackageGraph extends Map {
+  /**
+   * @param {import("@lerna/package").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.
+   * @param {boolean} [forceLocal] Force all local dependencies to be linked.
+   */
   constructor(packages, graphType = "allDependencies", forceLocal) {
     super(packages.map((pkg) => [pkg.name, new PackageGraphNode(pkg)]));
 
@@ -86,8 +91,7 @@ class PackageGraph extends Map {
    * they depend on. i.e if packageA depended on packageB `graph.addDependencies([packageA])`
    * would return [packageA, packageB].
    *
-   * @param {!Array.<Package>} filteredPackages The packages to include dependencies for.
-   * @return {Array.<Package>} The packages with any dependencies that weren't already included.
+   * @param {import("@lerna/package").Package[]} filteredPackages The packages to include dependencies for.
    */
   addDependencies(filteredPackages) {
     return this.extendList(filteredPackages, "localDependencies");
@@ -98,27 +102,26 @@ class PackageGraph extends Map {
    * that depend on them. i.e if packageC depended on packageD `graph.addDependents([packageD])`
    * would return [packageD, packageC].
    *
-   * @param {!Array.<Package>} filteredPackages The packages to include dependents for.
-   * @return {Array.<Package>} The packages with any dependents that weren't already included.
+   * @param {import("@lerna/package").Package[]} filteredPackages The packages to include dependents for.
    */
   addDependents(filteredPackages) {
     return this.extendList(filteredPackages, "localDependents");
   }
 
   /**
    * Extends a list of packages by traversing on a given property, which must refer to a
-   * `PackageGraphNode` property that is a collection of `PackageGraphNode`s
+   * `PackageGraphNode` property that is a collection of `PackageGraphNode`s.
+   * Returns input packages with any additional packages found by traversing `nodeProp`.
    *
-   * @param {!Array.<Package>} packageList The list of packages to extend
-   * @param {!String} nodeProp The property on `PackageGraphNode` used to traverse
-   * @return {Array.<Package>} The 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
    */
   extendList(packageList, nodeProp) {
     // the current list of packages we are expanding using breadth-first-search
     const search = new Set(packageList.map(({ name }) => this.get(name)));
 
     // an intermediate list of matched PackageGraphNodes
+    /** @type {PackageGraphNode[]} */
     const result = [];
 
     search.forEach((currentNode) => {
@@ -143,8 +146,8 @@ class PackageGraph extends Map {
    *
    * @deprecated Use collapseCycles instead.
    *
-   * @param {!boolean} rejectCycles Whether or not to reject cycles
-   * @returns [Set<String[]>, Set<PackageGraphNode>]
+   * @param {boolean} rejectCycles Whether or not to reject cycles
+   * @returns {[Set<string[]>, Set<PackageGraphNode>]}
    */
   partitionCycles(rejectCycles) {
     const cyclePaths = new Set();
@@ -199,14 +202,20 @@ class PackageGraph extends Map {
    * Returns the cycles of this graph. If two cycles share some elements, they will
    * be returned as a single cycle.
    *
-   * @param {!boolean} rejectCycles Whether or not to reject cycles
-   * @returns Set<CyclicPackageGraphNode>
+   * @param {boolean} rejectCycles Whether or not to reject cycles
+   * @returns {Set<CyclicPackageGraphNode>}
    */
   collapseCycles(rejectCycles) {
+    /** @type {string[]} */
     const cyclePaths = [];
+
+    /** @type {Map<PackageGraphNode, CyclicPackageGraphNode>} */
     const nodeToCycle = new Map();
+
+    /** @type {Set<CyclicPackageGraphNode>} */
     const cycles = new Set();
 
+    /** @type {(PackageGraphNode | CyclicPackageGraphNode)[]} */
     const walkStack = [];
 
     function visits(baseNode, dependentNode) {

core/package-graph/lib/cyclic-package-graph-node.js

@@ -6,29 +6,30 @@ let lastCollapsedNodeId = 0;
  * Represents a cyclic collection of nodes in a PackageGraph.
  * It is meant to be used as a black box, where the only exposed
  * information are the connections to the other nodes of the graph.
- * It can contains either `PackageGraphNode`s or other `CyclicPackageGraphNode`s.
+ * It can contain either `PackageGraphNode`s or other `CyclicPackageGraphNode`s.
+ *
+ * @extends {Map<string, import('..').PackageGraphNode | CyclicPackageGraphNode>}
  */
 class CyclicPackageGraphNode extends Map {
   constructor() {
     super();
 
+    this.name = `(cycle) ${(lastCollapsedNodeId += 1)}`;
+
+    /** @type {Map<string, import('..').PackageGraphNode | CyclicPackageGraphNode>} */
     this.localDependencies = new Map();
+
+    /** @type {Map<string, import('..').PackageGraphNode | CyclicPackageGraphNode>} */
     this.localDependents = new Map();
+  }
 
-    Object.defineProperties(this, {
-      // immutable properties
-      name: {
-        enumerable: true,
-        value: `(cycle) ${(lastCollapsedNodeId += 1)}`,
-      },
-      isCycle: {
-        value: true,
-      },
-    });
+  // eslint-disable-next-line class-methods-use-this
+  get isCycle() {
+    return true;
   }
 
   /**
-   * @returns {String} Returns a representation of a cycle, like like `A -> B -> C -> A`.
+   * @returns {string} A representation of a cycle, like like `A -> B -> C -> A`.
    */
   toString() {
     const parts = Array.from(this, ([key, node]) =>
@@ -43,10 +44,9 @@ class CyclicPackageGraphNode extends Map {
 
   /**
    * Flattens a CyclicPackageGraphNode (which can have multiple level of cycles).
-   *
-   * @returns {PackageGraphNode[]}
    */
   flatten() {
+    /** @type {import('..').PackageGraphNode[]} */
     const result = [];
 
     for (const node of this.values()) {
@@ -63,8 +63,8 @@ class CyclicPackageGraphNode extends Map {
   /**
    * Checks if a given node is contained in this cycle (or in a nested one)
    *
-   * @param {String} name The name of the package to search in this cycle
-   * @returns {Boolean}
+   * @param {string} name The name of the package to search in this cycle
+   * @returns {boolean}
    */
   contains(name) {
     for (const [currentName, currentNode] of this) {
@@ -82,7 +82,7 @@ class CyclicPackageGraphNode extends Map {
   /**
    * Adds a graph node, or a nested cycle, to this group.
    *
-   * @param {PackageGraphNode|CyclicPackageGraphNode} node
+   * @param {import('..').PackageGraphNode | CyclicPackageGraphNode} node
    */
   insert(node) {
     this.set(node.name, node);
@@ -103,7 +103,7 @@ class CyclicPackageGraphNode extends Map {
 
   /**
    * Remove pointers to candidate node from internal collections.
-   * @param {PackageGraphNode|CyclicPackageGraphNode} candidateNode instance to unlink
+   * @param {import('..').PackageGraphNode | CyclicPackageGraphNode} candidateNode instance to unlink
    */
   unlink(candidateNode) {
     // remove incoming edges ("indegree")

core/package-graph/lib/package-graph-node.js

@@ -3,44 +3,48 @@
 const semver = require("semver");
 const { prereleaseIdFromVersion } = require("@lerna/prerelease-id-from-version");
 
+const PKG = Symbol("pkg");
+
 /**
- * Represents a node in a PackageGraph.
- * @constructor
- * @param {!<Package>} pkg - A Package object to build the node from.
+ * A node in a PackageGraph.
  */
 class PackageGraphNode {
+  /**
+   * @param {import("@lerna/package").Package} pkg
+   */
   constructor(pkg) {
-    Object.defineProperties(this, {
-      // immutable properties
-      name: {
-        enumerable: true,
-        value: pkg.name,
-      },
-      location: {
-        value: pkg.location,
-      },
-      prereleaseId: {
-        // an existing prerelease ID only matters at the beginning
-        value: prereleaseIdFromVersion(pkg.version),
-      },
-      // properties that might change over time
-      version: {
-        get() {
-          return pkg.version;
-        },
-      },
-      pkg: {
-        get() {
-          return pkg;
-        },
-      },
-    });
+    this.name = pkg.name;
+    this[PKG] = pkg;
+
+    // omit raw pkg from default util.inspect() output
+    Object.defineProperty(this, PKG, { enumerable: false });
 
+    /** @type {Map<string, import("npm-package-arg").Result>} */
     this.externalDependencies = new Map();
+
+    /** @type {Map<string, import("npm-package-arg").Result>} */
     this.localDependencies = new Map();
+
+    /** @type {Map<string, PackageGraphNode>} */
     this.localDependents = new Map();
   }
 
+  get location() {
+    return this[PKG].location;
+  }
+
+  get pkg() {
+    return this[PKG];
+  }
+
+  get prereleaseId() {
+    return prereleaseIdFromVersion(this.version);
+  }
+
+  get version() {
+    return this[PKG].version;
+  }
+
   /**
    * Determine if the Node satisfies a resolved semver range.
    * @see https://github.com/npm/npm-package-arg#result-object