fs.Dir methods not documented

[jasnell]

The fs.Dir object that is returned by the fs.opendir APIs includes a couple of methods that are not documented. What is not clear is whether they are meant to be internal only or if they are meant to be public API

• processReadResult(path, result)
• readSyncRecursive(dirent)

Likewise, the constructor for the fs.Dir is callable without adequate type checking.

There's no documentation in the code so it would be helpful to get some clarification on what is intended here.

/cc @nodejs/fs @cjihrig @targos @addaleax @mcollina @anonrig

[targos]

Both methods were added in 7b39e80. It doesn't look like they are meant to be public.

[jasnell]

Ok, I'll open a PR that deprecates the public access to them and start working towards hiding them

[LiviaMedeiros]

IMHO these two methods do not require the deprecation cycle, they can not be used in any meaningful way in userland (both alter internal state of dir but don't return anything, and properties that could expose "results" are private), and seem to be never used in the wild (processReadResult, readSyncRecursive).

If #58672 won't have objections, we can just unexpose them as semver-patch.

[jasnell]

I think, unfortunately, per our deprecation policy we need to take them through a deprecation cycle at least in 24, then remove them entirely in 25.

[aduh95]

I think, unfortunately, per our deprecation policy we need to take them through a deprecation cycle at least in 24, then remove them entirely in 25.

I don't think that's accurate, IIUC undocumented features are not considered stable, they are considered internal (as long as they are not relied upon by the ecosystem)

node/doc/contributing/collaborator-guide.md

Lines 360 to 369 in 52425d2

	Existing stable public APIs that change in a backward-incompatible way must
	undergo deprecation. The exceptions to this rule are:
	* Adding or removing errors thrown or reported by a public API.
	* Emitting a runtime warning.
	* Changing error messages for errors without error code.
	* Altering the timing and non-internal side effects of the public API.
	* Changes to errors thrown by dependencies of Node.js, such as V8.
	* One-time exceptions granted by the TSC.

node/doc/contributing/collaborator-guide.md

Lines 319 to 342 in 52425d2

	### Internal vs. public API
	All functionality in the official Node.js documentation is part of the public
	API. Any undocumented object, property, method, argument, behavior, or event is
	internal. There are exceptions to this rule. Node.js users have come to rely on
	some undocumented behaviors. Collaborators treat many of those undocumented
	behaviors as public.
	All undocumented functionality exposed via `process.binding(...)` is internal.
	All undocumented functionality in `lib/internal/**/*.js` is internal. It is
	public, though, if it is re-exported by code in `lib/*.js`.
	Non-exported `Symbol` properties and methods are internal.
	Any undocumented object property or method that begins with `_` is internal.
	Any native C/C++ APIs/ABIs requiring the `NODE_WANT_INTERNALS` flag are
	internal.
	Sometimes, there is disagreement about whether functionality is internal or
	public. In those cases, the TSC makes a determination.
	For undocumented APIs that are public, open a pull request documenting the API.

[jasnell]

While that's the letter of the policy, unfortunately it's not the reality of it. There are stable APIs and options that sometimes we just forget to document but still need to be handled carefully to avoid breakage. In this case, I think the risk of breakage is low, so if there are no TSC objections to treating it as a semver-patch to hide these, then so be it.