feat(tar/unstable): add Symbol.asyncDispose to TarStreamEntry#7192
feat(tar/unstable): add Symbol.asyncDispose to TarStreamEntry#7192minato32 wants to merge 3 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.84% +0.19%
==========================================
Files 629 617 -12
Lines 51895 51677 -218
Branches 9373 9351 -22
==========================================
- Hits 49116 49011 -105
+ Misses 2211 2121 -90
+ Partials 568 545 -23 ☔ 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 |
| * .pipeThrough(new UntarStream()) | ||
| * ) { | ||
| * if (entry.path.endsWith(".log")) continue; | ||
| * // …consume entry.readable as usual… |
There was a problem hiding this comment.
can you use regular three dots instead of the non-ascii char here?
Addresses @bartlomieju's review feedback.
|
@bartlomieju done in 306cf40 — swapped the non-ascii ellipsis for plain |
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.