Skip to content

feat(node/stream): add 2nd options param to Readable.toWeb() - #71851

Merged
weswigham merged 1 commit into
DefinitelyTyped:masterfrom
hkleungai:node-stream-Readable-toWeb-update
Feb 13, 2025
Merged

weswigham merged 1 commit into
DefinitelyTyped:masterfrom
hkleungai:node-stream-Readable-toWeb-update

Conversation

@hkleungai

@hkleungai hkleungai commented Feb 5, 2025

Copy link
Copy Markdown
Contributor

Resolves #71828.

Resolves #71850

Also see my another PR on node repo, nodejs/node#56928.


Please fill in this template.

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: <>

@typescript-bot

typescript-bot commented Feb 5, 2025

Copy link
Copy Markdown
Contributor

@hkleungai Thank you for submitting this PR!

This is a live comment that I will keep updated.

1 package in this PR

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

Once every item on this list is checked, I'll ask you for permission to merge and publish the changes.

Inactive

This PR has been inactive for 7 days.


Diagnostic Information: What the bot saw about this PR
{
  "type": "info",
  "now": "-",
  "pr_number": 71851,
  "author": "hkleungai",
  "headCommitOid": "1ef0a94cfe47c964d78f79a83c6715fba47a726a",
  "mergeBaseOid": "58c738b2fd0d7550861c0422ebcaa022890841ea",
  "lastPushDate": "2025-02-05T19:31:08.000Z",
  "lastActivityDate": "2025-02-08T18:35:48.000Z",
  "hasMergeConflict": false,
  "isFirstContribution": false,
  "tooManyFiles": false,
  "hugeChange": false,
  "popularityLevel": "Critical",
  "pkgInfo": [
    {
      "name": "node",
      "kind": "edit",
      "files": [
        {
          "path": "types/node/stream.d.ts",
          "kind": "definition"
        },
        {
          "path": "types/node/test/stream.ts",
          "kind": "test"
        },
        {
          "path": "types/node/v18/stream.d.ts",
          "kind": "definition"
        },
        {
          "path": "types/node/v18/test/stream.ts",
          "kind": "test"
        },
        {
          "path": "types/node/v20/stream.d.ts",
          "kind": "definition"
        },
        {
          "path": "types/node/v20/test/stream.ts",
          "kind": "test"
        }
      ],
      "owners": [
        "Microsoft",
        "jkomyno",
        "alvis",
        "r3nya",
        "btoueg",
        "smac89",
        "touffy",
        "DeividasBakanas",
        "eyqs",
        "Hannes-Magnusson-CK",
        "hoo29",
        "kjin",
        "ajafff",
        "islishude",
        "mwiktorczyk",
        "mohsen1",
        "galkin",
        "parambirs",
        "eps1lon",
        "ThomasdenH",
        "WilcoBakker",
        "wwwy3y3",
        "samuela",
        "kuehlein",
        "bhongy",
        "chyzwar",
        "trivikr",
        "yoursunny",
        "qwelias",
        "ExE-Boss",
        "peterblazejewicz",
        "addaleax",
        "victorperin",
        "NodeJS",
        "LinusU",
        "wafuwafu13",
        "mcollina",
        "Semigradsky"
      ],
      "addedOwners": [],
      "deletedOwners": [],
      "popularityLevel": "Critical"
    }
  ],
  "reviews": [
    {
      "type": "stale",
      "reviewer": "Renegade334",
      "date": "2025-02-08T15:10:14.000Z",
      "abbrOid": "d2cd400"
    }
  ],
  "mainBotCommentID": 2637851070,
  "ciResult": "pass"
}

@typescript-bot

Copy link
Copy Markdown
Contributor

Comment thread types/node/stream.d.ts Outdated
@Renegade334

Copy link
Copy Markdown
Contributor

It looks like someone converted the discussion to an issue – would you mind linking this to resolve #71850?

@hkleungai
hkleungai force-pushed the node-stream-Readable-toWeb-update branch from d2cd400 to 1ef0a94 Compare February 8, 2025 18:32
@hkleungai

Copy link
Copy Markdown
Contributor Author

Seems I cannot link the issue by myself.

Screenshot 2025-02-09 at 2 34 42 AM

@Renegade334 Renegade334 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👍

Seems I cannot link the issue by myself.

Adding Resolves #xxxx to the PR description should do the trick.

@typescript-bot

Copy link
Copy Markdown
Contributor

@Renegade334 Thank you for reviewing this PR! The author has pushed new commits since your last review. Could you take another look and submit a fresh review?

@weswigham
weswigham merged commit 880d42b into DefinitelyTyped:master Feb 13, 2025
@hkleungai
hkleungai deleted the node-stream-Readable-toWeb-update branch February 14, 2025 01:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[@types/node] v22.13.1 missing 2nd optional parameter for Readable.toWeb() function

4 participants