fix: search for lockfile in parent directories (#1139)

Author: sorccu

packages/cli/src/services/check-parser/package-files/package-manager.ts

@@ -1,6 +1,8 @@
 import fs from 'node:fs/promises'
 import path from 'node:path'
 
+import { lineage } from './walk'
+
 export class Runnable {
   executable: string
   args: string[]
@@ -44,7 +46,8 @@ export abstract class PackageManagerDetector {
   abstract get name (): string
   abstract detectUserAgent (userAgent: string): boolean
   abstract detectRuntime (): boolean
-  abstract detectLockfile (dir: string): Promise<void>
+  abstract get representativeLockfile (): string | undefined
+  abstract detectLockfile (dir: string): Promise<string>
   abstract detectExecutable (lookup: PathLookup): Promise<void>
   abstract installCommand (): Runnable
   abstract execCommand (args: string[]): Runnable
@@ -63,8 +66,12 @@ export class NpmDetector extends PackageManagerDetector implements PackageManage
     return false
   }
 
-  async detectLockfile (dir: string): Promise<void> {
-    return await accessR(path.join(dir, 'package-lock.json'))
+  get representativeLockfile (): string {
+    return 'package-lock.json'
+  }
+
+  async detectLockfile (dir: string): Promise<string> {
+    return await accessR(path.join(dir, this.representativeLockfile))
   }
 
   async detectExecutable (lookup: PathLookup): Promise<void> {
@@ -93,8 +100,12 @@ export class CNpmDetector extends PackageManagerDetector implements PackageManag
     return false
   }
 
+  get representativeLockfile (): undefined {
+    return
+  }
+
   // eslint-disable-next-line require-await
-  async detectLockfile (): Promise<void> {
+  async detectLockfile (): Promise<string> {
     throw new NotDetectedError()
   }
 
@@ -124,8 +135,12 @@ export class PNpmDetector extends PackageManagerDetector implements PackageManag
     return false
   }
 
-  async detectLockfile (dir: string): Promise<void> {
-    return await accessR(path.join(dir, 'pnpm-lock.yaml'))
+  get representativeLockfile (): string {
+    return 'pnpm-lock.yaml'
+  }
+
+  async detectLockfile (dir: string): Promise<string> {
+    return await accessR(path.join(dir, this.representativeLockfile))
   }
 
   async detectExecutable (lookup: PathLookup): Promise<void> {
@@ -154,8 +169,12 @@ export class YarnDetector extends PackageManagerDetector implements PackageManag
     return false
   }
 
-  async detectLockfile (dir: string): Promise<void> {
-    return await accessR(path.join(dir, 'yarn.lock'))
+  get representativeLockfile (): string {
+    return 'yarn.lock'
+  }
+
+  async detectLockfile (dir: string): Promise<string> {
+    return await accessR(path.join(dir, this.representativeLockfile))
   }
 
   async detectExecutable (lookup: PathLookup): Promise<void> {
@@ -184,8 +203,12 @@ export class DenoDetector extends PackageManagerDetector implements PackageManag
     return process.versions.deno !== undefined
   }
 
-  async detectLockfile (dir: string): Promise<void> {
-    return await accessR(path.join(dir, 'deno.lock'))
+  get representativeLockfile (): string {
+    return 'deno.lock'
+  }
+
+  async detectLockfile (dir: string): Promise<string> {
+    return await accessR(path.join(dir, this.representativeLockfile))
   }
 
   async detectExecutable (lookup: PathLookup): Promise<void> {
@@ -214,8 +237,12 @@ export class BunDetector extends PackageManagerDetector implements PackageManage
     return process.versions.bun !== undefined
   }
 
-  async detectLockfile (dir: string): Promise<void> {
-    return await accessR(path.join(dir, 'bun.lockb'))
+  get representativeLockfile (): string {
+    return 'bun.lockb'
+  }
+
+  async detectLockfile (dir: string): Promise<string> {
+    return await accessR(path.join(dir, this.representativeLockfile))
   }
 
   async detectExecutable (lookup: PathLookup): Promise<void> {
@@ -231,17 +258,19 @@ export class BunDetector extends PackageManagerDetector implements PackageManage
   }
 }
 
-async function accessR (filePath: string): Promise<void> {
+async function accessR (filePath: string): Promise<string> {
   try {
     await fs.access(filePath, fs.constants.R_OK)
+    return filePath
   } catch {
     throw new NotDetectedError()
   }
 }
 
-async function accessX (filePath: string): Promise<void> {
+async function accessX (filePath: string): Promise<string> {
   try {
     await fs.access(filePath, fs.constants.X_OK)
+    return filePath
   } catch {
     throw new NotDetectedError()
   }
@@ -333,13 +362,13 @@ export const knownPackageManagers: PackageManagerDetector[] = [
   npmDetector,
 ]
 
-export interface DetectPackageManagerOptions {
+export interface DetectOptions {
   detectors?: PackageManagerDetector[]
 }
 
 export async function detectPackageManager (
   dir: string,
-  options?: DetectPackageManagerOptions,
+  options?: DetectOptions,
 ): Promise<PackageManager> {
   const detectors = options?.detectors ?? knownPackageManagers
 
@@ -362,10 +391,11 @@ export async function detectPackageManager (
 
   // Next, try to find a lockfile.
   try {
-    return await Promise.any(detectors.map(async detector => {
-      await detector.detectLockfile(dir)
-      return detector
-    }))
+    const { packageManager } = await detectNearestLockfile(dir, {
+      detectors,
+    })
+
+    return packageManager
   } catch {
     // Nothing detected.
   }
@@ -387,3 +417,61 @@ export async function detectPackageManager (
   // If all else fails, just assume npm.
   return npmDetector
 }
+
+export interface NearestLockFile {
+  packageManager: PackageManager
+  lockfile: string
+}
+
+export class NoLockfileFoundError extends Error {
+  searchPaths: string[]
+  lockfiles: string[]
+
+  constructor (searchPaths: string[], lockfiles: string[], options?: ErrorOptions) {
+    const message = `Unable to detect a lockfile in any of the following paths:`
+      + `\n\n`
+      + `${searchPaths.map(searchPath => `  ${searchPath}`).join('\n')}`
+      + `\n\n`
+      + `Lockfiles we looked for:`
+      + `\n\n`
+      + `${lockfiles.map(lockfile => `  ${lockfile}`).join('\n')}`
+    super(message, options)
+    this.name = 'NoLockfileFoundError'
+    this.searchPaths = searchPaths
+    this.lockfiles = lockfiles
+  }
+}
+
+export async function detectNearestLockfile (
+  dir: string,
+  options?: DetectOptions,
+): Promise<NearestLockFile> {
+  const detectors = options?.detectors ?? knownPackageManagers
+
+  const searchPaths: string[] = []
+
+  // Next, try to find a lockfile.
+  for (const searchPath of lineage(dir)) {
+    try {
+      searchPaths.push(searchPath)
+
+      // Assume that only a single kind of lockfile exists, which means the
+      // resolve order does not matter.
+      return await Promise.any(detectors.map(async detector => {
+        const lockfile = await detector.detectLockfile(searchPath)
+        return {
+          packageManager: detector,
+          lockfile,
+        }
+      }))
+    } catch {
+      // Nothing detected.
+    }
+  }
+
+  const lockfiles = detectors.reduce<string[]>((acc, detector) => {
+    return acc.concat(detector.representativeLockfile ?? [])
+  }, [])
+
+  throw new NoLockfileFoundError(searchPaths, lockfiles)
+}

packages/cli/src/services/check-parser/package-files/resolver.ts

@@ -8,6 +8,7 @@ import { isBuiltinPath, isLocalPath, PathResult } from './paths'
 import { FileLoader, LoadFile } from './loader'
 import { JsonSourceFile } from './json-source-file'
 import { LookupContext } from './lookup'
+import { walkUp, WalkUpOptions } from './walk'
 
 class PackageFilesCache {
   #sourceFileCache = new FileLoader(SourceFile.loadFromFilePath)
@@ -142,48 +143,6 @@ export type Dependencies = {
   local: LocalDependency[]
 }
 
-export interface WalkUpOptions {
-  root?: string
-  isDir?: boolean
-}
-
-async function walkUp (
-  filePath: string,
-  find: (dirPath: string) => Promise<boolean>,
-  options?: WalkUpOptions,
-): Promise<boolean> {
-  let currentPath = filePath
-
-  if (options?.isDir === true) {
-    // To keep things simple, just add a dummy component.
-    currentPath = path.join(currentPath, 'z')
-  }
-
-  while (true) {
-    const prevPath = currentPath
-
-    currentPath = path.dirname(prevPath)
-
-    // Bail out if we reach root.
-    if (prevPath === currentPath) {
-      break
-    }
-
-    const found = await find(currentPath)
-    if (found) {
-      return true
-    }
-
-    // Stop if we reach the user-specified root directory.
-    // TODO: I don't like a string comparison for this but it'll do for now.
-    if (currentPath === options?.root) {
-      break
-    }
-  }
-
-  return false
-}
-
 export class PackageFilesResolver {
   cache = new PackageFilesCache()
 

packages/cli/src/services/check-parser/package-files/walk.ts

@@ -0,0 +1,58 @@
+import { resolve, dirname } from 'node:path'
+
+export interface WalkUpOptions extends LineageOptions {
+  isDir?: boolean
+}
+
+export async function walkUp (
+  filePath: string,
+  find: (dirPath: string) => Promise<boolean>,
+  options?: WalkUpOptions,
+): Promise<boolean> {
+  let startPath = filePath
+
+  if (options?.isDir !== true) {
+    startPath = dirname(startPath)
+  }
+
+  for (const dirPath of lineage(startPath, options)) {
+    const found = await find(dirPath)
+    if (found) {
+      return true
+    }
+  }
+
+  return false
+}
+
+export interface LineageOptions {
+  root?: string
+}
+
+export function* lineage (path: string, options?: LineageOptions): Generator<string, void> {
+  let currentPath = resolve(path)
+
+  const stopRoot = options?.root && resolve(options.root)
+
+  // Lineage includes the starting path itself.
+  yield currentPath
+
+  while (true) {
+    const prevPath = currentPath
+
+    // Stop if we reach the user-specified root directory.
+    // TODO: I don't like a string comparison for this but it'll do for now.
+    if (prevPath === stopRoot) {
+      return
+    }
+
+    currentPath = dirname(prevPath)
+
+    // Bail out if we reach root.
+    if (prevPath === currentPath) {
+      return
+    }
+
+    yield currentPath
+  }
+}

packages/cli/src/services/util.ts

@@ -16,10 +16,11 @@ import { ChecklyConfig, PlaywrightSlimmedProp } from './checkly-config-loader'
 import { Parser } from './check-parser/parser'
 import * as JSON5 from 'json5'
 import { PlaywrightConfig } from './playwright-config'
-import { access, readFile } from 'fs/promises'
+import { readFile } from 'fs/promises'
 import { createHash } from 'crypto'
 import { Session } from '../constructs'
 import semver from 'semver'
+import { detectNearestLockfile } from './check-parser/package-files/package-manager'
 
 export interface GitInformation {
   commitId: string
@@ -208,13 +209,10 @@ export async function bundlePlayWrightProject (
   archive.pipe(output)
 
   const pwConfigParsed = new PlaywrightConfig(filePath, pwtConfig)
-  const lockFile = await findLockFile(dir)
-  if (!lockFile) {
-    throw new Error('No lock file found')
-  }
+  const { lockfile } = await detectNearestLockfile(dir)
 
   const [cacheHash, playwrightVersion] = await Promise.all([
-    getCacheHash(lockFile),
+    getCacheHash(lockfile),
     getPlaywrightVersion(dir),
     loadPlaywrightProjectFiles(dir, pwConfigParsed, include, archive),
   ])
@@ -263,21 +261,6 @@ export async function getPlaywrightVersion (projectDir: string): Promise<string
   }
 }
 
-async function findLockFile (dir: string): Promise<string | null> {
-  const lockFiles = ['package-lock.json', 'pnpm-lock.yaml', 'yarn.lock']
-
-  for (const lockFile of lockFiles) {
-    const filePath = path.join(dir, lockFile)
-    try {
-      await access(filePath)
-      return filePath
-    } catch {
-      // Ignore errors, just check the next file
-    }
-  }
-  return null // Return null if no lock file is found
-}
-
 export async function loadPlaywrightProjectFiles (
   dir: string, pwConfigParsed: PlaywrightConfig, include: string[], archive: Archiver,
 ) {