refactor: proper Node polyfill directory iteration now that Deno supports this (#4783)

std/node/_fs/_fs_dir.ts

@@ -2,8 +2,8 @@ import Dirent from "./_fs_dirent.ts";
 
 export default class Dir {
   private dirPath: string | Uint8Array;
-  private files: Dirent[] = [];
+  private syncIterator!: Iterator<Deno.DirEntry> | null;
-  private filesReadComplete = false;
+  private asyncIterator!: AsyncIterator<Deno.DirEntry> | null;
 
   constructor(path: string | Uint8Array) {
     this.dirPath = path;
@@ -16,35 +16,19 @@ export default class Dir {
     return this.dirPath;
   }
 
-  /**
-   * NOTE: Deno doesn't provide an interface to the filesystem like readdir
-   * where each call to readdir returns the next file.  This function simulates this
-   * behaviour by fetching all the entries on the first call, putting them on a stack
-   * and then popping them off the stack one at a time.
-   *
-   * TODO: Rework this implementation once https://github.com/denoland/deno/issues/4218
-   * is resolved.
-   */
   read(callback?: Function): Promise<Dirent | null> {
     return new Promise(async (resolve, reject) => {
       try {
-        if (this.initializationOfDirectoryFilesIsRequired()) {
+        if (!this.asyncIterator) {
-          const denoFiles: Deno.DirEntry[] = [];
+          this.asyncIterator = Deno.readdir(this.path)[Symbol.asyncIterator]();
-          for await (const dirEntry of Deno.readdir(this.path)) {
-            denoFiles.push(dirEntry);
-          }
-          this.files = denoFiles.map((file) => new Dirent(file));
-        }
-        const nextFile = this.files.pop();
-        if (nextFile) {
-          resolve(nextFile);
-          this.filesReadComplete = this.files.length === 0;
-        } else {
-          this.filesReadComplete = true;
-          resolve(null);
         }
+
+        const result: Dirent | null = await (await this.asyncIterator?.next())
+          .value;
+        resolve(result ? result : null);
+
         if (callback) {
-          callback(null, !nextFile ? null : nextFile);
+          callback(null, result ? result : null);
         }
       } catch (err) {
         if (callback) {
@@ -56,19 +40,13 @@ export default class Dir {
   }
 
   readSync(): Dirent | null {
-    if (this.initializationOfDirectoryFilesIsRequired()) {
+    if (!this.syncIterator) {
-      this.files.push(
+      this.syncIterator = Deno.readdirSync(this.path)![Symbol.iterator]();
-        ...[...Deno.readdirSync(this.path)].map((file) => new Dirent(file))
-      );
     }
-    const dirent: Dirent | undefined = this.files.pop();
-    this.filesReadComplete = this.files.length === 0;
 
-    return !dirent ? null : dirent;
+    const file: Deno.DirEntry = this.syncIterator.next().value;
-  }
 
-  private initializationOfDirectoryFilesIsRequired(): boolean {
+    return file ? new Dirent(file) : null;
-    return this.files.length === 0 && !this.filesReadComplete;
   }
 
   /**

std/node/_fs/_fs_dir_test.ts

@@ -90,6 +90,7 @@ test({
         }
       );
       const thirdRead: Dirent | null = await dir.read();
+      const fourthRead: Dirent | null = await dir.read();
 
       if (firstRead?.name === "foo.txt") {
         assertEquals(secondRead?.name, "bar.txt");
@@ -100,6 +101,7 @@ test({
       }
       assert(secondCallback);
       assert(thirdRead === null);
+      assert(fourthRead === null);
     } finally {
       Deno.removeSync(testDir, { recursive: true });
     }
@@ -120,6 +122,7 @@ test({
       const firstRead: Dirent | null = dir.readSync();
       const secondRead: Dirent | null = dir.readSync();
       const thirdRead: Dirent | null = dir.readSync();
+      const fourthRead: Dirent | null = dir.readSync();
 
       if (firstRead?.name === "foo.txt") {
         assertEquals(secondRead?.name, "bar.txt");
@@ -129,6 +132,7 @@ test({
         fail("File not found during read");
       }
       assert(thirdRead === null);
+      assert(fourthRead === null);
     } finally {
       Deno.removeSync(testDir, { recursive: true });
     }