fs: make processReadResult() and readSyncRecursive() private

[LiviaMedeiros]

These two methods are for internal use only. They alter dir's internal state and don't return anything, i.e. can't be used in userland in meaningful way.
Unless there are objections, these can be unexposed without deprecation cycle.

Closes: #58671
Refs: #41439

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 90.14%. Comparing base (bb99171) to head (5483950).
Report is 24 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #58672      +/-   ##
==========================================
- Coverage   90.17%   90.14%   -0.03%     
==========================================
  Files         636      636              
  Lines      188060   188060              
  Branches    36898    36898              
==========================================
- Hits       169579   169526      -53     
- Misses      11229    11285      +56     
+ Partials     7252     7249       -3     

Files with missing lines	Coverage Δ
lib/internal/fs/dir.js	94.56% <100.00%> (ø)

... and 32 files with indirect coverage changes

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[nodejs-github-bot]

CI: https://ci.nodejs.org/job/node-test-pull-request/67401/

[Ethan-Arrowood]

I agree, but can you do a GitHub code search check and see if anyone is in fact publicly using these methods?

[LiviaMedeiros]

I did that, couldn't find anything except copies of lib/internal/fs/dir.js itself.

[Ethan-Arrowood]

Excellent, then I fully support landing this as a fix instead of a breaking change.

[jasnell]

I'm not entirely sure we can just remove these. Would like to get some other opinions from @nodejs/tsc ... In the meantime I've also opened an alternative that runtime deprecates them See #58679

[targos]

I don't see the problem. I haven't verified it but I don't remember that our deprecation policy applies to undocumented internals.

[RaisinTen]

LGTM.

I don't think an entire runtime deprecation cycle for an undocumented API is necessary.

However, I would prefer to see this change highlighted in the changelog to make Node.js upgrades easier for people who might be relying on this privately, which won't show up on GitHub search.

Can we land it as semver-major PRs that contain breaking changes and should be released in the next major version. or mark this as a notable-change PRs with changes that should be highlighted in changelogs. ? I'm fine with both.

[nodejs-github-bot]

Landed in 823ca69