Skip to content

fix(GuildMember): cache based on partials options #10785

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 2 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
Original file line number Diff line number Diff line change
Expand Up @@ -21,11 +21,14 @@ module.exports = (client, { d: data }) => {

let member = guild.members.cache.get(user.id);
if (!member && data.status !== 'offline') {
member = guild.members._add({
user,
deaf: false,
mute: false,
});
member = guild.members._add(
{
user,
deaf: false,
mute: false,
},
client.options.partials.includes(Partials.GuildMember),
Copy link
Member

Choose a reason for hiding this comment

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

This needs more/better logic. The member should always be patched with that data, if cached before, but should never be newly added to cache. The guildMemberAvaliable event here should only emit if the partial is set. So add the partial check to the if conditions surrounding this block instead.

Copy link
Author

@Pavel-Boyazov Pavel-Boyazov Mar 2, 2025

Choose a reason for hiding this comment

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

The guildMemberAvaliable event here should only emit if the partial is set.

I think it also needs to be emit if member object cached before. Didn't?

Copy link
Member

Choose a reason for hiding this comment

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

No, if it's cache before then the !member condition would be false anyway.

);

client.emit(Events.GuildMemberAvailable, member);
}
Expand Down
6 changes: 3 additions & 3 deletions packages/discord.js/src/managers/GuildMemberManager.js
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ class GuildMemberManager extends CachedManager {

/**
* The cache of this Manager
* @type {Collection<Snowflake, GuildMember>}
* @type {Collection<Snowflake, GuildMember | PartialGuildMember>}
Copy link
Member

Choose a reason for hiding this comment

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

The whole idea of fixing this was that there won‘t be PartialGuildMember in the cache. And there is no PartialGuildMember in JS land anyway, so this would only make sense in the typings (but is wrong there for the above reason).
Same applies for all other JSDoc comments you added PartialGuildMember to

* @name GuildMemberManager#cache
*/

Expand All @@ -42,7 +42,7 @@ class GuildMemberManager extends CachedManager {
/**
* Resolves a {@link UserResolvable} to a {@link GuildMember} object.
* @param {UserResolvable} member The user that is part of the guild
* @returns {?GuildMember}
* @returns {?(GuildMember | PartialGuildMember)}
*/
resolve(member) {
const memberResolvable = super.resolve(member);
Expand Down Expand Up @@ -132,7 +132,7 @@ class GuildMemberManager extends CachedManager {

/**
* The client user as a GuildMember of this guild
* @type {?GuildMember}
* @type {?(GuildMember | PartialGuildMember)}
* @readonly
*/
get me() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ class GuildMemberRoleManager extends DataManager {

/**
* The GuildMember this manager belongs to
* @type {GuildMember}
* @type {GuildMember | PartialGuildMember}
*/
this.member = member;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ class BaseGuildVoiceChannel extends GuildChannel {

/**
* The members in this voice-based channel
* @type {Collection<Snowflake, GuildMember>}
* @type {Collection<Snowflake, GuildMember | PartialGuildMember>}
* @readonly
*/
get members() {
Expand Down
2 changes: 1 addition & 1 deletion packages/discord.js/src/structures/GuildChannel.js
Original file line number Diff line number Diff line change
Expand Up @@ -275,7 +275,7 @@ class GuildChannel extends BaseChannel {
* A collection of cached members of this channel, mapped by their ids.
* Members that can view this channel, if the channel is text-based.
* Members in the channel, if the channel is voice-based.
* @type {Collection<Snowflake, GuildMember>}
* @type {Collection<Snowflake, GuildMember | PartialGuildMember>}
* @readonly
*/
get members() {
Expand Down
2 changes: 1 addition & 1 deletion packages/discord.js/src/structures/Message.js
Original file line number Diff line number Diff line change
Expand Up @@ -497,7 +497,7 @@ class Message extends Base {
/**
* Represents the author of the message as a guild member.
* Only available if the message comes from a guild where the author is still a member
* @type {?GuildMember}
* @type {?(GuildMember | PartialGuildMember)}
* @readonly
*/
get member() {
Expand Down
4 changes: 2 additions & 2 deletions packages/discord.js/src/structures/MessageMentions.js
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,7 @@ class MessageMentions {

/**
* Cached members for {@link MessageMentions#members}
* @type {?Collection<Snowflake, GuildMember>}
* @type {?Collection<Snowflake, GuildMember | PartialGuildMember>}
* @private
*/
this._members = null;
Expand Down Expand Up @@ -190,7 +190,7 @@ class MessageMentions {
/**
* Any members that were mentioned (only in {@link Guild}s)
* <info>Order as received from the API, not as they appear in the message content</info>
* @type {?Collection<Snowflake, GuildMember>}
* @type {?Collection<Snowflake, GuildMember | PartialGuildMember>}
* @readonly
*/
get members() {
Expand Down
2 changes: 1 addition & 1 deletion packages/discord.js/src/structures/Presence.js
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ class Presence extends Base {

/**
* The member of this presence
* @type {?GuildMember}
* @type {?(GuildMember | PartialGuildMember)}
Copy link
Member

Choose a reason for hiding this comment

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

Here this.member should be turned into a property created from data passed in constructor instead of a getter. That way it could contain a partial guild member without putting that partial data into cache.

* @readonly
*/
get member() {
Expand Down
2 changes: 1 addition & 1 deletion packages/discord.js/src/structures/Role.js
Original file line number Diff line number Diff line change
Expand Up @@ -175,7 +175,7 @@ class Role extends Base {

/**
* The cached guild members that have this role
* @type {Collection<Snowflake, GuildMember>}
* @type {Collection<Snowflake, GuildMember | PartialGuildMember>}
* @readonly
*/
get members() {
Expand Down
2 changes: 1 addition & 1 deletion packages/discord.js/src/structures/Typing.js
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ class Typing extends Base {

/**
* The member who is typing
* @type {?GuildMember}
* @type {?(GuildMember | PartialGuildMember)}
* @readonly
*/
get member() {
Expand Down
2 changes: 1 addition & 1 deletion packages/discord.js/src/structures/VoiceState.js
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,7 @@ class VoiceState extends Base {

/**
* The member that this voice state belongs to
* @type {?GuildMember}
* @type {?(GuildMember | PartialGuildMember)}
* @readonly
*/
get member() {
Expand Down
57 changes: 37 additions & 20 deletions packages/discord.js/typings/index.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -677,7 +677,7 @@ export class BaseGuildVoiceChannel extends GuildChannel {
public bitrate: number;
public get full(): boolean;
public get joinable(): boolean;
public get members(): Collection<Snowflake, GuildMember>;
public get members(): Collection<Snowflake, GuildMember | PartialGuildMember>;
public nsfw: boolean;
public rateLimitPerUser: number | null;
public rtcRegion: string | null;
Expand Down Expand Up @@ -1541,7 +1541,10 @@ export class GuildBan extends Base {

export abstract class GuildChannel extends BaseChannel {
public constructor(guild: Guild, data?: RawGuildChannelData, client?: Client<true>, immediatePatch?: boolean);
private memberPermissions(member: GuildMember, checkAdmin: boolean): Readonly<PermissionsBitField>;
private memberPermissions(
member: GuildMember | PartialGuildMember,
checkAdmin: boolean,
): Readonly<PermissionsBitField>;
private rolePermissions(role: Role, checkAdmin: boolean): Readonly<PermissionsBitField>;
public get createdAt(): Date;
public get createdTimestamp(): number;
Expand All @@ -1550,7 +1553,7 @@ export abstract class GuildChannel extends BaseChannel {
public guild: Guild;
public guildId: Snowflake;
public get manageable(): boolean;
public get members(): Collection<Snowflake, GuildMember>;
public get members(): Collection<Snowflake, GuildMember | PartialGuildMember>;
public name: string;
public get parent(): CategoryChannel | null;
public parentId: Snowflake | null;
Expand All @@ -1565,7 +1568,10 @@ export abstract class GuildChannel extends BaseChannel {
public edit(options: GuildChannelEditOptions): Promise<this>;
public equals(channel: GuildChannel): boolean;
public lockPermissions(): Promise<this>;
public permissionsFor(memberOrRole: GuildMember | Role, checkAdmin?: boolean): Readonly<PermissionsBitField>;
public permissionsFor(
memberOrRole: GuildMember | PartialGuildMember | Role,
checkAdmin?: boolean,
): Readonly<PermissionsBitField>;
public permissionsFor(
memberOrRole: UserResolvable | RoleResolvable,
checkAdmin?: boolean,
Expand Down Expand Up @@ -1618,8 +1624,8 @@ export class GuildMember extends Base {
public get communicationDisabledUntil(): Date | null;
public communicationDisabledUntilTimestamp: number | null;
public flags: Readonly<GuildMemberFlagsBitField>;
public get joinedAt(): Date | null;
public joinedTimestamp: number | null;
public get joinedAt(): Date;
public joinedTimestamp: number;
public get kickable(): boolean;
public get manageable(): boolean;
public get moderatable(): boolean;
Expand Down Expand Up @@ -2135,7 +2141,7 @@ export class Message<InGuild extends boolean = boolean> extends Base {
public get hasThread(): boolean;
public id: Snowflake;
public interactionMetadata: MessageInteractionMetadata | null;
public get member(): GuildMember | null;
public get member(): GuildMember | PartialGuildMember | null;
public mentions: MessageMentions<InGuild>;
public nonce: string | number | null;
public get partial(): false;
Expand Down Expand Up @@ -2370,15 +2376,15 @@ export class MessageMentions<InGuild extends boolean = boolean> {
);
private _channels: Collection<Snowflake, Channel> | null;
private readonly _content: string;
private _members: Collection<Snowflake, GuildMember> | null;
private _members: Collection<Snowflake, GuildMember | PartialGuildMember> | null;
private _parsedUsers: Collection<Snowflake, User> | null;

public get channels(): Collection<Snowflake, Channel>;
public readonly client: Client;
public everyone: boolean;
public readonly guild: If<InGuild, Guild>;
public has(data: UserResolvable | RoleResolvable | ChannelResolvable, options?: MessageMentionsHasOptions): boolean;
public get members(): If<InGuild, Collection<Snowflake, GuildMember>>;
public get members(): If<InGuild, Collection<Snowflake, GuildMember | PartialGuildMember>>;
public get parsedUsers(): Collection<Snowflake, User>;
public repliedUser: User | null;
public roles: Collection<Snowflake, Role>;
Expand Down Expand Up @@ -2693,7 +2699,7 @@ export class Presence extends Base {
public activities: Activity[];
public clientStatus: ClientPresenceStatusData | null;
public guild: Guild | null;
public get member(): GuildMember | null;
public get member(): GuildMember | PartialGuildMember | null;
Copy link
Member

Choose a reason for hiding this comment

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

This should be turned into an actual property, not a getter.

public status: PresenceStatus;
public get user(): User | null;
public userId: Snowflake;
Expand Down Expand Up @@ -2803,7 +2809,7 @@ export class Role extends Base {
public hoist: boolean;
public id: Snowflake;
public managed: boolean;
public get members(): Collection<Snowflake, GuildMember>;
public get members(): Collection<Snowflake, GuildMember | PartialGuildMember>;
public mentionable: boolean;
public name: string;
public permissions: Readonly<PermissionsBitField>;
Expand Down Expand Up @@ -3355,7 +3361,10 @@ export class ThreadChannel<ThreadOnly extends boolean = boolean> extends BaseCha
public edit(options: ThreadEditOptions): Promise<this>;
public join(): Promise<this>;
public leave(): Promise<this>;
public permissionsFor(memberOrRole: GuildMember | Role, checkAdmin?: boolean): Readonly<PermissionsBitField>;
public permissionsFor(
memberOrRole: GuildMember | PartialGuildMember | Role,
checkAdmin?: boolean,
): Readonly<PermissionsBitField>;
public permissionsFor(
memberOrRole: UserResolvable | RoleResolvable,
checkAdmin?: boolean,
Expand Down Expand Up @@ -3403,7 +3412,7 @@ export class Typing extends Base {
public startedTimestamp: number;
public get startedAt(): Date;
public get guild(): Guild | null;
public get member(): GuildMember | null;
public get member(): GuildMember | PartialGuildMember | null;
public inGuild(): this is this & {
channel: TextChannel | AnnouncementChannel | ThreadChannel;
get guild(): Guild;
Expand Down Expand Up @@ -3586,7 +3595,7 @@ export class VoiceState extends Base {
public get deaf(): boolean | null;
public guild: Guild;
public id: Snowflake;
public get member(): GuildMember | null;
public get member(): GuildMember | PartialGuildMember | null;
public get mute(): boolean | null;
public selfDeaf: boolean | null;
public selfMute: boolean | null;
Expand Down Expand Up @@ -4224,10 +4233,10 @@ export interface AddOrRemoveGuildMemberRoleOptions {
reason?: string;
}

export class GuildMemberManager extends CachedManager<Snowflake, GuildMember, UserResolvable> {
export class GuildMemberManager extends CachedManager<Snowflake, GuildMember | PartialGuildMember, UserResolvable> {
private constructor(guild: Guild, iterable?: Iterable<RawGuildMemberData>);
public guild: Guild;
public get me(): GuildMember | null;
public get me(): GuildMember | PartialGuildMember | null;
Copy link
Member

Choose a reason for hiding this comment

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

We always get the full GuildMember data for the ClientUser on GUILD_CREATE, so this can never be a PartialGuildMember to begin with.

Copy link
Author

@Pavel-Boyazov Pavel-Boyazov Mar 2, 2025

Choose a reason for hiding this comment

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

GUILD_CREATE not emit if Guilds intent not set

Copy link
Member

Choose a reason for hiding this comment

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

Without Guilds intent you wouldn't have any GuildMembers cached and the whole GuildMemberManager would be inaccessible because you don't have a Guild having it as property to begin with.

Copy link
Author

Choose a reason for hiding this comment

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

What prevents us from using fetch to get and cache guilds with specific members exclude client member?

Copy link
Author

Choose a reason for hiding this comment

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

get me() {
return (
this.cache.get(this.client.user.id) ??
(this.client.options.partials.includes(Partials.GuildMember)
? this._add({ user: { id: this.client.user.id } }, true)
: null)
);
}

And if it like you say, why exist this functionality?

Copy link
Member

Choose a reason for hiding this comment

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

Okay, that's a good catch indeed. Guess GuildMemberManager#me can be a PartialGuildMember after all, if the GuildMemberManager cache is limited to 0 without exception for the ClientUser.
But about no Guilds intent: most of the typings rely on that being present. Because several helper methods need it to work anyway.

public add(
user: UserResolvable,
options: AddGuildMemberOptions & { fetchWhenExisting: false },
Expand Down Expand Up @@ -4314,14 +4323,14 @@ export class GuildStickerManager extends CachedManager<Snowflake, Sticker, Stick
}

export class GuildMemberRoleManager extends DataManager<Snowflake, Role, RoleResolvable> {
private constructor(member: GuildMember);
private constructor(member: GuildMember | PartialGuildMember);
public get hoist(): Role | null;
public get icon(): Role | null;
public get color(): Role | null;
public get highest(): Role;
public get premiumSubscriberRole(): Role | null;
public get botRole(): Role | null;
public member: GuildMember;
public member: GuildMember | PartialGuildMember;
Copy link
Member

Choose a reason for hiding this comment

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

While technically correct this degrades UX. Probably better to add a Partialized version of this manager overriding this property and overriding the roles getter on PartialGuildMember with that partial manager.

public guild: Guild;

public add(
Expand Down Expand Up @@ -5959,7 +5968,15 @@ export interface GuildMemberEditOptions {
reason?: string;
}

export type GuildResolvable = Guild | NonThreadGuildBasedChannel | GuildMember | GuildEmoji | Invite | Role | Snowflake;
export type GuildResolvable =
| Guild
| NonThreadGuildBasedChannel
| GuildMember
| PartialGuildMember
Copy link
Member

Choose a reason for hiding this comment

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

I don't think it makes much sense to resolve a Guild from a partial member, but that one's up to debate I'd say

| GuildEmoji
| Invite
| Role
| Snowflake;

export interface GuildPruneMembersOptions {
count?: boolean;
Expand Down Expand Up @@ -6864,7 +6881,7 @@ export type ThreadMemberResolvable = ThreadMember | UserResolvable;

export type UserMention = `<@${Snowflake}>`;

export type UserResolvable = User | Snowflake | Message | GuildMember | ThreadMember;
export type UserResolvable = User | Snowflake | Message | GuildMember | PartialGuildMember | ThreadMember;

export interface Vanity {
code: string | null;
Expand Down