Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(node/fs): support recursive option in readdir #27179

Merged
merged 1 commit into from
Dec 3, 2024
Merged

fix(node/fs): support recursive option in readdir #27179

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
105 changes: 73 additions & 32 deletions ext/node/polyfills/_fs/_fs_readdir.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,13 @@
// deno-lint-ignore-file prefer-primordials

import { TextDecoder, TextEncoder } from "ext:deno_web/08_text_encoding.js";
import { asyncIterableToCallback } from "ext:deno_node/_fs/_fs_watch.ts";
import Dirent from "ext:deno_node/_fs/_fs_dirent.ts";
import { denoErrorToNodeError } from "ext:deno_node/internal/errors.ts";
import { getValidatedPath } from "ext:deno_node/internal/fs/utils.mjs";
import { Buffer } from "node:buffer";
import { promisify } from "ext:deno_node/internal/util.mjs";
import { op_fs_read_dir_async, op_fs_read_dir_sync } from "ext:core/ops";
import { join, relative } from "node:path";

function toDirent(val: Deno.DirEntry & { parentPath: string }): Dirent {
return new Dirent(val);
Expand All @@ -18,6 +19,7 @@ function toDirent(val: Deno.DirEntry & { parentPath: string }): Dirent {
type readDirOptions = {
encoding?: string;
withFileTypes?: boolean;
recursive?: boolean;
};

type readDirCallback = (err: Error | null, files: string[]) => void;
Expand All @@ -30,12 +32,12 @@ type readDirBoth = (

export function readdir(
path: string | Buffer | URL,
options: { withFileTypes?: false; encoding?: string },
options: readDirOptions,
callback: readDirCallback,
): void;
export function readdir(
path: string | Buffer | URL,
options: { withFileTypes: true; encoding?: string },
options: readDirOptions,
callback: readDirCallbackDirent,
): void;
export function readdir(path: string | URL, callback: readDirCallback): void;
Expand All @@ -51,8 +53,7 @@ export function readdir(
const options = typeof optionsOrCallback === "object"
? optionsOrCallback
: null;
const result: Array<string | Dirent> = [];
path = getValidatedPath(path);
path = getValidatedPath(path).toString();

if (!callback) throw new Error("No callback function supplied");

Expand All @@ -66,24 +67,44 @@ export function readdir(
}
}

try {
path = path.toString();
asyncIterableToCallback(Deno.readDir(path), (val, done) => {
if (typeof path !== "string") return;
if (done) {
callback(null, result);
const result: Array<string | Dirent> = [];
const dirs = [path];
let current: string | undefined;
(async () => {
while ((current = dirs.shift()) !== undefined) {
try {
const entries = await op_fs_read_dir_async(current);

for (let i = 0; i < entries.length; i++) {
const entry = entries[i];
if (options?.recursive && entry.isDirectory) {
dirs.push(join(current, entry.name));
}

if (options?.withFileTypes) {
entry.parentPath = current;
result.push(toDirent(entry));
} else {
let name = decode(entry.name, options?.encoding);
if (options?.recursive) {
name = relative(path, join(current, name));
}
result.push(name);
}
}
} catch (err) {
callback(
denoErrorToNodeError(err as Error, {
syscall: "readdir",
path: current,
}),
);
return;
}
if (options?.withFileTypes) {
val.parentPath = path;
result.push(toDirent(val));
} else result.push(decode(val.name));
}, (e) => {
callback(denoErrorToNodeError(e as Error, { syscall: "readdir" }));
});
} catch (e) {
callback(denoErrorToNodeError(e as Error, { syscall: "readdir" }));
}
}

callback(null, result);
})();
}

function decode(str: string, encoding?: string): string {
Expand Down Expand Up @@ -118,8 +139,7 @@ export function readdirSync(
path: string | Buffer | URL,
options?: readDirOptions,
): Array<string | Dirent> {
const result = [];
path = getValidatedPath(path);
path = getValidatedPath(path).toString();

if (options?.encoding) {
try {
Expand All @@ -131,16 +151,37 @@ export function readdirSync(
}
}

try {
path = path.toString();
for (const file of Deno.readDirSync(path)) {
if (options?.withFileTypes) {
file.parentPath = path;
result.push(toDirent(file));
} else result.push(decode(file.name));
const result: Array<string | Dirent> = [];
const dirs = [path];
let current: string | undefined;
while ((current = dirs.shift()) !== undefined) {
try {
const entries = op_fs_read_dir_sync(current);

for (let i = 0; i < entries.length; i++) {
const entry = entries[i];
if (options?.recursive && entry.isDirectory) {
dirs.push(join(current, entry.name));
}

if (options?.withFileTypes) {
entry.parentPath = current;
result.push(toDirent(entry));
} else {
let name = decode(entry.name, options?.encoding);
if (options?.recursive) {
name = relative(path, join(current, name));
}
result.push(name);
}
}
} catch (e) {
throw denoErrorToNodeError(e as Error, {
syscall: "readdir",
path: current,
});
}
} catch (e) {
throw denoErrorToNodeError(e as Error, { syscall: "readdir" });
}

return result;
}
43 changes: 43 additions & 0 deletions tests/unit_node/_fs/_fs_readdir_test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,29 @@ Deno.test({
},
});

Deno.test("ASYNC: read dirs recursively", async () => {
const dir = Deno.makeTempDirSync();
Deno.writeTextFileSync(join(dir, "file1.txt"), "hi");
Deno.mkdirSync(join(dir, "sub"));
Deno.writeTextFileSync(join(dir, "sub", "file2.txt"), "hi");

try {
const files = await new Promise<string[]>((resolve, reject) => {
readdir(dir, { recursive: true }, (err, files) => {
if (err) reject(err);
resolve(files.map((f) => f.toString()));
});
});

assertEqualsArrayAnyOrder(
files,
["file1.txt", "sub", join("sub", "file2.txt")],
);
} finally {
Deno.removeSync(dir, { recursive: true });
}
});

Deno.test({
name: "SYNC: reading empty the directory",
fn() {
Expand All @@ -75,6 +98,26 @@ Deno.test({
},
});

Deno.test("SYNC: read dirs recursively", () => {
const dir = Deno.makeTempDirSync();
Deno.writeTextFileSync(join(dir, "file1.txt"), "hi");
Deno.mkdirSync(join(dir, "sub"));
Deno.writeTextFileSync(join(dir, "sub", "file2.txt"), "hi");

try {
const files = readdirSync(dir, { recursive: true }).map((f) =>
f.toString()
);

assertEqualsArrayAnyOrder(
files,
["file1.txt", "sub", join("sub", "file2.txt")],
);
} finally {
Deno.removeSync(dir, { recursive: true });
}
});

Deno.test("[std/node/fs] readdir callback isn't called twice if error is thrown", async () => {
// The correct behaviour is not to catch any errors thrown,
// but that means there'll be an uncaught error and the test will fail.
Expand Down