-
-
Notifications
You must be signed in to change notification settings - Fork 33.5k
fs: runtime deprecate mistakenly exposed fs.Dir
methods
#58679
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.
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
Conversation
The `processReadResult` and `readSyncRecursive` methods on the `fs.Dir` class were mistakenly exposed as public APIs. These methods are intended only for internal use. They are now runtime deprecated and will be removed.
Alternative to #58672 |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #58679 +/- ##
==========================================
- Coverage 90.15% 90.14% -0.01%
==========================================
Files 636 636
Lines 188061 188078 +17
Branches 36899 36903 +4
==========================================
+ Hits 169537 169552 +15
- Misses 11278 11280 +2
Partials 7246 7246
🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm totes fine with either
|
||
### DEP0198: `fs.Dir.prototype.readSyncRecursive()` and `fs.Dir.prototype.processReadResult` | ||
|
||
<!-- |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
<!-- | |
<!-- YAML |
<!-- | ||
changes: | ||
- version: REPLACEME | ||
pr-url: https://github.com/nodejs/node/pull/00000 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
pr-url: https://github.com/nodejs/node/pull/00000 | |
pr-url: https://github.com/nodejs/node/pull/58679 |
|
||
Type: Runtime | ||
|
||
The `fs.Dir.prototype.readSyncRecursive()` and `fs.Dir.prototype.processReadResult` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The `fs.Dir.prototype.readSyncRecursive()` and `fs.Dir.prototype.processReadResult` | |
The `fs.Dir.prototype.readSyncRecursive()` and `fs.Dir.prototype.processReadResult()` |
or
The `fs.Dir.prototype.readSyncRecursive()` and `fs.Dir.prototype.processReadResult` | |
The `fs.Dir.prototype.readSyncRecursive` and `fs.Dir.prototype.processReadResult` |
|
||
The [`util.types.isNativeError`][] API is deprecated. Please use [`Error.isError`][] instead. | ||
|
||
### DEP0198: `fs.Dir.prototype.readSyncRecursive()` and `fs.Dir.prototype.processReadResult` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
### DEP0198: `fs.Dir.prototype.readSyncRecursive()` and `fs.Dir.prototype.processReadResult` | |
### DEP0198: `fs.Dir.prototype.readSyncRecursive()` and `fs.Dir.prototype.processReadResult()` |
or
### DEP0198: `fs.Dir.prototype.readSyncRecursive()` and `fs.Dir.prototype.processReadResult` | |
### DEP0198: `fs.Dir.prototype.readSyncRecursive` and `fs.Dir.prototype.processReadResult` |
If it's undocumented and we find no use in the ecosystem, I don't think we need a deprecation, we can directly remove it in a semver-major IMO |
Let's get some other opinions from @nodejs/tsc on it... if there are no objections to handling it without a deprecation then ok |
I'm ok with landing without a deprecation, and just removing/hiding it. |
I concur. Landing #58672 as
semver-major
|
Closing in favor of #58672 |
The
processReadResult
andreadSyncRecursive
methods on thefs.Dir
class were mistakenly exposed as public APIs. These methods are intended only for internal use. They are now runtime deprecated and will be removed.There is an extremely slim possibility that this change could be breaking in that if someone out there happened to be monkeypatching these then the approach taken in this PR would break that. A github code search revealed no such cases and I believe it's unlikely enough that we shouldn't need to worry about it.