feat(tar/unstable): add Symbol.asyncDispose to TarStreamEntry#7192
feat(tar/unstable): add Symbol.asyncDispose to TarStreamEntry#7192minato32 wants to merge 2 commits into
Symbol.asyncDispose to TarStreamEntry#7192Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #7192 +/- ##
==========================================
- Coverage 94.64% 94.64% -0.01%
==========================================
Files 629 629
Lines 51895 51646 -249
Branches 9373 9324 -49
==========================================
- Hits 49116 48880 -236
+ Misses 2211 2202 -9
+ Partials 568 564 -4 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
While I have reservations about the The code itself looks fine and well implemented. I have no changes to recommend. I'm not sure whether the tests will pass in Deno v1 though, or how to tell the test runner to skip them when it's running. My main concerns with But These tradeoffs may be more desirable to everyone else here, but in my opinion it will just cause people to have two trains of thought for which one they're using, |
|
@BlackAsLight thanks for the review. On Deno v1 — no skip needed. On the |
Refs #6019.
The most common footgun with
UntarStream(raised by @lowlighter, @dsherret, and others on the issue) is that requesting the next entry hangs forever if the previousentry.readablewas neither consumed nor cancelled — e.g.if (entry.path.endsWith(…)) continue;in the loop body.This adds
[Symbol.asyncDispose]toTarStreamEntry, which cancelsentry.readableif it exists. Callers can then use the explicit-resource-management form to skip entries without remembering to callcancel():Existing manual
entry.readable?.cancel()usage keeps working unchanged; nothing else about the streaming contract changes.