@ckohen Thank you for submitting this PR!

This is a live comment that I will keep updated.

15 packages in this PR

• airbnb__node-memwatch — on npm, on unpkg
• hubot — on npm, on unpkg
• imap — on npm, on unpkg
• jake — on npm, on unpkg
• newman — on npm, on unpkg
• node-red — on npm, on unpkg
• node — on npm, on unpkg
• opossum — on npm, on unpkg
• rdf-store-fs — on npm, on unpkg
• readable-stream — on npm, on unpkg
• sane — on npm, on unpkg
• sse — on npm, on unpkg
• steam — on npm, on unpkg
• twitter — on npm, on unpkg
• xml-flow — on npm, on unpkg

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
• 🕐 A DT maintainer needs to approve changes that affect more than one package

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 13 days.

{
  "type": "info",
  "now": "-",
  "pr_number": 71060,
  "author": "ckohen",
  "headCommitOid": "08adb73dfaa313904fc066a2de9f06b7506116b0",
  "mergeBaseOid": "5ade627d2a6d5054327237fa3fee8bd07636f797",
  "lastPushDate": "2024-11-01T02:25:45.000Z",
  "lastActivityDate": "2025-09-30T08:45:59.000Z",
  "hasMergeConflict": true,
  "isFirstContribution": false,
  "tooManyFiles": false,
  "hugeChange": false,
  "popularityLevel": "Critical",
  "pkgInfo": [
    {
      "name": "airbnb__node-memwatch",
      "kind": "edit",
      "files": [
        {
          "path": "types/airbnb__node-memwatch/airbnb__node-memwatch-tests.ts",
          "kind": "test"
        }
      ],
      "owners": [
        "tylerlevine"
      ],
      "addedOwners": [],
      "deletedOwners": [],
      "popularityLevel": "Well-liked by everyone"
    },
    {
      "name": "hubot",
      "kind": "edit",
      "files": [
        {
          "path": "types/hubot/hubot-tests.ts",
          "kind": "test"
        }
      ],
      "owners": [
        "dirk",
        "KeesCBakker",
        "eeemil",
        "jphenow"
      ],
      "addedOwners": [],
      "deletedOwners": [],
      "popularityLevel": "Well-liked by everyone"
    },
    {
      "name": "imap",
      "kind": "edit",
      "files": [
        {
          "path": "types/imap/index.d.ts",
          "kind": "definition"
        }
      ],
      "owners": [
        "psnider"
      ],
      "addedOwners": [],
      "deletedOwners": [],
      "popularityLevel": "Popular"
    },
    {
      "name": "jake",
      "kind": "edit",
      "files": [
        {
          "path": "types/jake/index.d.ts",
          "kind": "definition"
        }
      ],
      "owners": [],
      "addedOwners": [],
      "deletedOwners": [],
      "popularityLevel": "Well-liked by everyone"
    },
    {
      "name": "newman",
      "kind": "edit",
      "files": [
        {
          "path": "types/newman/newman-tests.ts",
          "kind": "test"
        }
      ],
      "owners": [
        "LogvinovLeon"
      ],
      "addedOwners": [],
      "deletedOwners": [],
      "popularityLevel": "Well-liked by everyone"
    },
    {
      "name": "node-red",
      "kind": "edit",
      "files": [
        {
          "path": "types/node-red/node-red-tests.ts",
          "kind": "test"
        }
      ],
      "owners": [
        "andersea",
        "tbowmo",
        "bernardobelchior",
        "alexk111",
        "Shaquu"
      ],
      "addedOwners": [],
      "deletedOwners": [],
      "popularityLevel": "Well-liked by everyone"
    },
    {
      "name": "node",
      "kind": "edit",
      "files": [
        {
          "path": "types/node/events.d.ts",
          "kind": "definition"
        },
        {
          "path": "types/node/test/events.ts",
          "kind": "test"
        },
        {
          "path": "types/node/test/events_generic.ts",
          "kind": "test"
        },
        {
          "path": "types/node/v18/events.d.ts",
          "kind": "definition"
        },
        {
          "path": "types/node/v18/test/events.ts",
          "kind": "test"
        },
        {
          "path": "types/node/v18/test/events_generic.ts",
          "kind": "test"
        },
        {
          "path": "types/node/v20/events.d.ts",
          "kind": "definition"
        },
        {
          "path": "types/node/v20/test/events.ts",
          "kind": "test"
        },
        {
          "path": "types/node/v20/test/events_generic.ts",
          "kind": "test"
        },
        {
          "path": "types/node/v22/events.d.ts",
          "kind": "definition"
        },
        {
          "path": "types/node/v22/test/events.ts",
          "kind": "test"
        },
        {
          "path": "types/node/v22/test/events_generic.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",
        "Renegade334",
        "anonrig"
      ],
      "addedOwners": [],
      "deletedOwners": [],
      "popularityLevel": "Critical"
    },
    {
      "name": "opossum",
      "kind": "edit",
      "files": [
        {
          "path": "types/opossum/opossum-tests.ts",
          "kind": "test"
        }
      ],
      "owners": [
        "quinnlangille",
        "merufm",
        "lance",
        "mastermatt",
        "tjenkinson"
      ],
      "addedOwners": [],
      "deletedOwners": [],
      "popularityLevel": "Popular"
    },
    {
      "name": "rdf-store-fs",
      "kind": "edit",
      "files": [
        {
          "path": "types/rdf-store-fs/rdf-store-fs-tests.ts",
          "kind": "test"
        }
      ],
      "owners": [
        "tpluscode"
      ],
      "addedOwners": [],
      "deletedOwners": [],
      "popularityLevel": "Well-liked by everyone"
    },
    {
      "name": "readable-stream",
      "kind": "edit",
      "files": [
        {
          "path": "types/readable-stream/index.d.ts",
          "kind": "definition"
        }
      ],
      "owners": [
        "TeamworkGuy2",
        "markdreyer",
        "mcollina"
      ],
      "addedOwners": [],
      "deletedOwners": [],
      "popularityLevel": "Critical"
    },
    {
      "name": "sane",
      "kind": "edit",
      "files": [
        {
          "path": "types/sane/index.d.ts",
          "kind": "definition"
        }
      ],
      "owners": [
        "BendingBender"
      ],
      "addedOwners": [],
      "deletedOwners": [],
      "popularityLevel": "Well-liked by everyone"
    },
    {
      "name": "sse",
      "kind": "edit",
      "files": [
        {
          "path": "types/sse/sse-tests.ts",
          "kind": "test"
        }
      ],
      "owners": [
        "yutak23"
      ],
      "addedOwners": [],
      "deletedOwners": [],
      "popularityLevel": "Well-liked by everyone"
    },
    {
      "name": "steam",
      "kind": "edit",
      "files": [
        {
          "path": "types/steam/index.d.ts",
          "kind": "definition"
        }
      ],
      "owners": [
        "kant2002"
      ],
      "addedOwners": [],
      "deletedOwners": [],
      "popularityLevel": "Well-liked by everyone"
    },
    {
      "name": "twitter",
      "kind": "edit",
      "files": [
        {
          "path": "types/twitter/twitter-tests.ts",
          "kind": "test"
        }
      ],
      "owners": [
        "BendingBender"
      ],
      "addedOwners": [],
      "deletedOwners": [],
      "popularityLevel": "Well-liked by everyone"
    },
    {
      "name": "xml-flow",
      "kind": "edit",
      "files": [
        {
          "path": "types/xml-flow/xml-flow-tests.ts",
          "kind": "test"
        }
      ],
      "owners": [
        "Warerebel"
      ],
      "addedOwners": [],
      "deletedOwners": [],
      "popularityLevel": "Well-liked by everyone"
    }
  ],
  "reviews": [
    {
      "type": "stale",
      "reviewer": "Bashamega",
      "date": "2025-04-23T13:08:01.000Z",
      "abbrOid": "b56a3af"
    },
    {
      "type": "stale",
      "reviewer": "Renegade334",
      "date": "2025-04-04T11:05:06.000Z",
      "abbrOid": "b56a3af"
    },
    {
      "type": "stale",
      "reviewer": "jakebailey",
      "date": "2025-01-30T20:06:15.000Z",
      "abbrOid": "c02af4b"
    }
  ],
  "mainBotCommentID": 2451175166,
  "ciResult": "pass"
}

⚠️ There are too many reviewers for this PR change (64). Merging can only be handled by a DT maintainer.

@ckohen 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 that are failing do not end up on the list of PRs for the DT maintainers to review.

@ckohen 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 that are failing do not end up on the list of PRs for the DT maintainers to review.

Note a separate reported issue, #71059.

That is not the only root cause, and has nothing to do with the this type. This is observable without classes

type Foo<T> = {
    callable<K extends T>(str: K): number;
}

type Bar<T> = {
    callable<K extends T>(str: K): string;
}

type FooBar = Foo<string> | Bar<'abc'>;
declare const obj2: FooBar;
obj2.callable('abc'); // TS2349

It does work with the single definition, but that defeats the purpose of this PR.
The second overload allows IntelliSense to suggest valid events which is a critical piece of replacing the existing tooling for adding eventmaps. Without this, I expect to see very little adoption of native eventmaps which makes it harder on the compiler for no reason.

Re-ping @psnider, @LogvinovLeon, @andersea, @tbowmo, @bernardobelchior, @alexk111, @Shaquu, @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, @ZYSzys, @nodejs, @LinusU, @wafuwafu13, @mcollina, @Semigradsky, @quinnlangille, @MeruFM, @lance, @mastermatt, @tjenkinson, @tpluscode, @TeamworkGuy2, @markdreyer, @BendingBender, @yutak23, @kant2002, @drinchev, @mlamp, @trodi, @emmanuelgautier, @Warerebel:

This PR has been out for over a week, yet I haven't seen any reviews.

Could someone please give it some attention? Thanks!

It has been more than two weeks and this PR still has no reviews.

I'll bump it to the DT maintainer queue. Thank you for your patience, @ckohen.

(Ping @tylerlevine, @dirk, @KeesCBakker, @Eeemil, @jphenow, @psnider, @LogvinovLeon, @andersea, @tbowmo, @bernardobelchior, @alexk111, @Shaquu, @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, @Renegade334, @anonrig, @quinnlangille, @merufm, @lance, @mastermatt, @tjenkinson, @tpluscode, @TeamworkGuy2, @markdreyer, @BendingBender, @yutak23, @kant2002, @Warerebel.)

Given this is back on the table, does the PR contain tests from all of the breaks from last time to make sure that we aren't regressing anything a second time?

This PR does not contain the changes that caused the major issues with outdated targets, they will be added in a separate PR so that this cleanup can be done and then #70635 can land. Tbh I don't know how it would even be possible to test for having no es private properties anywhere other than attempting to compile to all possible targets (bigger question about when targets can be dropped from the support list, especially when the target should be strictly defined and matched to the version of node types you used).

The only thing left is the assignability issue (#71059). I have been thinking about this but have been very busy the last couple of weeks.

As outlined above #71060 (comment), the current behavior is very counter-intuitive and seems like a bug to me (should be an intersection of the types, but - maybe because type arguments don't play nice apparently - it seems to want the params to be bivariant which makes the intended effect of the simplistic example practically impossible)

The only thing left is the assignability issue (#71059).

A bit of digging relates this to microsoft/TypeScript#29011. Essentially, that patch introduced a fallback in getUnionSignatures() for resolving a single "hybrid" call signature from a union of dissimilar signatures, but it has constraints regarding signatures with type parameters:

interface EventMap {
	foo: [bar: string];
}

type Listener<Event extends string | symbol> = (...args: Event extends keyof EventMap ? EventMap[Event] : any[]) => void;

interface Emitter {
	method1<Event extends string | symbol>(event: Event, listener: Listener<Event>): this;

	method2<Event extends keyof EventMap>(event: Event, listener: Listener<Event>): this;
	method2<Event extends string | symbol>(event: Event, listener: Listener<Event>): this;
}

// Declare a union of non-mutually-assignable emitters
declare const union: (Emitter & { n: 1 }) | (Emitter & { n: 2 });

union.method1('blah', () => {});
union.method2('blah', () => {}); // error

In both of these method calls, findMatchingSignatures() fails to find any valid signatures. However, in the first instance, getUnionSignatures() uses fallback union signature resolution to create a valid merged signature, whereas the second instance isn't eligible for this, as more than one of the union members has more than one signature with type parameters.

I can't really see a way to get around this. If this paradigm is kept, it will cause errors for calls to affected EventEmitter methods on any unions of emitters, even where event maps aren't used at all. This doesn't appear to be too rare a usage case – it's present in high-exposure libraries like vite and hono, for example.

It seems this PR is illustrating some of the major shortcomings (and tbh bugs) of typescript.

You neglected the assignability issue that applies to instances that have generics set which has nothing to do with what you've mentioned. I have an idea to fix what you've described (remove the generic on the second overload) but it will not fix the assignability issue completely due to the other bug I mentioned. Will update when I know if it works.

Update: removing the generic negates the purpose since it then just allows any params all the time. Did find what I was looking for in string & {} though. It allows intellisense to work without needing a second overload (this trick was suggested here and works great. No side effects were mentioned and I haven't observed any). Pushing this change now to see if there's any issues with it.

(For the Listener-returning methods I went with the remove overload method since that makes the return types happier and it doesn't affect the passed params)

@ckohen 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 that are failing do not end up on the list of PRs for the DT maintainers to review.

And....that broke everything. Every single failed test illustrates exactly why this needed to be done natively instead of the current fragmented ecosystem. I'll revert this tomorrow so that people have a chance to look before it disappears from existence

@ckohen I haven't seen any activity on this PR in more than three weeks, and it still has problems that prevent it from being merged. The PR will be closed on Jul 10th (in a week) if the issues aren't addressed.

Not yet, bot

I'm back in the rotation this week and this PR is still here; once again I ask if there's anything left here. ☹️

We are two months off Node.js v25, a non-LTS release. I wonder if it might be an idea to include this changeset in @types/node v25, and only v25, as a litmus test.

Although the more issues this work has highlighted, I'm more and more of the opinion that this should never have been a feature for @types/node in the first place, and would have been far better suited to userland 😄

It's kinda sucky either way. You really can't do this with native non-generic emitters well (and half the packages that do / try to miss out on important methods and never keep up with the changes). But this solution is still technically half-baked as it's missing static methods and has a few things that are technically incorrect but help with assignability that is necessary.

The main purpose that I had made this PR for was because the way discord.js did events (arguably as proper as possible) broke (cause it was technically a bug), buts since this didn't go as planned we've moved to https://www.npmjs.com/package/@vladfrangu/async_event_emitter which is a near 1:1 replication of the built in EE but built to support async and event contracts (which is part of why I slowed down on pushing this a bit ago).

This is no longer a huge priority for me, but I still kept working on it because so many packages have done this poorly and it's really frustrating to deal with that sometimes.

@ckohen 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!

@ckohen To keep things tidy, we have to close PRs that aren't mergeable and don't have activity in the last month. No worries, though — please open a new PR if you'd like to continue with this change. Thank you!

Why... Didn't the bot warn like it's supposed to....

FWIW, I'm looking at probably rolling this changeset into v25 as a major change.

26? I don't think we've ever done an odd node types

Used to be the case, but the "only-LTS" practice started around v14-v16ish, due essentially to the maintenance burden AFAICT. The desire from the community to access new features from the active release meant that these always ended up as a blur of (vN..vN+1) anyway, which isn't ideal for a supposed LTS branch.

Superseded by #73882