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.

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

fs: allow 'withFileTypes' to be used with globs #52837

Merged
merged 8 commits into from
May 7, 2024
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
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
18 changes: 18 additions & 0 deletions doc/api/fs.md
Original file line number Diff line number Diff line change
Expand Up @@ -1073,6 +1073,10 @@ behavior is similar to `cp dir1/ dir2/`.

<!-- YAML
added: v22.0.0
changes:
- version: REPLACEME
pr-url: https://github.com/nodejs/node/pull/52837
description: Add support 'withFileTypes' as an option.
avivkeller marked this conversation as resolved.
Show resolved Hide resolved
-->

> Stability: 1 - Experimental
Expand All @@ -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.

Expand Down Expand Up @@ -3109,6 +3115,10 @@ descriptor. See [`fs.utimes()`][].

<!-- YAML
added: v22.0.0
changes:
- version: REPLACEME
pr-url: https://github.com/nodejs/node/pull/52837
description: Add support for 'withFileTypes' as an option.
avivkeller marked this conversation as resolved.
Show resolved Hide resolved
-->

> Stability: 1 - Experimental
Expand All @@ -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}
Expand Down Expand Up @@ -5603,6 +5615,10 @@ Synchronous version of [`fs.futimes()`][]. Returns `undefined`.

<!-- YAML
added: v22.0.0
changes:
- version: REPLACEME
pr-url: https://github.com/nodejs/node/pull/52837
description: Add support for 'withFileTypes' as an option.
avivkeller marked this conversation as resolved.
Show resolved Hide resolved
-->

> Stability: 1 - Experimental
Expand All @@ -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
Expand Down
46 changes: 32 additions & 14 deletions lib/internal/fs/glob.js
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -27,6 +27,7 @@ const {
validateString,
validateStringArray,
} = require('internal/validators');
const { DirentFromStats } = require('internal/fs/utils');

let minimatch;
function lazyMinimatch() {
Expand All @@ -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();
Expand All @@ -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;
}
Expand All @@ -58,7 +67,7 @@ class Cache {
}
let val;
try {
val = lstatSync(path);
val = getDirentSync(path);
} catch {
val = null;
}
Expand Down Expand Up @@ -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');
Expand Down Expand Up @@ -222,7 +233,14 @@ class Glob {
.forEach((patterns, path) => ArrayPrototypePush(this.#queue, { __proto__: null, path, patterns }));
this.#subpatterns.clear();
}
return ArrayFrom(this.#results);
return ArrayFrom(
this.#results,
this.#withFileTypes ? (path) => this.#cache.statSync(
isAbsolute(path) ?
path :
join(this.#root, path),
) : undefined,
);
}
#addSubpattern(path, pattern) {
if (!this.#subpatterns.has(path)) {
Expand Down Expand Up @@ -317,7 +335,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()) {
Expand Down Expand Up @@ -460,7 +478,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)) {
Expand All @@ -472,7 +490,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;
}
}

Expand Down Expand Up @@ -522,7 +540,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;
}
}

Expand All @@ -533,7 +551,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
Expand Down Expand Up @@ -569,14 +587,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;
}
}
}
Expand All @@ -592,7 +610,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);
Expand All @@ -605,7 +623,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);
Expand Down
1 change: 1 addition & 0 deletions lib/internal/fs/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -961,6 +961,7 @@ module.exports = {
BigIntStats, // for testing
copyObject,
Dirent,
DirentFromStats,
emitRecursiveRmdirWarning,
getDirent,
getDirents,
Expand Down
43 changes: 41 additions & 2 deletions test/parallel/test-fs-glob.mjs
Original file line number Diff line number Diff line change
@@ -1,12 +1,16 @@
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) {
assert.ok(dirents.every((dirent) => dirent instanceof Dirent));
}

tmpdir.refresh();

const fixtureDir = tmpdir.resolve('fixtures');
Expand Down Expand Up @@ -333,3 +337,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());
});
}
});
Loading