Skip to content

feat: support custom fs #136

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.

Sign up for GitHub

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

Merged
thecodrr merged 2 commits into from
Jul 4, 2025
+50 −6
Merged

feat: support custom fs #136

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
23089cb
feat: support custom fs
43081j Mar 10, 2025
9206def
chore: run format
43081j May 7, 2025
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
25 changes: 25 additions & 0 deletions __tests__/fdir.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -378,6 +378,31 @@ for (const type of apiTypes) {
.crawl("node_modules");
t.expect(globFunction).toHaveBeenCalledWith(["**/*.js"], "bleep");
});

test(`[${type}] using custom fs implementation`, async (t) => {
const readdirStub = vi.fn<Parameters<typeof fs.readdir>>(
(_path, _opts, cb) => {
cb(null, []);
}
);
const readdirSyncStub = vi.fn();
readdirSyncStub.mockReturnValue([]);
const fakeFs = {
...fs,
readdir: readdirStub,
readdirSync: readdirSyncStub,
} as unknown as typeof fs;

const api = new fdir({
fs: fakeFs,
}).crawl("node_modules");
await api[type]();
if (type === "withPromise") {
t.expect(readdirStub).toHaveBeenCalled();
} else {
t.expect(readdirSyncStub).toHaveBeenCalled();
}
});
}

// AbortController is not present on Node v14
Expand Down
6 changes: 4 additions & 2 deletions src/api/functions/resolve-symlink.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
import fs from "fs";
import type { Stats } from "fs";
import { WalkerState, Options } from "../../types";
import { dirname } from "path";

export type ResolveSymlinkFunction = (
path: string,
state: WalkerState,
callback: (stat: fs.Stats, path: string) => void
callback: (stat: Stats, path: string) => void
) => void;

const resolveSymlinksAsync: ResolveSymlinkFunction = function (
Expand All @@ -15,6 +15,7 @@ const resolveSymlinksAsync: ResolveSymlinkFunction = function (
) {
const {
queue,
fs,
options: { suppressErrors },
} = state;
queue.enqueue();
Expand All @@ -41,6 +42,7 @@ const resolveSymlinks: ResolveSymlinkFunction = function (
) {
const {
queue,
fs,
options: { suppressErrors },
} = state;
queue.enqueue();
Expand Down
9 changes: 6 additions & 3 deletions src/api/functions/walk-directory.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
import { WalkerState } from "../../types";
import fs from "fs";
import type { Dirent } from "fs";

export type WalkDirectoryFunction = (
state: WalkerState,
crawlPath: string,
directoryPath: string,
depth: number,
callback: (entries: fs.Dirent[], directoryPath: string, depth: number) => void
callback: (entries: Dirent[], directoryPath: string, depth: number) => void
) => void;

const readdirOpts = { withFileTypes: true } as const;
Expand All @@ -22,6 +22,8 @@ const walkAsync: WalkDirectoryFunction = (

if (currentDepth < 0) return state.queue.dequeue(null, state);

const { fs } = state;

state.visited.push(crawlPath);
state.counts.directories++;

Expand All @@ -41,11 +43,12 @@ const walkSync: WalkDirectoryFunction = (
currentDepth,
callback
) => {
const { fs } = state;
if (currentDepth < 0) return;
state.visited.push(crawlPath);
state.counts.directories++;

let entries: fs.Dirent[] = [];
let entries: Dirent[] = [];
try {
entries = fs.readdirSync(crawlPath || ".", readdirOpts);
} catch (e) {
Expand Down
4 changes: 3 additions & 1 deletion src/api/walker.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,8 @@ import * as resolveSymlink from "./functions/resolve-symlink";
import * as invokeCallback from "./functions/invoke-callback";
import * as walkDirectory from "./functions/walk-directory";
import { Queue } from "./queue";
import { Dirent } from "fs";
import type { Dirent } from "fs";
import * as nativeFs from "fs";
Comment on lines +13 to +14
Copy link
Copy Markdown
Contributor

@benmccann benmccann Apr 7, 2025

Choose a reason for hiding this comment

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

just a style question probably for @thecodrr, but I wonder if these two lines should be combined into a single line

Copy link
Copy Markdown
Owner

@thecodrr thecodrr Jun 30, 2025

Choose a reason for hiding this comment

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

We can't import fs anywhere in the project (importing the types are okay since they are stripped). It needs to be imported conditionally otherwise anyone trying to use fdir outside node won't be able to use it - defeating the whole point of this PR.

Perhaps we can use require but if we generate esm builds, that won't work afaik. await import is also not an option since build options are synchronous.

Copy link
Copy Markdown
Contributor Author

@43081j 43081j Jun 30, 2025

Choose a reason for hiding this comment

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

we already have this problem - the walker imports path

are we sure that's the point of this PR? afaik nobody is expecting to run fdir in browsers (which is the only place that doesn't have path and fs, since all node-like runtimes provide them)

if we want to avoid depending on any node built ins, its a bigger job since we'd need to pull in pathe or something (agnostic path library) instead of path

@SuperchupuDev what does tinyglobby need this for?

Copy link
Copy Markdown
Contributor

@SuperchupuDev SuperchupuDev Jun 30, 2025

Choose a reason for hiding this comment

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

We can't import fs anywhere in the project (importing the types are okay since they are stripped). It needs to be imported conditionally otherwise anyone trying to use fdir outside node won't be able to use it - defeating the whole point of this PR.

Perhaps we can use require but if we generate esm builds, that won't work afaik. await import is also not an option since build options are synchronous.

we can try to use require for conditionally importing it like we already do with picomatch. tsdown (the tool thats used in the dual build pr) automatically defines a require function with import { createRequire } from 'node:module'; createRequire(import.meta.url) for esm builds

Copy link
Copy Markdown
Contributor

@SuperchupuDev SuperchupuDev Jun 30, 2025

Choose a reason for hiding this comment

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

we already have this problem - the walker imports path

are we sure that's the point of this PR? afaik nobody is expecting to run fdir in browsers (which is the only place that doesn't have path and fs, since all node-like runtimes provide them)

if we want to avoid depending on any node built ins, its a bigger job since we'd need to pull in pathe or something (agnostic path library) instead of path

@SuperchupuDev what does tinyglobby need this for?

iirc the library that requested the feature wants it so that they can use when globbing their custom fs wrapper that does caching

Copy link
Copy Markdown
Contributor Author

@43081j 43081j Jun 30, 2025

Choose a reason for hiding this comment

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

yeah lets not go down a rabbit hole trying to hack in some require calls 😅

fdir currently relies on Node libraries (it imports path)

we can do this with fs too and satisfy the feature request with the PR in its current state afaict

nobody can use fdir in a browser right now without bundling it. so we haven't changed that

let me know if im missing something though

Copy link
Copy Markdown
Contributor

@SuperchupuDev SuperchupuDev Jun 30, 2025

Choose a reason for hiding this comment

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

i agree, not using require would be ideal

import { Output } from "../types";
import { Counter } from "./counter";
import { Aborter } from "./aborter";
Expand Down Expand Up @@ -50,6 +51,7 @@ export class Walker<TOutput extends Output> {
symlinks: new Map(),
visited: [""].slice(0, 0),
controller: new Aborter(),
fs: options.fs ?? nativeFs,
};

/*
Expand Down
12 changes: 12 additions & 0 deletions src/types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { Aborter } from "./api/aborter";
import { Queue } from "./api/queue";
import type * as nativeFs from "fs";

export type Counts = {
files: number;
Expand All @@ -26,6 +27,15 @@ export type PathsOutput = string[];

export type Output = OnlyCountsOutput | PathsOutput | GroupOutput;

export type FSLike = {
Copy link
Copy Markdown
Contributor Author

@43081j 43081j Mar 10, 2025

Choose a reason for hiding this comment

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

this is purposely explicit rather than just FSLike = typeof nativeFs since the fs module has all sorts of exports (both types and actual exports) which we don't want to have to stub out to satisfy the type when bringing our own fs-like object

readdir: typeof nativeFs.readdir;
readdirSync: typeof nativeFs.readdirSync;
realpath: typeof nativeFs.realpath;
realpathSync: typeof nativeFs.realpathSync;
stat: typeof nativeFs.stat;
statSync: typeof nativeFs.statSync;
};

export type WalkerState = {
root: string;
paths: string[];
Expand All @@ -34,6 +44,7 @@ export type WalkerState = {
options: Options;
queue: Queue;
controller: Aborter;
fs: FSLike;

symlinks: Map<string, string>;
visited: string[];
Expand Down Expand Up @@ -67,6 +78,7 @@ export type Options<TGlobFunction = unknown> = {
pathSeparator: PathSeparator;
signal?: AbortSignal;
globFunction?: TGlobFunction;
fs?: FSLike;
};

export type GlobMatcher = (test: string) => boolean;
Expand Down