Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

Commit 5006627

Browse filesBrowse files
Trottaduh95
authored andcommitted
fs: apply exclude function to root path
In at least some situations, the root path was being added to the results of globbing even if it matched the optional exclude function. In the relevant test file, a variable was renamed for consistency. (There was a patterns and pattern2, rather than patterns2.) The test case is added to patterns2. Fixes: #56260 PR-URL: #57420 Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com> Reviewed-By: Jason Zhang <xzha4350@gmail.com> Reviewed-By: LiviaMedeiros <livia@cirno.name> Reviewed-By: James M Snell <jasnell@gmail.com>
1 parent 6ee15c6 commit 5006627
Copy full SHA for 5006627

File tree

Expand file treeCollapse file tree

2 files changed

+33
-5
lines changed
Open diff view settings
Filter options
Expand file treeCollapse file tree

2 files changed

+33
-5
lines changed
Open diff view settings
Collapse file

‎lib/internal/fs/glob.js‎

Copy file name to clipboardExpand all lines: lib/internal/fs/glob.js
+25-1Lines changed: 25 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ const {
99
ArrayPrototypePop,
1010
ArrayPrototypePush,
1111
ArrayPrototypeSome,
12+
Promise,
1213
PromisePrototypeThen,
1314
SafeMap,
1415
SafeSet,
@@ -125,7 +126,8 @@ class Cache {
125126
}
126127
statSync(path) {
127128
const cached = this.#statsCache.get(path);
128-
if (cached) {
129+
// Do not return a promise from a sync function.
130+
if (cached && !(cached instanceof Promise)) {
129131
return cached;
130132
}
131133
const val = getDirentSync(path);
@@ -326,6 +328,28 @@ class Glob {
326328
if (this.#isExcluded(path)) {
327329
return;
328330
}
331+
const fullpath = resolve(this.#root, path);
332+
333+
// If path is a directory, add trailing slash and test patterns again.
334+
// TODO(Trott): Would running #isExcluded() first and checking isDirectory() only
335+
// if it matches be more performant in the typical use case? #isExcluded()
336+
// is often ()=>false which is about as optimizable as a function gets.
337+
if (this.#cache.statSync(fullpath).isDirectory() && this.#isExcluded(`${fullpath}/`)) {
338+
return;
339+
}
340+
341+
if (this.#exclude) {
342+
if (this.#withFileTypes) {
343+
const stat = this.#cache.statSync(path);
344+
if (stat !== null) {
345+
if (this.#exclude(stat)) {
346+
return;
347+
}
348+
}
349+
} else if (this.#exclude(path)) {
350+
return;
351+
}
352+
}
329353
if (!this.#subpatterns.has(path)) {
330354
this.#subpatterns.set(path, [pattern]);
331355
} else {
Collapse file

‎test/parallel/test-fs-glob.mjs‎

Copy file name to clipboardExpand all lines: test/parallel/test-fs-glob.mjs
+8-4Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -388,7 +388,7 @@ describe('fsPromises glob - withFileTypes', function() {
388388
});
389389

390390
// [pattern, exclude option, expected result]
391-
const pattern2 = [
391+
const patterns2 = [
392392
['a/{b,c}*', ['a/*c'], ['a/b', 'a/cb']],
393393
['a/{a,b,c}*', ['a/*bc*', 'a/cb'], ['a/b', 'a/c']],
394394
['a/**/[cg]', ['**/c'], ['a/abcdef/g', 'a/abcfed/g']],
@@ -427,6 +427,10 @@ const pattern2 = [
427427
[`${absDir}/*{a,q}*`, './a/*{c,b}*/*'],
428428
[`${absDir}/foo`, 'a/c', ...(common.isWindows ? [] : ['a/symlink/a/b/c'])],
429429
],
430+
[ 'a/**', () => true, [] ],
431+
[ 'a/**', [ '*' ], [] ],
432+
[ 'a/**', [ '**' ], [] ],
433+
[ 'a/**', [ 'a/**' ], [] ],
430434
];
431435

432436
describe('globSync - exclude', function() {
@@ -436,7 +440,7 @@ describe('globSync - exclude', function() {
436440
assert.strictEqual(actual.length, 0);
437441
});
438442
}
439-
for (const [pattern, exclude, expected] of pattern2) {
443+
for (const [pattern, exclude, expected] of patterns2) {
440444
test(`${pattern} - exclude: ${exclude}`, () => {
441445
const actual = globSync(pattern, { cwd: fixtureDir, exclude }).sort();
442446
const normalized = expected.filter(Boolean).map((item) => item.replaceAll('/', sep)).sort();
@@ -453,7 +457,7 @@ describe('glob - exclude', function() {
453457
assert.strictEqual(actual.length, 0);
454458
});
455459
}
456-
for (const [pattern, exclude, expected] of pattern2) {
460+
for (const [pattern, exclude, expected] of patterns2) {
457461
test(`${pattern} - exclude: ${exclude}`, async () => {
458462
const actual = (await promisified(pattern, { cwd: fixtureDir, exclude })).sort();
459463
const normalized = expected.filter(Boolean).map((item) => item.replaceAll('/', sep)).sort();
@@ -471,7 +475,7 @@ describe('fsPromises glob - exclude', function() {
471475
assert.strictEqual(actual.length, 0);
472476
});
473477
}
474-
for (const [pattern, exclude, expected] of pattern2) {
478+
for (const [pattern, exclude, expected] of patterns2) {
475479
test(`${pattern} - exclude: ${exclude}`, async () => {
476480
const actual = [];
477481
for await (const item of asyncGlob(pattern, { cwd: fixtureDir, exclude })) actual.push(item);

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.