Skip to content
Closed
Show file tree
Hide file tree
Changes from all 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
122 changes: 109 additions & 13 deletions ext/node/polyfills/fs.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3220,19 +3220,110 @@
return fsWatcher;
}

// Mirrors Node's `validateIgnoreOption` /
// `createIgnoreMatcher` from `lib/internal/fs/watchers.js`.
// Accepts a string (minimatch glob), RegExp, function, or array of those.
// Returns a function `(filename) => boolean` (or `null` if `ignore` is nullish).
type IgnoreOption =
| string
| RegExp
| ((filename: string) => boolean)
| (string | RegExp | ((filename: string) => boolean))[]
| undefined
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor — let _lazyMinimatch: any = null is using any for the lazy-loader handle. This is contagious: _lazyMinimatch ??= core.createLazyLoader(...) then _lazyMinimatch() returns any, and getMinimatch().default is therefore any too.

Not blocking, but typing it as something narrower (e.g. () => typeof import("...") or just () => unknown and then asserting at the destructure site) would catch a mistyped property access at compile time. The current shape would silently break if Minimatch got renamed in the dep.

| null;

let _lazyMinimatch: any = null;
function getMinimatch() {
_lazyMinimatch ??= core.createLazyLoader("ext:deno_node/deps/minimatch.js");
return _lazyMinimatch();
}

function validateIgnoreOptionElement(value: unknown, name: string) {
if (typeof value === "string") {
if (value.length === 0) {
throw new ERR_INVALID_ARG_VALUE(
name,
value,
"must be a non-empty string",
);
}
return;
}
if (value instanceof RegExp) return;

Check failure on line 3252 in ext/node/polyfills/fs.ts

View workflow job for this annotation

GitHub Actions / lint debug linux-x86_64

Don't use the global intrinsic

Check failure on line 3252 in ext/node/polyfills/fs.ts

View workflow job for this annotation

GitHub Actions / lint debug linux-x86_64

Don't use `instanceof` operator

Check failure on line 3252 in ext/node/polyfills/fs.ts

View workflow job for this annotation

GitHub Actions / lint debug windows-x86_64

Don't use the global intrinsic

Check failure on line 3252 in ext/node/polyfills/fs.ts

View workflow job for this annotation

GitHub Actions / lint debug windows-x86_64

Don't use `instanceof` operator
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lint blocker — prefer-primordials rejects all four hits in this PR:

  • L3252: value instanceof RegExpObjectPrototypeIsPrototypeOf(RegExpPrototype, value)
  • L3263: Array.isArray(value)ArrayIsArray(value)
  • L3276: Array.isArray(ignore)ArrayIsArray(ignore)
  • L3295: matcher instanceof RegExpObjectPrototypeIsPrototypeOf(RegExpPrototype, matcher)

The file already imports ObjectPrototypeIsPrototypeOf from primordials at line 174. After #33574 landed, RegExpPrototype and ArrayIsArray should also already be in the destructure — if not, add them.

Full failure: https://github.com/denoland/deno/actions/runs/25018807704/job/73273702258

#33574 had the exact same pattern fixed in the same way; you can crib that fix verbatim.

if (typeof value === "function") return;
throw new ERR_INVALID_ARG_TYPE(
name,
["string", "RegExp", "Function"],
value,
);
}

function validateIgnoreOption(value: unknown, name: string) {
if (value == null) return;
if (Array.isArray(value)) {

Check failure on line 3263 in ext/node/polyfills/fs.ts

View workflow job for this annotation

GitHub Actions / lint debug linux-x86_64

Don't use the global intrinsic

Check failure on line 3263 in ext/node/polyfills/fs.ts

View workflow job for this annotation

GitHub Actions / lint debug windows-x86_64

Don't use the global intrinsic
for (let i = 0; i < value.length; i++) {
validateIgnoreOptionElement(value[i], `${name}[${i}]`);
}
return;
}
validateIgnoreOptionElement(value, name);
}

function createIgnoreMatcher(
ignore: IgnoreOption,
): ((filename: string) => boolean) | null {
if (ignore == null) return null;
const matchers = Array.isArray(ignore) ? ignore : [ignore];

Check failure on line 3276 in ext/node/polyfills/fs.ts

View workflow job for this annotation

GitHub Actions / lint debug linux-x86_64

Don't use the global intrinsic

Check failure on line 3276 in ext/node/polyfills/fs.ts

View workflow job for this annotation

GitHub Actions / lint debug windows-x86_64

Don't use the global intrinsic
const compiled: Array<(filename: string) => boolean> = [];

for (let i = 0; i < matchers.length; i++) {
const matcher = matchers[i];
if (typeof matcher === "string") {
const { Minimatch } = getMinimatch().default;
const mm = new Minimatch(matcher, {
nocase: isMacOS || isWindows,
windowsPathsNoEscape: true,
nonegate: true,
nocomment: true,
optimizationLevel: 2,
platform: isWindows ? "win32" : "posix",
// Allow patterns without slashes to match the basename
// e.g. '*.log' matches 'subdir/file.log'.
matchBase: true,
});
compiled.push((filename: string) => mm.match(filename));

Check failure on line 3294 in ext/node/polyfills/fs.ts

View workflow job for this annotation

GitHub Actions / lint debug linux-x86_64

Don't use the global intrinsic

Check failure on line 3294 in ext/node/polyfills/fs.ts

View workflow job for this annotation

GitHub Actions / lint debug linux-x86_64

Don't use the global intrinsic

Check failure on line 3294 in ext/node/polyfills/fs.ts

View workflow job for this annotation

GitHub Actions / lint debug windows-x86_64

Don't use the global intrinsic

Check failure on line 3294 in ext/node/polyfills/fs.ts

View workflow job for this annotation

GitHub Actions / lint debug windows-x86_64

Don't use the global intrinsic
} else if (matcher instanceof RegExp) {

Check failure on line 3295 in ext/node/polyfills/fs.ts

View workflow job for this annotation

GitHub Actions / lint debug linux-x86_64

Don't use the global intrinsic

Check failure on line 3295 in ext/node/polyfills/fs.ts

View workflow job for this annotation

GitHub Actions / lint debug linux-x86_64

Don't use `instanceof` operator

Check failure on line 3295 in ext/node/polyfills/fs.ts

View workflow job for this annotation

GitHub Actions / lint debug windows-x86_64

Don't use the global intrinsic

Check failure on line 3295 in ext/node/polyfills/fs.ts

View workflow job for this annotation

GitHub Actions / lint debug windows-x86_64

Don't use `instanceof` operator
compiled.push((filename: string) => matcher.test(filename));

Check failure on line 3296 in ext/node/polyfills/fs.ts

View workflow job for this annotation

GitHub Actions / lint debug linux-x86_64

Don't use the global intrinsic

Check failure on line 3296 in ext/node/polyfills/fs.ts

View workflow job for this annotation

GitHub Actions / lint debug windows-x86_64

Don't use the global intrinsic
} else {
// Function
compiled.push(matcher as (filename: string) => boolean);

Check failure on line 3299 in ext/node/polyfills/fs.ts

View workflow job for this annotation

GitHub Actions / lint debug linux-x86_64

Don't use the global intrinsic

Check failure on line 3299 in ext/node/polyfills/fs.ts

View workflow job for this annotation

GitHub Actions / lint debug windows-x86_64

Don't use the global intrinsic
}
}

return (filename: string) => {
for (let i = 0; i < compiled.length; i++) {
if (compiled[i](filename)) return true;
}
return false;
};
}

function watchPromise(
filename: string | Buffer | URL,
options?: {
persistent?: boolean;
recursive?: boolean;
encoding?: string;
signal?: AbortSignal;
ignore?: IgnoreOption;
},
): AsyncIterable<{ eventType: string; filename: string | Buffer | null }> {
// deno-lint-ignore prefer-primordials
const watchPath = getValidatedPath(filename).toString();

const recursive = options?.recursive ?? false;
validateIgnoreOption(options?.ignore, "options.ignore");
const ignoreMatcher = createIgnoreMatcher(options?.ignore);
const watcher = Deno.watchFs(watchPath, {
recursive,
});
Expand All @@ -3255,20 +3346,25 @@
async next(): Promise<
IteratorResult<{ eventType: string; filename: string | Buffer | null }>
> {
// deno-lint-ignore prefer-primordials
const iterResult = await fsIterable.next();
if (iterResult.done) return iterResult;
while (true) {
// deno-lint-ignore prefer-primordials
const iterResult = await fsIterable.next();
if (iterResult.done) return iterResult;

const eventType = convertDenoFsEventToNodeFsEvent(
iterResult.value.kind,
);
const fname = recursive
? relative(resolvedWatchPath, iterResult.value.paths[0])
: basename(iterResult.value.paths[0]);
return {
value: { eventType, filename: fname },
done: false,
};
const eventType = convertDenoFsEventToNodeFsEvent(
iterResult.value.kind,
);
const fname = recursive
? relative(resolvedWatchPath, iterResult.value.paths[0])
: basename(iterResult.value.paths[0]);
if (ignoreMatcher !== null && ignoreMatcher(fname)) {
continue;
}
return {
value: { eventType, filename: fname },
done: false,
};
}
},
// deno-lint-ignore no-explicit-any
return(value?: any): Promise<IteratorResult<any>> {
Expand Down
5 changes: 5 additions & 0 deletions tests/node_compat/config.jsonc
Original file line number Diff line number Diff line change
Expand Up @@ -1089,6 +1089,11 @@
"parallel/test-fs-promises-readfile-empty.js": {},
"parallel/test-fs-promises-readfile-with-fd.js": {},
"parallel/test-fs-promises-statfs-validate-path.js": {},
"parallel/test-fs-promises-watch-ignore-function.mjs": {},
"parallel/test-fs-promises-watch-ignore-glob.mjs": {},
"parallel/test-fs-promises-watch-ignore-invalid.mjs": {},
"parallel/test-fs-promises-watch-ignore-mixed.mjs": {},
"parallel/test-fs-promises-watch-ignore-regexp.mjs": {},
"parallel/test-fs-promises-watch-iterator.js": {},
"parallel/test-fs-promises-write-optional-params.js": {},
"parallel/test-fs-promises-writefile-typedarray.js": {},
Expand Down
Loading