perf: remove find-up dependency, upgrade file-type dependency (#8195)

AlessioGr · web-flow · commit 6d1a287dd1e1 · 2024-09-15T16:53:53.000Z
Fixes #8111 and #8113 Before: 132 dependencies After: 123 dependencies This PR also contains a small performance optimization during telemetry startup: By using the async `fs.promises.readFile` instead of `readFileSync` we're not blocking the entire thread anymore and are allowing other stuff to happen while the file is being read. Also, in our dependency checker, this moves some variables out of loops, to the module scope, as they only need to be calculated once. We have to pin file-type to 19.3.0 and cannot upgrade it further (latest is 19.5.0). See reasoning in #8111 (comment)
diff --git a/jest.config.js b/jest.config.js
@@ -5,14 +5,14 @@ const esModules = [
   'readable-web-to-node-stream',
   'token-types',
   'peek-readable',
-  'find-up',
   'locate-path',
   'p-locate',
   'p-limit',
   'yocto-queue',
   'unicorn-magic',
   'path-exists',
   'qs-esm',
+  'uint8array-extras',
 ].join('|')
 
 /** @type {import('jest').Config}  */
diff --git a/packages/next/package.json b/packages/next/package.json
@@ -70,7 +70,7 @@
     "@payloadcms/translations": "workspace:*",
     "@payloadcms/ui": "workspace:*",
     "busboy": "^1.6.0",
-    "file-type": "17.1.6",
+    "file-type": "19.3.0",
     "graphql-http": "^1.22.0",
     "graphql-playground-html": "1.6.30",
     "http-status": "1.6.2",
diff --git a/packages/payload/package.json b/packages/payload/package.json
@@ -94,8 +94,7 @@
     "console-table-printer": "2.11.2",
     "dataloader": "2.2.2",
     "deepmerge": "4.3.1",
-    "file-type": "17.1.6",
-    "find-up": "7.0.0",
+    "file-type": "19.3.0",
     "get-tsconfig": "^4.7.2",
     "http-status": "1.6.2",
     "image-size": "^1.1.1",
diff --git a/packages/payload/src/bin/loadEnv.ts b/packages/payload/src/bin/loadEnv.ts
@@ -1,6 +1,7 @@
 import nextEnvImport from '@next/env'
+
+import { findUpSync } from '../utilities/findUp.js'
 const { loadEnvConfig } = nextEnvImport
-import { findUpStop, findUpSync } from 'find-up'
 
 /**
  * Try to find user's env files and load it. Uses the same algorithm next.js uses to parse env files, meaning this also supports .env.local, .env.development, .env.production, etc.
@@ -15,11 +16,14 @@ export function loadEnv(path?: string) {
 
   if (!loadedEnvFiles?.length) {
     // use findUp to find the env file. So, run loadEnvConfig for every directory upwards
-    findUpSync((dir) => {
-      const { loadedEnvFiles } = loadEnvConfig(dir, true)
-      if (loadedEnvFiles?.length) {
-        return findUpStop
-      }
+    findUpSync({
+      condition: (dir) => {
+        const { loadedEnvFiles } = loadEnvConfig(dir, true)
+        if (loadedEnvFiles?.length) {
+          return true
+        }
+      },
+      dir: process.cwd(),
     })
   }
 }
diff --git a/packages/payload/src/config/find.ts b/packages/payload/src/config/find.ts
@@ -1,7 +1,13 @@
-import { findUpSync, pathExistsSync } from 'find-up'
 import { getTsconfig } from 'get-tsconfig'
 import path from 'path'
 
+import { findUpSync } from '../utilities/findUp.js'
+
+/**
+ * List of all filenames to detect as a Payload configuration file.
+ */
+export const payloadConfigFileNames = ['payload.config.js', 'payload.config.ts']
+
 /**
  * Returns the source and output paths from the nearest tsconfig.json file.
  * If no tsconfig.json file is found, returns the current working directory.
@@ -75,26 +81,10 @@ export const findConfig = (): string => {
       continue
     }
 
-    const configPath = findUpSync(
-      (dir) => {
-        const tsPath = path.join(dir, 'payload.config.ts')
-        const hasTS = pathExistsSync(tsPath)
-
-        if (hasTS) {
-          return tsPath
-        }
-
-        const jsPath = path.join(dir, 'payload.config.js')
-        const hasJS = pathExistsSync(jsPath)
-
-        if (hasJS) {
-          return jsPath
-        }
-
-        return undefined
-      },
-      { cwd: searchPath },
-    )
+    const configPath = findUpSync({
+      dir: searchPath,
+      fileNames: payloadConfigFileNames,
+    })
 
     if (configPath) {
       return configPath
@@ -104,16 +94,18 @@ export const findConfig = (): string => {
   // If no config file is found in the directories defined by tsconfig.json,
   // try searching in the 'src' and 'dist' directory as a last resort, as they are most commonly used
   if (process.env.NODE_ENV === 'production') {
-    const distConfigPath = findUpSync(['payload.config.js', 'payload.config.ts'], {
-      cwd: path.resolve(process.cwd(), 'dist'),
+    const distConfigPath = findUpSync({
+      dir: path.resolve(process.cwd(), 'dist'),
+      fileNames: ['payload.config.js'],
     })
 
     if (distConfigPath) {
       return distConfigPath
     }
   } else {
-    const srcConfigPath = findUpSync(['payload.config.js', 'payload.config.ts'], {
-      cwd: path.resolve(process.cwd(), 'src'),
+    const srcConfigPath = findUpSync({
+      dir: path.resolve(process.cwd(), 'src'),
+      fileNames: payloadConfigFileNames,
     })
 
     if (srcConfigPath) {
diff --git a/packages/payload/src/index.ts b/packages/payload/src/index.ts
@@ -1005,6 +1005,12 @@ export {
   deepMergeWithSourceArrays,
 } from './utilities/deepMerge.js'
 export { getDependencies } from './utilities/dependencies/getDependencies.js'
+export {
+  findUp,
+  findUpSync,
+  pathExistsAndIsAccessible,
+  pathExistsAndIsAccessibleSync,
+} from './utilities/findUp.js'
 export { default as flattenTopLevelFields } from './utilities/flattenTopLevelFields.js'
 export { formatLabels, formatNames, toWords } from './utilities/formatLabels.js'
 export { getCollectionIDFieldTypes } from './utilities/getCollectionIDFieldTypes.js'
diff --git a/packages/payload/src/utilities/dependencies/getDependencies.ts b/packages/payload/src/utilities/dependencies/getDependencies.ts
@@ -14,16 +14,23 @@
   THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
 */
 
-import { findUp } from 'find-up'
-import { existsSync, promises as fs } from 'fs'
+import { promises as fs } from 'fs'
 import path from 'path'
 import { fileURLToPath } from 'url'
 
+import { findUp } from '../findUp.js'
 import { resolveFrom } from './resolveFrom.js'
 
 const filename = fileURLToPath(import.meta.url)
 const dirname = path.dirname(filename)
 
+const payloadPkgDirname = path.resolve(dirname, '../../../') // pkg dir (outside src)
+// if node_modules is in payloadPkgDirname, go to parent dir which contains node_modules
+if (payloadPkgDirname.includes('node_modules')) {
+  payloadPkgDirname.split('node_modules').slice(0, -1)
+}
+const resolvedCwd = path.resolve(process.cwd())
+
 export type NecessaryDependencies = {
   missing: string[]
   resolved: Map<
@@ -52,35 +59,29 @@ export async function getDependencies(
     requiredPackages.map(async (pkg) => {
       try {
         const pkgPath = await fs.realpath(resolveFrom(baseDir, pkg))
-
         const pkgDir = path.dirname(pkgPath)
 
         let packageJsonFilePath = null
 
-        const processCwd = process.cwd()
-        const payloadPkgDirname = path.resolve(dirname, '../../../') // pkg dir (outside src)
+        const foundPackageJsonDir = await findUp({
+          dir: pkgDir,
+          fileNames: ['package.json'],
+        })
 
-        // if node_modules is in payloadPkgDirname, go to parent dir which contains node_modules
-        if (payloadPkgDirname.includes('node_modules')) {
-          payloadPkgDirname.split('node_modules').slice(0, -1)
-        }
+        if (foundPackageJsonDir) {
+          const resolvedFoundPath = path.resolve(foundPackageJsonDir)
 
-        await findUp('package.json', { type: 'file', cwd: pkgDir }).then((foundPath) => {
-          if (foundPath) {
-            const resolvedFoundPath = path.resolve(foundPath)
-            const resolvedCwd = path.resolve(processCwd)
-
-            if (
-              resolvedFoundPath.startsWith(resolvedCwd) ||
-              resolvedFoundPath.startsWith(payloadPkgDirname)
-            ) {
-              // We don't want to match node modules outside the user's project. Checking for both process.cwd and dirname is a reliable way to do this.
-              packageJsonFilePath = foundPath
-            }
+          if (
+            resolvedFoundPath.startsWith(resolvedCwd) ||
+            resolvedFoundPath.startsWith(payloadPkgDirname)
+          ) {
+            // We don't want to match node modules outside the user's project. Checking for both process.cwd and dirname is a reliable way to do this.
+            packageJsonFilePath = resolvedFoundPath
           }
-        })
+        }
 
-        if (packageJsonFilePath && existsSync(packageJsonFilePath)) {
+        // No need to check if packageJsonFilePath exists - findUp checks that for us
+        if (packageJsonFilePath) {
           // parse version
           const packageJson = JSON.parse(await fs.readFile(packageJsonFilePath, 'utf8'))
           const version = packageJson.version
diff --git a/packages/payload/src/utilities/findUp.ts b/packages/payload/src/utilities/findUp.ts
@@ -0,0 +1,118 @@
+import fs from 'fs'
+import path from 'path'
+
+/**
+ * Synchronously walks up parent directories until a condition is met and/or one of the file names within the fileNames array is found.
+ */
+export function findUpSync({
+  condition,
+  dir,
+  fileNames,
+}: {
+  condition?: (dir: string) => boolean | Promise<boolean | string> | string
+  dir: string
+  fileNames?: string[]
+}): null | string {
+  const { root } = path.parse(dir)
+
+  while (true) {
+    if (fileNames?.length) {
+      let found = false
+      for (const fileName of fileNames) {
+        const filePath = path.join(dir, fileName)
+        const exists = pathExistsAndIsAccessibleSync(filePath)
+        if (exists) {
+          if (!condition) {
+            return filePath
+          }
+          found = true
+          break
+        }
+      }
+      if (!found) {
+        dir = path.dirname(dir) // Move up one directory level.
+        continue
+      }
+    }
+    const result = condition(dir)
+    if (result === true) {
+      return dir
+    }
+    if (typeof result === 'string' && result?.length) {
+      return result
+    }
+    if (dir === root) {
+      return null // Reached the root directory without a match.
+    }
+    dir = path.dirname(dir) // Move up one directory level.
+  }
+}
+
+/**
+ * Asynchronously walks up parent directories until a condition is met and/or one of the file names within the fileNames array is found.
+ */
+export async function findUp({
+  condition,
+  dir,
+  fileNames,
+}: {
+  condition?: (dir: string) => boolean | Promise<boolean | string> | string
+  dir: string
+  fileNames?: string[]
+}): Promise<null | string> {
+  const { root } = path.parse(dir)
+
+  while (true) {
+    if (fileNames?.length) {
+      let found = false
+      for (const fileName of fileNames) {
+        const filePath = path.resolve(dir, fileName)
+        const exists = await pathExistsAndIsAccessible(filePath)
+        if (exists) {
+          if (!condition) {
+            return filePath
+          }
+          found = true
+          break
+        }
+      }
+      if (!found) {
+        dir = path.dirname(dir) // Move up one directory level.
+        continue
+      }
+    }
+    const result = await condition(dir)
+    if (result === true) {
+      return dir
+    }
+    if (typeof result === 'string' && result?.length) {
+      return result
+    }
+    if (dir === root) {
+      return null // Reached the root directory without a match.
+    }
+    dir = path.dirname(dir) // Move up one directory level.
+  }
+}
+
+// From https://github.com/sindresorhus/path-exists/blob/main/index.js
+// fs.accessSync is preferred over fs.existsSync as it's usually a good idea
+// to check if the process has permission to read/write to a file before doing so.
+// Also see https://github.com/nodejs/node/issues/39960
+export function pathExistsAndIsAccessibleSync(path: string) {
+  try {
+    fs.accessSync(path)
+    return true
+  } catch {
+    return false
+  }
+}
+
+export async function pathExistsAndIsAccessible(path: string) {
+  try {
+    await fs.promises.access(path)
+    return true
+  } catch {
+    return false
+  }
+}
diff --git a/packages/payload/src/utilities/telemetry/index.ts b/packages/payload/src/utilities/telemetry/index.ts
@@ -1,7 +1,6 @@
 import { execSync } from 'child_process'
 import ciInfo from 'ci-info'
 import { randomBytes } from 'crypto'
-import { findUp } from 'find-up'
 import fs from 'fs'
 import path from 'path'
 import { fileURLToPath } from 'url'
@@ -10,6 +9,7 @@ import type { Payload } from '../../types/index.js'
 import type { AdminInitEvent } from './events/adminInit.js'
 import type { ServerInitEvent } from './events/serverInit.js'
 
+import { findUp } from '../findUp.js'
 import { Conf } from './conf/index.js'
 import { oneWayHash } from './oneWayHash.js'
 
@@ -136,13 +136,15 @@ const getPackageJSON = async (): Promise<{
     // Old logic
     const filename = fileURLToPath(import.meta.url)
     const dirname = path.dirname(filename)
-    packageJSONPath = await findUp('package.json', { cwd: dirname })
-    const jsonContent: PackageJSON = JSON.parse(fs.readFileSync(packageJSONPath, 'utf-8'))
-    return { packageJSON: jsonContent, packageJSONPath }
+    packageJSONPath = await findUp({
+      dir: dirname,
+      fileNames: ['package.json'],
+    })
   }
 
-  const packageJSON: PackageJSON = JSON.parse(fs.readFileSync(packageJSONPath, 'utf-8'))
-  return { packageJSON, packageJSONPath }
+  const jsonContentString = await fs.promises.readFile(packageJSONPath, 'utf-8')
+  const jsonContent: PackageJSON = JSON.parse(jsonContentString)
+  return { packageJSON: jsonContent, packageJSONPath }
 }
 
 const getPackageJSONID = (payload: Payload, packageJSON: PackageJSON): string => {
diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml
diff --git a/test/package.json b/test/package.json
