refactor(GuildMemberManager): Tidy up fetching guild members (#8972)

* refactor(GuildMemberManager): tidy up fetching guild members

* refactor: no destructure

* fix: throw `nonce` error correctly

* refactor: simplify `resolve()` with ternary

* refactor: prioritise `nonce` check

* fix: allow single user

* refactor: do not use `null`

This is not documented to request over the gateway.

* refactor: better name

* fix: extract correct property

packages/discord.js/src/managers/GuildMemberManager.js

@@ -159,20 +159,18 @@ class GuildMemberManager extends CachedManager {
   /**
    * Options used to fetch multiple members from a guild.
    * @typedef {Object} FetchMembersOptions
-   * @property {UserResolvable|UserResolvable[]} user The user(s) to fetch
-   * @property {?string} query Limit fetch to members with similar usernames
+   * @property {UserResolvable|UserResolvable[]} [user] The user(s) to fetch
+   * @property {?string} [query] Limit fetch to members with similar usernames
    * @property {number} [limit=0] Maximum number of members to request
-   * @property {boolean} [withPresences=false] Whether or not to include the presences
+   * @property {boolean} [withPresences=false] Whether to include the presences
    * @property {number} [time=120e3] Timeout for receipt of members
-   * @property {?string} nonce Nonce for this request (32 characters max - default to base 16 now timestamp)
-   * @property {boolean} [force=false] Whether to skip the cache check and request the API
+   * @property {?string} [nonce] Nonce for this request (32 characters max - default to base 16 now timestamp)
    */
 
   /**
-   * Fetches member(s) from Discord, even if they're offline.
-   * @param {UserResolvable|FetchMemberOptions|FetchMembersOptions} [options] If a UserResolvable, the user to fetch.
-   * If undefined, fetches all members.
-   * If a query, it limits the results to users with similar usernames.
+   * Fetches member(s) from a guild.
+   * @param {UserResolvable|FetchMemberOptions|FetchMembersOptions} [options] Options for fetching member(s).
+   * Omitting the parameter or providing `undefined` will fetch all members.
    * @returns {Promise<GuildMember|Collection<Snowflake, GuildMember>>}
    * @example
    * // Fetch all members from a guild
@@ -207,18 +205,70 @@ class GuildMemberManager extends CachedManager {
    */
   fetch(options) {
     if (!options) return this._fetchMany();
-    const user = this.client.users.resolveId(options);
-    if (user) return this._fetchSingle({ user, cache: true });
-    if (options.user) {
-      if (Array.isArray(options.user)) {
-        options.user = options.user.map(u => this.client.users.resolveId(u));
-        return this._fetchMany(options);
-      } else {
-        options.user = this.client.users.resolveId(options.user);
-      }
-      if (!options.limit && !options.withPresences) return this._fetchSingle(options);
+    const { user: users, limit, withPresences, cache, force } = options;
+    const resolvedUser = this.client.users.resolveId(users ?? options);
+    if (resolvedUser && !limit && !withPresences) return this._fetchSingle({ user: resolvedUser, cache, force });
+    const resolvedUsers = users?.map?.(user => this.client.users.resolveId(user)) ?? resolvedUser ?? undefined;
+    return this._fetchMany({ ...options, users: resolvedUsers });
+  }
+
+  async _fetchSingle({ user, cache, force = false }) {
+    if (!force) {
+      const existing = this.cache.get(user);
+      if (existing && !existing.partial) return existing;
     }
-    return this._fetchMany(options);
+
+    const data = await this.client.rest.get(Routes.guildMember(this.guild.id, user));
+    return this._add(data, cache);
+  }
+
+  _fetchMany({
+    limit = 0,
+    withPresences: presences,
+    users,
+    query,
+    time = 120e3,
+    nonce = DiscordSnowflake.generate().toString(),
+  } = {}) {
+    if (nonce.length > 32) return Promise.reject(new DiscordjsRangeError(ErrorCodes.MemberFetchNonceLength));
+
+    return new Promise((resolve, reject) => {
+      if (!query && !users) query = '';
+      this.guild.shard.send({
+        op: GatewayOpcodes.RequestGuildMembers,
+        d: {
+          guild_id: this.guild.id,
+          presences,
+          user_ids: users,
+          query,
+          nonce,
+          limit,
+        },
+      });
+      const fetchedMembers = new Collection();
+      let i = 0;
+      const handler = (members, _, chunk) => {
+        if (chunk.nonce !== nonce) return;
+        timeout.refresh();
+        i++;
+        for (const member of members.values()) {
+          fetchedMembers.set(member.id, member);
+        }
+        if (members.size < 1_000 || (limit && fetchedMembers.size >= limit) || i === chunk.count) {
+          clearTimeout(timeout);
+          this.client.removeListener(Events.GuildMembersChunk, handler);
+          this.client.decrementMaxListeners();
+          resolve(users && !Array.isArray(users) && fetchedMembers.size ? fetchedMembers.first() : fetchedMembers);
+        }
+      };
+      const timeout = setTimeout(() => {
+        this.client.removeListener(Events.GuildMembersChunk, handler);
+        this.client.decrementMaxListeners();
+        reject(new DiscordjsError(ErrorCodes.GuildMembersTimeout));
+      }, time).unref();
+      this.client.incrementMaxListeners();
+      this.client.on(Events.GuildMembersChunk, handler);
+    });
   }
 
   /**
@@ -485,66 +535,6 @@ class GuildMemberManager extends CachedManager {
 
     return this.resolve(user) ?? this.client.users.resolve(user) ?? userId;
   }
-
-  async _fetchSingle({ user, cache, force = false }) {
-    if (!force) {
-      const existing = this.cache.get(user);
-      if (existing && !existing.partial) return existing;
-    }
-
-    const data = await this.client.rest.get(Routes.guildMember(this.guild.id, user));
-    return this._add(data, cache);
-  }
-
-  _fetchMany({
-    limit = 0,
-    withPresences: presences = false,
-    user: user_ids,
-    query,
-    time = 120e3,
-    nonce = DiscordSnowflake.generate().toString(),
-  } = {}) {
-    return new Promise((resolve, reject) => {
-      if (!query && !user_ids) query = '';
-      if (nonce.length > 32) throw new DiscordjsRangeError(ErrorCodes.MemberFetchNonceLength);
-      this.guild.shard.send({
-        op: GatewayOpcodes.RequestGuildMembers,
-        d: {
-          guild_id: this.guild.id,
-          presences,
-          user_ids,
-          query,
-          nonce,
-          limit,
-        },
-      });
-      const fetchedMembers = new Collection();
-      let i = 0;
-      const handler = (members, _, chunk) => {
-        timeout.refresh();
-        if (chunk.nonce !== nonce) return;
-        i++;
-        for (const member of members.values()) {
-          fetchedMembers.set(member.id, member);
-        }
-        if (members.size < 1_000 || (limit && fetchedMembers.size >= limit) || i === chunk.count) {
-          clearTimeout(timeout);
-          this.client.removeListener(Events.GuildMembersChunk, handler);
-          this.client.decrementMaxListeners();
-          let fetched = fetchedMembers;
-          if (user_ids && !Array.isArray(user_ids) && fetched.size) fetched = fetched.first();
-          resolve(fetched);
-        }
-      };
-      const timeout = setTimeout(() => {
-        this.client.removeListener(Events.GuildMembersChunk, handler);
-        this.client.decrementMaxListeners();
-        reject(new DiscordjsError(ErrorCodes.GuildMembersTimeout));
-      }, time).unref();
-      this.client.incrementMaxListeners();
-      this.client.on(Events.GuildMembersChunk, handler);
-    });
-  }
 }
 
 module.exports = GuildMemberManager;

packages/discord.js/typings/index.d.ts

@@ -5167,7 +5167,6 @@ export interface FetchMembersOptions {
   withPresences?: boolean;
   time?: number;
   nonce?: string;
-  force?: boolean;
 }
 
 export interface FetchMessageOptions extends BaseFetchOptions {

packages/discord.js/typings/index.test-d.ts

@@ -158,6 +158,7 @@ import {
   AutoModerationRuleManager,
   PrivateThreadChannel,
   PublicThreadChannel,
+  GuildMemberManager,
   GuildMemberFlagsBitField,
 } from '.';
 import { expectAssignable, expectNotAssignable, expectNotType, expectType } from 'tsd';
@@ -1486,6 +1487,28 @@ declare const guildTextThreadManager: GuildTextThreadManager<
 >;
 expectType<TextChannel | NewsChannel>(guildTextThreadManager.channel);
 
+declare const guildMemberManager: GuildMemberManager;
+{
+  expectType<Promise<GuildMember>>(guildMemberManager.fetch('12345678901234567'));
+  expectType<Promise<GuildMember>>(guildMemberManager.fetch({ user: '12345678901234567' }));
+  expectType<Promise<GuildMember>>(guildMemberManager.fetch({ user: '12345678901234567', cache: true, force: false }));
+  expectType<Promise<GuildMember>>(guildMemberManager.fetch({ user: '12345678901234567', cache: true, force: false }));
+  expectType<Promise<Collection<Snowflake, GuildMember>>>(guildMemberManager.fetch());
+  expectType<Promise<Collection<Snowflake, GuildMember>>>(guildMemberManager.fetch({}));
+  expectType<Promise<Collection<Snowflake, GuildMember>>>(guildMemberManager.fetch({ user: ['12345678901234567'] }));
+  expectType<Promise<Collection<Snowflake, GuildMember>>>(guildMemberManager.fetch({ withPresences: false }));
+  expectType<Promise<GuildMember>>(guildMemberManager.fetch({ user: '12345678901234567', withPresences: true }));
+
+  expectType<Promise<Collection<Snowflake, GuildMember>>>(
+    guildMemberManager.fetch({ query: 'test', user: ['12345678901234567'], nonce: 'test' }),
+  );
+
+  // @ts-expect-error The cache & force options have no effect here.
+  guildMemberManager.fetch({ cache: true, force: false });
+  // @ts-expect-error The force option has no effect here.
+  guildMemberManager.fetch({ user: ['12345678901234567'], cache: true, force: false });
+}
+
 declare const messageManager: MessageManager;
 {
   expectType<Promise<Message>>(messageManager.fetch('1234567890'));