effect icon indicating copy to clipboard operation
effect copied to clipboard

The ‘recursive’ option for FileSystem.readDirectory does nothing on Node 18

Open strogonoff opened this issue 1 year ago • 3 comments

What version of Effect is running?

2.0.0-next.62

What steps can reproduce the bug?

Attempt using fs.readDirectory() with recursive option on a directory with nested directories and files.

What is the expected behavior?

The listing is recursive, or the option is not listed in docs in the first place.

What do you see instead?

The resulting listing is not recursive and no error is thrown, neither at compile nor at run time.

Additional information

To be clear, from my tests this is the same with vanilla Node 18’s fs.readdir(). I.e., it appears to be either a bug or misdocumentation in Node (I wonder if in this issue someone fat-finger added option in docs of older Node version where it is not supported). However, since this package appears to “proxy” Node docs the deficiency applies here as well.

I guess either:

  • The option should not be listed in docstring (if Node docs are mistaken, they should not be proxied)
  • Dropping Node 18 would fix it (assuming the option works in Node 20), though to be honest I am personally relying on Effect‘s Node 18 support currently…
  • The option is implemented in Effect/platform, to compensate for Node’s lack, so that it works as docs say

strogonoff avatar Dec 28 '23 07:12 strogonoff

Ran into this issue too. Adding { recursive: true } as a second argument to readdirSync('.') does not change the return value (there are subfolders in the folder).

lolmaus avatar Jan 18 '24 09:01 lolmaus

I use this for the time being, seems to work but obviously is suboptimal and inefficient:

const readdirRecursive = (
  /** Directory to list. */
  dir: string,
  /**
   * Directory to output paths relative to.
   * (Don’t specify, used for recursion.)
   */
  relativeTo?: string,
):
Effect.Effect<FileSystem.FileSystem, PlatformError, readonly string[]> =>
Effect.gen(function * (_) {
  const fs = yield * _(FileSystem.FileSystem);
  const dirEntries = yield * _(
    fs.readDirectory(dir),
    Effect.map(basenames => basenames.map(name => join(dir, name))),
  );

  const dirEntryStats:
  Record<string, FileSystem.File.Info> =
  yield * _(Effect.reduceEffect(
    dirEntries.map(path => pipe(
      fs.stat(path),
      Effect.map(stat => ({ [path]: stat })),
    )),
    Effect.succeed({}),
    (accum, item) => ({ ...accum, ...item }),
    { concurrency: 10 },
  ));

  const recursiveListings = dirEntries.map(path =>
    dirEntryStats[path]?.type === 'Directory'
      ? readdirRecursive(path, relativeTo ?? dir)
      : Effect.succeed([relative(relativeTo ?? dir, path)])
    );

  const entries = yield * _(
    Effect.all(recursiveListings, { concurrency: 10 }),
    Effect.map(resultLists => resultLists.flat()),
  );

  return entries;
});

strogonoff avatar Jan 18 '24 10:01 strogonoff

got the same issue with node 18.15, updated to node 18.19 and it fixed the issue :)

You don't need to update to node 20 :) latest version of node 18 are fine.

BastLast avatar Feb 26 '24 20:02 BastLast