feat(tar/unstable): add Symbol.asyncDispose to TarStreamEntry#7192
feat(tar/unstable): add Symbol.asyncDispose to TarStreamEntry#7192minato32 wants to merge 1 commit 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.57% 94.56% -0.01%
==========================================
Files 636 637 +1
Lines 52142 52153 +11
Branches 9401 9402 +1
==========================================
+ Hits 49315 49321 +6
- Misses 2249 2254 +5
Partials 578 578 ☔ 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.