fix comments

packages/app-builder-lib/src/node-module-collector/index.ts

@@ -1,7 +1,7 @@
 import { NpmNodeModulesCollector } from "./npmNodeModulesCollector"
 import { PnpmNodeModulesCollector } from "./pnpmNodeModulesCollector"
 import { YarnNodeModulesCollector } from "./yarnNodeModulesCollector"
-import { detect, PM, getNpmVersion } from "./packageManager"
+import { detect, PM, getPackageManagerVersion } from "./packageManager"
 import { NodeModuleInfo } from "./types"
 import { log } from "builder-util"
 
@@ -15,7 +15,7 @@ async function getCollectorByPackageManager(rootDir: string) {
     case "yarn":
       return new YarnNodeModulesCollector(rootDir)
     default:
-      log.warn({ rootDir }, `Cannot detect lock file to determine node_modules structure. Assuming plain structure.`)
+      log.filePath(rootDir)
       return new NpmNodeModulesCollector(rootDir)
   }
 }
@@ -25,4 +25,4 @@ export async function getNodeModules(rootDir: string): Promise<NodeModuleInfo[]>
   return collector.getNodeModules()
 }
 
-export { detect, getNpmVersion, PM }
+export { detect, getPackageManagerVersion, PM }

packages/app-builder-lib/src/node-module-collector/nodeModulesCollector.ts

@@ -2,17 +2,15 @@ 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 { log } from "builder-util"
+import { exec, log } from "builder-util"
 
 export abstract class NodeModulesCollector {
   private nodeModules: NodeModuleInfo[]
-  protected rootDir: string
   protected dependencyPathMap: Map<string, string>
 
-  constructor(rootDir: string) {
+  constructor(private readonly rootDir: string) {
     this.dependencyPathMap = new Map()
     this.nodeModules = []
-    this.rootDir = rootDir
   }
 
   private transToHoisterTree(obj: DependencyGraph, key: string = `.`, nodes: Map<string, HoisterTree> = new Map()): HoisterTree {
@@ -43,13 +41,13 @@ export abstract class NodeModulesCollector {
       } else {
         return filePath
       }
-    } catch (error) {
-      log.error({ filePath, error }, "Error resolving path")
+    } catch (error: any) {
+      log.debug({ message: error.message || error.stack }, "error resolving path")
       return filePath
     }
   }
 
-  public TransToDependencyGraph(tree: DependencyTree): DependencyGraph {
+  public convertToDependencyGraph(tree: DependencyTree): DependencyGraph {
     const result: DependencyGraph = { ".": {} }
 
     const flatten = (node: DependencyTree, parentKey = ".") => {
@@ -75,20 +73,36 @@ export abstract class NodeModulesCollector {
     return result
   }
 
-  abstract getDependenciesTree(): DependencyTree
-  abstract getPMCommand(): string
+  abstract getCommand(): string
+  abstract getArgs(): string[]
+
+  protected async getDependenciesTree(): Promise<DependencyTree> {
+    const command = this.getCommand()
+    const args = this.getArgs()
+    const dependencies = await exec(command, args, {
+      cwd: this.rootDir,
+    })
+    const dependencyTree: DependencyTree | DependencyTree[] = JSON.parse(dependencies)
+    return Array.isArray(dependencyTree) ? dependencyTree[0] : dependencyTree
+  }
 
   private _getNodeModules(dependencies: Set<HoisterResult>, result: NodeModuleInfo[]) {
-    if (dependencies.size === 0) return
+    if (dependencies.size === 0) {
+      return
+    }
 
     for (const d of dependencies.values()) {
       const reference = [...d.references][0]
       const p = this.dependencyPathMap.get(`${d.name}@${reference}`)
-      const node = {
+      if (p === undefined) {
+        log.debug({ name: d.name, reference }, "cannot find path for dependency")
+        continue
+      }
+      const node: NodeModuleInfo = {
         name: d.name,
         version: reference,
         dir: p,
-      } as NodeModuleInfo
+      }
       result.push(node)
       if (d.dependencies.size > 0) {
         node["dependencies"] = []
@@ -109,10 +123,10 @@ export abstract class NodeModulesCollector {
     return tree
   }
 
-  public getNodeModules(): NodeModuleInfo[] {
-    const tree = this.getDependenciesTree()
+  public async getNodeModules(): Promise<NodeModuleInfo[]> {
+    const tree = await this.getDependenciesTree()
     const realTree = this.getTreeFromWorkspaces(tree)
-    const dependencyGraph = this.TransToDependencyGraph(realTree)
+    const dependencyGraph = this.convertToDependencyGraph(realTree)
     const hoisterResult = hoist(this.transToHoisterTree(dependencyGraph), { check: true })
     this._getNodeModules(hoisterResult.dependencies, this.nodeModules)
     return this.nodeModules

packages/app-builder-lib/src/node-module-collector/npmNodeModulesCollector.ts

@@ -1,25 +1,15 @@
-import { execSync } from "child_process"
 import { NodeModulesCollector } from "./nodeModulesCollector"
-import { DependencyTree } from "./types"
 
 export class NpmNodeModulesCollector extends NodeModulesCollector {
   constructor(rootDir: string) {
     super(rootDir)
   }
 
-  getPMCommand(): string {
-    const cmd = process.platform === "win32" ? "npm.cmd" : "npm"
-    return `${cmd} list --omit dev -a --json --long`
+  getCommand(): string {
+    return process.platform === "win32" ? "npm.cmd" : "npm"
   }
 
-  getDependenciesTree(): DependencyTree {
-    const npmListOutput = execSync(this.getPMCommand(), {
-      cwd: this.rootDir,
-      encoding: "utf-8",
-      maxBuffer: 1024 * 1024 * 100,
-    })
-
-    const dependencyTree: DependencyTree = JSON.parse(npmListOutput)
-    return dependencyTree
+  getArgs(): string[] {
+    return ["list", "--omit", "dev", "-a", "--json", "--long"]
   }
 }

packages/app-builder-lib/src/node-module-collector/packageManager.ts

@@ -1,70 +1,55 @@
 // copy from https://github.com/egoist/detect-package-manager/blob/main/src/index.ts
 // and merge https://github.com/egoist/detect-package-manager/pull/9 to support Monorepo
-import { promises as fs } from "fs"
 import { resolve, dirname } from "path"
-import { exec } from "child_process"
-import { promisify } from "util"
-
-const execa = promisify(exec)
+import { exec, exists } from "builder-util"
 
 export type PM = "npm" | "yarn" | "pnpm" | "bun"
 
-/**
- * Check if a path exists
- */
-async function pathExists(p: string) {
-  try {
-    await fs.access(p)
-    return true
-  } catch {
-    return false
-  }
-}
-
 const cache = new Map()
+const globalInstallationCache = new Map<string, boolean>()
+const lockfileCache = new Map<string, PM>()
 
 /**
  * Check if a global pm is available
  */
 function hasGlobalInstallation(pm: PM): Promise<boolean> {
   const key = `has_global_${pm}`
-  if (cache.has(key)) {
-    return Promise.resolve(cache.get(key))
+  if (globalInstallationCache.has(key)) {
+    return Promise.resolve(globalInstallationCache.get(key)!)
   }
-  const execa = promisify(exec)
 
-  return execa(`${pm} --version`)
+  return exec(pm, ["--version"])
     .then(res => {
-      return /^\d+.\d+.\d+$/.test(res.stdout)
+      return /^\d+.\d+.\d+$/.test(res)
     })
     .then(value => {
-      cache.set(key, value)
+      globalInstallationCache.set(key, value)
       return value
     })
     .catch(() => false)
 }
 
-function getTypeofLockFile(cwd = "."): Promise<PM | null> {
+function getTypeofLockFile(cwd = process.cwd()): Promise<PM> {
   const key = `lockfile_${cwd}`
-  if (cache.has(key)) {
-    return Promise.resolve(cache.get(key))
+  if (lockfileCache.has(key)) {
+    return Promise.resolve(lockfileCache.get(key)!)
   }
 
   return Promise.all([
-    pathExists(resolve(cwd, "yarn.lock")),
-    pathExists(resolve(cwd, "package-lock.json")),
-    pathExists(resolve(cwd, "pnpm-lock.yaml")),
-    pathExists(resolve(cwd, "bun.lockb")),
-  ]).then(([isYarn, isNpm, isPnpm, isBun]) => {
-    let value: PM | null = null
+    exists(resolve(cwd, "yarn.lock")),
+    exists(resolve(cwd, "package-lock.json")),
+    exists(resolve(cwd, "pnpm-lock.yaml")),
+    exists(resolve(cwd, "bun.lockb")),
+  ]).then(([isYarn, _, isPnpm, isBun]) => {
+    let value: PM
 
     if (isYarn) {
       value = "yarn"
     } else if (isPnpm) {
       value = "pnpm"
     } else if (isBun) {
       value = "bun"
-    } else if (isNpm) {
+    } else {
       value = "npm"
     }
 
@@ -73,7 +58,7 @@ function getTypeofLockFile(cwd = "."): Promise<PM | null> {
   })
 }
 
-const detect = async ({ cwd, includeGlobalBun }: { cwd?: string; includeGlobalBun?: boolean } = {}) => {
+export const detect = async ({ cwd, includeGlobalBun }: { cwd?: string; includeGlobalBun?: boolean } = {}) => {
   let type = await getTypeofLockFile(cwd)
   if (type) {
     return type
@@ -88,23 +73,21 @@ const detect = async ({ cwd, includeGlobalBun }: { cwd?: string; includeGlobalBu
     }
   }
 
-  const [hasYarn, hasPnpm, hasBun] = await Promise.all([hasGlobalInstallation("yarn"), hasGlobalInstallation("pnpm"), includeGlobalBun && hasGlobalInstallation("bun")])
-  if (hasYarn) {
+  if (await hasGlobalInstallation("yarn")) {
     return "yarn"
   }
-  if (hasPnpm) {
-    return "pnpm"
+  if (await hasGlobalInstallation("pnpm")) {
+    return "yarn"
   }
-  if (hasBun) {
+
+  if (includeGlobalBun && (await hasGlobalInstallation("bun"))) {
     return "bun"
   }
   return "npm"
 }
 
-export { detect }
-
-export function getNpmVersion(pm: PM) {
-  return execa(`${pm} --version`).then(res => res.stdout.trim())
+export function getPackageManagerVersion(pm: PM) {
+  return exec(pm, ["--version"]).then(res => res.trim())
 }
 
 export function clearCache() {

packages/app-builder-lib/src/node-module-collector/pnpmNodeModulesCollector.ts

@@ -1,25 +1,15 @@
-import { execSync } from "child_process"
 import { NodeModulesCollector } from "./nodeModulesCollector"
-import { DependencyTree } from "./types"
 
 export class PnpmNodeModulesCollector extends NodeModulesCollector {
   constructor(rootDir: string) {
     super(rootDir)
   }
 
-  getPMCommand(): string {
-    const cmd = process.platform === "win32" ? "pnpm.cmd" : "pnpm"
-    return `${cmd} list --prod --json --long --depth Infinity`
+  getCommand(): string {
+    return process.platform === "win32" ? "pnpm.cmd" : "pnpm"
   }
 
-  getDependenciesTree() {
-    const pnpmListOutput = execSync(this.getPMCommand(), {
-      cwd: this.rootDir,
-      encoding: "utf-8",
-      maxBuffer: 1024 * 1024 * 100,
-    })
-
-    const dependencyTree: DependencyTree = JSON.parse(pnpmListOutput)[0]
-    return dependencyTree
+  getArgs(): string[] {
+    return ["list", "--prod", "--json", "--long", "--depth", "Infinity"]
   }
 }

packages/app-builder-lib/src/node-module-collector/yarnNodeModulesCollector.ts

@@ -1,35 +1,15 @@
-import { execSync } from "child_process"
-import * as path from "path"
 import { NodeModulesCollector } from "./nodeModulesCollector"
-import { DependencyTree } from "./types"
-import { log } from "builder-util"
 
 export class YarnNodeModulesCollector extends NodeModulesCollector {
   constructor(rootDir: string) {
     super(rootDir)
   }
 
-  getPMCommand(): string {
-    const cmd = process.platform === "win32" ? "npm.cmd" : "npm"
-    return `${cmd} list --omit dev -a --json --long`
+  getCommand(): string {
+    return process.platform === "win32" ? "npm.cmd" : "npm"
   }
 
-  getDependenciesTree(): DependencyTree {
-    try {
-      const stdout = execSync(this.getPMCommand(), {
-        cwd: this.rootDir,
-        encoding: "utf-8",
-        maxBuffer: 1024 * 1024 * 100,
-      })
-      return JSON.parse(stdout) as DependencyTree
-    } catch (error) {
-      log.debug({ error }, "npm list failed in yarn project, but will be ignored")
-      if ((error as any).stdout) {
-        const stdout = (error as any).stdout
-        return JSON.parse(stdout) as DependencyTree
-      }
-    }
-
-    return require(path.join(this.rootDir, "package.json"))
+  getArgs(): string[] {
+    return ["list", "--omit", "dev", "-a", "--json", "--long", "--silent"]
   }
 }

packages/app-builder-lib/src/util/appFileCopier.ts

@@ -181,7 +181,7 @@ function validateFileSet(fileSet: ResolvedFileSet): ResolvedFileSet {
 
 /** @internal */
 export async function computeNodeModuleFileSets(platformPackager: PlatformPackager<any>, mainMatcher: FileMatcher): Promise<Array<ResolvedFileSet>> {
-  const deps = (await getNodeModules(platformPackager.info.appDir)) as NodeModuleInfo[]
+  const deps = await getNodeModules(platformPackager.info.appDir)
   log.debug({ nodeModules: deps }, "collected node modules")
 
   const nodeModuleExcludedExts = getNodeModuleExcludedExts(platformPackager)

packages/app-builder-lib/src/util/yarn.ts

@@ -10,7 +10,7 @@ import { getProjectRootPath } from "@electron/rebuild/lib/search-module"
 import { rebuild as remoteRebuild } from "./rebuild/rebuild"
 import { executeAppBuilderAndWriteJson } from "./appBuilder"
 import { RebuildMode } from "@electron/rebuild/lib/types"
-import { PM, detect, getNpmVersion } from "../node-module-collector"
+import { PM, detect, getPackageManagerVersion } from "../node-module-collector"
 
 export async function installOrRebuild(config: Configuration, appDir: string, options: RebuildOptions, forceInstall = false) {
   const effectiveOptions: RebuildOptions = {
@@ -83,7 +83,7 @@ async function checkYarnBerry(pm: PM) {
   if (pm !== "yarn") {
     return false
   }
-  const version = await getNpmVersion(pm)
+  const version = await getPackageManagerVersion(pm)
   if (version == null || version.split(".").length < 1) {
     return false
   }
@@ -146,14 +146,11 @@ export async function nodeGypRebuild(platform: NodeJS.Platform, arch: string, fr
 }
 
 function getPackageToolPath(pm: PM) {
-  const suffix = process.platform === "win32" ? ".cmd" : ""
   let cmd = pm
-
   if (process.env.FORCE_YARN === "true") {
     cmd = "yarn"
   }
-
-  return cmd + suffix
+  return `${cmd}${process.platform === "win32" ? ".cmd" : ""}`
 }
 
 function isRunningYarn(pm: PM) {