From 88149ec66c375607722c5e0685c6cd52b87a367e Mon Sep 17 00:00:00 2001 From: Wyatt Johnson Date: Sun, 27 Aug 2023 17:23:16 -0600 Subject: [PATCH 01/12] feat: added support for symbolic link resolving --- .../file-reader/default-file-reader.ts | 31 +++++++++++++++++-- 1 file changed, 29 insertions(+), 2 deletions(-) diff --git a/packages/next/src/server/future/route-matcher-providers/dev/helpers/file-reader/default-file-reader.ts b/packages/next/src/server/future/route-matcher-providers/dev/helpers/file-reader/default-file-reader.ts index 1372d198778d8..7ed5eab6912d5 100644 --- a/packages/next/src/server/future/route-matcher-providers/dev/helpers/file-reader/default-file-reader.ts +++ b/packages/next/src/server/future/route-matcher-providers/dev/helpers/file-reader/default-file-reader.ts @@ -1,7 +1,8 @@ -import { type Dirent } from 'fs' +import type { FileReader } from './file-reader' +import type { Dirent } from 'fs' + import fs from 'fs/promises' import path from 'path' -import { FileReader } from './file-reader' export class DefaultFileReader implements FileReader { public async read(dir: string): Promise> { @@ -33,6 +34,9 @@ export class DefaultFileReader implements FileReader { // 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 { files, directory } of results) { // And for each file in it... @@ -44,6 +48,29 @@ export class DefaultFileReader implements FileReader { // they'll be scanned on a later pass. if (file.isDirectory()) { directories.push(pathname) + } else if (file.isSymbolicLink()) { + links.push(pathname) + } else { + pathnames.push(pathname) + } + } + } + + // Resolve all the symbolic links we found if any. + if (links.length > 0) { + const resolved = await Promise.all( + links.map(async (pathname) => { + const stats = await fs.stat(pathname) + return stats + }) + ) + + for (let i = 0; i < links.length; i++) { + const pathname = links[i] + const stats = resolved[i] + + if (stats.isDirectory()) { + directories.push(pathname) } else { pathnames.push(pathname) } From 19d2628ce82b72483062d88912718644af4bb445 Mon Sep 17 00:00:00 2001 From: Wyatt Johnson Date: Sun, 27 Aug 2023 18:07:02 -0600 Subject: [PATCH 02/12] feat: moved file reading impl into fn --- bench/readdir/.gitignore | 1 - bench/readdir/create-fixtures.sh | 4 - bench/readdir/glob.js | 24 --- bench/readdir/recursive-readdir.js | 21 --- packages/next/src/lib/recursive-readdir.ts | 135 ++++++++++------ .../file-reader/default-file-reader.ts | 148 ++++++++++-------- .../src/server/lib/recursive-readdir-sync.ts | 31 ---- .../src/server/lib/router-utils/filesystem.ts | 12 +- 8 files changed, 171 insertions(+), 205 deletions(-) delete mode 100644 bench/readdir/.gitignore delete mode 100644 bench/readdir/create-fixtures.sh delete mode 100644 bench/readdir/glob.js delete mode 100644 bench/readdir/recursive-readdir.js delete mode 100644 packages/next/src/server/lib/recursive-readdir-sync.ts diff --git a/bench/readdir/.gitignore b/bench/readdir/.gitignore deleted file mode 100644 index 116caa12788c7..0000000000000 --- a/bench/readdir/.gitignore +++ /dev/null @@ -1 +0,0 @@ -fixtures diff --git a/bench/readdir/create-fixtures.sh b/bench/readdir/create-fixtures.sh deleted file mode 100644 index afda00b81dda1..0000000000000 --- a/bench/readdir/create-fixtures.sh +++ /dev/null @@ -1,4 +0,0 @@ -# Uses https://github.com/divmain/fuzzponent -mkdir fixtures -cd fixtures -fuzzponent -d 2 -s 20 diff --git a/bench/readdir/glob.js b/bench/readdir/glob.js deleted file mode 100644 index 170f4fb050eb5..0000000000000 --- a/bench/readdir/glob.js +++ /dev/null @@ -1,24 +0,0 @@ -import { join } from 'path' -import { promisify } from 'util' -import globMod from 'glob' - -const glob = promisify(globMod) -const resolveDataDir = join(__dirname, 'fixtures', '**/*') - -async function test() { - const time = process.hrtime() - await glob(resolveDataDir) - - const hrtime = process.hrtime(time) - const nanoseconds = hrtime[0] * 1e9 + hrtime[1] - const milliseconds = nanoseconds / 1e6 - console.log(milliseconds) -} - -async function run() { - for (let i = 0; i < 50; i++) { - await test() - } -} - -run() diff --git a/bench/readdir/recursive-readdir.js b/bench/readdir/recursive-readdir.js deleted file mode 100644 index 2167f30336553..0000000000000 --- a/bench/readdir/recursive-readdir.js +++ /dev/null @@ -1,21 +0,0 @@ -import { join } from 'path' -import { recursiveReadDir } from 'next/dist/lib/recursive-readdir' -const resolveDataDir = join(__dirname, 'fixtures') - -async function test() { - const time = process.hrtime() - await recursiveReadDir(resolveDataDir, /\.js$/) - - const hrtime = process.hrtime(time) - const nanoseconds = hrtime[0] * 1e9 + hrtime[1] - const milliseconds = nanoseconds / 1e6 - console.log(milliseconds) -} - -async function run() { - for (let i = 0; i < 50; i++) { - await test() - } -} - -run() diff --git a/packages/next/src/lib/recursive-readdir.ts b/packages/next/src/lib/recursive-readdir.ts index 251bf397195ee..720c3071e87d3 100644 --- a/packages/next/src/lib/recursive-readdir.ts +++ b/packages/next/src/lib/recursive-readdir.ts @@ -1,59 +1,100 @@ -import { Dirent, promises } from 'fs' -import { join } from 'path' +import type { Dirent } from 'fs' + +import fs from 'fs/promises' +import path from 'path' /** - * Recursively read directory - * Returns array holding all relative paths + * + * @param dir 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 + * @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 + pathnameFilter?: (absoluteFilePath: string) => boolean, + ignoreFilter?: (absoluteFilePath: string) => boolean, + ignorePartFilter?: (partName: string) => boolean ): Promise { - 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() - } + const pathnames: string[] = [] - if (isDirectory) { - await recursiveReadDir( - absolutePath, - filter, - ignore, - ignorePart, - arr, - rootDir - ) - return - } + let directories: string[] = [dir] + + while (directories.length > 0) { + // Load all the files in each directory at the same time. + const results = await Promise.all( + directories.map(async (directory) => { + let files: Dirent[] + try { + files = await fs.readdir(directory, { withFileTypes: true }) + } 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 + + // The error occurred, so abandon reading this directory. + files = [] + } + + return { directory, files } + }) + ) + + // Empty the directories, we'll fill it later if some of the files are + // directories. + directories = [] - if (!filter(absolutePath)) { - return + // Keep track of any symbolic links we find, we'll resolve them later. + const links = [] + + // For each result of directory scans... + for (const { files, directory } of results) { + // And for each file in it... + for (const file of files) { + // If enabled, ignore the file if it matches the ignore filter. + if (ignorePartFilter && ignorePartFilter(file.name)) { + continue + } + + // Handle each file. + const pathname = path.join(directory, file.name) + + // If enabled, ignore the file if it matches the ignore filter. + if (ignoreFilter && ignoreFilter(pathname)) { + 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()) { + directories.push(pathname) + } else if (file.isSymbolicLink()) { + links.push(pathname) + } else if (!pathnameFilter || pathnameFilter(pathname)) { + pathnames.push(pathname) + } } + } - arr.push(relativePath) - }) - ) + // Resolve all the symbolic links we found if any. + if (links.length > 0) { + const resolved = await Promise.all( + links.map(async (pathname) => fs.stat(pathname)) + ) + + for (let i = 0; i < links.length; i++) { + const pathname = links[i] + const stats = resolved[i] + + if (stats.isDirectory()) { + directories.push(pathname) + } else if (!pathnameFilter || pathnameFilter(pathname)) { + pathnames.push(pathname) + } + } + } + } - return arr.sort() + return pathnames } diff --git a/packages/next/src/server/future/route-matcher-providers/dev/helpers/file-reader/default-file-reader.ts b/packages/next/src/server/future/route-matcher-providers/dev/helpers/file-reader/default-file-reader.ts index 7ed5eab6912d5..7c4916643e801 100644 --- a/packages/next/src/server/future/route-matcher-providers/dev/helpers/file-reader/default-file-reader.ts +++ b/packages/next/src/server/future/route-matcher-providers/dev/helpers/file-reader/default-file-reader.ts @@ -1,83 +1,95 @@ import type { FileReader } from './file-reader' -import type { Dirent } from 'fs' -import fs from 'fs/promises' -import path from 'path' +import { recursiveReadDir } from '../../../../../../lib/recursive-readdir' -export class DefaultFileReader implements FileReader { - public async read(dir: string): Promise> { - const pathnames: string[] = [] - - let directories: string[] = [dir] +type FilterInput = ((pathname: string) => boolean) | RegExp - while (directories.length > 0) { - // Load all the files in each directory at the same time. - const results = await Promise.all( - directories.map(async (directory) => { - let files: Dirent[] - try { - files = await fs.readdir(directory, { withFileTypes: true }) - } 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 +type Filter = (pathname: string) => boolean - // The error occurred, so abandon reading this directory. - files = [] - } +function createFilter(input?: FilterInput): Filter | undefined { + if (typeof input === 'undefined') return + if (typeof input === 'function') return input - return { directory, files } - }) - ) - - // Empty the directories, we'll fill it later if some of the files are - // directories. - directories = [] + return (pathname) => input.test(pathname) +} - // Keep track of any symbolic links we find, we'll resolve them later. - const links = [] +/** + * Reads all the files in the directory and its subdirectories following any + * symbolic links. + */ +export class DefaultFileReader implements FileReader { + /** + * Filter to ignore files with absolute pathnames. If undefined, no files are + * ignored. + */ + private readonly pathnameFilter: Filter | undefined - // For each result of directory scans... - for (const { files, directory } of results) { - // And for each file in it... - for (const file of files) { - // Handle each file. - const pathname = path.join(directory, file.name) + /** + * Filter to ignore files and directories with absolute pathnames. If + * undefined, no files are ignored. + */ + private readonly ignoreFilter: Filter | undefined - // 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()) { - directories.push(pathname) - } else if (file.isSymbolicLink()) { - links.push(pathname) - } else { - pathnames.push(pathname) - } - } - } + /** + * Filter to ignore files and directories with the pathname part. If + * undefined, no files are ignored. + */ + private readonly ignorePartFilter: Filter | undefined - // Resolve all the symbolic links we found if any. - if (links.length > 0) { - const resolved = await Promise.all( - links.map(async (pathname) => { - const stats = await fs.stat(pathname) - return stats - }) - ) + /** + * Creates a new file reader. + * + * @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 + */ + constructor( + pathnameFilter?: FilterInput, + ignoreFilter?: FilterInput, + ignorePartFilter?: FilterInput + ) { + this.pathnameFilter = createFilter(pathnameFilter) + this.ignoreFilter = createFilter(ignoreFilter) + this.ignorePartFilter = createFilter(ignorePartFilter) + } - for (let i = 0; i < links.length; i++) { - const pathname = links[i] - const stats = resolved[i] + /** + * Reads all the files in the directory and its subdirectories following any + * symbolic links. + * + * @param dir 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 + */ + public static read( + dir: string, + pathnameFilter?: FilterInput, + ignoreFilter?: FilterInput, + ignorePartFilter?: FilterInput + ): Promise> { + const reader = new DefaultFileReader( + pathnameFilter, + ignoreFilter, + ignorePartFilter + ) - if (stats.isDirectory()) { - directories.push(pathname) - } else { - pathnames.push(pathname) - } - } - } - } + return reader.read(dir) + } - return pathnames + /** + * Reads all the files in the directory and its subdirectories following any + * symbolic links. + * + * @param dir the directory to read + * @returns a promise that resolves to the list of files + */ + public async read(dir: string): Promise> { + return recursiveReadDir( + dir, + this.pathnameFilter, + this.ignoreFilter, + this.ignorePartFilter + ) } } diff --git a/packages/next/src/server/lib/recursive-readdir-sync.ts b/packages/next/src/server/lib/recursive-readdir-sync.ts deleted file mode 100644 index 07549a88bbc36..0000000000000 --- a/packages/next/src/server/lib/recursive-readdir-sync.ts +++ /dev/null @@ -1,31 +0,0 @@ -import fs from 'fs' -import { sep } from 'path' - -/** - * Recursively read directory - * Returns array holding all relative paths - */ -export function recursiveReadDirSync( - /** The directory to read */ - dir: string, - /** This doesn't have to be provided, it's used for the recursion */ - arr: string[] = [], - /** Used to remove the initial path suffix and leave only the relative, faster than path.relative. */ - rootDirLength = dir.length -): string[] { - // Use opendirSync for better memory usage - const result = fs.opendirSync(dir) - - let part: fs.Dirent | null - while ((part = result.readSync())) { - const absolutePath = dir + sep + part.name - if (part.isDirectory()) { - recursiveReadDirSync(absolutePath, arr, rootDirLength) - } else { - arr.push(absolutePath.slice(rootDirLength)) - } - } - - result.closeSync() - return arr -} diff --git a/packages/next/src/server/lib/router-utils/filesystem.ts b/packages/next/src/server/lib/router-utils/filesystem.ts index 5930967833c0a..df75df71fb14d 100644 --- a/packages/next/src/server/lib/router-utils/filesystem.ts +++ b/packages/next/src/server/lib/router-utils/filesystem.ts @@ -141,7 +141,7 @@ export async function setupFsCheck(opts: { buildId = await fs.readFile(buildIdPath, 'utf8') try { - for (let file of await recursiveReadDir(publicFolderPath, () => true)) { + for (let file of await recursiveReadDir(publicFolderPath)) { file = normalizePathSep(file) // ensure filename is encoded publicFolderItems.add(encodeURI(file)) @@ -153,10 +153,7 @@ export async function setupFsCheck(opts: { } try { - for (let file of await recursiveReadDir( - legacyStaticFolderPath, - () => true - )) { + for (let file of await recursiveReadDir(legacyStaticFolderPath)) { file = normalizePathSep(file) // ensure filename is encoded legacyStaticFolderItems.add(encodeURI(file)) @@ -171,10 +168,7 @@ export async function setupFsCheck(opts: { } try { - for (let file of await recursiveReadDir( - nextStaticFolderPath, - () => true - )) { + for (let file of await recursiveReadDir(nextStaticFolderPath)) { file = normalizePathSep(file) // ensure filename is encoded nextStaticFolderItems.add( From 26400167181e3d46643da089d98e5adba23a6c4f Mon Sep 17 00:00:00 2001 From: Wyatt Johnson Date: Sun, 27 Aug 2023 18:12:53 -0600 Subject: [PATCH 03/12] fix: restore sorting to function results --- packages/next/src/lib/recursive-readdir.ts | 9 ++++++++- .../dev/helpers/file-reader/default-file-reader.ts | 5 ++++- 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/packages/next/src/lib/recursive-readdir.ts b/packages/next/src/lib/recursive-readdir.ts index 720c3071e87d3..4adf2505e9928 100644 --- a/packages/next/src/lib/recursive-readdir.ts +++ b/packages/next/src/lib/recursive-readdir.ts @@ -9,13 +9,15 @@ import path from 'path' * @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 * @returns */ export async function recursiveReadDir( dir: string, pathnameFilter?: (absoluteFilePath: string) => boolean, ignoreFilter?: (absoluteFilePath: string) => boolean, - ignorePartFilter?: (partName: string) => boolean + ignorePartFilter?: (partName: string) => boolean, + sortPathnames: boolean = true ): Promise { const pathnames: string[] = [] @@ -96,5 +98,10 @@ export async function recursiveReadDir( } } + // Sort the pathnames in place if requested. + if (sortPathnames) { + pathnames.sort() + } + return pathnames } diff --git a/packages/next/src/server/future/route-matcher-providers/dev/helpers/file-reader/default-file-reader.ts b/packages/next/src/server/future/route-matcher-providers/dev/helpers/file-reader/default-file-reader.ts index 7c4916643e801..39df779fc5d0c 100644 --- a/packages/next/src/server/future/route-matcher-providers/dev/helpers/file-reader/default-file-reader.ts +++ b/packages/next/src/server/future/route-matcher-providers/dev/helpers/file-reader/default-file-reader.ts @@ -89,7 +89,10 @@ export class DefaultFileReader implements FileReader { dir, this.pathnameFilter, this.ignoreFilter, - this.ignorePartFilter + this.ignorePartFilter, + // We don't need to sort the results because we're not depending on the + // order of the results. + false ) } } From a35a740b52a1caac4a342bf1886f966c5c2be421 Mon Sep 17 00:00:00 2001 From: Wyatt Johnson Date: Sun, 27 Aug 2023 18:13:06 -0600 Subject: [PATCH 04/12] refactor: rename cached -> batched file reader --- packages/next/src/server/dev/next-dev-server.ts | 4 ++-- ...file-reader.test.ts => batched-file-reader.test.ts} | 6 +++--- .../{cached-file-reader.ts => batched-file-reader.ts} | 10 +++++----- 3 files changed, 10 insertions(+), 10 deletions(-) rename packages/next/src/server/future/route-matcher-providers/dev/helpers/file-reader/{cached-file-reader.test.ts => batched-file-reader.test.ts} (91%) rename packages/next/src/server/future/route-matcher-providers/dev/helpers/file-reader/{cached-file-reader.ts => batched-file-reader.ts} (93%) diff --git a/packages/next/src/server/dev/next-dev-server.ts b/packages/next/src/server/dev/next-dev-server.ts index 5e366a5982a52..2649096c35b40 100644 --- a/packages/next/src/server/dev/next-dev-server.ts +++ b/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) { diff --git a/packages/next/src/server/future/route-matcher-providers/dev/helpers/file-reader/cached-file-reader.test.ts b/packages/next/src/server/future/route-matcher-providers/dev/helpers/file-reader/batched-file-reader.test.ts similarity index 91% rename from packages/next/src/server/future/route-matcher-providers/dev/helpers/file-reader/cached-file-reader.test.ts rename to packages/next/src/server/future/route-matcher-providers/dev/helpers/file-reader/batched-file-reader.test.ts index 16f2aa32e54bb..36342762bfce9 100644 --- a/packages/next/src/server/future/route-matcher-providers/dev/helpers/file-reader/cached-file-reader.test.ts +++ b/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('/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) => { diff --git a/packages/next/src/server/future/route-matcher-providers/dev/helpers/file-reader/cached-file-reader.ts b/packages/next/src/server/future/route-matcher-providers/dev/helpers/file-reader/batched-file-reader.ts similarity index 93% rename from packages/next/src/server/future/route-matcher-providers/dev/helpers/file-reader/cached-file-reader.ts rename to packages/next/src/server/future/route-matcher-providers/dev/helpers/file-reader/batched-file-reader.ts index c9db00d97953b..c6ad88b2f6f35 100644 --- a/packages/next/src/server/future/route-matcher-providers/dev/helpers/file-reader/cached-file-reader.ts +++ b/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 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: [], From 6a6a60bb85530eabf6c0a5664f50efca87fa1e2b Mon Sep 17 00:00:00 2001 From: Wyatt Johnson Date: Sun, 27 Aug 2023 18:43:39 -0600 Subject: [PATCH 05/12] refactor: removed unused function --- .../file-reader/default-file-reader.ts | 45 +++---------------- 1 file changed, 6 insertions(+), 39 deletions(-) diff --git a/packages/next/src/server/future/route-matcher-providers/dev/helpers/file-reader/default-file-reader.ts b/packages/next/src/server/future/route-matcher-providers/dev/helpers/file-reader/default-file-reader.ts index 39df779fc5d0c..8517a9725326c 100644 --- a/packages/next/src/server/future/route-matcher-providers/dev/helpers/file-reader/default-file-reader.ts +++ b/packages/next/src/server/future/route-matcher-providers/dev/helpers/file-reader/default-file-reader.ts @@ -2,17 +2,8 @@ import type { FileReader } from './file-reader' import { recursiveReadDir } from '../../../../../../lib/recursive-readdir' -type FilterInput = ((pathname: string) => boolean) | RegExp - type Filter = (pathname: string) => boolean -function createFilter(input?: FilterInput): Filter | undefined { - if (typeof input === 'undefined') return - if (typeof input === 'function') return input - - return (pathname) => input.test(pathname) -} - /** * Reads all the files in the directory and its subdirectories following any * symbolic links. @@ -44,37 +35,13 @@ export class DefaultFileReader implements FileReader { * @param ignorePartFilter filter to ignore files and directories with the pathname part, false to ignore */ constructor( - pathnameFilter?: FilterInput, - ignoreFilter?: FilterInput, - ignorePartFilter?: FilterInput + pathnameFilter?: Filter, + ignoreFilter?: Filter, + ignorePartFilter?: Filter ) { - this.pathnameFilter = createFilter(pathnameFilter) - this.ignoreFilter = createFilter(ignoreFilter) - this.ignorePartFilter = createFilter(ignorePartFilter) - } - - /** - * Reads all the files in the directory and its subdirectories following any - * symbolic links. - * - * @param dir 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 - */ - public static read( - dir: string, - pathnameFilter?: FilterInput, - ignoreFilter?: FilterInput, - ignorePartFilter?: FilterInput - ): Promise> { - const reader = new DefaultFileReader( - pathnameFilter, - ignoreFilter, - ignorePartFilter - ) - - return reader.read(dir) + this.pathnameFilter = pathnameFilter + this.ignoreFilter = ignoreFilter + this.ignorePartFilter = ignorePartFilter } /** From 28bb24b33e7d3194f71e8e280515fabdd44cce40 Mon Sep 17 00:00:00 2001 From: Wyatt Johnson Date: Sun, 27 Aug 2023 22:19:09 -0600 Subject: [PATCH 06/12] fix: ensure default beheviour is to return relative pathnames --- packages/next/src/lib/recursive-readdir.ts | 65 ++++++++++++++----- .../file-reader/default-file-reader.ts | 3 + 2 files changed, 53 insertions(+), 15 deletions(-) diff --git a/packages/next/src/lib/recursive-readdir.ts b/packages/next/src/lib/recursive-readdir.ts index 4adf2505e9928..e09b40ea9afc4 100644 --- a/packages/next/src/lib/recursive-readdir.ts +++ b/packages/next/src/lib/recursive-readdir.ts @@ -3,6 +3,8 @@ import type { Dirent } from 'fs' import fs from 'fs/promises' import path from 'path' +type Filter = (pathname: string) => boolean + /** * * @param dir the directory to read @@ -10,17 +12,33 @@ import path from 'path' * @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( dir: string, - pathnameFilter?: (absoluteFilePath: string) => boolean, - ignoreFilter?: (absoluteFilePath: string) => boolean, - ignorePartFilter?: (partName: string) => boolean, - sortPathnames: boolean = true + pathnameFilter?: Filter, + ignoreFilter?: Filter, + ignorePartFilter?: Filter, + sortPathnames: boolean = true, + relativePathnames: boolean = true ): Promise { + // The list of pathnames to return. const pathnames: string[] = [] + /** + * Pushes the pathname to the list of pathnames and coerces it to be relative + * if requested. + */ + const push = relativePathnames + ? (pathname: string) => { + pathnames.push(pathname.replace(dir, '')) + } + : (pathname: string) => { + pathnames.push(pathname) + } + + // The queue of directories to scan. let directories: string[] = [dir] while (directories.length > 0) { @@ -60,21 +78,21 @@ export async function recursiveReadDir( } // Handle each file. - const pathname = path.join(directory, file.name) + const absolutePathname = path.join(directory, file.name) // If enabled, ignore the file if it matches the ignore filter. - if (ignoreFilter && ignoreFilter(pathname)) { + 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()) { - directories.push(pathname) + directories.push(absolutePathname) } else if (file.isSymbolicLink()) { - links.push(pathname) - } else if (!pathnameFilter || pathnameFilter(pathname)) { - pathnames.push(pathname) + links.push(absolutePathname) + } else if (!pathnameFilter || pathnameFilter(absolutePathname)) { + push(absolutePathname) } } } @@ -82,17 +100,34 @@ export async function recursiveReadDir( // Resolve all the symbolic links we found if any. if (links.length > 0) { const resolved = await Promise.all( - links.map(async (pathname) => fs.stat(pathname)) + 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 pathname = links[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 (stats.isDirectory()) { - directories.push(pathname) - } else if (!pathnameFilter || pathnameFilter(pathname)) { - pathnames.push(pathname) + directories.push(absolutePathname) + } else if (!pathnameFilter || pathnameFilter(absolutePathname)) { + push(absolutePathname) } } } diff --git a/packages/next/src/server/future/route-matcher-providers/dev/helpers/file-reader/default-file-reader.ts b/packages/next/src/server/future/route-matcher-providers/dev/helpers/file-reader/default-file-reader.ts index 8517a9725326c..ecec91c160240 100644 --- a/packages/next/src/server/future/route-matcher-providers/dev/helpers/file-reader/default-file-reader.ts +++ b/packages/next/src/server/future/route-matcher-providers/dev/helpers/file-reader/default-file-reader.ts @@ -59,6 +59,9 @@ export class DefaultFileReader implements FileReader { this.ignorePartFilter, // We don't need to sort the results because we're not depending on the // order of the results. + false, + // We want absolute pathnames because we're going to be comparing them + // with other absolute pathnames. false ) } From da52b5b9953973506d9540aecc94de546b2096e1 Mon Sep 17 00:00:00 2001 From: Wyatt Johnson Date: Mon, 28 Aug 2023 08:27:42 -0600 Subject: [PATCH 07/12] fix: throw error when scan root directory isn't found --- packages/next/src/lib/recursive-readdir.ts | 97 +++++++++++-------- packages/next/src/lib/wait.ts | 8 ++ .../src/server/lib/router-utils/filesystem.ts | 21 ++-- packages/next/src/server/load-components.ts | 62 ++++++------ 4 files changed, 105 insertions(+), 83 deletions(-) create mode 100644 packages/next/src/lib/wait.ts diff --git a/packages/next/src/lib/recursive-readdir.ts b/packages/next/src/lib/recursive-readdir.ts index e09b40ea9afc4..17f5ad2772f72 100644 --- a/packages/next/src/lib/recursive-readdir.ts +++ b/packages/next/src/lib/recursive-readdir.ts @@ -5,9 +5,15 @@ import path from 'path' type Filter = (pathname: string) => boolean +type Result = { + directories: string[] + files: string[] + links: string[] +} + /** * - * @param dir the directory to read + * @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 @@ -16,7 +22,7 @@ type Filter = (pathname: string) => boolean * @returns */ export async function recursiveReadDir( - dir: string, + rootDirectory: string, pathnameFilter?: Filter, ignoreFilter?: Filter, ignorePartFilter?: Filter, @@ -27,37 +33,61 @@ export async function recursiveReadDir( const pathnames: string[] = [] /** - * Pushes the pathname to the list of pathnames and coerces it to be relative - * if requested. + * Coerces the pathname to be relative if requested. */ - const push = relativePathnames - ? (pathname: string) => { - pathnames.push(pathname.replace(dir, '')) - } - : (pathname: string) => { - pathnames.push(pathname) - } + const coerce = relativePathnames + ? (pathname: string) => pathname.replace(rootDirectory, '') + : (pathname: string) => pathname // The queue of directories to scan. - let directories: string[] = [dir] + 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) => { - let files: Dirent[] + const result: Result = { directories: [], files: [], links: [] } + try { - files = await fs.readdir(directory, { withFileTypes: true }) + 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') throw err + if (err.code !== 'ENOENT' || directory === rootDirectory) throw err // The error occurred, so abandon reading this directory. - files = [] + return { directories: [], files: [], links: [] } } - return { directory, files } + return result }) ) @@ -69,32 +99,15 @@ export async function recursiveReadDir( const links = [] // For each result of directory scans... - for (const { files, directory } of results) { - // And for each file in it... - for (const file of files) { - // If enabled, ignore the file if it matches the ignore filter. - if (ignorePartFilter && ignorePartFilter(file.name)) { - continue - } + for (const result of results) { + // Add any directories to the list of directories to scan. + directories.push(...result.directories) - // Handle each file. - const absolutePathname = path.join(directory, file.name) + // Add any symbolic links to the list of symbolic links to resolve. + links.push(...result.links) - // 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()) { - directories.push(absolutePathname) - } else if (file.isSymbolicLink()) { - links.push(absolutePathname) - } else if (!pathnameFilter || pathnameFilter(absolutePathname)) { - push(absolutePathname) - } - } + // Add any file pathnames to the list of pathnames. + pathnames.push(...result.files) } // Resolve all the symbolic links we found if any. @@ -127,7 +140,7 @@ export async function recursiveReadDir( if (stats.isDirectory()) { directories.push(absolutePathname) } else if (!pathnameFilter || pathnameFilter(absolutePathname)) { - push(absolutePathname) + pathnames.push(coerce(absolutePathname)) } } } diff --git a/packages/next/src/lib/wait.ts b/packages/next/src/lib/wait.ts new file mode 100644 index 0000000000000..4476631bc989d --- /dev/null +++ b/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)) +} diff --git a/packages/next/src/server/lib/router-utils/filesystem.ts b/packages/next/src/server/lib/router-utils/filesystem.ts index df75df71fb14d..e09af618061d7 100644 --- a/packages/next/src/server/lib/router-utils/filesystem.ts +++ b/packages/next/src/server/lib/router-utils/filesystem.ts @@ -141,10 +141,9 @@ export async function setupFsCheck(opts: { buildId = await fs.readFile(buildIdPath, 'utf8') try { - for (let file of await recursiveReadDir(publicFolderPath)) { - file = normalizePathSep(file) - // ensure filename is encoded - publicFolderItems.add(encodeURI(file)) + for (const file of await recursiveReadDir(publicFolderPath)) { + // Ensure filename is encoded and normalized. + publicFolderItems.add(encodeURI(normalizePathSep(file))) } } catch (err: any) { if (err.code !== 'ENOENT') { @@ -153,10 +152,9 @@ export async function setupFsCheck(opts: { } try { - for (let file of await recursiveReadDir(legacyStaticFolderPath)) { - file = normalizePathSep(file) - // ensure filename is encoded - legacyStaticFolderItems.add(encodeURI(file)) + for (const file of await recursiveReadDir(legacyStaticFolderPath)) { + // Ensure filename is encoded and normalized. + legacyStaticFolderItems.add(encodeURI(normalizePathSep(file))) } Log.warn( `The static directory has been deprecated in favor of the public directory. https://nextjs.org/docs/messages/static-dir-deprecated` @@ -168,11 +166,10 @@ export async function setupFsCheck(opts: { } try { - for (let file of await recursiveReadDir(nextStaticFolderPath)) { - file = normalizePathSep(file) - // ensure filename is encoded + for (const file of await recursiveReadDir(nextStaticFolderPath)) { + // Ensure filename is encoded and normalized. nextStaticFolderItems.add( - path.posix.join('/_next/static', encodeURI(file)) + path.posix.join('/_next/static', encodeURI(normalizePathSep(file))) ) } } catch (err) { diff --git a/packages/next/src/server/load-components.ts b/packages/next/src/server/load-components.ts index 26ac8b843e7fd..45f190c589cca 100644 --- a/packages/next/src/server/load-components.ts +++ b/packages/next/src/server/load-components.ts @@ -25,6 +25,7 @@ import { interopDefault } from '../lib/interop-default' import { getTracer } from './lib/trace/tracer' import { LoadComponentsSpan } from './lib/trace/constants' import { loadManifest } from './load-manifest' +import { wait } from '../lib/wait' export type ManifestItem = { id: number | string files: string[] @@ -51,32 +52,6 @@ export type LoadComponentsReturnType = { pathname: string } -async function loadDefaultErrorComponentsImpl( - distDir: string -): Promise { - const Document = interopDefault(require('next/dist/pages/_document')) - const AppMod = require('next/dist/pages/_app') - const App = interopDefault(AppMod) - - // Load the compiled route module for this builtin error. - // TODO: (wyattjoh) replace this with just exporting the route module when the transition is complete - const ComponentMod = - require('./future/route-modules/pages/builtin/_error') as typeof import('./future/route-modules/pages/builtin/_error') - const Component = ComponentMod.routeModule.userland.default - - return { - App, - Document, - Component, - pageConfig: {}, - buildManifest: require(join(distDir, `fallback-${BUILD_MANIFEST}`)), - reactLoadableManifest: {}, - ComponentMod, - pathname: '/_error', - routeModule: ComponentMod.routeModule, - } -} - /** * Load manifest file with retries, defaults to 3 attempts. */ @@ -90,7 +65,8 @@ async function loadManifestWithRetries( } catch (err) { attempts-- if (attempts <= 0) throw err - await new Promise((resolve) => setTimeout(resolve, 100)) + + await wait(100) } } } @@ -98,19 +74,47 @@ async function loadManifestWithRetries( async function loadJSManifest( manifestPath: string, name: string, - entryname: string + entryName: string ): Promise { process.env.NEXT_MINIMAL ? // @ts-ignore __non_webpack_require__(manifestPath) : require(manifestPath) try { - return JSON.parse((globalThis as any)[name][entryname]) as T + return JSON.parse((globalThis as any)[name][entryName]) as T } catch (err) { return undefined } } +async function loadDefaultErrorComponentsImpl( + distDir: string +): Promise { + const Document = interopDefault(require('next/dist/pages/_document')) + const AppMod = require('next/dist/pages/_app') + const App = interopDefault(AppMod) + + // Load the compiled route module for this builtin error. + // TODO: (wyattjoh) replace this with just exporting the route module when the transition is complete + const ComponentMod = + require('./future/route-modules/pages/builtin/_error') as typeof import('./future/route-modules/pages/builtin/_error') + const Component = ComponentMod.routeModule.userland.default + + return { + App, + Document, + Component, + pageConfig: {}, + buildManifest: await loadManifestWithRetries( + join(distDir, `fallback-${BUILD_MANIFEST}`) + ), + reactLoadableManifest: {}, + ComponentMod, + pathname: '/_error', + routeModule: ComponentMod.routeModule, + } +} + async function loadComponentsImpl({ distDir, pathname, From 6843e3b3c49ad3b7658ed0f17dba8ddb922e647a Mon Sep 17 00:00:00 2001 From: Wyatt Johnson Date: Mon, 28 Aug 2023 08:31:13 -0600 Subject: [PATCH 08/12] fix: linting --- packages/next/src/lib/recursive-readdir.ts | 2 -- 1 file changed, 2 deletions(-) diff --git a/packages/next/src/lib/recursive-readdir.ts b/packages/next/src/lib/recursive-readdir.ts index 17f5ad2772f72..c6af116a110e2 100644 --- a/packages/next/src/lib/recursive-readdir.ts +++ b/packages/next/src/lib/recursive-readdir.ts @@ -1,5 +1,3 @@ -import type { Dirent } from 'fs' - import fs from 'fs/promises' import path from 'path' From 74a65b6962d34c5ac3dd83a3c2f026eb4302b03a Mon Sep 17 00:00:00 2001 From: Wyatt Johnson Date: Mon, 28 Aug 2023 09:39:21 -0600 Subject: [PATCH 09/12] fix: move options to object --- packages/next/src/build/index.ts | 20 ++++--- packages/next/src/lib/recursive-readdir.ts | 53 +++++++++++++++---- .../src/lib/typescript/getTypeScriptIntent.ts | 9 ++-- .../file-reader/default-file-reader.ts | 17 +++--- 4 files changed, 64 insertions(+), 35 deletions(-) diff --git a/packages/next/src/build/index.ts b/packages/next/src/build/index.ts index a86398b9d5658..6449c5f781349 100644 --- a/packages/next/src/build/index.ts +++ b/packages/next/src/build/index.ts @@ -448,11 +448,11 @@ export default async function build( const pagesPaths = !appDirOnly && pagesDir - ? await nextBuildSpan - .traceChild('collect-pages') - .traceAsyncFn(() => - recursiveReadDir(pagesDir, validFileMatcher.isPageFile) - ) + ? await nextBuildSpan.traceChild('collect-pages').traceAsyncFn(() => + recursiveReadDir(pagesDir, { + pathnameFilter: validFileMatcher.isPageFile, + }) + ) : [] const middlewareDetectionRegExp = new RegExp( @@ -512,16 +512,14 @@ export default async function build( const appPaths = await nextBuildSpan .traceChild('collect-app-paths') .traceAsyncFn(() => - recursiveReadDir( - appDir, - (absolutePath) => + recursiveReadDir(appDir, { + pathnameFilter: (absolutePath) => validFileMatcher.isAppRouterPage(absolutePath) || // For now we only collect the root /not-found page in the app // directory as the 404 fallback validFileMatcher.isRootNotFound(absolutePath), - undefined, - (part) => part.startsWith('_') - ) + ignorePartFilter: (part) => part.startsWith('_'), + }) ) mappedAppPages = nextBuildSpan diff --git a/packages/next/src/lib/recursive-readdir.ts b/packages/next/src/lib/recursive-readdir.ts index c6af116a110e2..d8eca65bf7274 100644 --- a/packages/next/src/lib/recursive-readdir.ts +++ b/packages/next/src/lib/recursive-readdir.ts @@ -9,24 +9,55 @@ type Result = { links: string[] } +type RecursiveReadDirOptions = { + /** + * Filter to ignore files with absolute pathnames, false to ignore. + */ + pathnameFilter: Filter + + /** + * Filter to ignore files and directories with absolute pathnames, false to + * ignore. + */ + ignoreFilter: Filter + + /** + * Filter to ignore files and directories with the pathname part, false to + * ignore. + */ + ignorePartFilter: Filter + + /** + * Whether to sort the results, true by default. + */ + sortPathnames: boolean + + /** + * Whether to return relative pathnames, true by default. + */ + relativePathnames: boolean +} + /** + * Recursively reads a directory and returns the list of pathnames. * * @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 + * @param options options to control the behavior of the recursive read + * @returns the list of pathnames */ export async function recursiveReadDir( rootDirectory: string, - pathnameFilter?: Filter, - ignoreFilter?: Filter, - ignorePartFilter?: Filter, - sortPathnames: boolean = true, - relativePathnames: boolean = true + options: Partial = {} ): Promise { + // Grab our options. + const { + pathnameFilter, + ignoreFilter, + ignorePartFilter, + sortPathnames = true, + relativePathnames = true, + } = options + // The list of pathnames to return. const pathnames: string[] = [] diff --git a/packages/next/src/lib/typescript/getTypeScriptIntent.ts b/packages/next/src/lib/typescript/getTypeScriptIntent.ts index bb103a10ebed8..919274352f47a 100644 --- a/packages/next/src/lib/typescript/getTypeScriptIntent.ts +++ b/packages/next/src/lib/typescript/getTypeScriptIntent.ts @@ -32,11 +32,10 @@ export async function getTypeScriptIntent( const tsFilesRegex = /.*\.(ts|tsx)$/ const excludedRegex = /(node_modules|.*\.d\.ts$)/ for (const dir of intentDirs) { - const typescriptFiles = await recursiveReadDir( - dir, - (name) => tsFilesRegex.test(name), - (name) => excludedRegex.test(name) - ) + const typescriptFiles = await recursiveReadDir(dir, { + pathnameFilter: (name) => tsFilesRegex.test(name), + ignoreFilter: (name) => excludedRegex.test(name), + }) if (typescriptFiles.length) { return { firstTimeSetup: true } } diff --git a/packages/next/src/server/future/route-matcher-providers/dev/helpers/file-reader/default-file-reader.ts b/packages/next/src/server/future/route-matcher-providers/dev/helpers/file-reader/default-file-reader.ts index ecec91c160240..772e712d335d2 100644 --- a/packages/next/src/server/future/route-matcher-providers/dev/helpers/file-reader/default-file-reader.ts +++ b/packages/next/src/server/future/route-matcher-providers/dev/helpers/file-reader/default-file-reader.ts @@ -52,17 +52,18 @@ export class DefaultFileReader implements FileReader { * @returns a promise that resolves to the list of files */ public async read(dir: string): Promise> { - return recursiveReadDir( - dir, - this.pathnameFilter, - this.ignoreFilter, - this.ignorePartFilter, + return recursiveReadDir(dir, { + pathnameFilter: this.pathnameFilter, + ignoreFilter: this.ignoreFilter, + ignorePartFilter: this.ignorePartFilter, + // We don't need to sort the results because we're not depending on the // order of the results. - false, + sortPathnames: false, + // We want absolute pathnames because we're going to be comparing them // with other absolute pathnames. - false - ) + relativePathnames: false, + }) } } From 47529f64aa5e87c8ed4a2d0d75a72e9ed5eb89f0 Mon Sep 17 00:00:00 2001 From: Wyatt Johnson Date: Mon, 28 Aug 2023 10:26:43 -0600 Subject: [PATCH 10/12] feat: remove iterator to improve performance --- packages/next/src/lib/recursive-readdir.ts | 17 +++++++---------- 1 file changed, 7 insertions(+), 10 deletions(-) diff --git a/packages/next/src/lib/recursive-readdir.ts b/packages/next/src/lib/recursive-readdir.ts index d8eca65bf7274..db5595e86db36 100644 --- a/packages/next/src/lib/recursive-readdir.ts +++ b/packages/next/src/lib/recursive-readdir.ts @@ -5,7 +5,7 @@ type Filter = (pathname: string) => boolean type Result = { directories: string[] - files: string[] + pathnames: string[] links: string[] } @@ -75,14 +75,11 @@ export async function recursiveReadDir( // 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: [] } + const result: Result = { directories: [], pathnames: [], 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) { + const dir = await fs.readdir(directory, { withFileTypes: true }) + for (const file of dir) { // If enabled, ignore the file if it matches the ignore filter. if (ignorePartFilter && ignorePartFilter(file.name)) { continue @@ -103,7 +100,7 @@ export async function recursiveReadDir( } else if (file.isSymbolicLink()) { result.links.push(absolutePathname) } else if (!pathnameFilter || pathnameFilter(absolutePathname)) { - result.files.push(coerce(absolutePathname)) + result.pathnames.push(coerce(absolutePathname)) } } } catch (err: any) { @@ -113,7 +110,7 @@ export async function recursiveReadDir( if (err.code !== 'ENOENT' || directory === rootDirectory) throw err // The error occurred, so abandon reading this directory. - return { directories: [], files: [], links: [] } + return { directories: [], pathnames: [], links: [] } } return result @@ -136,7 +133,7 @@ export async function recursiveReadDir( links.push(...result.links) // Add any file pathnames to the list of pathnames. - pathnames.push(...result.files) + pathnames.push(...result.pathnames) } // Resolve all the symbolic links we found if any. From 65b8b5e6c9b9a8341daf1fbb1972f79833ec24e0 Mon Sep 17 00:00:00 2001 From: Wyatt Johnson Date: Mon, 28 Aug 2023 11:01:21 -0600 Subject: [PATCH 11/12] fix: handled other test suites that needed updating --- packages/next/src/lib/recursive-readdir.ts | 19 +++++---- .../next/src/server/dev/next-dev-server.ts | 23 ++++++++++- .../file-reader/default-file-reader.ts | 40 +++++++------------ .../build-output/test/index.test.js | 6 +-- .../test/index.test.js | 5 +-- .../integration/production/test/index.test.js | 4 +- test/integration/production/test/security.js | 6 +-- test/unit/recursive-delete.test.ts | 12 ++---- test/unit/recursive-readdir.test.ts | 4 +- 9 files changed, 62 insertions(+), 57 deletions(-) diff --git a/packages/next/src/lib/recursive-readdir.ts b/packages/next/src/lib/recursive-readdir.ts index db5595e86db36..d67effb698a65 100644 --- a/packages/next/src/lib/recursive-readdir.ts +++ b/packages/next/src/lib/recursive-readdir.ts @@ -9,33 +9,33 @@ type Result = { links: string[] } -type RecursiveReadDirOptions = { +export type RecursiveReadDirOptions = { /** * Filter to ignore files with absolute pathnames, false to ignore. */ - pathnameFilter: Filter + pathnameFilter?: Filter /** * Filter to ignore files and directories with absolute pathnames, false to * ignore. */ - ignoreFilter: Filter + ignoreFilter?: Filter /** * Filter to ignore files and directories with the pathname part, false to * ignore. */ - ignorePartFilter: Filter + ignorePartFilter?: Filter /** * Whether to sort the results, true by default. */ - sortPathnames: boolean + sortPathnames?: boolean /** * Whether to return relative pathnames, true by default. */ - relativePathnames: boolean + relativePathnames?: boolean } /** @@ -47,7 +47,7 @@ type RecursiveReadDirOptions = { */ export async function recursiveReadDir( rootDirectory: string, - options: Partial = {} + options: RecursiveReadDirOptions = {} ): Promise { // Grab our options. const { @@ -110,7 +110,7 @@ export async function recursiveReadDir( if (err.code !== 'ENOENT' || directory === rootDirectory) throw err // The error occurred, so abandon reading this directory. - return { directories: [], pathnames: [], links: [] } + return null } return result @@ -126,6 +126,9 @@ export async function recursiveReadDir( // For each result of directory scans... for (const result of results) { + // If the directory was removed, then skip it. + if (!result) continue + // Add any directories to the list of directories to scan. directories.push(...result.directories) diff --git a/packages/next/src/server/dev/next-dev-server.ts b/packages/next/src/server/dev/next-dev-server.ts index 2649096c35b40..e399050c64412 100644 --- a/packages/next/src/server/dev/next-dev-server.ts +++ b/packages/next/src/server/dev/next-dev-server.ts @@ -208,10 +208,17 @@ export default class DevServer extends Server { this.dir ) const extensions = this.nextConfig.pageExtensions - const fileReader = new BatchedFileReader(new DefaultFileReader()) + const extensionsExpression = new RegExp(`\\.(?:${extensions.join('|')})$`) // If the pages directory is available, then configure those matchers. if (pagesDir) { + const fileReader = new BatchedFileReader( + new DefaultFileReader({ + // Only allow files that have the correct extensions. + pathnameFilter: (pathname) => extensionsExpression.test(pathname), + }) + ) + matchers.push( new DevPagesRouteMatcherProvider( pagesDir, @@ -231,6 +238,20 @@ export default class DevServer extends Server { } if (appDir) { + // We create a new file reader for the app directory because we don't want + // to include any folders or files starting with an underscore. This will + // prevent the reader from wasting time reading files that we know we + // don't care about. + const fileReader = new BatchedFileReader( + new DefaultFileReader({ + // Only allow files that have the correct extensions. + pathnameFilter: (pathname) => extensionsExpression.test(pathname), + + // Ignore any directory prefixed with an underscore. + ignorePartFilter: (part) => part.startsWith('_'), + }) + ) + matchers.push( new DevAppPageRouteMatcherProvider(appDir, extensions, fileReader) ) diff --git a/packages/next/src/server/future/route-matcher-providers/dev/helpers/file-reader/default-file-reader.ts b/packages/next/src/server/future/route-matcher-providers/dev/helpers/file-reader/default-file-reader.ts index 772e712d335d2..96a463fe3b83c 100644 --- a/packages/next/src/server/future/route-matcher-providers/dev/helpers/file-reader/default-file-reader.ts +++ b/packages/next/src/server/future/route-matcher-providers/dev/helpers/file-reader/default-file-reader.ts @@ -1,8 +1,14 @@ import type { FileReader } from './file-reader' -import { recursiveReadDir } from '../../../../../../lib/recursive-readdir' +import { + RecursiveReadDirOptions, + recursiveReadDir, +} from '../../../../../../lib/recursive-readdir' -type Filter = (pathname: string) => boolean +export type DefaultFileReaderOptions = Pick< + RecursiveReadDirOptions, + 'pathnameFilter' | 'ignoreFilter' | 'ignorePartFilter' +> /** * Reads all the files in the directory and its subdirectories following any @@ -13,19 +19,7 @@ export class DefaultFileReader implements FileReader { * Filter to ignore files with absolute pathnames. If undefined, no files are * ignored. */ - private readonly pathnameFilter: Filter | undefined - - /** - * Filter to ignore files and directories with absolute pathnames. If - * undefined, no files are ignored. - */ - private readonly ignoreFilter: Filter | undefined - - /** - * Filter to ignore files and directories with the pathname part. If - * undefined, no files are ignored. - */ - private readonly ignorePartFilter: Filter | undefined + private readonly options: Readonly /** * Creates a new file reader. @@ -34,14 +28,8 @@ export class DefaultFileReader implements FileReader { * @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 */ - constructor( - pathnameFilter?: Filter, - ignoreFilter?: Filter, - ignorePartFilter?: Filter - ) { - this.pathnameFilter = pathnameFilter - this.ignoreFilter = ignoreFilter - this.ignorePartFilter = ignorePartFilter + constructor(options: Readonly) { + this.options = options } /** @@ -53,9 +41,9 @@ export class DefaultFileReader implements FileReader { */ public async read(dir: string): Promise> { return recursiveReadDir(dir, { - pathnameFilter: this.pathnameFilter, - ignoreFilter: this.ignoreFilter, - ignorePartFilter: this.ignorePartFilter, + pathnameFilter: this.options.pathnameFilter, + ignoreFilter: this.options.ignoreFilter, + ignorePartFilter: this.options.ignorePartFilter, // We don't need to sort the results because we're not depending on the // order of the results. diff --git a/test/integration/build-output/test/index.test.js b/test/integration/build-output/test/index.test.js index 9c9742d943b6a..ecefcf8b726e7 100644 --- a/test/integration/build-output/test/index.test.js +++ b/test/integration/build-output/test/index.test.js @@ -183,9 +183,9 @@ describe('Build Output', () => { }) it('should not emit extracted comments', async () => { - const files = await recursiveReadDir(join(appDir, '.next'), (f) => - /\.txt|\.LICENSE\./.test(f) - ) + const files = await recursiveReadDir(join(appDir, '.next'), { + pathnameFilter: (f) => /\.txt|\.LICENSE\./.test(f), + }) expect(files).toEqual([]) }) }) diff --git a/test/integration/production-browser-sourcemaps/test/index.test.js b/test/integration/production-browser-sourcemaps/test/index.test.js index b8a8fe3dc6140..29439848660f9 100644 --- a/test/integration/production-browser-sourcemaps/test/index.test.js +++ b/test/integration/production-browser-sourcemaps/test/index.test.js @@ -8,10 +8,7 @@ const appDir = join(__dirname, '../') function runTests() { it('includes sourcemaps for all browser files', async () => { - const browserFiles = await recursiveReadDir( - join(appDir, '.next', 'static'), - (f) => /.*/.test(f) - ) + const browserFiles = await recursiveReadDir(join(appDir, '.next', 'static')) const jsFiles = browserFiles.filter( (file) => file.endsWith('.js') && file.includes('/pages/') ) diff --git a/test/integration/production/test/index.test.js b/test/integration/production/test/index.test.js index d41fd51b6f339..ed861434b6c1a 100644 --- a/test/integration/production/test/index.test.js +++ b/test/integration/production/test/index.test.js @@ -648,13 +648,13 @@ describe('Production Usage', () => { const cssStaticAssets = await recursiveReadDir( join(__dirname, '..', '.next', 'static'), - (f) => /\.css$/.test(f) + { pathnameFilter: (f) => /\.css$/.test(f) } ) expect(cssStaticAssets.length).toBeGreaterThanOrEqual(1) expect(cssStaticAssets[0]).toMatch(/[\\/]css[\\/]/) const mediaStaticAssets = await recursiveReadDir( join(__dirname, '..', '.next', 'static'), - (f) => /\.svg$/.test(f) + { pathnameFilter: (f) => /\.svg$/.test(f) } ) expect(mediaStaticAssets.length).toBeGreaterThanOrEqual(1) expect(mediaStaticAssets[0]).toMatch(/[\\/]media[\\/]/) diff --git a/test/integration/production/test/security.js b/test/integration/production/test/security.js index 2d2d7a35bf65f..edb1692f0d068 100644 --- a/test/integration/production/test/security.js +++ b/test/integration/production/test/security.js @@ -98,9 +98,9 @@ module.exports = (context) => { const buildId = readFileSync(join(__dirname, '../.next/BUILD_ID'), 'utf8') const readPath = join(__dirname, `../.next/static/${buildId}`) - const buildFiles = await recursiveReadDir(readPath, (f) => - /\.js$/.test(f) - ) + const buildFiles = await recursiveReadDir(readPath, { + pathnameFilter: (f) => /\.js$/.test(f), + }) if (buildFiles.length < 1) { throw new Error('Could not locate any build files') diff --git a/test/unit/recursive-delete.test.ts b/test/unit/recursive-delete.test.ts index 495d98991462f..237790bbf8905 100644 --- a/test/unit/recursive-delete.test.ts +++ b/test/unit/recursive-delete.test.ts @@ -21,9 +21,7 @@ describe('recursiveDelete', () => { await recursiveCopy(resolveDataDir, testResolveDataDir) await fs.symlink('./aa', join(testResolveDataDir, 'symlink')) await recursiveDelete(testResolveDataDir) - const result = await recursiveReadDir(testResolveDataDir, (f) => - /.*/.test(f) - ) + const result = await recursiveReadDir(testResolveDataDir) expect(result.length).toBe(0) } finally { await recursiveDelete(testResolveDataDir) @@ -39,17 +37,13 @@ describe('recursiveDelete', () => { // preserve cache dir await recursiveDelete(testpreservefileDir, /^cache/) - const result = await recursiveReadDir(testpreservefileDir, (f) => - /.*/.test(f) - ) + const result = await recursiveReadDir(testpreservefileDir) expect(result.length).toBe(1) } finally { // Ensure test cleanup await recursiveDelete(testpreservefileDir) - const cleanupResult = await recursiveReadDir(testpreservefileDir, (f) => - /.*/.test(f) - ) + const cleanupResult = await recursiveReadDir(testpreservefileDir) expect(cleanupResult.length).toBe(0) } }) diff --git a/test/unit/recursive-readdir.test.ts b/test/unit/recursive-readdir.test.ts index 666ea770d7ced..a07cba7bd9791 100644 --- a/test/unit/recursive-readdir.test.ts +++ b/test/unit/recursive-readdir.test.ts @@ -8,7 +8,9 @@ const dirWithPages = join(resolveDataDir, 'readdir', 'pages') describe('recursiveReadDir', () => { it('should work', async () => { - const result = await recursiveReadDir(dirWithPages, (f) => /\.js/.test(f)) + const result = await recursiveReadDir(dirWithPages, { + pathnameFilter: (f) => /\.js/.test(f), + }) const pages = [ /^[\\/]index\.js/, /^[\\/]prefered\.js/, From 29b42046a2175376878cef95c59a258cb1083800 Mon Sep 17 00:00:00 2001 From: Wyatt Johnson Date: Mon, 28 Aug 2023 11:17:51 -0600 Subject: [PATCH 12/12] fix: remove file extension test for app dir routes to support static metadata files --- packages/next/src/server/dev/next-dev-server.ts | 3 --- 1 file changed, 3 deletions(-) diff --git a/packages/next/src/server/dev/next-dev-server.ts b/packages/next/src/server/dev/next-dev-server.ts index e399050c64412..6c75285d77afe 100644 --- a/packages/next/src/server/dev/next-dev-server.ts +++ b/packages/next/src/server/dev/next-dev-server.ts @@ -244,9 +244,6 @@ export default class DevServer extends Server { // don't care about. const fileReader = new BatchedFileReader( new DefaultFileReader({ - // Only allow files that have the correct extensions. - pathnameFilter: (pathname) => extensionsExpression.test(pathname), - // Ignore any directory prefixed with an underscore. ignorePartFilter: (part) => part.startsWith('_'), })