Volume implementation of opendir method

[ddziara]

Implementation of opendir() and fs.Dir

1. A new file Dir.ts implementing fs.Dir added
2. In volume.ts opendirSync(), opendir(), opendir(callback) added
3. in node/options.ts methods getOpendirOptions() & getOpendirOptsAndCb() added.
4. Tests in __tests__/volume.test.ts added

[G-Rath]

This looks awesome! would you mind running Prettier?

[ddziara]

I run Prettier.

[G-Rath]

looking good, just not sure what to do about that constructor...

[G-Rath]

fwiw it seems that Dir does have a constructor, though it's not documented

> new fs.Dir()
Uncaught TypeError [ERR_MISSING_ARGS]: The "handle" argument must be specified
    at new Dir (node:internal/fs/dir:49:31) {
  code: 'ERR_MISSING_ARGS'
}

I'm not sure if it's worth trying to figure out it's params though, but I can't decide between marking it as private or if we actually try to use it with our own signature (and maybe mark it as internal...?) - as either way a class always has a constructor

[ddziara]

Not sure... "fs.d.ts" (node:fs) doesn't seem to have constructor signature defined. Then again memfs module Dirent.ts defining Dirent class also doesn't define constructor. The constructor seems to be private... or I'm getting you wrong.

[G-Rath]

The constructor seems to be private

If you mean the constructor for the native Dirent, yes it is "documented" as private (by way of not being documented) though technically you can call it because JS doesn't have a concept of private constructors at runtime.

Classes in JS always have a constructor, even if you don't define one - and class properties, methods, and constructors are public by default in TypeScript unless you declare them as private.

That's what I'm getting at - we need a way to construct a Dirent ourselves in order to use them, hence why you've implemented ~build which is effectively a constructor; so I'm debating if we should use the constructor since it is always defined anyway and Node doesn't define it themselves...

Pro: we have one less method
Con: even though Node doesn't document a constructor, the native Dirent does have one, and so it might be confusing if we define one with a different signature...?

[ddziara]

I though we were talking about constructor of Dir. :) I use to think "class factory" about static methods like "build" constructing and returning object of given class.
I believe that undocumented methods should be regarded non-existing in general - it's a little bit risky to use them. And you're right that signature would be different that makes them even less feasible. I would mark private the constructor (Dir)... although actually "build" also should be unavailable (if it would be possible)

[G-Rath]

can we remove this? it doesn't seem like it should be needed - compileOnSave looks like a vscode specific feature which may or may not exist, and even if it does, it shouldn't be set in tsconfig.json because not everyone uses vscode

[ddziara]

I've read someone's comment claiming "compileOnSave" didn't matter in VS Code, only in "full Visual Studio". Then again tsconfig.ts in __tests__ directory seems to help - the typings are again available.

[G-Rath]

Might be best if you describe more what the actual problem you're having is, that leads you think we need this.

My concern is not everyone uses VS Code so including a file for it specifically means it might not be properly maintained, and other changes could be made that impact that editing experience without realizing it

[ddziara]

If I am not wrong, when __test__ is excluded in main tsconfig.json then typescript stops to recognize jest types in test files but adding another tsconfig.json to __test__ dir seems to solve the problem.
Actually I followed these instructions:
(https://intellij-support.jetbrains.com/hc/en-us/community/posts/360000432259-Setting-up-a-typescript-compile-scope-to-exclude-tests-folder-from-being-compiled-doesn-t-work)

No need to specify test patterns... I'd suggest excluding "__tests__" from the root tsconfig.json, adding a separate tsconfig.json to __tests__ folder, with

"compileOnSave": false

PS.
I use VS Code so I'm not aware of behaviours in other environments.
PS2.
It also may be my specific configuration, pnp yarn + monorepo with workspaces + yarn plugin for typescript (workspace typescript used)
PS3.
Of course we can remove it.

[ddziara]

I removed tsconfig.json from __tests__ directory.

[streamich]

Thank you, good work!

[github-actions]

🎉 This PR is included in version 4.11.0 🎉

The release is available on:

• npm package (@latest dist-tag)
• GitHub release

Your semantic-release bot 📦🚀