-
-
Notifications
You must be signed in to change notification settings - Fork 4.1k
Description
ℹ️ This goes hand-in-hand with #6567, and as such, the linked PR must be solved before this one.
❓ The problem
Currently, the way we handle channels in the library is overwhelmingly complex, for example, just to create the channel class, we need a lot of checks:
discord.js/packages/discord.js/src/structures/Channel.js
Lines 192 to 251 in c10afea
| static create(client, data, guild, { allowUnknownGuild, fromInteraction } = {}) { | |
| CategoryChannel ??= require('./CategoryChannel'); | |
| DMChannel ??= require('./DMChannel'); | |
| NewsChannel ??= require('./NewsChannel'); | |
| StageChannel ??= require('./StageChannel'); | |
| StoreChannel ??= require('./StoreChannel'); | |
| TextChannel ??= require('./TextChannel'); | |
| ThreadChannel ??= require('./ThreadChannel'); | |
| VoiceChannel ??= require('./VoiceChannel'); | |
| let channel; | |
| if (!data.guild_id && !guild) { | |
| if ((data.recipients && data.type !== ChannelType.GroupDM) || data.type === ChannelType.DM) { | |
| channel = new DMChannel(client, data); | |
| } else if (data.type === ChannelType.GroupDM) { | |
| const PartialGroupDMChannel = require('./PartialGroupDMChannel'); | |
| channel = new PartialGroupDMChannel(client, data); | |
| } | |
| } else { | |
| guild ??= client.guilds.cache.get(data.guild_id); | |
| if (guild || allowUnknownGuild) { | |
| switch (data.type) { | |
| case ChannelType.GuildText: { | |
| channel = new TextChannel(guild, data, client); | |
| break; | |
| } | |
| case ChannelType.GuildVoice: { | |
| channel = new VoiceChannel(guild, data, client); | |
| break; | |
| } | |
| case ChannelType.GuildCategory: { | |
| channel = new CategoryChannel(guild, data, client); | |
| break; | |
| } | |
| case ChannelType.GuildNews: { | |
| channel = new NewsChannel(guild, data, client); | |
| break; | |
| } | |
| case ChannelType.GuildStore: { | |
| channel = new StoreChannel(guild, data, client); | |
| break; | |
| } | |
| case ChannelType.GuildStageVoice: { | |
| channel = new StageChannel(guild, data, client); | |
| break; | |
| } | |
| case ChannelType.GuildNewsThread: | |
| case ChannelType.GuildPublicThread: | |
| case ChannelType.GuildPrivateThread: { | |
| channel = new ThreadChannel(guild, data, client, fromInteraction); | |
| if (!allowUnknownGuild) channel.parent?.threads.cache.set(channel.id, channel); | |
| break; | |
| } | |
| } | |
| if (channel && !allowUnknownGuild) guild.channels?.cache.set(channel.id, channel); | |
| } | |
| } | |
| return channel; | |
| } |
(And let's not mention it's also a class that requires quite a workaround to work, as seen below):
| if (data && immediatePatch) this._patch(data); |
Furthermore, changing channel types is not easy:
discord.js/packages/discord.js/src/client/actions/ChannelUpdate.js
Lines 14 to 19 in c10afea
| if (channel.type !== data.type) { | |
| const newChannel = Channel.create(this.client, data, channel.guild); | |
| for (const [id, message] of channel.messages.cache) newChannel.messages.cache.set(id, message); | |
| channel = newChannel; | |
| this.client.channels.cache.set(channel.id, channel); | |
| } |
On a large scale, those checks cause the shard ready quite slower.
✅ The solution
We can make a single class following whichever design we take with the aforementioned PR, change the extended classes to interfaces, and use #6957 to cast Channel to those interfaces.
Some stuff such as the line below:
discord.js/packages/discord.js/src/structures/GuildChannel.js
Lines 27 to 31 in c10afea
| /** | |
| * The guild the channel is in | |
| * @type {Guild} | |
| */ | |
| this.guild = guild; |
Would just be a getter, following the design from the Message class:
discord.js/packages/discord.js/src/structures/Message.js
Lines 37 to 41 in c10afea
| /** | |
| * The id of the guild the message was sent in, if any | |
| * @type {?Snowflake} | |
| */ | |
| this.guildId = data.guild_id ?? this.channel?.guild?.id ?? null; |
discord.js/packages/discord.js/src/structures/Message.js
Lines 397 to 399 in c10afea
| get guild() { | |
| return this.client.guilds.resolve(this.guildId) ?? this.channel?.guild ?? null; | |
| } |
⬆️ Upsides
- Lowers performance impact when instanting large amounts of channels in a short period of time (e.g. a bot on +10 internal shards).
- Much easier type-changing mechanism - as easy as simply patching it.
- Future-proofing messages and webhooks, PRs like feat(VoiceChannel): add support for text in voice #6921 would be significantly easier, or even not need to exist.
- No mixins, makes documentation easier.
- No need for inheritance trees, middle-ground shared classes (
BaseGuildTextChannel,BaseGuildVoiceChannel, andGuildChannel) and future ones that might arise in the future. - (?) Less memory usage due to channel managers storing only 1 class as opposed to using a lot of different classes with different shapes (https://mathiasbynens.be/notes/shapes-ics).
⬇️ Downsides
- A single class for everything means a very complex class.
- How do we manage message caches? Should we remove them? Issue soon!
Metadata
Metadata
Assignees
Type
Projects
Status