fix: support maxFiles without slicing

We currently exceed `maxFiles` and slice the result later. This doesn't
work in an iterator since there is no end "result".

This change instead makes the walker stop walking when `maxFiles` is
met, so we already have the correctly sized result set.

Author: 43081j

__tests__/fdir.test.ts

@@ -1,12 +1,11 @@
-import { fdir, IterableOutput } from "../src/index";
+import { fdir } from "../src/index";
 import fs from "fs";
 import mock from "mock-fs";
-import { test, beforeEach, TestContext, vi } from "vitest";
+import { test, beforeEach, vi } from "vitest";
 import path, { sep } from "path";
 import { convertSlashes } from "../src/utils";
 import picomatch from "picomatch";
-import { apiTypes, APITypes, cwd, restricted, root } from "./utils";
-import { APIBuilder } from "../src/builder/api-builder";
+import { apiTypes, APITypes, cwd, restricted, root, execute } from "./utils";
 
 beforeEach(() => {
   mock.restore();
@@ -26,22 +25,6 @@ test(`crawl single depth directory with callback`, (t) => {
   });
 });
 
-async function execute<T extends IterableOutput>(
-  api: APIBuilder<T>,
-  type: APITypes
-): Promise<T> {
-  let files: T[number][] = [];
-
-  if (type === "withIterator") {
-    for await (const file of api[type]()) {
-      files.push(file);
-    }
-  } else {
-    files = await api[type]();
-  }
-  return files as T;
-}
-
 async function crawl(type: APITypes, path: string) {
   const api = new fdir().crawl(path);
   return execute(api, type);
@@ -270,7 +253,7 @@ for (const type of apiTypes) {
       .onlyDirs()
       .filter((path) => path.includes("api"))
       .crawl("./src");
-    const result = await api[type]();
+    const result = await execute(api, type);
     t.expect(result).toHaveLength(2);
   });
 

__tests__/symlinks.test.ts

@@ -1,7 +1,7 @@
-import { afterAll, beforeAll, beforeEach, describe, test } from "vitest";
-import { apiTypes, normalize, root } from "./utils";
+import { afterAll, beforeAll, describe, test } from "vitest";
+import { apiTypes, normalize, root, execute } from "./utils";
 import mock from "mock-fs";
-import { fdir, Options } from "../src";
+import { fdir } from "../src";
 import path from "path";
 
 const fsWithRelativeSymlinks = {
@@ -154,7 +154,7 @@ for (const type of apiTypes) {
 
     test(`resolve symlinks`, async (t) => {
       const api = new fdir().withSymlinks().crawl("/some/dir");
-      const files = await api[type]();
+      const files = await execute(api, type);
       t.expect(files.sort()).toStrictEqual(
         normalize([
           "/other/dir/file-2",
@@ -166,7 +166,7 @@ for (const type of apiTypes) {
 
     test(`resolve recursive symlinks`, async (t) => {
       const api = new fdir().withSymlinks().crawl("/recursive");
-      const files = await api[type]();
+      const files = await execute(api, type);
       t.expect(files.sort()).toStrictEqual(
         normalize([
           "/double/recursive/another-file",
@@ -184,7 +184,7 @@ for (const type of apiTypes) {
       const api = new fdir()
         .withSymlinks({ resolvePaths: false })
         .crawl("/recursive");
-      const files = await api[type]();
+      const files = await execute(api, type);
       t.expect(files.sort()).toStrictEqual(
         normalize([
           "/recursive/dir/not-recursive/another-file",
@@ -234,7 +234,7 @@ for (const type of apiTypes) {
         .withRelativePaths()
         .withErrors()
         .crawl("./recursive");
-      const files = await api[type]();
+      const files = await execute(api, type);
       t.expect(files.sort()).toStrictEqual(
         normalize([
           "dir/not-recursive/another-file",
@@ -284,7 +284,7 @@ for (const type of apiTypes) {
         .withRelativePaths()
         .withErrors()
         .crawl("./recursive");
-      const files = await api[type]();
+      const files = await execute(api, type);
       t.expect(files.sort()).toStrictEqual(
         normalize([
           "..//double/recursive/another-file",
@@ -302,7 +302,7 @@ for (const type of apiTypes) {
       const api = new fdir()
         .withSymlinks({ resolvePaths: false })
         .crawl("/some/dir");
-      const files = await api[type]();
+      const files = await execute(api, type);
       t.expect(files.sort()).toStrictEqual(
         normalize([
           "/some/dir/dirSymlink/file-1",
@@ -317,7 +317,7 @@ for (const type of apiTypes) {
         .withSymlinks({ resolvePaths: false })
         .withRelativePaths()
         .crawl("/some/dir");
-      const files = await api[type]();
+      const files = await execute(api, type);
       t.expect(files.sort()).toStrictEqual(
         normalize([
           "dirSymlink/file-1",
@@ -332,7 +332,7 @@ for (const type of apiTypes) {
         .withSymlinks()
         .withRelativePaths()
         .crawl("./relative/dir");
-      const files = await api[type]();
+      const files = await execute(api, type);
       t.expect(files.sort()).toStrictEqual(
         normalize([
           "../../../../other-relative/dir/file-2",
@@ -347,7 +347,7 @@ for (const type of apiTypes) {
         .withSymlinks()
         .exclude((_name, path) => path === resolveSymlinkRoot("/sym/linked/"))
         .crawl("/some/dir");
-      const files = await api[type]();
+      const files = await execute(api, type);
       t.expect(files.sort()).toStrictEqual(normalize(["/other/dir/file-2"]));
     });
 
@@ -358,31 +358,31 @@ for (const type of apiTypes) {
           (_name, path) => path === resolveSymlinkRoot("/some/dir/dirSymlink/")
         )
         .crawl("/some/dir");
-      const files = await api[type]();
+      const files = await execute(api, type);
       t.expect(files.sort()).toStrictEqual(
         normalize(["/some/dir/fileSymlink"])
       );
     });
 
     test(`do not resolve symlinks`, async (t) => {
       const api = new fdir().crawl("/some/dir");
-      const files = await api[type]();
+      const files = await execute(api, type);
       t.expect(files.sort()).toStrictEqual(
         normalize(["dirSymlink", "fileSymlink", "fileSymlink2"])
       );
     });
 
     test(`exclude symlinks`, async (t) => {
       const api = new fdir({ excludeSymlinks: true }).crawl("/some/dir");
-      const files = await api[type]();
+      const files = await execute(api, type);
       t.expect(files).toHaveLength(0);
     });
 
     test(
       "doesn't hang when resolving symlinks in the root directory",
       async (t) => {
         const api = new fdir().withSymlinks({ resolvePaths: false }).crawl("/");
-        const files = await api[type]();
+        const files = await execute(api, type);
         const expectedFiles = normalize(["/lib/file-1", "/usr/lib/file-1"]);
         for (const expectedFile of expectedFiles) {
           t.expect(files).toContain(expectedFile);

__tests__/utils.ts

@@ -1,4 +1,6 @@
 import path from "path";
+import type { APIBuilder } from "../src/builder/api-builder";
+import type { IterableOutput } from "../src/types";
 
 export type APITypes = (typeof apiTypes)[number];
 export const apiTypes = ["withPromise", "sync", "withIterator"] as const;
@@ -22,3 +24,19 @@ export function normalize(paths: string[]) {
     path.isAbsolute(p) ? path.resolve(p) : path.normalize(p)
   );
 }
+
+export async function execute<T extends IterableOutput>(
+  api: APIBuilder<T>,
+  type: APITypes
+): Promise<T> {
+  let files: T[number][] = [];
+
+  if (type === "withIterator") {
+    for await (const file of api[type]()) {
+      files.push(file);
+    }
+  } else {
+    files = await api[type]();
+  }
+  return files as T;
+}

src/api/functions/push-directory.ts

@@ -1,46 +1,53 @@
-import { FilterPredicate, Options } from "../../types";
+import { FilterPredicate, Options, Counts } from "../../types";
 
 export type PushDirectoryFunction = (
   directoryPath: string,
   paths: string[],
   pushPath: (path: string, arr: string[]) => void,
+  counts: Counts,
   filters?: FilterPredicate[]
 ) => void;
 
 function pushDirectoryWithRelativePath(root: string): PushDirectoryFunction {
-  return function (directoryPath, paths, pushPath) {
+  return function (directoryPath, paths, pushPath, counts) {
     pushPath(directoryPath.substring(root.length) || ".", paths);
+    counts.directories++;
   };
 }
 
 function pushDirectoryFilterWithRelativePath(
   root: string
 ): PushDirectoryFunction {
-  return function (directoryPath, paths, pushPath, filters) {
+  return function (directoryPath, paths, pushPath, counts, filters) {
     const relativePath = directoryPath.substring(root.length) || ".";
     if (filters!.every((filter) => filter(relativePath, true))) {
       pushPath(relativePath, paths);
+      counts.directories++;
     }
   };
 }
 
 const pushDirectory: PushDirectoryFunction = (
   directoryPath,
   paths,
-  pushPath
+  pushPath,
+  counts
 ) => {
   pushPath(directoryPath || ".", paths);
+  counts.directories++;
 };
 
 const pushDirectoryFilter: PushDirectoryFunction = (
   directoryPath,
   paths,
   pushPath,
+  counts,
   filters
 ) => {
   const path = directoryPath || ".";
   if (filters!.every((filter) => filter(path, true))) {
     pushPath(path, paths);
+    counts.directories++;
   }
 };
 

src/api/functions/push-file.ts

@@ -22,11 +22,13 @@ const pushFileFilter: PushFileFunction = (
   filename,
   paths,
   pushPath,
-  _counts,
+  counts,
   filters
 ) => {
-  if (filters!.every((filter) => filter(filename, false)))
+  if (filters!.every((filter) => filter(filename, false))) {
     pushPath(filename, paths);
+    counts.files++;
+  }
 };
 
 const pushFileCount: PushFileFunction = (
@@ -39,8 +41,9 @@ const pushFileCount: PushFileFunction = (
   counts.files++;
 };
 
-const pushFile: PushFileFunction = (filename, paths, pushPath) => {
+const pushFile: PushFileFunction = (filename, paths, pushPath, counts) => {
   pushPath(filename, paths);
+  counts.files++;
 };
 
 const empty: PushFileFunction = () => {};

src/api/functions/walk-directory.ts

@@ -23,7 +23,6 @@ const walkAsync: WalkDirectoryFunction = (
   if (currentDepth < 0) return state.queue.dequeue(null, state);
 
   state.visited.push(crawlPath);
-  state.counts.directories++;
 
   // Perf: Node >= 10 introduced withFileTypes that helps us
   // skip an extra fs.stat call.
@@ -43,7 +42,6 @@ const walkSync: WalkDirectoryFunction = (
 ) => {
   if (currentDepth < 0) return;
   state.visited.push(crawlPath);
-  state.counts.directories++;
 
   let entries: fs.Dirent[] = [];
   try {

src/api/iterator.ts

@@ -6,6 +6,7 @@ class WalkerIterator<TOutput extends IterableOutput> {
   #walker: Walker<TOutput>;
   #currentGroup?: string[];
   #queue: TOutput[number][] = [];
+  #error?: unknown;
 
   public constructor(root: string, options: Options) {
     const pushPath = options.group ? this.#pushPath : this.#pushResult;
@@ -34,9 +35,12 @@ class WalkerIterator<TOutput extends IterableOutput> {
     }
   };
 
-  #onComplete = () => {
+  #onComplete = (err: unknown) => {
     this.#currentGroup = undefined;
     this.#complete = true;
+    if (err) {
+      this.#error = err;
+    }
     if (this.#resolver) {
       const resolver = this.#resolver;
       this.#resolver = undefined;
@@ -51,6 +55,10 @@ class WalkerIterator<TOutput extends IterableOutput> {
       yield* this.#queue;
       this.#queue = [];
 
+      if (this.#error) {
+        throw this.#error;
+      }
+
       if (this.#complete) {
         return;
       }

src/api/walker.ts

@@ -84,6 +84,7 @@ export class Walker<TOutput extends Output> {
       this.root,
       this.state.paths,
       this.pushPath,
+      this.state.counts,
       this.state.options.filters
     );
     this.walkDirectory(
@@ -98,7 +99,7 @@ export class Walker<TOutput extends Output> {
 
   private walk = (entries: Dirent[], directoryPath: string, depth: number) => {
     const {
-      paths,
+      counts,
       options: {
         filters,
         resolveSymlinks,
@@ -115,12 +116,16 @@ export class Walker<TOutput extends Output> {
     if (
       controller.signal.aborted ||
       (signal && signal.aborted) ||
-      (maxFiles && paths.length > maxFiles)
+      (maxFiles && counts.directories + counts.files > maxFiles)
     )
       return;
 
     const files = this.getArray(this.state.paths);
     for (let i = 0; i < entries.length; ++i) {
+      if (maxFiles && counts.directories + counts.files >= maxFiles) {
+        break;
+      }
+
       const entry = entries[i];
 
       if (
@@ -142,7 +147,7 @@ export class Walker<TOutput extends Output> {
           this.state.options.pathSeparator
         );
         if (exclude && exclude(entry.name, path)) continue;
-        this.pushDirectory(path, files, this.pushPath, filters);
+        this.pushDirectory(path, files, this.pushPath, counts, filters);
         this.walkDirectory(this.state, path, path, depth - 1, this.walk);
       } else if (this.resolveSymlink && entry.isSymbolicLink()) {
         let path = joinPath.joinPathWithBasePath(entry.name, directoryPath);