discordjs / discord.js

A powerful JavaScript library for interacting with the Discord API
https://discord.js.org
Apache License 2.0
25.36k stars 3.97k forks source link

Possibility of same old & new structure for `Client#threadMembersUpdate` #6513

Closed Jiralite closed 2 years ago

Jiralite commented 3 years ago

Please describe the problem you are having in as much detail as possible: <Client>.threadMembersUpdate has the possibility of emitting the same Collection for both the oldMembers & newMembers parameters.

This is achieved by the following:

  1. Join a thread
  2. Restart your bot to ensure reproducibility. Do not fetch active threads!
  3. Leave the thread

Upon leaving the thread, oldMembers and newMembers will emit the same Collection. Needless to say, this is quite... pointless! This is because the thread channel's members didn't have the member in question cached, ergo the same structures.

Include a reproducible code sample here, if possible:

const client = new (require("discord.js").Client)({
  intents: [
    "GUILDS",
    "GUILD_MEMBERS"
  ]
});

client.once("ready", () => {
  const thread = client.channels.resolve("thread_id"); // Thread
  console.log(thread.members.cache); // Collection(0) [Map] {}
});

// Emitting from thread_id
client.on("threadMembersUpdate", (oldMembers, newMembers) => {
  console.log(oldMembers.equals(newMembers)); // true
});

client.login("token");

Possible Fixes: I guess the simple answer is the make the oldMembers parameter nullable after these operations: https://github.com/discordjs/discord.js/blob/70cc0295f833cd988ea627b37d20536f73e21630/src/client/actions/ThreadMembersUpdate.js#L11-L20 We could just check to see if the size is equal and return null if so. By sending out null, we are essentially saying that an event occurred that we did not have enough data about (the old cache was unreliable).

I guess another way to do it would be to emit a "fake" ThreadMember that we create solely for it to be removed here just to ensure that the event fires with an actual ThreadMember. I'm unsure if this is possible.

Further details:

Relevant client options:

DTrombett commented 2 years ago

In my opinion, the problem is that both the old and new members may be partials, as Discord only sends us the added/removed members, so, if a member is added in a thread of which members are not cached, the emitted event will only contain eventually added members and, incredibly, if another member is added, the newMembers collection will contain the new member, the previously added member, but not all the members added before, which seems quite useless. A solution would be to emit the event with two parameters: a collection of removed members and a collection of added members, as they're surely sent by the API, but this would probably be a breaking change.