From 1515357d7f6955d8ea2869cdc1872eb085b20a26 Mon Sep 17 00:00:00 2001 From: RedYetiDev <38299977+RedYetiDev@users.noreply.github.com> Date: Sat, 4 May 2024 21:02:16 -0400 Subject: [PATCH 1/8] fs: allow 'withFileTypes' to be used with globs --- doc/api/fs.md | 18 +++++++++++++ lib/internal/fs/glob.js | 47 ++++++++++++++++++++++++---------- lib/internal/fs/utils.js | 1 + test/parallel/test-fs-glob.mjs | 45 ++++++++++++++++++++++++++++++-- 4 files changed, 95 insertions(+), 16 deletions(-) diff --git a/doc/api/fs.md b/doc/api/fs.md index d6afc9d5ea9403..7982be0c93ca1b 100644 --- a/doc/api/fs.md +++ b/doc/api/fs.md @@ -1073,6 +1073,10 @@ behavior is similar to `cp dir1/ dir2/`. > Stability: 1 - Experimental @@ -1082,6 +1086,8 @@ added: v22.0.0 * `cwd` {string} current working directory. **Default:** `process.cwd()` * `exclude` {Function} Function to filter out files/directories. Return `true` to exclude the item, `false` to include it. **Default:** `undefined`. + * `withFileTypes` {boolean} `true` if the glob should return paths as Dirents, + `false` otherwise. **Default:** `false`. * Returns: {AsyncIterator} An AsyncIterator that yields the paths of files that match the pattern. @@ -3109,6 +3115,10 @@ descriptor. See [`fs.utimes()`][]. > Stability: 1 - Experimental @@ -3119,6 +3129,8 @@ added: v22.0.0 * `cwd` {string} current working directory. **Default:** `process.cwd()` * `exclude` {Function} Function to filter out files/directories. Return `true` to exclude the item, `false` to include it. **Default:** `undefined`. + * `withFileTypes` {boolean} `true` if the glob should return paths as Dirents, + `false` otherwise. **Default:** `false`. * `callback` {Function} * `err` {Error} @@ -5603,6 +5615,10 @@ Synchronous version of [`fs.futimes()`][]. Returns `undefined`. > Stability: 1 - Experimental @@ -5612,6 +5628,8 @@ added: v22.0.0 * `cwd` {string} current working directory. **Default:** `process.cwd()` * `exclude` {Function} Function to filter out files/directories. Return `true` to exclude the item, `false` to include it. **Default:** `undefined`. + * `withFileTypes` {boolean} `true` if the glob should return paths as Dirents, + `false` otherwise. **Default:** `false`. * Returns: {string\[]} paths of files that match the pattern. ```mjs diff --git a/lib/internal/fs/glob.js b/lib/internal/fs/glob.js index cdf8c4dfbc296b..c63d9e81a17ffd 100644 --- a/lib/internal/fs/glob.js +++ b/lib/internal/fs/glob.js @@ -16,7 +16,7 @@ const { const { lstatSync, readdirSync } = require('fs'); const { lstat, readdir } = require('fs/promises'); -const { join, resolve } = require('path'); +const { join, resolve, basename, isAbsolute } = require('path'); const { kEmptyObject, @@ -27,6 +27,7 @@ const { validateString, validateStringArray, } = require('internal/validators'); +const { DirentFromStats } = require('internal/fs/utils'); let minimatch; function lazyMinimatch() { @@ -37,6 +38,14 @@ function lazyMinimatch() { const isWindows = process.platform === 'win32'; const isOSX = process.platform === 'darwin'; +async function getDirent(path) { + return new DirentFromStats(basename(path), await lstat(path), path); +} + +function getDirentSync(path) { + return new DirentFromStats(basename(path), lstatSync(path), path); +} + class Cache { #cache = new SafeMap(); #statsCache = new SafeMap(); @@ -47,7 +56,7 @@ class Cache { if (cached) { return cached; } - const promise = PromisePrototypeThen(lstat(path), null, () => null); + const promise = PromisePrototypeThen(getDirent(path), null, () => null); this.#statsCache.set(path, promise); return promise; } @@ -58,7 +67,7 @@ class Cache { } let val; try { - val = lstatSync(path); + val = getDirentSync(path); } catch { val = null; } @@ -175,14 +184,16 @@ class Glob { #queue = []; #subpatterns = new SafeMap(); #patterns; + #withFileTypes; constructor(pattern, options = kEmptyObject) { validateObject(options, 'options'); - const { exclude, cwd } = options; + const { exclude, cwd, withFileTypes } = options; if (exclude != null) { validateFunction(exclude, 'options.exclude'); } this.#root = cwd ?? '.'; this.#exclude = exclude; + this.#withFileTypes = !!withFileTypes; let patterns; if (typeof pattern === 'object') { validateStringArray(pattern, 'patterns'); @@ -222,7 +233,15 @@ class Glob { .forEach((patterns, path) => ArrayPrototypePush(this.#queue, { __proto__: null, path, patterns })); this.#subpatterns.clear(); } - return ArrayFrom(this.#results); + return this.#withFileTypes ? + ArrayPrototypeMap( + ArrayFrom(this.#results), + (path) => this.#cache.statSync( + isAbsolute(path) ? + path : + join(this.#root, path), + ), + ) : ArrayFrom(this.#results); } #addSubpattern(path, pattern) { if (!this.#subpatterns.has(path)) { @@ -317,7 +336,7 @@ class Glob { const fromSymlink = pattern.symlinks.has(index); if (current === lazyMinimatch().GLOBSTAR) { - if (entry.name[0] === '.' || (this.#exclude && this.#exclude(entry.name))) { + if (entry.name[0] === '.' || (this.#exclude && this.#exclude(this.#withFileTypes ? entry : entry.name))) { continue; } if (!fromSymlink && entry.isDirectory()) { @@ -460,7 +479,7 @@ class Glob { const result = join(path, p); if (!this.#results.has(result)) { this.#results.add(result); - yield result; + yield this.#withFileTypes ? stat : result; } } if (pattern.indexes.size === 1 && pattern.indexes.has(last)) { @@ -472,7 +491,7 @@ class Glob { // if path is ".", add it only if pattern starts with "." or pattern is exactly "**" if (!this.#results.has(path)) { this.#results.add(path); - yield path; + yield this.#withFileTypes ? stat : path; } } @@ -522,7 +541,7 @@ class Glob { // If ** is last, add to results if (!this.#results.has(entryPath)) { this.#results.add(entryPath); - yield entryPath; + yield this.#withFileTypes ? entry : entryPath; } } @@ -533,7 +552,7 @@ class Glob { // If next pattern is the last one, add to results if (!this.#results.has(entryPath)) { this.#results.add(entryPath); - yield entryPath; + yield this.#withFileTypes ? entry : entryPath; } } else if (nextMatches && entry.isDirectory()) { // Pattern mached, meaning two patterns forward @@ -569,14 +588,14 @@ class Glob { this.#cache.add(path, pattern.child(new SafeSet().add(nextIndex))); if (!this.#results.has(path)) { this.#results.add(path); - yield path; + yield this.#withFileTypes ? this.#cache.statSync(fullpath) : path; } } if (!this.#cache.seen(path, pattern, nextIndex) || !this.#cache.seen(parent, pattern, nextIndex)) { this.#cache.add(parent, pattern.child(new SafeSet().add(nextIndex))); if (!this.#results.has(parent)) { this.#results.add(parent); - yield parent; + yield this.#withFileTypes ? this.#cache.statSync(join(this.#root, parent)) : parent; } } } @@ -592,7 +611,7 @@ class Glob { if (nextIndex === last) { if (!this.#results.has(entryPath)) { this.#results.add(entryPath); - yield entryPath; + yield this.#withFileTypes ? entry : entryPath; } } else { subPatterns.add(nextIndex + 1); @@ -605,7 +624,7 @@ class Glob { if (index === last) { if (!this.#results.has(entryPath)) { this.#results.add(entryPath); - yield entryPath; + yield this.#withFileTypes ? entry : entryPath; } } else if (entry.isDirectory()) { subPatterns.add(nextIndex); diff --git a/lib/internal/fs/utils.js b/lib/internal/fs/utils.js index 18c6f8d419d47a..1ff6d8a3dd5158 100644 --- a/lib/internal/fs/utils.js +++ b/lib/internal/fs/utils.js @@ -961,6 +961,7 @@ module.exports = { BigIntStats, // for testing copyObject, Dirent, + DirentFromStats, emitRecursiveRmdirWarning, getDirent, getDirents, diff --git a/test/parallel/test-fs-glob.mjs b/test/parallel/test-fs-glob.mjs index c01edf0431849d..cc7a66f6176bde 100644 --- a/test/parallel/test-fs-glob.mjs +++ b/test/parallel/test-fs-glob.mjs @@ -1,12 +1,18 @@ import * as common from '../common/index.mjs'; import tmpdir from '../common/tmpdir.js'; -import { resolve, dirname, sep } from 'node:path'; +import { resolve, dirname, sep, basename } from 'node:path'; import { mkdir, writeFile, symlink, glob as asyncGlob } from 'node:fs/promises'; -import { glob, globSync } from 'node:fs'; +import { glob, globSync, Dirent } from 'node:fs'; import { test, describe } from 'node:test'; import { promisify } from 'node:util'; import assert from 'node:assert'; +function assertDirents(dirents) { + dirents.forEach((dirent) => { + assert(dirent instanceof Dirent); + }); +} + tmpdir.refresh(); const fixtureDir = tmpdir.resolve('fixtures'); @@ -333,3 +339,38 @@ describe('fsPromises glob', function() { }); } }); + +describe('glob - withFileTypes', function() { + const promisified = promisify(glob); + for (const [pattern, expected] of Object.entries(patterns)) { + test(pattern, async () => { + const actual = await promisified(pattern, { cwd: fixtureDir, withFileTypes: true }); + assertDirents(actual); + const normalized = expected.filter(Boolean).map((item) => basename(item)).sort(); + assert.deepStrictEqual(actual.map((dirent) => dirent.name).sort(), normalized.sort()); + }); + } +}); + +describe('globSync - withFileTypes', function() { + for (const [pattern, expected] of Object.entries(patterns)) { + test(pattern, () => { + const actual = globSync(pattern, { cwd: fixtureDir, withFileTypes: true }); + assertDirents(actual); + const normalized = expected.filter(Boolean).map((item) => basename(item)).sort(); + assert.deepStrictEqual(actual.map((dirent) => dirent.name).sort(), normalized.sort()); + }); + } +}); + +describe('fsPromises glob - withFileTypes', function() { + for (const [pattern, expected] of Object.entries(patterns)) { + test(pattern, async () => { + const actual = []; + for await (const item of asyncGlob(pattern, { cwd: fixtureDir, withFileTypes: true })) actual.push(item); + assertDirents(actual); + const normalized = expected.filter(Boolean).map((item) => basename(item)).sort(); + assert.deepStrictEqual(actual.map((dirent) => dirent.name).sort(), normalized.sort()); + }); + } +}); From 2dc41bf3908357fe780c5b89ba8a113c3d9752a6 Mon Sep 17 00:00:00 2001 From: Aviv Keller <38299977+RedYetiDev@users.noreply.github.com> Date: Sat, 4 May 2024 21:04:49 -0400 Subject: [PATCH 2/8] doc: add PR URL --- doc/api/fs.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/doc/api/fs.md b/doc/api/fs.md index 7982be0c93ca1b..6dab74659525c4 100644 --- a/doc/api/fs.md +++ b/doc/api/fs.md @@ -1075,7 +1075,7 @@ behavior is similar to `cp dir1/ dir2/`. added: v22.0.0 changes: - version: REPLACEME - pr-url: TODO + pr-url: https://github.com/nodejs/node/pull/52837 description: Add support 'withFileTypes' as an option. --> @@ -3117,7 +3117,7 @@ descriptor. See [`fs.utimes()`][]. added: v22.0.0 changes: - version: REPLACEME - pr-url: TODO + pr-url: https://github.com/nodejs/node/pull/52837 description: Add support for 'withFileTypes' as an option. --> @@ -5617,7 +5617,7 @@ Synchronous version of [`fs.futimes()`][]. Returns `undefined`. added: v22.0.0 changes: - version: REPLACEME - pr-url: TODO + pr-url: https://github.com/nodejs/node/pull/52837 description: Add support for 'withFileTypes' as an option. --> From b70ecc490381b81b89a062607a8bba7806beb35f Mon Sep 17 00:00:00 2001 From: Aviv Keller <38299977+RedYetiDev@users.noreply.github.com> Date: Sun, 5 May 2024 11:39:28 -0400 Subject: [PATCH 3/8] Update glob.js Co-authored-by: Antoine du Hamel --- lib/internal/fs/glob.js | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/lib/internal/fs/glob.js b/lib/internal/fs/glob.js index c63d9e81a17ffd..9d096fd71bc6f2 100644 --- a/lib/internal/fs/glob.js +++ b/lib/internal/fs/glob.js @@ -233,15 +233,14 @@ class Glob { .forEach((patterns, path) => ArrayPrototypePush(this.#queue, { __proto__: null, path, patterns })); this.#subpatterns.clear(); } - return this.#withFileTypes ? - ArrayPrototypeMap( - ArrayFrom(this.#results), - (path) => this.#cache.statSync( + return ArrayFrom( + this.#results, + this.#withFileTypes ? (path) => this.#cache.statSync( isAbsolute(path) ? path : join(this.#root, path), - ), - ) : ArrayFrom(this.#results); + ) : undefined, + ); } #addSubpattern(path, pattern) { if (!this.#subpatterns.has(path)) { From 0bd1e1b2ee1f00b8d6b98b200009bc075de8c5fc Mon Sep 17 00:00:00 2001 From: Aviv Keller <38299977+RedYetiDev@users.noreply.github.com> Date: Sun, 5 May 2024 11:39:45 -0400 Subject: [PATCH 4/8] Update test-fs-glob.mjs Co-authored-by: Antoine du Hamel --- test/parallel/test-fs-glob.mjs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/test/parallel/test-fs-glob.mjs b/test/parallel/test-fs-glob.mjs index cc7a66f6176bde..7dcb8ecc8373a3 100644 --- a/test/parallel/test-fs-glob.mjs +++ b/test/parallel/test-fs-glob.mjs @@ -8,9 +8,7 @@ import { promisify } from 'node:util'; import assert from 'node:assert'; function assertDirents(dirents) { - dirents.forEach((dirent) => { - assert(dirent instanceof Dirent); - }); + assert.ok(dirents.every((dirent) => dirent instanceof Dirent)); } tmpdir.refresh(); From 9d377e15185b854df37506813fba379f47dfa35b Mon Sep 17 00:00:00 2001 From: RedYetiDev <38299977+RedYetiDev@users.noreply.github.com> Date: Sun, 5 May 2024 13:09:39 -0400 Subject: [PATCH 5/8] lint --- lib/internal/fs/glob.js | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/lib/internal/fs/glob.js b/lib/internal/fs/glob.js index 9d096fd71bc6f2..4dd46ed0b6cb67 100644 --- a/lib/internal/fs/glob.js +++ b/lib/internal/fs/glob.js @@ -234,13 +234,13 @@ class Glob { this.#subpatterns.clear(); } return ArrayFrom( - this.#results, - this.#withFileTypes ? (path) => this.#cache.statSync( - isAbsolute(path) ? - path : - join(this.#root, path), - ) : undefined, - ); + this.#results, + this.#withFileTypes ? (path) => this.#cache.statSync( + isAbsolute(path) ? + path : + join(this.#root, path), + ) : undefined, + ); } #addSubpattern(path, pattern) { if (!this.#subpatterns.has(path)) { From 75041e2523658e6036b491ab847db8a9691bde07 Mon Sep 17 00:00:00 2001 From: Aviv Keller <38299977+RedYetiDev@users.noreply.github.com> Date: Sun, 5 May 2024 14:26:45 -0400 Subject: [PATCH 6/8] Update doc/api/fs.md MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Michaël Zasso --- doc/api/fs.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/api/fs.md b/doc/api/fs.md index 6dab74659525c4..f4e2052cd3c18f 100644 --- a/doc/api/fs.md +++ b/doc/api/fs.md @@ -1076,7 +1076,7 @@ added: v22.0.0 changes: - version: REPLACEME pr-url: https://github.com/nodejs/node/pull/52837 - description: Add support 'withFileTypes' as an option. + description: Add support for `withFileTypes` as an option. --> > Stability: 1 - Experimental From bd621ca1edf6edde4f54178bea7a3439f9655296 Mon Sep 17 00:00:00 2001 From: Aviv Keller <38299977+RedYetiDev@users.noreply.github.com> Date: Sun, 5 May 2024 14:26:51 -0400 Subject: [PATCH 7/8] Update doc/api/fs.md MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Michaël Zasso --- doc/api/fs.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/api/fs.md b/doc/api/fs.md index f4e2052cd3c18f..3ebc4e2e7b8013 100644 --- a/doc/api/fs.md +++ b/doc/api/fs.md @@ -3118,7 +3118,7 @@ added: v22.0.0 changes: - version: REPLACEME pr-url: https://github.com/nodejs/node/pull/52837 - description: Add support for 'withFileTypes' as an option. + description: Add support for `withFileTypes` as an option. --> > Stability: 1 - Experimental From 8d2e90532ace09f765973d20cd0b4222a2190be2 Mon Sep 17 00:00:00 2001 From: Aviv Keller <38299977+RedYetiDev@users.noreply.github.com> Date: Sun, 5 May 2024 14:26:57 -0400 Subject: [PATCH 8/8] Update doc/api/fs.md MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Michaël Zasso --- doc/api/fs.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/api/fs.md b/doc/api/fs.md index 3ebc4e2e7b8013..4b301f49614684 100644 --- a/doc/api/fs.md +++ b/doc/api/fs.md @@ -5618,7 +5618,7 @@ added: v22.0.0 changes: - version: REPLACEME pr-url: https://github.com/nodejs/node/pull/52837 - description: Add support for 'withFileTypes' as an option. + description: Add support for `withFileTypes` as an option. --> > Stability: 1 - Experimental