fix: use fs.existsSync to avoid race condition (#56387)

Using `await fs.access` has couple of downsides. It creates unnecessary async contexts where async scope can be removed. Also, it creates the possibility of race conditions such as `Time-of-Check to Time-of-Use`. It would be nice if someone can benchmark this. I'm rooting for a performance improvement. Some updates from Node.js land: - There is an open pull request to add V8 Fast API to `existsSync` method - nodejs/node#49893 - Non-existing `existsSync` executions became 30% faster - nodejs/node#49593 Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com>
vercel · Oct 3, 2023 · a46c5af · a46c5af
1 parent 09b0ca4
commit a46c5af
Show file tree

Hide file tree

Showing 16 changed files with 63 additions and 81 deletions.
diff --git a/packages/next/src/build/entries.ts b/packages/next/src/build/entries.ts
@@ -15,6 +15,7 @@ import type { AppLoaderOptions } from './webpack/loaders/next-app-loader'
 import { cyan } from '../lib/picocolors'
 import { posix, join, dirname, extname } from 'path'
 import { stringify } from 'querystring'
+import fs from 'fs'
 import {
   PAGES_DIR_ALIAS,
   ROOT_DIR_ALIAS,
@@ -51,7 +52,6 @@ import { encodeMatchers } from './webpack/loaders/next-middleware-loader'
 import { EdgeFunctionLoaderOptions } from './webpack/loaders/next-edge-function-loader'
 import { isAppRouteRoute } from '../lib/is-app-route-route'
 import { normalizeMetadataRoute } from '../lib/metadata/get-metadata-route'
-import { fileExists } from '../lib/file-exists'
 import { getRouteLoaderEntry } from './webpack/loaders/next-route-loader'
 import {
   isInternalComponent,
@@ -123,7 +123,7 @@ export async function getStaticInfoIncludingLayouts({
     while (dir.startsWith(appDir)) {
       for (const potentialLayoutFile of potentialLayoutFiles) {
         const layoutFile = join(dir, potentialLayoutFile)
-        if (!(await fileExists(layoutFile))) {
+        if (!fs.existsSync(layoutFile)) {
           continue
         }
         layoutFiles.unshift(layoutFile)

diff --git a/packages/next/src/build/get-babel-config-file.ts b/packages/next/src/build/get-babel-config-file.ts
@@ -1,5 +1,5 @@
-import { fileExists } from '../lib/file-exists'
 import { join } from 'path'
+import { existsSync } from 'fs'
 
 const BABEL_CONFIG_FILES = [
   '.babelrc',
@@ -13,12 +13,10 @@ const BABEL_CONFIG_FILES = [
   'babel.config.cjs',
 ]
 
-export async function getBabelConfigFile(
-  dir: string
-): Promise<string | undefined> {
+export function getBabelConfigFile(dir: string): string | undefined {
   for (const filename of BABEL_CONFIG_FILES) {
     const configFilePath = join(dir, filename)
-    const exists = await fileExists(configFilePath)
+    const exists = existsSync(configFilePath)
     if (!exists) {
       continue
     }

diff --git a/packages/next/src/build/index.ts b/packages/next/src/build/index.ts
@@ -12,7 +12,7 @@ import { loadEnvConfig } from '@next/env'
 import { bold, yellow, green } from '../lib/picocolors'
 import crypto from 'crypto'
 import { isMatch, makeRe } from 'next/dist/compiled/micromatch'
-import fs from 'fs/promises'
+import { existsSync, promises as fs } from 'fs'
 import os from 'os'
 import { Worker } from '../lib/worker'
 import { defaultConfig } from '../server/config-shared'
@@ -408,7 +408,7 @@ export default async function build(
 
       const cacheDir = path.join(distDir, 'cache')
       if (ciEnvironment.isCI && !ciEnvironment.hasNextSupport) {
-        const hasCache = await fileExists(cacheDir)
+        const hasCache = existsSync(cacheDir)
 
         if (!hasCache) {
           // Intentionally not piping to stderr in case people fail in CI when
@@ -433,7 +433,7 @@ export default async function build(
       const isSrcDir = path
         .relative(dir, pagesDir || appDir || '')
         .startsWith('src')
-      const hasPublicDir = await fileExists(publicDir)
+      const hasPublicDir = existsSync(publicDir)
 
       telemetry.record(
         eventCliSession(dir, config, {
@@ -719,7 +719,7 @@ export default async function build(
         mappedPages['/_error'].startsWith(PAGES_DIR_ALIAS)
 
       if (hasPublicDir) {
-        const hasPublicUnderScoreNextDir = await fileExists(
+        const hasPublicUnderScoreNextDir = existsSync(
           path.join(publicDir, '_next')
         )
         if (hasPublicUnderScoreNextDir) {
@@ -2426,7 +2426,7 @@ export default async function build(
                   .join('pages', '404.html')
                   .replace(/\\/g, '/')
 
-                if (await fileExists(orig)) {
+                if (existsSync(orig)) {
                   await fs.copyFile(
                     orig,
                     path.join(distDir, 'server', updatedRelativeDest)
@@ -2901,7 +2901,7 @@ export default async function build(
                   SERVER_DIRECTORY,
                   'app'
                 )
-                if (await fileExists(originalServerApp)) {
+                if (existsSync(originalServerApp)) {
                   await recursiveCopy(
                     originalServerApp,
                     path.join(

diff --git a/packages/next/src/build/load-jsconfig.ts b/packages/next/src/build/load-jsconfig.ts
@@ -1,5 +1,5 @@
 import path from 'path'
-import { fileExists } from '../lib/file-exists'
+import fs from 'fs'
 import { NextConfigComplete } from '../server/config-shared'
 import * as Log from './output/log'
 import { getTypeScriptConfiguration } from '../lib/typescript/getTypeScriptConfiguration'
@@ -53,9 +53,7 @@ export default async function loadJsConfig(
     typeScriptPath = deps.resolved.get('typescript')
   } catch {}
   const tsConfigPath = path.join(dir, config.typescript.tsconfigPath)
-  const useTypeScript = Boolean(
-    typeScriptPath && (await fileExists(tsConfigPath))
-  )
+  const useTypeScript = Boolean(typeScriptPath && fs.existsSync(tsConfigPath))
 
   let implicitBaseurl
   let jsConfig: { compilerOptions: Record<string, any> } | undefined
@@ -78,7 +76,7 @@ export default async function loadJsConfig(
   }
 
   const jsConfigPath = path.join(dir, 'jsconfig.json')
-  if (!useTypeScript && (await fileExists(jsConfigPath))) {
+  if (!useTypeScript && fs.existsSync(jsConfigPath)) {
     jsConfig = parseJsonFile(jsConfigPath)
     implicitBaseurl = path.dirname(jsConfigPath)
   }

diff --git a/packages/next/src/build/webpack-config.ts b/packages/next/src/build/webpack-config.ts
@@ -496,7 +496,7 @@ export default async function getBaseWebpackConfig(
     ? '-experimental'
     : ''
 
-  const babelConfigFile = await getBabelConfigFile(dir)
+  const babelConfigFile = getBabelConfigFile(dir)
   const distDir = path.join(dir, config.distDir)
 
   let useSWCLoader = !babelConfigFile || config.experimental.forceSwcTransforms

diff --git a/packages/next/src/build/webpack/loaders/next-metadata-image-loader.ts b/packages/next/src/build/webpack/loaders/next-metadata-image-loader.ts
@@ -7,12 +7,11 @@ import type {
   MetadataImageModule,
   PossibleImageFileNameConvention,
 } from './metadata/types'
-import fs from 'fs/promises'
+import { existsSync, promises as fs } from 'fs'
 import path from 'path'
 import loaderUtils from 'next/dist/compiled/loader-utils3'
 import { getImageSize } from '../../../server/image-optimizer'
 import { imageExtMimeTypeMap } from '../../../lib/mime-type'
-import { fileExists } from '../../../lib/file-exists'
 import { WEBPACK_RESOURCE_QUERIES } from '../../../lib/constants'
 import { normalizePathSep } from '../../../shared/lib/page-path/normalize-path-sep'
 
@@ -181,7 +180,7 @@ async function nextMetadataImageLoader(this: any, content: Buffer) {
       fileNameBase + '.alt.txt'
     )
 
-    if (await fileExists(altPath)) {
+    if (existsSync(altPath)) {
       imageData.alt = await fs.readFile(altPath, 'utf8')
     }
   }

diff --git a/packages/next/src/export/index.ts b/packages/next/src/export/index.ts
@@ -9,7 +9,7 @@ import type { PagesManifest } from '../build/webpack/plugins/pages-manifest-plug
 
 import { bold, yellow } from '../lib/picocolors'
 import findUp from 'next/dist/compiled/find-up'
-import fs from 'fs/promises'
+import { existsSync, promises as fs } from 'fs'
 
 import '../server/require-hook'
 
@@ -52,7 +52,6 @@ import { isAppRouteRoute } from '../lib/is-app-route-route'
 import { isAppPageRoute } from '../lib/is-app-page-route'
 import isError from '../lib/is-error'
 import { needsExperimentalReact } from '../lib/needs-experimental-react'
-import { fileExists } from '../lib/file-exists'
 import { formatManifest } from '../build/manifests/formatter/format-manifest'
 
 function divideSegments(number: number, segments: number): number[] {
@@ -240,7 +239,7 @@ export async function exportAppImpl(
       )
       return null
     }
-    if (await fileExists(join(distDir, 'server', 'app'))) {
+    if (existsSync(join(distDir, 'server', 'app'))) {
       throw new ExportError(
         '"next export" does not work with App Router. Please use "output: export" in next.config.js https://nextjs.org/docs/advanced-features/static-html-export'
       )
@@ -284,7 +283,7 @@ export async function exportAppImpl(
 
   const buildIdFile = join(distDir, BUILD_ID_FILE)
 
-  if (!(await fileExists(buildIdFile))) {
+  if (!existsSync(buildIdFile)) {
     throw new ExportError(
       `Could not find a production build in the '${distDir}' directory. Try building your app with 'next build' before starting the static export. https://nextjs.org/docs/messages/next-export-no-build-id`
     )
@@ -407,7 +406,7 @@ export async function exportAppImpl(
   )
 
   // Copy static directory
-  if (!options.buildExport && (await fileExists(join(dir, 'static')))) {
+  if (!options.buildExport && existsSync(join(dir, 'static'))) {
     if (!options.silent) {
       Log.info('Copying "static" directory')
     }
@@ -421,7 +420,7 @@ export async function exportAppImpl(
   // Copy .next/static directory
   if (
     !options.buildExport &&
-    (await fileExists(join(distDir, CLIENT_STATIC_FILES_PATH)))
+    existsSync(join(distDir, CLIENT_STATIC_FILES_PATH))
   ) {
     if (!options.silent) {
       Log.info('Copying "static build" directory')
@@ -664,7 +663,7 @@ export async function exportAppImpl(
 
   const publicDir = join(dir, CLIENT_PUBLIC_FILES_PATH)
   // Copy public directory
-  if (!options.buildExport && (await fileExists(publicDir))) {
+  if (!options.buildExport && existsSync(publicDir)) {
     if (!options.silent) {
       Log.info('Copying "public" directory')
     }
@@ -818,7 +817,7 @@ export async function exportAppImpl(
         const handlerSrc = `${orig}.body`
         const handlerDest = join(outDir, route)
 
-        if (isAppRouteHandler && (await fileExists(handlerSrc))) {
+        if (isAppRouteHandler && existsSync(handlerSrc)) {
           await fs.mkdir(dirname(handlerDest), { recursive: true })
           await fs.copyFile(handlerSrc, handlerDest)
           return
@@ -852,7 +851,7 @@ export async function exportAppImpl(
         await fs.copyFile(htmlSrc, htmlDest)
         await fs.copyFile(jsonSrc, jsonDest)
 
-        if (await fileExists(`${orig}.amp.html`)) {
+        if (existsSync(`${orig}.amp.html`)) {
           await fs.mkdir(dirname(ampHtmlDest), { recursive: true })
           await fs.copyFile(`${orig}.amp.html`, ampHtmlDest)
         }

diff --git a/packages/next/src/lib/download-swc.ts b/packages/next/src/lib/download-swc.ts
@@ -8,7 +8,6 @@ const { fetch } = require('next/dist/compiled/undici') as {
 const { WritableStream } = require('node:stream/web') as {
   WritableStream: typeof global.WritableStream
 }
-import { fileExists } from './file-exists'
 import { getRegistry } from './helpers/get-registry'
 import { getCacheDirectory } from './helpers/get-cache-directory'
 
@@ -19,20 +18,19 @@ async function extractBinary(
   pkgName: string,
   tarFileName: string
 ) {
-  const cacheDirectory = await getCacheDirectory(
+  const cacheDirectory = getCacheDirectory(
     'next-swc',
     process.env['NEXT_SWC_PATH']
   )
 
-  const extractFromTar = async () => {
-    await tar.x({
+  const extractFromTar = () =>
+    tar.x({
       file: path.join(cacheDirectory, tarFileName),
       cwd: outputDirectory,
       strip: 1,
     })
-  }
 
-  if (!(await fileExists(path.join(cacheDirectory, tarFileName)))) {
+  if (!fs.existsSync(path.join(cacheDirectory, tarFileName))) {
     Log.info(`Downloading swc package ${pkgName}...`)
     await fs.promises.mkdir(cacheDirectory, { recursive: true })
     const tempFile = path.join(
@@ -99,7 +97,7 @@ export async function downloadNativeNextSwc(
     const tarFileName = `${pkgName.substring(6)}-${version}.tgz`
     const outputDirectory = path.join(bindingsDirectory, pkgName)
 
-    if (await fileExists(outputDirectory)) {
+    if (fs.existsSync(outputDirectory)) {
       // if the package is already downloaded a different
       // failure occurred than not being present
       return
@@ -119,7 +117,7 @@ export async function downloadWasmSwc(
   const tarFileName = `${pkgName.substring(6)}-${version}.tgz`
   const outputDirectory = path.join(wasmDirectory, pkgName)
 
-  if (await fileExists(outputDirectory)) {
+  if (fs.existsSync(outputDirectory)) {
     // if the package is already downloaded a different
     // failure occurred than not being present
     return

diff --git a/packages/next/src/lib/file-exists.ts b/packages/next/src/lib/file-exists.ts
@@ -1,4 +1,4 @@
-import { constants, promises } from 'fs'
+import { existsSync, promises } from 'fs'
 import isError from './is-error'
 
 export enum FileType {
@@ -17,10 +17,9 @@ export async function fileExists(
     } else if (type === FileType.Directory) {
       const stats = await promises.stat(fileName)
       return stats.isDirectory()
-    } else {
-      await promises.access(fileName, constants.F_OK)
     }
-    return true
+
+    return existsSync(fileName)
   } catch (err) {
     if (
       isError(err) &&

diff --git a/packages/next/src/lib/has-necessary-dependencies.ts b/packages/next/src/lib/has-necessary-dependencies.ts
@@ -1,5 +1,4 @@
-import { promises as fs } from 'fs'
-import { fileExists } from './file-exists'
+import { existsSync, promises as fs } from 'fs'
 import { resolveFrom } from './resolve-from'
 import { dirname, join, relative } from 'path'
 
@@ -33,7 +32,7 @@ export async function hasNecessaryDependencies(
           const fileNameToVerify = relative(p.pkg, p.file)
           if (fileNameToVerify) {
             const fileToVerify = join(pkgDir, fileNameToVerify)
-            if (await fileExists(fileToVerify)) {
+            if (existsSync(fileToVerify)) {
               resolutions.set(p.pkg, fileToVerify)
             } else {
               return missingPackages.push(p)

diff --git a/packages/next/src/lib/helpers/get-cache-directory.ts b/packages/next/src/lib/helpers/get-cache-directory.ts
@@ -1,13 +1,10 @@
 import os from 'os'
 import path from 'path'
-import { fileExists } from '../file-exists'
+import fs from 'fs'
 
 // get platform specific cache directory adapted from playwright's handling
 // https://github.com/microsoft/playwright/blob/7d924470d397975a74a19184c136b3573a974e13/packages/playwright-core/src/utils/registry.ts#L141
-export async function getCacheDirectory(
-  fileDirectory: string,
-  envPath?: string
-) {
+export function getCacheDirectory(fileDirectory: string, envPath?: string) {
   let result
 
   if (envPath) {
@@ -29,7 +26,7 @@ export async function getCacheDirectory(
           path.join(os.homedir(), '.cache'),
           path.join(os.tmpdir()),
         ]) {
-          if (await fileExists(dir)) {
+          if (fs.existsSync(dir)) {
             systemCacheDirectory = dir
             break
           }

diff --git a/packages/next/src/lib/mkcert.ts b/packages/next/src/lib/mkcert.ts
@@ -36,7 +36,7 @@ function getBinaryName() {
 async function downloadBinary() {
   try {
     const binaryName = getBinaryName()
-    const cacheDirectory = await getCacheDirectory('mkcert')
+    const cacheDirectory = getCacheDirectory('mkcert')
     const binaryPath = path.join(cacheDirectory, binaryName)
 
     if (fs.existsSync(binaryPath)) {

diff --git a/packages/next/src/lib/typescript/getTypeScriptIntent.ts b/packages/next/src/lib/typescript/getTypeScriptIntent.ts
@@ -1,6 +1,5 @@
-import { promises as fs } from 'fs'
+import { existsSync, promises as fs } from 'fs'
 import path from 'path'
-import { fileExists } from '../file-exists'
 import { recursiveReadDir } from '../recursive-readdir'
 
 export type TypeScriptIntent = { firstTimeSetup: boolean }
@@ -14,7 +13,7 @@ export async function getTypeScriptIntent(
 
   // The integration turns on if we find a `tsconfig.json` in the user's
   // project.
-  const hasTypeScriptConfiguration = await fileExists(resolvedTsConfigPath)
+  const hasTypeScriptConfiguration = existsSync(resolvedTsConfigPath)
   if (hasTypeScriptConfiguration) {
     const content = await fs
       .readFile(resolvedTsConfigPath, { encoding: 'utf8' })