Skip to content

Commit

Permalink
refactor(dependencies-hierarchy): merge isPartiallyVisited and height
Browse files Browse the repository at this point in the history
The numerical `height` value isn't used if `isPartiallyVisited` is set.
Merging these two fields makes that more clear.

The definition of the `height` field used in getTreeHelper is also
updated to include the parent node in this commit. This makes the field
consistent with the `height` definition in DependenciesCache.
  • Loading branch information
gluxon authored and zkochan committed Jan 1, 2023
1 parent a9304ac commit 37c818d
Show file tree
Hide file tree
Showing 2 changed files with 23 additions and 26 deletions.
7 changes: 2 additions & 5 deletions reviewing/dependencies-hierarchy/src/DependenciesCache.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,7 @@ export interface TraversalResultPartiallyVisited {

export interface CacheHit {
readonly dependencies: PackageNode[]
readonly height: number
readonly isPartiallyVisited: boolean
readonly height: number | 'unknown'
// Circular dependencies are not stored in the cache.
readonly circular: false
}
Expand Down Expand Up @@ -92,7 +91,6 @@ export class DependenciesCache {
return {
dependencies: fullyVisitedEntry.dependencies,
height: fullyVisitedEntry.height,
isPartiallyVisited: false,
circular: false,
}
}
Expand All @@ -101,8 +99,7 @@ export class DependenciesCache {
if (partiallyVisitedEntry != null) {
return {
dependencies: partiallyVisitedEntry,
height: args.requestedDepth,
isPartiallyVisited: true,
height: 'unknown',
circular: false,
}
}
Expand Down
42 changes: 21 additions & 21 deletions reviewing/dependencies-hierarchy/src/getTree.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,16 +25,15 @@ interface DependencyInfo {
circular?: true

/**
* Whether or not the dependencies array was fully enumerated. This may not be
* the case if a max depth was hit.
* The number of edges along the longest path, including the parent node.
*
* - `"unknown"` if traversal was limited by a max depth option, therefore
* making the true height of a package undetermined.
* - `0` if the dependencies array is empty.
* - `1` if the dependencies array has at least 1 element and no child
* dependencies.
*/
isPartiallyVisited: boolean

/**
* The number of edges along longest path. null if the dependencies array is
* empty.
*/
height: number | null
height: number | 'unknown'
}

export function getTree (
Expand All @@ -54,11 +53,11 @@ function getTreeHelper (
parentId: string
): DependencyInfo {
if (opts.maxDepth <= 0) {
return { dependencies: [], isPartiallyVisited: true, height: null }
return { dependencies: [], height: 'unknown' }
}

if (!opts.currentPackages?.[parentId]) {
return { dependencies: [], isPartiallyVisited: false, height: null }
return { dependencies: [], height: 0 }
}

const deps = !opts.includeOptionalDependencies
Expand All @@ -69,7 +68,7 @@ function getTreeHelper (
}

if (deps == null) {
return { dependencies: [], isPartiallyVisited: false, height: null }
return { dependencies: [], height: 0 }
}

const childTreeMaxDepth = opts.maxDepth - 1
Expand All @@ -81,9 +80,8 @@ function getTreeHelper (
const peers = new Set(Object.keys(opts.currentPackages[parentId].peerDependencies ?? {}))

const resultDependencies: PackageNode[] = []
let resultHeight: number | null = null
let resultHeight: number | 'unknown' = 0
let resultCircular: boolean = false
let resultIsPartiallyVisited = false

Object.entries(deps).forEach(([alias, ref]) => {
const { packageInfo, packageAbsolutePath } = getPkgInfo({
Expand Down Expand Up @@ -116,25 +114,28 @@ function getTreeHelper (
const cacheEntry = dependenciesCache.get({ packageAbsolutePath, requestedDepth: childTreeMaxDepth })
const children = cacheEntry ?? getChildrenTree(keypath.concat([relativeId]), relativeId)

const heightOfCurrentDepNode = children.height == null ? 0 : children.height + 1

if (cacheEntry == null && !children.circular) {
if (children.isPartiallyVisited) {
if (children.height === 'unknown') {
dependenciesCache.addPartiallyVisitedResult(packageAbsolutePath, {
dependencies: children.dependencies,
depth: childTreeMaxDepth,
})
} else {
dependenciesCache.addFullyVisitedResult(packageAbsolutePath, {
dependencies: children.dependencies,
height: heightOfCurrentDepNode,
height: children.height,
})
}
}

const heightOfCurrentDepNode = children.height === 'unknown'
? 'unknown'
: children.height + 1

dependencies = children.dependencies
resultIsPartiallyVisited = resultIsPartiallyVisited || children.isPartiallyVisited
resultHeight = Math.max(resultHeight ?? 0, heightOfCurrentDepNode)
resultHeight = resultHeight === 'unknown' || heightOfCurrentDepNode === 'unknown'
? 'unknown'
: Math.max(resultHeight, heightOfCurrentDepNode)
resultCircular = resultCircular || (children.circular ?? false)
}

Expand All @@ -161,7 +162,6 @@ function getTreeHelper (

const result: DependencyInfo = {
dependencies: resultDependencies,
isPartiallyVisited: resultIsPartiallyVisited,
height: resultHeight,
}

Expand Down

0 comments on commit 37c818d

Please sign in to comment.