feat(Client): add typed wrapper methods for EventEmitter.{on,once}
#10581
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
So...
Historically, the discord.js typings have overloaded
EventEmitter.on()
andEventEmitter.once()
with signatures that return typed tuples from ClientEvents, if the target is a client object and the event is in ClientEvents. This allowed users to, for example, do things likeconst [ role ] = await EventEmitter.once(client, 'roleCreate')
and have the yielded values be automatically typed without needing any manual coercion.These overloads had to be removed, due to TypeScript patching out the bug that was being exploited to add them to EventEmitter itself. They were moved downstream to
Client.{on,once}
in #10360 as a temporising measure, but the overload signatures themselves remained the same.Unfortunately,
EventEmitter.on()
returns an async iterator.Iterators are in a state of heavy flux within the TS library at the moment, and hence also within @types/node, which recently changed the types of its returned iterators across the entire library. These were formerly
[Async]IterableIterator
s, but are now internal interfaces which extend TypeScript's[Async]IteratorObject
in TS 5.6+.Since those interfaces change shape depending on which TypeScript libs are loaded, this has the potential to cause subtle assignability errors with the Client overloads (eg. #10577) that are conditional on both TS and @types/node versions and also on which libs are loaded, and are therefore not picked up within the library's build tests. Even if the Client overloads are fixed now, they're going to be vulnerable to further breakage in the future if those interfaces continue to evolve.
However, there is a different approach entirely: declare new client instance methods, which wrap the EventEmitter static methods.
Instead of playing with EventEmitter definitions, wrapper methods are declared on the BaseClient prototype (naming up for discussion):
.eventIterator(eventName[, options])
as a wrapper forEventEmitter.on(client, eventName[, options])
.awaitEvent(eventName[, options])
as a wrapper forEventEmitter.once(client, eventName[, options])
This has a number of advantages:
client.awaitEvent(event)
rather thanEventEmitter.once(client, event)
.Promise
andAsyncIterableIterator
).Removing the client-specific
Client.{on,once}
overload definitions from the .d.ts won't break any existing builds, since the only potential change from reverting to the native @types/node signatures is to widen the types of the yielded/resolved arrays (from typed tuples toany[]
).Resolves: #10577
Status and versioning classification: