File Reader Improvements

packages/next/src/lib/recursive-readdir.ts

@@ -1,59 +1,153 @@
-import { Dirent, promises } from 'fs'
-import { join } from 'path'
+import fs from 'fs/promises'
+import path from 'path'
+
+type Filter = (pathname: string) => boolean
+
+type Result = {
+  directories: string[]
+  files: string[]
+  links: string[]
+}
 
 /**
- * Recursively read directory
- * Returns array holding all relative paths
+ *
+ * @param rootDirectory the directory to read
+ * @param pathnameFilter filter to ignore files with absolute pathnames, false to ignore
+ * @param ignoreFilter filter to ignore files and directories with absolute pathnames, false to ignore
+ * @param ignorePartFilter filter to ignore files and directories with the pathname part, false to ignore
+ * @param sortPathnames whether to sort the results, true by default
+ * @param relativePathnames whether to return relative pathnames, true by default
+ * @returns
  */
 export async function recursiveReadDir(
-  /** Directory to read */
-  dir: string,
-  /** Filter for the file path */
-  filter: (absoluteFilePath: string) => boolean,
-  /** Filter for the file path */
-  ignore?: (absoluteFilePath: string) => boolean,
-  ignorePart?: (partName: string) => boolean,
-  /** This doesn't have to be provided, it's used for the recursion */
-  arr: string[] = [],
-  /** Used to replace the initial path, only the relative path is left, it's faster than path.relative. */
-  rootDir: string = dir
+  rootDirectory: string,
+  pathnameFilter?: Filter,
+  ignoreFilter?: Filter,
+  ignorePartFilter?: Filter,
+  sortPathnames: boolean = true,
+  relativePathnames: boolean = true
 ): Promise<string[]> {
-  const result = await promises.readdir(dir, { withFileTypes: true })
-
-  await Promise.all(
-    result.map(async (part: Dirent) => {
-      const absolutePath = join(dir, part.name)
-      const relativePath = absolutePath.replace(rootDir, '')
-      if (ignore && ignore(absolutePath)) return
-      if (ignorePart && ignorePart(part.name)) return
-
-      // readdir does not follow symbolic links
-      // if part is a symbolic link, follow it using stat
-      let isDirectory = part.isDirectory()
-      if (part.isSymbolicLink()) {
-        const stats = await promises.stat(absolutePath)
-        isDirectory = stats.isDirectory()
-      }
+  // The list of pathnames to return.
+  const pathnames: string[] = []
 
-      if (isDirectory) {
-        await recursiveReadDir(
-          absolutePath,
-          filter,
-          ignore,
-          ignorePart,
-          arr,
-          rootDir
-        )
-        return
-      }
+  /**
+   * Coerces the pathname to be relative if requested.
+   */
+  const coerce = relativePathnames
+    ? (pathname: string) => pathname.replace(rootDirectory, '')
+    : (pathname: string) => pathname
+
+  // The queue of directories to scan.
+  let directories: string[] = [rootDirectory]
+
+  while (directories.length > 0) {
+    // Load all the files in each directory at the same time.
+    const results = await Promise.all(
+      directories.map(async (directory) => {
+        const result: Result = { directories: [], files: [], links: [] }
+
+        try {
+          const dir = await fs.opendir(directory, {
+            // Buffer up to 100 files at a time for the iterator.
+            bufferSize: 100,
+          })
+          for await (const file of dir) {
+            // If enabled, ignore the file if it matches the ignore filter.
+            if (ignorePartFilter && ignorePartFilter(file.name)) {
+              continue
+            }
+
+            // Handle each file.
+            const absolutePathname = path.join(directory, file.name)
+
+            // If enabled, ignore the file if it matches the ignore filter.
+            if (ignoreFilter && ignoreFilter(absolutePathname)) {
+              continue
+            }
+
+            // If the file is a directory, then add it to the list of directories,
+            // they'll be scanned on a later pass.
+            if (file.isDirectory()) {
+              result.directories.push(absolutePathname)
+            } else if (file.isSymbolicLink()) {
+              result.links.push(absolutePathname)
+            } else if (!pathnameFilter || pathnameFilter(absolutePathname)) {
+              result.files.push(coerce(absolutePathname))
+            }
+          }
+        } catch (err: any) {
+          // This can only happen when the underlying directory was removed. If
+          // anything other than this error occurs, re-throw it.
+          // if (err.code !== 'ENOENT') throw err
+          if (err.code !== 'ENOENT' || directory === rootDirectory) throw err
+
+          // The error occurred, so abandon reading this directory.
+          return { directories: [], files: [], links: [] }
+        }
+
+        return result
+      })
+    )
+
+    // Empty the directories, we'll fill it later if some of the files are
+    // directories.
+    directories = []
+
+    // Keep track of any symbolic links we find, we'll resolve them later.
+    const links = []
+
+    // For each result of directory scans...
+    for (const result of results) {
+      // Add any directories to the list of directories to scan.
+      directories.push(...result.directories)
+
+      // Add any symbolic links to the list of symbolic links to resolve.
+      links.push(...result.links)
+
+      // Add any file pathnames to the list of pathnames.
+      pathnames.push(...result.files)
+    }
+
+    // Resolve all the symbolic links we found if any.
+    if (links.length > 0) {
+      const resolved = await Promise.all(
+        links.map(async (absolutePathname) => {
+          try {
+            return await fs.stat(absolutePathname)
+          } catch (err: any) {
+            // This can only happen when the underlying link was removed. If
+            // anything other than this error occurs, re-throw it.
+            if (err.code !== 'ENOENT') throw err
+
+            // The error occurred, so abandon reading this directory.
+            return null
+          }
+        })
+      )
+
+      for (let i = 0; i < links.length; i++) {
+        const stats = resolved[i]
+
+        // If the link was removed, then skip it.
+        if (!stats) continue
+
+        // We would have already ignored the file if it matched the ignore
+        // filter, so we don't need to check it again.
+        const absolutePathname = links[i]
 
-      if (!filter(absolutePath)) {
-        return
+        if (stats.isDirectory()) {
+          directories.push(absolutePathname)
+        } else if (!pathnameFilter || pathnameFilter(absolutePathname)) {
+          pathnames.push(coerce(absolutePathname))
+        }
       }
+    }
+  }
 
-      arr.push(relativePath)
-    })
-  )
+  // Sort the pathnames in place if requested.
+  if (sortPathnames) {
+    pathnames.sort()
+  }
 
-  return arr.sort()
+  return pathnames
 }

packages/next/src/lib/wait.ts

@@ -0,0 +1,8 @@
+/**
+ * Wait for a given number of milliseconds and then resolve.
+ *
+ * @param ms the number of milliseconds to wait
+ */
+export async function wait(ms: number) {
+  return new Promise((resolve) => setTimeout(resolve, ms))
+}

packages/next/src/server/dev/next-dev-server.ts

@@ -54,7 +54,7 @@ import { DevAppPageRouteMatcherProvider } from '../future/route-matcher-provider
 import { DevAppRouteRouteMatcherProvider } from '../future/route-matcher-providers/dev/dev-app-route-route-matcher-provider'
 import { PagesManifest } from '../../build/webpack/plugins/pages-manifest-plugin'
 import { NodeManifestLoader } from '../future/route-matcher-providers/helpers/manifest-loaders/node-manifest-loader'
-import { CachedFileReader } from '../future/route-matcher-providers/dev/helpers/file-reader/cached-file-reader'
+import { BatchedFileReader } from '../future/route-matcher-providers/dev/helpers/file-reader/batched-file-reader'
 import { DefaultFileReader } from '../future/route-matcher-providers/dev/helpers/file-reader/default-file-reader'
 import { NextBuildContext } from '../../build/build-context'
 import { IncrementalCache } from '../lib/incremental-cache'
@@ -208,7 +208,7 @@ export default class DevServer extends Server {
       this.dir
     )
     const extensions = this.nextConfig.pageExtensions
-    const fileReader = new CachedFileReader(new DefaultFileReader())
+    const fileReader = new BatchedFileReader(new DefaultFileReader())
 
     // If the pages directory is available, then configure those matchers.
     if (pagesDir) {

packages/next/src/server/future/route-matcher-providers/dev/helpers/file-reader/cached-file-reader.test.ts → packages/next/src/server/future/route-matcher-providers/dev/helpers/file-reader/batched-file-reader.test.ts

@@ -1,4 +1,4 @@
-import { CachedFileReader } from './cached-file-reader'
+import { BatchedFileReader } from './batched-file-reader'
 import { FileReader } from './file-reader'
 
 describe('CachedFileReader', () => {
@@ -18,7 +18,7 @@ describe('CachedFileReader', () => {
         }
       }),
     }
-    const cached = new CachedFileReader(reader)
+    const cached = new BatchedFileReader(reader)
 
     const results = await Promise.all([
       cached.read('<root>/pages'),
@@ -49,7 +49,7 @@ describe('CachedFileReader', () => {
         }
       }),
     }
-    const cached = new CachedFileReader(reader)
+    const cached = new BatchedFileReader(reader)
 
     await Promise.all(
       ['reject', 'resolve', 'reject', 'resolve'].map(async (directory) => {

packages/next/src/server/future/route-matcher-providers/dev/helpers/file-reader/cached-file-reader.ts → packages/next/src/server/future/route-matcher-providers/dev/helpers/file-reader/batched-file-reader.ts

@@ -1,6 +1,6 @@
 import { FileReader } from './file-reader'
 
-interface CachedFileReaderBatch {
+interface FileReaderBatch {
   completed: boolean
   directories: Array<string>
   callbacks: Array<{
@@ -13,8 +13,8 @@ interface CachedFileReaderBatch {
  * CachedFileReader will deduplicate requests made to the same folder structure
  * to scan for files.
  */
-export class CachedFileReader implements FileReader {
-  private batch?: CachedFileReaderBatch
+export class BatchedFileReader implements FileReader {
+  private batch?: FileReaderBatch
 
   constructor(private readonly reader: FileReader) {}
 
@@ -30,13 +30,13 @@ export class CachedFileReader implements FileReader {
     })
   }
 
-  private getOrCreateBatch(): CachedFileReaderBatch {
+  private getOrCreateBatch(): FileReaderBatch {
     // If there is an existing batch and it's not completed, then reuse it.
     if (this.batch && !this.batch.completed) {
       return this.batch
     }
 
-    const batch: CachedFileReaderBatch = {
+    const batch: FileReaderBatch = {
       completed: false,
       directories: [],
       callbacks: [],