From 4717af9edd8058d6779f575a1830ed6562be7348 Mon Sep 17 00:00:00 2001 From: Don Isaac Date: Thu, 23 Jan 2025 14:37:44 -0800 Subject: [PATCH] pr feedback --- src/js/builtins.d.ts | 1 + src/js/internal/fs/glob.ts | 75 +++++++++++++++++++ src/js/internal/validators.ts | 2 +- src/js/node/fs.promises.ts | 7 +- src/js/node/fs.ts | 25 +++++++ test/harness.ts | 17 +++++ test/js/node/fs/glob.test.ts | 132 ++++++++++++++++++++++++++++++++++ 7 files changed, 255 insertions(+), 4 deletions(-) create mode 100644 src/js/internal/fs/glob.ts create mode 100644 test/js/node/fs/glob.test.ts diff --git a/src/js/builtins.d.ts b/src/js/builtins.d.ts index 5405daea909a72..42c9ef427ab624 100644 --- a/src/js/builtins.d.ts +++ b/src/js/builtins.d.ts @@ -363,6 +363,7 @@ declare function $isAbortSignal(signal: unknown): signal is AbortSignal; declare function $isAbsolute(): TODO; declare function $isDisturbed(): TODO; declare function $isPaused(): TODO; +/** FIXME: does not exist. */ declare function $isWindows(): TODO; declare function $join(): TODO; declare function $kind(): TODO; diff --git a/src/js/internal/fs/glob.ts b/src/js/internal/fs/glob.ts new file mode 100644 index 00000000000000..294313fcbc3046 --- /dev/null +++ b/src/js/internal/fs/glob.ts @@ -0,0 +1,75 @@ +import type { GlobScanOptions } from "bun"; +const { validateObject, validateString, validateFunction } = require("internal/validators"); + +const isWindows = process.platform === "win32"; + +interface GlobOptions { + /** @default process.cwd() */ + cwd?: string; + exclude?: (ent: string) => boolean; + /** + * Should glob return paths as {@link Dirent} objects. `false` for strings. + * @default false */ + withFileTypes?: boolean; +} + +interface ExtendedGlobOptions extends GlobScanOptions { + exclude(ent: string): boolean; +} + +async function* glob(pattern: string | string[], options: GlobOptions): AsyncGenerator { + pattern = validatePattern(pattern); + const globOptions = mapOptions(options); + let it = new Bun.Glob(pattern).scan(globOptions); + const exclude = globOptions.exclude; + + for await (const ent of it) { + if (exclude(ent)) continue; + yield ent; + } +} + +function* globSync(pattern: string | string[], options: GlobOptions): Generator { + pattern = validatePattern(pattern); + const globOptions = mapOptions(options); + const g = new Bun.Glob(pattern); + const exclude = globOptions.exclude; + for (const ent of g.scanSync(globOptions)) { + if (exclude(ent)) continue; + yield ent; + } +} + +function validatePattern(pattern: string | string[]): string { + if ($isArray(pattern)) { + throw new TypeError("fs.glob does not support arrays of patterns yet. Please open an issue on GitHub."); + } + validateString(pattern, "pattern"); + return isWindows ? pattern.replaceAll("/", "\\") : pattern; +} + +function mapOptions(options: GlobOptions): ExtendedGlobOptions { + validateObject(options, "options"); + + const exclude = options.exclude ?? no; + validateFunction(exclude, "options.exclude"); + + if (options.withFileTypes) { + throw new TypeError("fs.glob does not support options.withFileTypes yet. Please open an issue on GitHub."); + } + + return { + // NOTE: this is subtly different from Glob's default behavior. + // `process.cwd()` may be overridden by JS code, but native code will used the + // cached `getcwd` on BunProcess. + cwd: options?.cwd ?? process.cwd(), + // https://github.com/nodejs/node/blob/a9546024975d0bfb0a8ae47da323b10fb5cbb88b/lib/internal/fs/glob.js#L655 + followSymlinks: true, + exclude, + }; +} + +// `var` avoids TDZ checks. +var no = _ => false; + +export default { glob, globSync }; diff --git a/src/js/internal/validators.ts b/src/js/internal/validators.ts index 2df37ba6eaae72..fdf61f7a40c115 100644 --- a/src/js/internal/validators.ts +++ b/src/js/internal/validators.ts @@ -68,7 +68,7 @@ function validateLinkHeaderValue(hints) { } hideFromStack(validateLinkHeaderValue); // TODO: do it in NodeValidator.cpp -function validateObject(value, name) { +function validateObject(value: unknown, name: string): asserts value is object { if (typeof value !== "object" || value === null) throw $ERR_INVALID_ARG_TYPE(name, "object", value); } hideFromStack(validateObject); diff --git a/src/js/node/fs.promises.ts b/src/js/node/fs.promises.ts index 4ea642a6acb8e0..e3b4ab770ac1af 100644 --- a/src/js/node/fs.promises.ts +++ b/src/js/node/fs.promises.ts @@ -1,8 +1,8 @@ // Hardcoded module "node:fs/promises" -import type { Dirent } from "fs"; const types = require("node:util/types"); const EventEmitter = require("node:events"); const fs = $zig("node_fs_binding.zig", "createBinding"); +const { glob } = require("internal/fs/glob"); const constants = $processBindingConstants.fs; var PromisePrototypeFinally = Promise.prototype.finally; //TODO @@ -22,7 +22,7 @@ const kDeserialize = Symbol("kDeserialize"); const kEmptyObject = ObjectFreeze({ __proto__: null }); const kFlag = Symbol("kFlag"); -const { validateObject, validateInteger } = require("internal/validators"); +const { validateInteger } = require("internal/validators"); function watch( filename: string | Buffer | URL, @@ -112,7 +112,7 @@ function cp(src, dest, options) { } async function opendir(dir: string, options) { - return new (require('node:fs').Dir)(1, dir, options); + return new (require("node:fs").Dir)(1, dir, options); } const private_symbols = { @@ -152,6 +152,7 @@ const exports = { fdatasync: asyncWrap(fs.fdatasync, "fdatasync"), ftruncate: asyncWrap(fs.ftruncate, "ftruncate"), futimes: asyncWrap(fs.futimes, "futimes"), + glob, lchmod: asyncWrap(fs.lchmod, "lchmod"), lchown: asyncWrap(fs.lchown, "lchown"), link: asyncWrap(fs.link, "link"), diff --git a/src/js/node/fs.ts b/src/js/node/fs.ts index f3fd95a9cb6e49..e2ddab09f3e433 100644 --- a/src/js/node/fs.ts +++ b/src/js/node/fs.ts @@ -3,6 +3,9 @@ import type { Stats as StatsType } from "fs"; const EventEmitter = require("node:events"); const promises = require("node:fs/promises"); const types = require("node:util/types"); +const { validateFunction } = require("internal/validators"); + +const kEmptyObject = Object.freeze(Object.create(null)); const isDate = types.isDate; @@ -11,6 +14,10 @@ const isDate = types.isDate; const { fs } = promises.$data; const constants = $processBindingConstants.fs; +var _lazyGlob; +function lazyGlob() { + return (_lazyGlob ??= require("internal/fs/glob")); +} function ensureCallback(callback) { if (!$isCallable(callback)) { @@ -1076,6 +1083,22 @@ class Dir { } } +function glob(pattern: string | string[], options, callback) { + if (typeof options === "function") { + callback = options; + options = undefined; + } + validateFunction(callback, "callback"); + + Array.fromAsync(lazyGlob().glob(pattern, options ?? kEmptyObject)) + .then(result => callback(null, result)) + .catch(callback); +} + +function globSync(pattern: string | string[], options): string[] { + return Array.from(lazyGlob().globSync(pattern, options ?? kEmptyObject)); +} + var exports = { appendFile, appendFileSync, @@ -1109,6 +1132,8 @@ var exports = { ftruncateSync, futimes, futimesSync, + glob, + globSync, lchown, lchownSync, lchmod, diff --git a/test/harness.ts b/test/harness.ts index dcb84f3e8886cf..a736ee149de629 100644 --- a/test/harness.ts +++ b/test/harness.ts @@ -186,6 +186,23 @@ export async function makeTree(base: string, tree: DirectoryTree) { } } +/** + * Recursively create files within a new temporary directory. + * + * @param basename prefix of the new temporary directory + * @param files directory tree. Each key is a folder or file, and each value is the contents of the file. Use objects for directories. + * @returns an absolute path to the new temporary directory + * + * @example + * ```ts + * const dir = tempDirWithFiles("my-test", { + * "index.js": `import foo from "./src/foo";`, + * "src": { + * "foo.js": `export default "foo";`, + * }, + * }); + * ``` + */ export function tempDirWithFiles(basename: string, files: DirectoryTree): string { const base = fs.mkdtempSync(join(fs.realpathSync(os.tmpdir()), basename + "_")); makeTree(base, files); diff --git a/test/js/node/fs/glob.test.ts b/test/js/node/fs/glob.test.ts new file mode 100644 index 00000000000000..93cd38063fe765 --- /dev/null +++ b/test/js/node/fs/glob.test.ts @@ -0,0 +1,132 @@ +/** + * @note `fs.glob` et. al. are powered by {@link Bun.Glob}, which is extensively + * tested elsewhere. These tests check API compatibility with Node.js. + */ +import fs from "node:fs"; +import { describe, beforeAll, afterAll, it, expect } from "bun:test"; +import { isWindows, tempDirWithFiles } from "harness"; + +let tmp: string; +beforeAll(() => { + tmp = tempDirWithFiles("fs-glob", { + "foo.txt": "foo", + a: { + "bar.txt": "bar", + "baz.js": "baz", + }, + }); +}); + +afterAll(() => { + return fs.promises.rmdir(tmp, { recursive: true }); +}); + +describe("fs.glob", () => { + it("has a length of 3", () => { + expect(fs).toHaveProperty("glob"); + expect(typeof fs.glob).toEqual("function"); + expect(fs.glob).toHaveLength(3); + }); + + it("is named 'glob'", () => { + expect(fs.glob.name).toEqual("glob"); + }); + + it("when successful, passes paths to the callback", done => { + fs.glob("*.txt", { cwd: tmp }, (err, paths) => { + expect(err).toBeNull(); + expect(paths.sort()).toStrictEqual(["foo.txt"]); + done(); + }); + }); + + it("can filter out files", done => { + const exclude = (path: string) => path.endsWith(".js"); + fs.glob("a/*", { cwd: tmp, exclude }, (err, paths) => { + if (err) done(err); + if (isWindows) { + expect(paths).toStrictEqual(["a\\bar.txt"]); + } else { + expect(paths).toStrictEqual(["a/bar.txt"]); + } + done(); + }); + }); + + describe("invalid arguments", () => { + it("throws if no callback is provided", () => { + expect(() => fs.glob("*.txt")).toThrow(TypeError); + expect(() => fs.glob("*.txt", undefined)).toThrow(TypeError); + expect(() => fs.glob("*.txt", { cwd: tmp })).toThrow(TypeError); + expect(() => fs.glob("*.txt", { cwd: tmp }, undefined)).toThrow(TypeError); + }); + }); +}); // + +describe("fs.globSync", () => { + it("has a length of 2", () => { + expect(fs).toHaveProperty("globSync"); + expect(typeof fs.globSync).toBe("function"); + expect(fs.globSync).toHaveLength(2); + }); + + it("is named 'globSync'", () => { + expect(fs.globSync.name).toEqual("globSync"); + }); + + it.each([ + ["*.txt", ["foo.txt"]], + ["a/**", isWindows ? ["a\\bar.txt", "a\\baz.js"] : ["a/bar.txt", "a/baz.js"]], + ])("fs.glob(%p, { cwd: /tmp/fs-glob }) === %p", (pattern, expected) => { + expect(fs.globSync(pattern, { cwd: tmp }).sort()).toStrictEqual(expected); + }); + + describe("when process.cwd() is set", () => { + let oldProcessCwd: () => string; + beforeAll(() => { + oldProcessCwd = process.cwd; + process.cwd = () => tmp; + }); + afterAll(() => { + process.cwd = oldProcessCwd; + }); + + it("respects the new cwd", () => { + expect(fs.globSync("*.txt")).toStrictEqual(["foo.txt"]); + }); + }); + + it("can filter out files", () => { + const exclude = (path: string) => path.endsWith(".js"); + const expected = isWindows ? ["a\\bar.txt"] : ["a/bar.txt"]; + expect(fs.globSync("a/*", { cwd: tmp, exclude })).toStrictEqual(expected); + }); + + describe("invalid arguments", () => { + // TODO: GlobSet + it("does not support arrays of patterns yet", () => { + expect(() => fs.globSync(["*.txt"])).toThrow(TypeError); + }); + }); +}); // + +describe("fs.promises.glob", () => { + it("has a length of 2", () => { + expect(fs.promises).toHaveProperty("glob"); + expect(typeof fs.promises.glob).toBe("function"); + expect(fs.promises.glob).toHaveLength(2); + }); + + it("is named 'glob'", () => { + expect(fs.promises.glob.name).toEqual("glob"); + }); + + it("returns an AsyncIterable over matched paths", async () => { + const iter = fs.promises.glob("*.txt", { cwd: tmp }); + // FIXME: .toHaveProperty does not support symbol keys + expect(iter[Symbol.asyncIterator]).toBeDefined(); + for await (const path of iter) { + expect(path).toMatch(/\.txt$/); + } + }); +}); //