Uh oh!
There was an error while loading. Please reload this page.
fs: runtime deprecate closing fs.Dir on garbage collection - #58850
fs: runtime deprecate closing fs.Dir on garbage collection#58850LiviaMedeiros wants to merge 1 commit into
fs.Dir on garbage collection#58850Conversation
719ebb4 to
39e8ae5CompareCodecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@## main #58850 +/- ##
==========================================
- Coverage 88.28% 88.26% -0.02%
==========================================
Files 702 702 Lines 206760 206773 +13 Branches 39776 39777 +1 ==========================================
- Hits 182531 182508 -23 - Misses 16235 16269 +34 - Partials 7994 7996 +2
🚀 New features to boost your workflow:
|
| import { opendir } from 'node:fs/promises'; | ||
| { | ||
| await using dir = await opendir('/async/disposable/directory'); | ||
| } // Closed by dir[Symbol.asyncDispose]() | ||
| { | ||
| using dir = await opendir('/sync/disposable/directory'); | ||
| } // Closed by dir[Symbol.dispose]() | ||
| { | ||
| let dir; | ||
| try { | ||
| dir = await opendir('/legacy/closeable/directory'); | ||
| } finally { | ||
| await dir?.close(); | ||
| } | ||
| } |
There was a problem hiding this comment.
I think this is lacking the (IMO) most prominent use-case for opendir: iterating on it. Can you add a test that iterates on a Dir and validates that it does not trigger any warning?
There was a problem hiding this comment.
Can you add the iteration example in this snippet as well please?
39e8ae5 to
c60424bCompare
aduh95
left a comment
There was a problem hiding this comment.
LGTM. We should probably land this as doc-only first, if you can open another PR that lands first, that'd be great, otherwise I can do it for you
LiviaMedeiros
commented
Sep 10, 2025
Sure, i'll make PR with doc-only version of this (or rather, formalizing already existing runtime warning as deprecation). |
741088b to
8ff0b20Comparenodejs-github-bot
commented
Sep 12, 2025
aduh95
commented
Sep 12, 2025
@nodejs/tsc do we want this in v25.x? If so, we'd need at least one more approval |
This pull request has been marked as stale due to 90 days of inactivity. |
The DEP0137 is going to be End-of-Life in the next major release.
I think we should align closing
fs.Diron GC and deprecate it as well. Now that explicit resource management is available, closing dirs properly should become easier for userland.Given that it already spams
Warnings in stderr, it would make sense to make this deprecation runtime right away.cc @nodejs/fs @nodejs/tsc