refactor(dependencies-hierarchy): simplify max depth recursion (#5858)

* test(dependencies-hierarchy): start currentDepth at 1 in tests Calls to `getTree` from the `buildDependenciesHierarchy` function always start the `currentDepth` at `1` rather than `0`. Updating `currentDepth` calls in test to also start at `1` for consistency with production code. This required incrementing the `maxDepth` argument in a few cases as well. * refactor(dependencies-hierarchy): simplify max depth recursion
pnpm · Dec 31, 2022 · 7eb7056 · 7eb7056
1 parent 00dea18
commit 7eb7056
Show file tree

Hide file tree

Showing 3 changed files with 19 additions and 26 deletions.
diff --git a/reviewing/dependencies-hierarchy/src/buildDependenciesHierarchy.ts b/reviewing/dependencies-hierarchy/src/buildDependenciesHierarchy.ts
@@ -106,7 +106,6 @@ async function dependenciesHierarchyForPackage (
   const wantedLockfile = await readWantedLockfile(opts.lockfileDir, { ignoreIncompatible: false }) ?? { packages: {} }
 
   const getChildrenTree = getTree.bind(null, {
-    currentDepth: 1,
     currentPackages: currentLockfile.packages ?? {},
     includeOptionalDependencies: opts.include.optionalDependencies,
     lockfileDir: opts.lockfileDir,

diff --git a/reviewing/dependencies-hierarchy/src/getTree.ts b/reviewing/dependencies-hierarchy/src/getTree.ts
@@ -9,7 +9,6 @@ import { getPkgInfo } from './getPkgInfo'
 import { DependenciesCache } from './DependenciesCache'
 
 interface GetTreeOpts {
-  currentDepth: number
   maxDepth: number
   modulesDir: string
   includeOptionalDependencies: boolean
@@ -54,7 +53,7 @@ function getTreeHelper (
   keypath: string[],
   parentId: string
 ): DependencyInfo {
-  if (opts.currentDepth > opts.maxDepth) {
+  if (opts.maxDepth <= 0) {
     return { dependencies: [], isPartiallyVisited: true, height: null }
   }
 
@@ -75,7 +74,7 @@ function getTreeHelper (
 
   const getChildrenTree = getTreeHelper.bind(null, dependenciesCache, {
     ...opts,
-    currentDepth: opts.currentDepth + 1,
+    maxDepth: opts.maxDepth - 1,
   })
 
   const peers = new Set(Object.keys(opts.currentPackages[parentId].peerDependencies ?? {}))
@@ -113,7 +112,7 @@ function getTreeHelper (
       if (circular) {
         dependencies = []
       } else {
-        const requestedDepth = opts.maxDepth - opts.currentDepth
+        const requestedDepth = opts.maxDepth
         dependencies = dependenciesCache.get({ packageAbsolutePath, requestedDepth })
 
         if (dependencies == null) {

diff --git a/reviewing/dependencies-hierarchy/test/getTree.test.ts b/reviewing/dependencies-hierarchy/test/getTree.test.ts
@@ -94,7 +94,7 @@ describe('getTree', () => {
     }
 
     test('full test case to print when max depth is large', () => {
-      const result = normalizePackageNodeForTesting(getTree({ ...getTreeArgs, currentDepth: 0, maxDepth: 9999 }, [], startingDepPath))
+      const result = normalizePackageNodeForTesting(getTree({ ...getTreeArgs, maxDepth: 9999 }, [], startingDepPath))
 
       expect(result).toEqual([
         expect.objectContaining({
@@ -114,12 +114,12 @@ describe('getTree', () => {
     })
 
     test('no result when current depth exceeds max depth', () => {
-      const result = getTree({ ...getTreeArgs, currentDepth: 1, maxDepth: 0 }, [], startingDepPath)
+      const result = getTree({ ...getTreeArgs, maxDepth: 0 }, [], startingDepPath)
       expect(result).toEqual([])
     })
 
-    test('max depth of 0 to print flat dependencies', () => {
-      const result = getTree({ ...getTreeArgs, currentDepth: 0, maxDepth: 0 }, [], startingDepPath)
+    test('max depth of 1 to print flat dependencies', () => {
+      const result = getTree({ ...getTreeArgs, maxDepth: 1 }, [], startingDepPath)
 
       expect(normalizePackageNodeForTesting(result)).toEqual([
         expect.objectContaining({ alias: 'b1', dependencies: undefined }),
@@ -128,8 +128,8 @@ describe('getTree', () => {
       ])
     })
 
-    test('max depth of 1 to print a1 -> b1 -> c1, but not d1', () => {
-      const result = getTree({ ...getTreeArgs, currentDepth: 0, maxDepth: 1 }, [], startingDepPath)
+    test('max depth of 2 to print a1 -> b1 -> c1, but not d1', () => {
+      const result = getTree({ ...getTreeArgs, maxDepth: 2 }, [], startingDepPath)
 
       expect(normalizePackageNodeForTesting(result)).toEqual([
         expect.objectContaining({
@@ -165,7 +165,7 @@ describe('getTree', () => {
 
     test('revisiting package at lower depth prints dependenices not previously printed', () => {
       // This tests the "glob" npm package on a subset of its dependency tree.
-      // Requested depth (max depth - current depth) shown in square brackets.
+      // Max depth shown in square brackets.
       //
       // root
       // └─┬ glob [2]
@@ -185,8 +185,7 @@ describe('getTree', () => {
 
       const result = getTree({
         ...commonMockGetTreeArgs,
-        currentDepth: 0,
-        maxDepth: 2,
+        maxDepth: 3,
         currentPackages,
         wantedPackages: currentPackages,
       }, [rootDepPath], rootDepPath)
@@ -225,7 +224,7 @@ describe('getTree', () => {
 
     test('revisiting package at higher depth does not print extra dependenices', () => {
       // This tests the "glob" npm package on a subset of its dependency tree.
-      // Requested depth (max depth - current depth) shown in square brackets.
+      // Max depth shown in square brackets.
       //
       // root
       // └─┬ a [2]
@@ -244,8 +243,7 @@ describe('getTree', () => {
 
       const result = getTree({
         ...commonMockGetTreeArgs,
-        currentDepth: 0,
-        maxDepth: 2,
+        maxDepth: 3,
         currentPackages,
         wantedPackages: currentPackages,
       }, [rootDepPath], rootDepPath)
@@ -299,7 +297,7 @@ describe('getTree', () => {
 
     // The fully visited cache can be used in this situation.
     test('height < requestedDepth', () => {
-      // Requested depth (max depth - current depth) shown in square brackets.
+      // Max depth shown in square brackets.
       //
       // root
       // ├─┬ a [3]
@@ -317,8 +315,7 @@ describe('getTree', () => {
 
       const result = getTree({
         ...commonMockGetTreeArgs,
-        currentDepth: 0,
-        maxDepth: 3,
+        maxDepth: 4,
         currentPackages,
         wantedPackages: currentPackages,
       }, [rootDepPath], rootDepPath)
@@ -350,7 +347,7 @@ describe('getTree', () => {
     })
 
     test('height === requestedDepth', () => {
-      // Requested depth (max depth - current depth) shown in square brackets.
+      // Max depth shown in square brackets.
       //
       // root
       // ├─┬ a [3]       <-- 1st time "a" is seen, its dependencies are recorded to the cache with a height of 1.
@@ -370,8 +367,7 @@ describe('getTree', () => {
 
       const result = getTree({
         ...commonMockGetTreeArgs,
-        currentDepth: 0,
-        maxDepth: 3,
+        maxDepth: 4,
         currentPackages,
         wantedPackages: currentPackages,
       }, [rootDepPath], rootDepPath)
@@ -403,7 +399,7 @@ describe('getTree', () => {
     })
 
     test('height > requestedDepth', () => {
-      // Requested depth (max depth - current depth) shown in square brackets.
+      // Max depth shown in square brackets.
       //
       // root
       // ├─┬ a [3]       <-- 1st time "a" is seen. Its dependencies are recorded to the cache with a height of 1.
@@ -427,8 +423,7 @@ describe('getTree', () => {
 
       const result = getTree({
         ...commonMockGetTreeArgs,
-        currentDepth: 0,
-        maxDepth: 3,
+        maxDepth: 4,
         currentPackages,
         wantedPackages: currentPackages,
       }, [rootDepPath], rootDepPath)