Skip to content

feat(Collector): add iterators and getters #9165

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

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
225 changes: 220 additions & 5 deletions packages/discord.js/src/structures/interfaces/Collector.js
Original file line number Diff line number Diff line change
Expand Up @@ -157,7 +157,7 @@ class Collector extends EventEmitter {
}

/**
* Returns a promise that resolves with the next collected element;
* Returns a promise that resolves with the next collected, disposed, or ignored elements;
* rejects with collected elements if the collector finishes without receiving a next element
* @type {Promise}
* @readonly
Expand All @@ -171,12 +171,59 @@ class Collector extends EventEmitter {

const cleanup = () => {
this.removeListener('collect', onCollect);
this.removeListener('dispose', onDispose);
this.removeListener('ignore', onIgnore);
this.removeListener('end', onEnd);
};

const onCollect = item => {
const onCollect = (...items) => {
cleanup();
resolve(item);
resolve(['collecting', ...items]);
};

const onDispose = (...items) => {
cleanup();
resolve(['disposing', ...items]);
};

const onIgnore = (...items) => {
cleanup();
resolve(['ignoring', ...items]);
};

const onEnd = () => {
cleanup();
reject(this.collected);
};

this.on('collect', onCollect);
this.on('dispose', onDispose);
this.on('ignore', onIgnore);
this.on('end', onEnd);
});
}

/**
* Returns a promise that resolves with the next collected elements;
* rejects with collected elements if the collector finishes without receiving a next element
* @type {Promise}
* @readonly
*/
get nextCollecting() {
Copy link
Member

Choose a reason for hiding this comment

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

I feel like we shouldn't have async getters. What about waitForCollect()? Same for the other methods.

I also think that it'd be useful to accept an AbortSignal so we can cancel and unregister the listeners earlier.

return new Promise((resolve, reject) => {
if (this.ended) {
reject(this.collected);
return;
}

const cleanup = () => {
this.removeListener('collect', onCollect);
this.removeListener('end', onEnd);
};

const onCollect = (...items) => {
cleanup();
resolve(items);
};

const onEnd = () => {
Expand All @@ -189,6 +236,72 @@ class Collector extends EventEmitter {
});
}

/**
* Returns a promise that resolves with the next disposed elements;
* rejects with collected elements if the collector finishes without receiving a next element
* @type {Promise}
* @readonly
*/
get nextDisposing() {
return new Promise((resolve, reject) => {
if (this.ended) {
reject(this.collected);
return;
}

const cleanup = () => {
this.removeListener('dispose', onDispose);
this.removeListener('end', onEnd);
};

const onDispose = (...items) => {
cleanup();
resolve(items);
};

const onEnd = () => {
cleanup();
reject(this.collected);
};

this.on('dispose', onDispose);
this.on('end', onEnd);
});
}

/**
* Returns a promise that resolves with the next ignored elements;
* rejects with collected elements if the collector finishes without receiving a next element
* @type {Promise}
* @readonly
*/
get nextIgnoring() {
return new Promise((resolve, reject) => {
if (this.ended) {
reject(this.collected);
return;
}

const cleanup = () => {
this.removeListener('ignore', onIgnore);
this.removeListener('end', onEnd);
};

const onIgnore = (...items) => {
cleanup();
resolve(items);
};

const onEnd = () => {
cleanup();
reject(this.collected);
};

this.on('ignore', onIgnore);
this.on('end', onEnd);
});
}

/**
* Stops this collector and emits the `end` event.
* @param {string} [reason='user'] The reason this collector is ending
Expand Down Expand Up @@ -251,13 +364,17 @@ class Collector extends EventEmitter {
}

/**
* Allows collectors to be consumed with for-await-of loops
* Allows collectors to be consumed with for-await-of loop for collected, disposed, and ignored elements
* @see {@link https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Statements/for-await...of}
*/
async *[Symbol.asyncIterator]() {
const queue = [];
const onCollect = (...item) => queue.push(item);
const onCollect = (...items) => queue.push(['collecting', ...items]);
const onDispose = (...items) => queue.push(['disposing', ...items]);
const onIgnore = (...items) => queue.push(['ignoring', ...items]);
this.on('collect', onCollect);
this.on('dispose', onDispose);
this.on('ignore', onIgnore);

try {
while (queue.length || !this.ended) {
Expand All @@ -268,16 +385,114 @@ class Collector extends EventEmitter {
await new Promise(resolve => {
const tick = () => {
this.removeListener('collect', tick);
this.removeListener('dispose', tick);
this.removeListener('end', tick);
Comment on lines 387 to 389
Copy link
Member

Choose a reason for hiding this comment

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

This isn't removing the ignore listener that's registering in line 394.

return resolve();
};
this.on('collect', tick);
this.on('dispose', tick);
this.on('ignore', tick);
this.on('end', tick);
});
}
}
} finally {
this.removeListener('collect', onCollect);
this.removeListener('dispose', onDispose);
this.removeListener('ignore', onIgnore);
}
}

/**
* Allows collectors to be consumed with for-await-of loop for collected elements
* @see {@link https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Statements/for-await...of}
*/
async *collectings() {
const queue = [];
const onCollect = (...items) => queue.push(items);
this.on('collect', onCollect);

try {
while (queue.length || !this.ended) {
if (queue.length) {
yield queue.shift();
} else {
// eslint-disable-next-line no-await-in-loop
await new Promise(resolve => {
const tick = () => {
this.removeListener('collect', tick);
this.removeListener('end', tick);
return resolve();
};
this.on('collect', tick);
this.on('end', tick);
});
}
}
} finally {
this.removeListener('collect', onCollect);
}
}

/**
* Allows collectors to be consumed with for-await-of loop for disposed elements
* @see {@link https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Statements/for-await...of}
*/
async *disposings() {
const queue = [];
const onDispose = (...items) => queue.push(items);
this.on('dispose', onDispose);

try {
while (queue.length || !this.ended) {
if (queue.length) {
yield queue.shift();
} else {
// eslint-disable-next-line no-await-in-loop
await new Promise(resolve => {
const tick = () => {
this.removeListener('dispose', tick);
this.removeListener('end', tick);
return resolve();
};
this.on('dispose', tick);
this.on('end', tick);
});
}
}
} finally {
this.removeListener('dispose', onDispose);
}
}

/**
* Allows collectors to be consumed with for-await-of loop for ignored elements
* @see {@link https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Statements/for-await...of}
*/
async *ignorings() {
const queue = [];
const onIgnore = (...items) => queue.push(items);
this.on('ignore', onIgnore);

try {
while (queue.length || !this.ended) {
if (queue.length) {
yield queue.shift();
} else {
// eslint-disable-next-line no-await-in-loop
await new Promise(resolve => {
const tick = () => {
this.removeListener('ignore', tick);
this.removeListener('end', tick);
return resolve();
};
this.on('ignore', tick);
this.on('end', tick);
});
}
}
} finally {
this.removeListener('ignore', onIgnore);
}
}

Expand Down
12 changes: 10 additions & 2 deletions packages/discord.js/typings/index.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1040,6 +1040,8 @@ export interface CollectorEventTypes<K, V, F extends unknown[] = []> {
end: [collected: Collection<K, V>, reason: string];
}

export type CollectorEventType = 'collecting' | 'disposing' | 'ignoring';

export abstract class Collector<K, V, F extends unknown[] = []> extends EventEmitter {
protected constructor(client: Client<true>, options?: CollectorOptions<[V, ...F]>);
private _timeout: NodeJS.Timeout | null;
Expand All @@ -1051,14 +1053,20 @@ export abstract class Collector<K, V, F extends unknown[] = []> extends EventEmi
public ended: boolean;
public get endReason(): string | null;
public filter: CollectorFilter<[V, ...F]>;
public get next(): Promise<V>;
public get next(): Promise<[CollectorEventType, V, ...F]>;
public get nextCollecting(): Promise<[V, ...F]>;
public get nextDisposing(): Promise<[V, ...F]>;
public get nextIgnoring(): Promise<[V, ...F]>;
public options: CollectorOptions<[V, ...F]>;
public checkEnd(): boolean;
public collectings(): AsyncIterableIterator<[V, ...F]>;
public disposings(): AsyncIterableIterator<[V, ...F]>;
public handleCollect(...args: unknown[]): Promise<void>;
public handleDispose(...args: unknown[]): Promise<void>;
public ignorings(): AsyncIterableIterator<[V, ...F]>;
public stop(reason?: string): void;
public resetTimer(options?: CollectorResetTimerOptions): void;
public [Symbol.asyncIterator](): AsyncIterableIterator<[V, ...F]>;
public [Symbol.asyncIterator](): AsyncIterableIterator<[CollectorEventType, V, ...F]>;
public toJSON(): unknown;

protected listener: (...args: any[]) => void;
Expand Down
37 changes: 37 additions & 0 deletions packages/discord.js/typings/index.test-d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -160,6 +160,7 @@ import {
PublicThreadChannel,
GuildMemberManager,
GuildMemberFlagsBitField,
CollectorEventType,
} from '.';
import { expectAssignable, expectNotAssignable, expectNotType, expectType } from 'tsd';
import type { ContextMenuCommandBuilder, SlashCommandBuilder } from '@discordjs/builders';
Expand Down Expand Up @@ -1311,6 +1312,18 @@ messageCollector.on('collect', (...args) => {

(async () => {
for await (const value of messageCollector) {
expectType<[CollectorEventType, Message<boolean>, Collection<Snowflake, Message>]>(value);
}

for await (const value of messageCollector.collectings()) {
expectType<[Message<boolean>, Collection<Snowflake, Message>]>(value);
}

for await (const value of messageCollector.disposings()) {
expectType<[Message<boolean>, Collection<Snowflake, Message>]>(value);
}

for await (const value of messageCollector.ignorings()) {
expectType<[Message<boolean>, Collection<Snowflake, Message>]>(value);
}
})();
Expand All @@ -1322,6 +1335,18 @@ reactionCollector.on('dispose', (...args) => {

(async () => {
for await (const value of reactionCollector) {
expectType<[CollectorEventType, MessageReaction, User]>(value);
}

for await (const value of reactionCollector.collectings()) {
expectType<[MessageReaction, User]>(value);
}

for await (const value of reactionCollector.disposings()) {
expectType<[MessageReaction, User]>(value);
}

for await (const value of reactionCollector.ignorings()) {
expectType<[MessageReaction, User]>(value);
}
})();
Expand Down Expand Up @@ -1916,6 +1941,18 @@ collector.on('end', (collection, reason) => {

(async () => {
for await (const value of collector) {
expectType<[CollectorEventType, Interaction, ...string[]]>(value);
}

for await (const value of collector.collectings()) {
expectType<[Interaction, ...string[]]>(value);
}

for await (const value of collector.disposings()) {
expectType<[Interaction, ...string[]]>(value);
}

for await (const value of collector.ignorings()) {
expectType<[Interaction, ...string[]]>(value);
}
})();
Expand Down