feat(@types/node): add `stream/consumers` #55311

favna · 2021-08-22T21:34:14Z

This is a set of new functions added in Node 16.7. As the types are now 16.7.x, we can add them.

NodeJS docs: https://nodejs.org/dist/latest-v16.x/docs/api/webstreams.html#webstreams_utility_consumers

Please fill in this template.

Use a meaningful title for the pull request. Include the name of the package modified.
Test the change in your own code. (Compile and run.)
Add or edit tests to reflect the change.
Follow the advice from the readme.
Avoid common mistakes.
Run npm test <package to test>.

Select one of these and delete the others:

If changing an existing definition:

Provide a URL to documentation or source code which provides context for the suggested changes: https://nodejs.org/dist/latest-v16.x/docs/api/webstreams.html#webstreams_utility_consumers
~~If this PR brings the type definitions up to date with a new version of the JS library, update the version number in the header.~~

typescript-bot · 2021-08-22T21:34:46Z

@favna Thank you for submitting this PR!

This is a live comment which I will keep updated.

1 package in this PR

node — on npm, on unpkg
- owner-approval: @peterblazejewicz, @LinusU

Code Reviews

Because this is a widely-used package, a DT maintainer will need to review it before it can be merged.

You can test the changes of this PR in the Playground.

Status

✅ No merge conflicts
✅ Continuous integration tests have passed
✅ Most recent commit is approved by a DT maintainer

All of the items on the list are green. To merge, you need to post a comment including the string "Ready to merge" to bring in your changes.

Diagnostic Information: What the bot saw about this PR

{
  "type": "info",
  "now": "-",
  "pr_number": 55311,
  "author": "Favna",
  "headCommitOid": "028811c99b8fa33b2181f725639a385ded81bb29",
  "lastPushDate": "2021-08-26T10:41:33.000Z",
  "lastActivityDate": "2021-08-27T18:00:33.000Z",
  "mergeOfferDate": "2021-08-27T09:32:42.000Z",
  "mergeRequestDate": "2021-08-27T18:00:33.000Z",
  "mergeRequestUser": "Favna",
  "hasMergeConflict": false,
  "isFirstContribution": false,
  "tooManyFiles": false,
  "popularityLevel": "Critical",
  "pkgInfo": [
    {
      "name": "node",
      "kind": "edit",
      "files": [
        {
          "path": "types/node/index.d.ts",
          "kind": "definition"
        },
        {
          "path": "types/node/stream.d.ts",
          "kind": "definition"
        },
        {
          "path": "types/node/stream/consumers.d.ts",
          "kind": "definition"
        },
        {
          "path": "types/node/test/stream.ts",
          "kind": "test"
        }
      ],
      "owners": [
        "Microsoft",
        "DefinitelyTyped",
        "jkomyno",
        "alvis",
        "r3nya",
        "btoueg",
        "smac89",
        "touffy",
        "DeividasBakanas",
        "eyqs",
        "Hannes-Magnusson-CK",
        "KSXGitHub",
        "hoo29",
        "kjin",
        "ajafff",
        "islishude",
        "mwiktorczyk",
        "mohsen1",
        "n-e",
        "galkin",
        "parambirs",
        "eps1lon",
        "SimonSchick",
        "ThomasdenH",
        "WilcoBakker",
        "wwwy3y3",
        "samuela",
        "kuehlein",
        "bhongy",
        "chyzwar",
        "trivikr",
        "nguymin4",
        "yoursunny",
        "qwelias",
        "ExE-Boss",
        "Ryan-Willpower",
        "peterblazejewicz",
        "addaleax",
        "JasonHK",
        "victorperin",
        "ZYSzys",
        "NodeJS",
        "LinusU"
      ],
      "addedOwners": [],
      "deletedOwners": [],
      "popularityLevel": "Critical"
    }
  ],
  "reviews": [
    {
      "type": "approved",
      "reviewer": "peterblazejewicz",
      "date": "2021-08-27T09:32:06.000Z",
      "isMaintainer": true
    },
    {
      "type": "approved",
      "reviewer": "LinusU",
      "date": "2021-08-26T15:18:50.000Z",
      "isMaintainer": false
    }
  ],
  "mainBotCommentID": 903333595,
  "ciResult": "pass"
}

typescript-bot · 2021-08-22T21:34:48Z

🔔 @microsoft @DefinitelyTyped @jkomyno @alvis @r3nya @btoueg @smac89 @Touffy @DeividasBakanas @eyqs @Hannes-Magnusson-CK @KSXGitHub @hoo29 @kjin @ajafff @islishude @mwiktorczyk @mohsen1 @n-e @galkin @parambirs @eps1lon @SimonSchick @ThomasdenH @WilcoBakker @wwwy3y3 @samuela @kuehlein @bhongy @chyzwar @trivikr @nguymin4 @yoursunny @qwelias @ExE-Boss @Ryan-Willpower @peterblazejewicz @addaleax @JasonHK @victorperin @ZYSzys @nodejs @LinusU — please review this PR in the next few days. Be sure to explicitly select Approve or Request Changes in the GitHub UI so I know what's going on.

typescript-bot · 2021-08-22T22:39:26Z

@favna The CI build failed! Please review the logs for more information.

Once you've pushed the fixes, the build will automatically re-run. Thanks!

Note: builds which are failing do not end up on the list of PRs for the DT maintainers to review.

typescript-bot · 2021-08-23T00:50:02Z

@favna The CI build failed! Please review the logs for more information.

Once you've pushed the fixes, the build will automatically re-run. Thanks!

Note: builds which are failing do not end up on the list of PRs for the DT maintainers to review.

typescript-bot · 2021-08-23T01:21:56Z

@favna The CI build failed! Please review the logs for more information.

Once you've pushed the fixes, the build will automatically re-run. Thanks!

Note: builds which are failing do not end up on the list of PRs for the DT maintainers to review.

SimonSchick · 2021-08-23T05:11:10Z

ts3.6 is being phased out, see #54895

You can safely ignore these errors and we can proceed once this PR is submitted.

SimonSchick · 2021-08-23T14:39:25Z

I'd revert them as you won't really have to do any of it.

typescript-bot · 2021-08-26T01:49:34Z

@favna Unfortunately, this pull request currently has a merge conflict 😥. Please update your PR branch to be up-to-date with respect to master. Have a nice day!

This is a set of new functions added in Node 16.7. As the types are now 16.7.1, we can add them. NodeJS docs: https://nodejs.org/dist/latest-v16.x/docs/api/webstreams.html#webstreams_utility_consumers

favna · 2021-08-26T10:44:48Z

@SimonSchick PR has been rebased on top of master. Thanks for your contributions in your PR.

LinusU

Nice! 👍

favna · 2021-08-27T18:00:33Z

Ready to merge

typescript-bot · 2021-08-27T18:32:39Z

I just published @types/node@16.7.4 to npm.

alecgibson · 2022-01-19T11:18:46Z

types/node/stream/consumers.d.ts

+// This interface is not, and should not be, exported.
+interface Blob {
+    readonly size: number;
+    readonly type: string;
+    arrayBuffer(): Promise<ArrayBuffer>;
+    slice(start?: number, end?: number, contentType?: string): Blob;
+    stream(): NodeJS.ReadableStream;
+    text(): Promise<string>;
+}


I'm not sure this comment is accurate? It looks like this interface is leaking into global definitions:

I can't remember where I read it (maybe in a linter rule or something?), but I'm pretty sure all interfaces in a .d.ts file are exported, so that consumers have access to references to them. Very happy to be proven wrong on this point, though.

Not only is that comment (about not being exported) inaccurate, the comment above it isn't accurate either. The interface is not a "duplicate" of Blob from lib.dom.ts, because the DOM Blob#stream method returns a W3C ReadableStream, which is not the same thing as a NodeJS.ReadableStream.

As you point out, simply having @types/node in your node_modules pollutes frontend code with this Node-specific global type definition, which is merged with the real (DOM) Blob definition. As a result, I've had to any-cast the return value whenever I call .stream().

Good news, the correct types have been added in the interim so this erroneous "duplicate" is no longer needed. I just opened #59905 which I think will resolve this error.

@thw0rted With the benefit of the doubt I will think you didn't mean it as such but I feel called out here anyway, and so I feel a need to defend myself.

I specifically did not add myself to the contributors list for node types because I have no interest in being pinged for reviews and I have hardly any interest in Node's types to begin with as long as they work, and this has for many months.

Furthermore, even though yes it might be wrong that I made this change, that change was made in a pull request just like your own. That means it was reviewed and it was merged by a DT maintainer. Simply denotsting that I did bad here is therefore coming over as shallow because clearly other people were involved as well.

Lastly, as I have also stated elsewhere, this review comment was made after the pull request was already merged. It was therefore quite obviously never taken into full consideration and mostly completely dismissed.

You're right that I didn't mean it as a personal attack -- reading back through my comment I can definitely see how you'd take it that way, and I'm sorry. (I can sympathize about not signing yourself up as a contributor, the scope is unlike anything else on DT!)

The situation is very confusing, and I don't know that anybody can really be blamed directly -- Node chose to add this partial implementation of some web APIs, and worse they're reusing web class names for incomplete implementations that aren't available at the same scope. When you made your PR you didn't even have a "Node Blob" to reference, so your approach was totally reasonable; bleeding into global was an honest mistake.

To make matters worse, I don't see any way to write a test (or I guess a "meta" test?) that would prevent this. Maybe the package.json or tsconfig file could have a field with a list of globals introduced by the package, and if anything else bleeds out, the test would fail? I don't really know enough about TS internals to suggest how to implement something like that but it might be worth kicking the idea around in Discussions -- I'll take a look.

At the moment, some users' builds fail, because `Blob` isn't defined as a global in Node.js - it sits in the [`buffer`][1] package. This change adds an import from `buffer` to fix this issue. Note that I think the build was incorrectly passing before because of a [leaky interface in `stream/consumers`][2]. [1]: https://nodejs.org/api/buffer.html#class-blob [2]: DefinitelyTyped#55311 (comment)

At the moment, some users' builds fail, because `Blob` isn't defined as a global in Node.js - it sits in the [`buffer`][1] package. This change adds an import from `buffer` to fix this issue. Note that I think the build was incorrectly passing before because of a [leaky interface in `stream/consumers`][2]. Note that I've had to disable the linter's `no-outside-dependencies` because of a [known false positive on `buffer`][3]. [1]: https://nodejs.org/api/buffer.html#class-blob [2]: DefinitelyTyped#55311 (comment) [3]: microsoft/dtslint#315

At the moment, some users' builds fail, because `Blob` isn't defined as a global in Node.js - it sits in the [`buffer`][1] package. This change adds an import from `buffer` to fix this issue. Note that I think the build was incorrectly passing before because of a [leaky interface in `stream/consumers`][2]. Note that I've had to disable the linter's `no-outside-dependencies` because of a [known false positive on `buffer`][3]. [1]: https://nodejs.org/api/buffer.html#class-blob [2]: #55311 (comment) [3]: microsoft/dtslint#315

At the moment, some users' builds fail, because `Blob` isn't defined as a global in Node.js - it sits in the [`buffer`][1] package. This change adds an import from `buffer` to fix this issue. Note that I think the build was incorrectly passing before because of a [leaky interface in `stream/consumers`][2]. Note that I've had to disable the linter's `no-outside-dependencies` because of a [known false positive on `buffer`][3]. [1]: https://nodejs.org/api/buffer.html#class-blob [2]: DefinitelyTyped#55311 (comment) [3]: microsoft/dtslint#315

favna requested a review from peterblazejewicz as a code owner August 22, 2021 21:34

typescript-bot added the Critical package label Aug 22, 2021

typescript-bot added this to Waiting for Code Reviews in New Pull Request Status Board Aug 22, 2021

typescript-bot added the The CI failed When GH Actions fails label Aug 22, 2021

typescript-bot moved this from Waiting for Code Reviews to Needs Author Action in New Pull Request Status Board Aug 22, 2021

typescript-bot removed the The CI failed When GH Actions fails label Aug 23, 2021

typescript-bot moved this from Needs Author Action to Waiting for Code Reviews in New Pull Request Status Board Aug 23, 2021

typescript-bot added the The CI failed When GH Actions fails label Aug 23, 2021

typescript-bot moved this from Waiting for Code Reviews to Needs Author Action in New Pull Request Status Board Aug 23, 2021

typescript-bot added Check Config Changes a module config files and removed The CI failed When GH Actions fails labels Aug 23, 2021

typescript-bot moved this from Needs Author Action to Waiting for Code Reviews in New Pull Request Status Board Aug 23, 2021

This comment has been minimized.

Sign in to view

typescript-bot added the The CI failed When GH Actions fails label Aug 23, 2021

typescript-bot moved this from Waiting for Code Reviews to Needs Author Action in New Pull Request Status Board Aug 23, 2021

typescript-bot removed the The CI failed When GH Actions fails label Aug 23, 2021

typescript-bot moved this from Needs Author Action to Waiting for Code Reviews in New Pull Request Status Board Aug 23, 2021

typescript-bot moved this from Waiting for Code Reviews to Needs Maintainer Review in New Pull Request Status Board Aug 23, 2021

This comment has been minimized.

Sign in to view

favna force-pushed the feat/add-stream-consumers-to-node branch from f1a9830 to e624d76 Compare August 23, 2021 14:42

typescript-bot added The CI failed When GH Actions fails and removed Check Config Changes a module config files labels Aug 23, 2021

typescript-bot moved this from Needs Maintainer Review to Needs Author Action in New Pull Request Status Board Aug 23, 2021

feat(node): add stream/consumers

028811c

This is a set of new functions added in Node 16.7. As the types are now 16.7.1, we can add them. NodeJS docs: https://nodejs.org/dist/latest-v16.x/docs/api/webstreams.html#webstreams_utility_consumers

favna force-pushed the feat/add-stream-consumers-to-node branch from e624d76 to 028811c Compare August 26, 2021 10:41

typescript-bot removed Has Merge Conflict This PR can't be merged because it has a merge conflict. The author needs to update it. The CI failed When GH Actions fails labels Aug 26, 2021

typescript-bot moved this from Needs Author Action to Waiting for Code Reviews in New Pull Request Status Board Aug 26, 2021

typescript-bot moved this from Waiting for Code Reviews to Needs Maintainer Review in New Pull Request Status Board Aug 26, 2021

favna mentioned this pull request Aug 26, 2021

refactor: switch to v13 skyra-project/skyra#1998

Merged

6 tasks

LinusU approved these changes Aug 26, 2021

View reviewed changes

typescript-bot added the Owner Approved A listed owner of this package signed off on the pull request. label Aug 26, 2021

peterblazejewicz approved these changes Aug 27, 2021

View reviewed changes

typescript-bot added Maintainer Approved Self Merge This PR can now be self-merged by the PR author or an owner labels Aug 27, 2021

typescript-bot moved this from Needs Maintainer Review to Waiting for Author to Merge in New Pull Request Status Board Aug 27, 2021

typescript-bot moved this from Waiting for Author to Merge to Recently Merged in New Pull Request Status Board Aug 27, 2021

typescript-bot merged commit 07e9b3d into DefinitelyTyped:master Aug 27, 2021

favna deleted the feat/add-stream-consumers-to-node branch August 27, 2021 18:01

typescript-bot removed this from Recently Merged in New Pull Request Status Board Aug 27, 2021

alecgibson reviewed Jan 19, 2022

View reviewed changes

alecgibson mentioned this pull request Jan 19, 2022

[superagent] Fix Blob error #58312

Merged

8 tasks

Gozala mentioned this pull request Mar 1, 2022

fix: Blob.stream return type which is incorrect and conflicts with lib.dom.d.ts #59057

Closed

17 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(@types/node): add `stream/consumers` #55311

feat(@types/node): add `stream/consumers` #55311

favna commented Aug 22, 2021 •

edited

typescript-bot commented Aug 22, 2021 •

edited

typescript-bot commented Aug 22, 2021

typescript-bot commented Aug 22, 2021

typescript-bot commented Aug 23, 2021

This comment has been minimized.

typescript-bot commented Aug 23, 2021

SimonSchick commented Aug 23, 2021

This comment has been minimized.

SimonSchick commented Aug 23, 2021

typescript-bot commented Aug 26, 2021

favna commented Aug 26, 2021

LinusU left a comment

favna commented Aug 27, 2021

typescript-bot commented Aug 27, 2021

alecgibson Jan 19, 2022

thw0rted Apr 14, 2022

thw0rted Apr 14, 2022

favna Apr 14, 2022 •

edited

thw0rted Apr 15, 2022

feat(@types/node): add stream/consumers #55311

feat(@types/node): add stream/consumers #55311

Conversation

favna commented Aug 22, 2021 • edited

typescript-bot commented Aug 22, 2021 • edited

1 package in this PR

Code Reviews

Status

typescript-bot commented Aug 22, 2021

typescript-bot commented Aug 22, 2021

typescript-bot commented Aug 23, 2021

This comment has been minimized.

typescript-bot commented Aug 23, 2021

SimonSchick commented Aug 23, 2021

This comment has been minimized.

SimonSchick commented Aug 23, 2021

typescript-bot commented Aug 26, 2021

favna commented Aug 26, 2021

LinusU left a comment

Choose a reason for hiding this comment

favna commented Aug 27, 2021

typescript-bot commented Aug 27, 2021

alecgibson Jan 19, 2022

Choose a reason for hiding this comment

thw0rted Apr 14, 2022

Choose a reason for hiding this comment

thw0rted Apr 14, 2022

Choose a reason for hiding this comment

favna Apr 14, 2022 • edited

Choose a reason for hiding this comment

thw0rted Apr 15, 2022

Choose a reason for hiding this comment

feat(@types/node): add `stream/consumers` #55311

feat(@types/node): add `stream/consumers` #55311

favna commented Aug 22, 2021 •

edited

typescript-bot commented Aug 22, 2021 •

edited

favna Apr 14, 2022 •

edited