Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

node: WriteStream and ReadStream Types much more concise and less error-prone now. #71520

Merged
merged 2 commits into from
Jan 14, 2025

Conversation

chwoerz
Copy link
Contributor

@chwoerz chwoerz commented Dec 28, 2024

I just updated how the existing definition is provided. I did not change any API.

This way no copying of the events like "read", "ready" and so one is required.

There now exist two new types for ReadStreamEvents and WriteStreamEvents. These are reusable for addListener, on, once, prependListener and prependOnceListener. Before all the events where copied for all these functions.

@typescript-bot
Copy link
Contributor

typescript-bot commented Dec 28, 2024

@chwoerz Thank you for submitting this PR! I see this is your first time submitting to DefinitelyTyped 👋 — I'm the local bot who will help you through the process of getting things through.

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

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": 71520,
  "author": "chwoerz",
  "headCommitOid": "027e31be8f8d4af30c374e29daa5112f09df3573",
  "mergeBaseOid": "bf3d8cb5de3e6909e881bf3c0214c925c600292c",
  "lastPushDate": "2024-12-28T21:38:18.000Z",
  "lastActivityDate": "2025-01-14T18:16:07.000Z",
  "mergeOfferDate": "2025-01-14T17:38:16.000Z",
  "mergeRequestDate": "2025-01-14T18:16:07.000Z",
  "mergeRequestUser": "chwoerz",
  "hasMergeConflict": false,
  "isFirstContribution": true,
  "tooManyFiles": false,
  "hugeChange": false,
  "popularityLevel": "Critical",
  "pkgInfo": [
    {
      "name": "node",
      "kind": "edit",
      "files": [
        {
          "path": "types/node/v20/fs.d.ts",
          "kind": "definition"
        },
        {
          "path": "types/node/v20/test/fs.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": "approved",
      "reviewer": "RyanCavanaugh",
      "date": "2025-01-14T17:37:35.000Z",
      "isMaintainer": true
    }
  ],
  "mainBotCommentID": 2564487723,
  "ciResult": "pass"
}

@typescript-bot typescript-bot added the Unreviewed No one showed up to review this PR, so it'll be reviewed by a DT maintainer. label Jan 9, 2025
@typescript-bot
Copy link
Contributor

@typescript-bot typescript-bot added Maintainer Approved Self Merge This PR can now be self-merged by the PR author or an owner and removed Unreviewed No one showed up to review this PR, so it'll be reviewed by a DT maintainer. labels Jan 14, 2025
@chwoerz
Copy link
Contributor Author

chwoerz commented Jan 14, 2025

Ready to merge

@typescript-bot typescript-bot merged commit 5a1a97f into DefinitelyTyped:master Jan 14, 2025
4 checks passed
@pardeep889
Copy link

This PR is makes a lot of sense

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Critical package Maintainer Approved Self Merge This PR can now be self-merged by the PR author or an owner
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants