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.

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

chore: refactor node module collector, extract explicit DependencyTree, update types #8872

Merged
merged 18 commits into from
Feb 20, 2025
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
Next Next commit
build and tests run
  • Loading branch information
mmaietta committed Feb 15, 2025
commit 1fce5848b17dc936c9c46842918db40c02b5b15f
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { hoist, type HoisterTree, type HoisterResult } from "./hoist"
import * as path from "path"
import * as fs from "fs"
import { NodeModuleInfo, DependencyTree, DependencyGraph } from "./types"
import { NodeModuleInfo, DependencyTree, DependencyGraph, NpmDependency } from "./types"
import { exec, log } from "builder-util"

export abstract class NodeModulesCollector {
Expand Down Expand Up @@ -48,34 +48,37 @@ export abstract class NodeModulesCollector {
}
}

private convertToDependencyGraph(tree: DependencyTree): DependencyGraph {
const result: DependencyGraph = { ".": {} }

const flatten = (node: DependencyTree, parentKey = ".") => {
const dependencies = node.dependencies || {}

for (const [key, value] of Object.entries(dependencies)) {
// Skip empty dependencies(like some optionalDependencies)
if (Object.keys(value).length === 0) {
continue
}
const version = value.version || ""
const newKey = `${key}@${version}`
this.dependencyPathMap.set(newKey, path.normalize(this.resolvePath(value.path)))
if (!result[parentKey]?.dependencies) {
result[parentKey] = { dependencies: [] }
}
result[parentKey].dependencies!.push(newKey)

if (node.__circularDependencyDetected) {
continue
}
flatten(value, newKey)
private convertToDependencyGraph(tree: DependencyTree, parentKey = "."): DependencyGraph {
return Object.entries(tree.dependencies || {}).reduce<DependencyGraph>((acc, curr) => {
const [packageName, dependencies] = curr
// Skip empty dependencies(like some optionalDependencies)
if (Object.keys(dependencies).length === 0) {
return acc
}
const version = dependencies.version || ""
const newKey = `${packageName}@${version}`
// Map dependency details: name, version and path to the dependency tree
this.dependencyPathMap.set(newKey, path.normalize(this.resolvePath(dependencies.path)))
if (!acc[parentKey]) {
acc[parentKey] = { dependencies: [] }
}
acc[parentKey].dependencies.push(newKey)
if (tree.circularDependencyDetected) {
log.debug(
{
dependency: packageName,
version,
path: dependencies.path,
parentModule: tree.name,
parentVersion: tree.version,
},
"evaluated circluar dependency; skipping dependency flattening step"
)
return acc
}
}

flatten(tree)
return result
return { ...acc, ...this.convertToDependencyGraph(dependencies, newKey) }
}, {})
}

getAllDependencies(tree: DependencyTree) {
Expand All @@ -88,18 +91,18 @@ export abstract class NodeModulesCollector {
}
}

abstract getCommand(): string
abstract getArgs(): string[]
abstract removeNonProductionDependencie(tree: DependencyTree): void
protected abstract getCommand(): string
protected abstract getArgs(): string[]
protected abstract removeNonProductionDependencies(tree: DependencyTree): void

protected async getDependenciesTree(): Promise<DependencyTree> {
protected async getDependenciesTree(): Promise<NpmDependency> {
const command = this.getCommand()
const args = this.getArgs()
const dependencies = await exec(command, args, {
cwd: this.rootDir,
shell: true,
})
const dependencyTree: DependencyTree | DependencyTree[] = JSON.parse(dependencies)
const dependencyTree: NpmDependency | NpmDependency[] = JSON.parse(dependencies)

// pnpm returns an array of dependency trees
if (Array.isArray(dependencyTree)) {
Expand Down Expand Up @@ -155,7 +158,7 @@ export abstract class NodeModulesCollector {
const tree = await this.getDependenciesTree()
const realTree = this.getTreeFromWorkspaces(tree)
this.getAllDependencies(realTree)
this.removeNonProductionDependencie(realTree)
this.removeNonProductionDependencies(realTree)
const dependencyGraph = this.convertToDependencyGraph(realTree)
const hoisterResult = hoist(this.transToHoisterTree(dependencyGraph), { check: true })
this._getNodeModules(hoisterResult.dependencies, this.nodeModules)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,12 +15,12 @@ export class NpmNodeModulesCollector extends NodeModulesCollector {
return ["list", "-a", "--include", "prod", "--include", "optional", "--omit", "dev", "--json", "--long", "--silent"]
}

removeNonProductionDependencie(tree: DependencyTree) {
removeNonProductionDependencies(tree: DependencyTree) {
const dependencies = tree.dependencies || {}
const _dependencies = tree._dependencies || {}
if (Object.keys(_dependencies).length > 0 && Object.keys(dependencies).length === 0) {
tree.dependencies = this.allDependencies.get(`${tree.name}@${tree.version}`)?.dependencies || {}
tree.__circularDependencyDetected = true
tree.circularDependencyDetected = true
log.debug({ name: tree.name, version: tree.version }, "circular dependency detected")
return
}
Expand All @@ -30,7 +30,7 @@ export class NpmNodeModulesCollector extends NodeModulesCollector {
delete dependencies[key]
continue
}
this.removeNonProductionDependencie(value)
this.removeNonProductionDependencies(value)
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,10 @@ export class PnpmNodeModulesCollector extends NodeModulesCollector {
return ["list", "--prod", "--json", "--depth", "Infinity"]
}

removeNonProductionDependencie(tree: DependencyTree) {
removeNonProductionDependencies(tree: DependencyTree) {
const dependencies = tree.dependencies || {}
const p = path.normalize(this.resolvePath(tree.path))
const pJson: Dependency = require(path.join(p, "package.json"))
const pJson: Dependency<string> = require(path.join(p, "package.json"))
const _dependencies = pJson.dependencies || {}
const _optionalDependencies = pJson.optionalDependencies || {}
const prodDependencies = { ..._dependencies, ..._optionalDependencies }
Expand All @@ -27,7 +27,7 @@ export class PnpmNodeModulesCollector extends NodeModulesCollector {
delete dependencies[key]
continue
}
this.removeNonProductionDependencie(value)
this.removeNonProductionDependencies(value)
}
}
}
42 changes: 24 additions & 18 deletions packages/app-builder-lib/src/node-module-collector/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,33 +5,39 @@ export interface NodeModuleInfo {
dependencies?: Array<NodeModuleInfo>
}

export interface DependencyTree {
export interface DependencyTree extends Dependency<DependencyTree>, ParsedDependencyTree {
// I hate this, but this needs to be optional to convert NpmDependency=>DependencyTree before this value can get set
// We can't set this with an initial value due to the need to set it recursively, and this can't be recursively applied beforehand without a `RangeError: Maximum call stack size exceeded`
circularDependencyDetected?: boolean
}

export interface ParsedDependencyTree {
readonly name: string
readonly version: string
readonly path: string
readonly workspaces?: string[]
__circularDependencyDetected?: boolean
}

export interface NpmDependency extends Dependency<NpmDependency>, ParsedDependencyTree {
// npm-specific: for `npm list --json` to detect circular dependencies
// _dependencies?: {
// [packageName: string]: NpmDependency
// }
}

export type Dependency<T> = {
dependencies?: {
[packageName: string]: DependencyTree
}
// for npm list --json
_dependencies?: {
[packageName: string]: string
[packageName: string]: T
}
optionalDependencies?: {
[packageName: string]: DependencyTree
[packageName: string]: T
}
peerDependencies?: {
[packageName: string]: DependencyTree
[packageName: string]: T
}
}

export interface Dependency {
dependencies?: {
[packageName: string]: string
}
optionalDependencies?: {
[packageName: string]: string
// npm-specific: for `npm list --json` to detect circular dependencies
_dependencies?: {
[packageName: string]: T
}
}

Expand All @@ -40,5 +46,5 @@ export interface DependencyGraph {
}

interface PackageDependencies {
dependencies?: string[]
dependencies: string[]
}