stream: support dispose in writable by benjamingr · Pull Request #48547 · nodejs/node · GitHub

Add support to Symbol.asyncDispose in writable streams. Additionally add a test for writable, transform and duplex streams (that inherit from readable/writable) to avoid breakage.

cc @nodejs/streams @MoLow @atlowChemi

Review requested:

@nodejs/streams

LGTM

CI: https://ci.nodejs.org/job/node-test-pull-request/52472/

common.mustCall accepts a count argument: common.mustCall(fn, times)

CI: https://ci.nodejs.org/job/node-test-pull-request/52513/

CI: https://ci.nodejs.org/job/node-test-pull-request/52555/

This needs a rebase.

lgtm

Add support to Symbol.asyncDispose in writable streams. Additionally add a test for writable, transform and duplex streams who inherit from readable/writable to avoid breakage. Co-authored-by: Robert Nagy <ronagy@icloud.com> Co-authored-by: atlowChemi <chemi@atlow.co.il>

lgtm

CI: https://ci.nodejs.org/job/node-test-pull-request/59786/

CI: https://ci.nodejs.org/job/node-test-pull-request/59798/

@benjamingr as we discussed, moved this PR forward, would you mind taking a look? 🙂

LGTM 😅

Landed in 75fe4f3

Add support to Symbol.asyncDispose in writable streams. Additionally add a test for writable, transform and duplex streams who inherit from readable/writable to avoid breakage. Co-authored-by: Robert Nagy <ronagy@icloud.com> Co-authored-by: atlowChemi <chemi@atlow.co.il> PR-URL: #48547 Reviewed-By: Chemi Atlow <chemi@atlow.co.il> Reviewed-By: Moshe Atlow <moshe@atlow.co.il> Reviewed-By: Robert Nagy <ronagy@icloud.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com>

Add support to Symbol.asyncDispose in writable streams. Additionally add a test for writable, transform and duplex streams who inherit from readable/writable to avoid breakage. Co-authored-by: Robert Nagy <ronagy@icloud.com> Co-authored-by: atlowChemi <chemi@atlow.co.il> PR-URL: nodejs#48547 Reviewed-By: Chemi Atlow <chemi@atlow.co.il> Reviewed-By: Moshe Atlow <moshe@atlow.co.il> Reviewed-By: Robert Nagy <ronagy@icloud.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com>

Add support to Symbol.asyncDispose in writable streams. Additionally add a test for writable, transform and duplex streams who inherit from readable/writable to avoid breakage. Co-authored-by: Robert Nagy <ronagy@icloud.com> Co-authored-by: atlowChemi <chemi@atlow.co.il> PR-URL: #48547 Reviewed-By: Chemi Atlow <chemi@atlow.co.il> Reviewed-By: Moshe Atlow <moshe@atlow.co.il> Reviewed-By: Robert Nagy <ronagy@icloud.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com>

nodejs-github-bot added the needs-ci PRs that need a full CI run. label Jun 25, 2023

benjamingr force-pushed the add-abortsignal-writable branch from f38134b to 63aad0a Compare June 25, 2023 17:16

benjamingr added stream Issues and PRs related to the stream subsystem. request-ci Add this label to start a Jenkins CI on a PR. labels Jun 25, 2023

github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Jun 25, 2023

This comment was marked as outdated.

Sign in to view

atlowChemi approved these changes Jun 25, 2023

View reviewed changes

benjamingr commented Jun 25, 2023

View reviewed changes

Comment thread test/parallel/test-stream-transform-destroy.js Outdated

benjamingr added the request-ci Add this label to start a Jenkins CI on a PR. label Jun 25, 2023

ronag reviewed Jun 25, 2023

View reviewed changes

Comment thread lib/internal/streams/writable.js Outdated

github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Jun 25, 2023

This comment was marked as outdated.

Sign in to view

benjamingr added the request-ci Add this label to start a Jenkins CI on a PR. label Jun 25, 2023

github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Jun 25, 2023

MoLow reviewed Jun 25, 2023

View reviewed changes

MoLow approved these changes Jun 25, 2023

View reviewed changes

This was referenced Jun 26, 2023

CI Reliability 2023-06-26 nodejs/reliability#599

Open

CI Reliability 2023-06-27 nodejs/reliability#600

Open

ronag reviewed Jun 27, 2023

View reviewed changes

Comment thread lib/internal/streams/writable.js Outdated

This was referenced Jun 28, 2023

CI Reliability 2023-06-28 nodejs/reliability#601

Open

CI Reliability 2023-06-29 nodejs/reliability#602

Open

benjamingr added the request-ci Add this label to start a Jenkins CI on a PR. label Jun 29, 2023

github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Jun 29, 2023

This was referenced Jun 30, 2023

CI Reliability 2023-06-30 nodejs/reliability#603

Open

CI Reliability 2023-07-01 nodejs/reliability#604

Open

CI Reliability 2023-07-02 nodejs/reliability#605

Open

github-actions Bot mentioned this pull request Jul 8, 2023

CI Reliability 2023-07-08 nodejs/reliability#611

Open

23 tasks

ronag approved these changes May 11, 2024

View reviewed changes

mcollina approved these changes May 12, 2024

View reviewed changes

atlowChemi reviewed May 12, 2024

View reviewed changes

Comment thread lib/internal/streams/writable.js Outdated

atlowChemi force-pushed the add-abortsignal-writable branch from 0ec7691 to 271a702 Compare June 14, 2024 15:20

fixup! stream: support dispose in writable

a4c2961

mcollina approved these changes Jun 14, 2024

View reviewed changes

ronag approved these changes Jun 14, 2024

View reviewed changes

atlowChemi added the request-ci Add this label to start a Jenkins CI on a PR. label Jun 14, 2024

github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Jun 14, 2024

atlowChemi added the author ready PRs that have at least one approval, no outstanding review comments, and a CI started. label Jun 15, 2024

benjamingr commented Jun 15, 2024

View reviewed changes

atlowChemi added the commit-queue Add this label to land a pull request using GitHub Actions. label Jun 15, 2024

nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Jun 15, 2024

nodejs-github-bot merged commit 75fe4f3 into main Jun 15, 2024

nodejs-github-bot deleted the add-abortsignal-writable branch June 15, 2024 23:42

targos mentioned this pull request Jun 25, 2024

v22.4.0 release proposal #53583

Merged

Karliz24 mentioned this pull request Jul 9, 2024

## 2024-07-02, Version 22.4.0 (Current), @targos Karliz24/Marley-#16

Open

marco-ippolito mentioned this pull request Jul 19, 2024

v20.16.0 proposal #53945

Merged

+                });
+                let count = 0;
+                duplex.on('error', common.mustCall((e) => {
+                  assert.strictEqual(count++, 0); // Ensure not called twice

stream: support dispose in writable by benjamingr · Pull Request #48547 · nodejs/node · GitHub

Uh oh!

Uh oh!

stream: support dispose in writable - #48547

stream: support dispose in writable#48547nodejs-github-bot merged 2 commits intomainnodejs/node:mainfrom add-abortsignal-writablenodejs/node:add-abortsignal-writableCopy head branch name to clipboard

Conversation

benjamingr commented Jun 25, 2023

Uh oh!

nodejs-github-bot commented Jun 25, 2023

Uh oh!

This comment was marked as outdated.

atlowChemi left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

This comment was marked as outdated.

nodejs-github-bot commented Jun 25, 2023

Uh oh!

MoLow Jun 25, 2023

Choose a reason for hiding this comment

Uh oh!

nodejs-github-bot commented Jun 27, 2023

Uh oh!

Uh oh!

nodejs-github-bot commented Jun 29, 2023

Uh oh!

aduh95 commented May 11, 2024

Uh oh!

mcollina left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

mcollina left a comment

Choose a reason for hiding this comment

Uh oh!

nodejs-github-bot commented Jun 14, 2024

Uh oh!

nodejs-github-bot commented Jun 15, 2024

Uh oh!

atlowChemi commented Jun 15, 2024

Uh oh!

benjamingr left a comment

Choose a reason for hiding this comment

Uh oh!

nodejs-github-bot commented Jun 15, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

stream: support dispose in writable#48547
nodejs-github-bot merged 2 commits into
mainfrom
add-abortsignal-writable

atlowChemi left a comment •
edited
Loading