Structures

[xhyrom]

https://github.com/discordjs/discord.js/pull/6027

Hello, I am writing here as a user of the structures and I agree that the structures should not be changed much but they should stay here. You are an open-source library so you should take a look from people as well. More people wanted the structures to stay but you changed them anyway. Structures also have a lot of advantages when, for example, you just want to get the prefix in your handler via <guild>.prefix instead of <database>.getGuildPrefix(guild.id). It's especially better in that it's simpler and for npm packages when someone is doing a handler for example, it makes much more sense to simply use <guild>.prefix than to use <database>.getGuildPrefix(guild.id). I also know that you can add a variable prefix when you start a bot that fornets all guilds and adds it there but it's useless and a bot that is also on a lot of servers for example so it's load crap. For example if I want to add some things to the .send function to make my job easier how do I do that? Should I make some custom structure and custom message event? Please this doesn't make logic to ruin a pretty useful thing. If you're going to argue that you can't add .prefix to typings then you can. Just fix it:

import { Guild } from 'discord.js'

declare module 'discord.js' {
   interface Guild {
     prefix: string;
   }
}

and there you have it. You simply have the prefix added to the guild structure in typings as well. By doing this, you will cause at most, in my opinion, to break a lot of libraries like Commando. We can say that there were more changes from djs v11 to djsv12 but in this case not even. Just add cache and some other crap. Due to the destruction of the structures, npm packag developers will have to completely redo some things and it will be maximally more difficult for people.

Also think about people's feedback otherwise it makes no sense to do a library.

[iCrawl]

Structures also have a lot of advantages when, for example, you just want to get the prefix in your handler via .prefix instead of .getGuildPrefix(guild.id)

I see no advantage here, merely a preference.

It's especially better in that it's simpler and for npm packages when someone is doing a handler for example Due to the destruction of the structures, npm packag developers will have to completely redo some things and it will be maximally more difficult for people.

You did read the 2nd point about this here right? https://github.com/discordjs/discord.js/pull/6027 If multiple custom guilds or structures of the same type get used (something those libraries like to do), there is no guarantee they will work properly with each other.

For example if I want to add some things to the .send function to make my job easier how do I do that?

export function send(message, my, custom, params) {
  // Do some cool fancy stuff here

  return message.channel.send({ content: my, embeds: [custom] })
}

send(message, your, custom, params);

[xhyrom]

Structures also have a lot of advantages when, for example, you just want to get the prefix in your handler via .prefix instead of .getGuildPrefix(guild.id)

I see no advantage here, merely a preference.

It's especially better in that it's simpler and for npm packages when someone is doing a handler for example Due to the destruction of the structures, npm packag developers will have to completely redo some things and it will be maximally more difficult for people.

You did read the 2nd point about this here right? #6027 If multiple custom guilds or structures of the same type get used (something those libraries like to do), there is no guarantee they will work properly with each other.

Yes I know that but it could be solved that it only goes to extend 1x. And when you have an npm plugin you probably won't use 30 npm plugins for example for the command handler framework when you only need 1.

For example if I want to add some things to the .send function to make my job easier how do I do that?

export function send(message, my, custom, params) {
  // Do some cool fancy stuff here

  return message.channel.send({ content: my, embeds: [custom] })
}

send(message, your, custom, params);

Well okay maybe like this but there's still the fact that if I want to add it directly to <message> or if I want to add a completely custom function to <message> then I can't. And this method of yours is functional but it doesn't change the fact that I can't put it directly in <message>.

And more and more people were against the removal so you should take note of that.

[kyranet]

You install a library that overrides a class's send method to add internal tracking for auto-editable messages, you also install a library that also overrides it to construct an extended version of MessagePayload so you can send other fields (that we don't support).

Which one would you be using? The one with auto-editable messages? Or the one with the extended MessagePayload? What if we want to use one or another based on specific logic?

Now, that's just the tip of the iceberg. There are libraries that do not extend, but override discord.js's internals, such as discord-buttons and discord.js-light, both of which override internal logic in ways that we cannot support as the code becomes harder to predict, which is a burden to our support members.

Keep in mind that Structures was always intended to extend, not override or replace, and while the initial concept was good, it has caused many questions in the support team as they become exposed to properties and methods that do not exist in the library itself.

---

Furthermore, Structures was always limited, there are structures we can extend, and structures that because inheritance limitations, we can not, which leads to inconsistencies, for example, the ClientUser class was exported as a getter because it extends User, which by itself is a structure. That way, when ClientUser is imported for the first time, it'll extend the last extension of User.

Problem is: ESM. Getters don't exist in ESM due to the exports being always static, so ClientUser was always imported no matter what, that results on it being always extending User, and never the extended ones, this is unfixable and the types (provided you use module augmentation) become wrong and misleading.

This inheritance limitation isn't limited to this case only, and overall it has caused major pains when structuring the code.

---

If you want to go for an OOP approach, no matter the cost, you should use Reflect.setPrototypeOf and Reflect.defineProperty, be wary though, because support may get confused when those are used, and prototype mutations are in general, seen as a bad practice (specially since ES6).

Keep in mind that this last bit is basically "I understand the consequences this may cause", which is something very powerful that's built into JavaScript for many years, we do not condone the use of extensions for many reasons (specially support and the issues that arise with them), but you have perfectly valid alternatives.

What we'll support the most is the approach @iCrawl mentioned, of using functions, since they're guaranteed to be unique, non-conflicting, composable, and support members don't get confused by the appearance of arbitrary properties or methods that do not exist in the base library.

---

And at last, the staff team doesn't take decisions very lightly, some of them have been discussed over and over through the years (#6036's change has been discussed internally for 2 to 3 years) and we usually discuss with a large set of proficients and contributors before doing this kind of breaking change.

Of course, the community doesn't see this, some just thumbs down our code changes because they dislike changes, even if it's what the team sees as a step to the right direction.

If you really want a any change to be rethinked of, or even denied, please provide a better argument than "a lot of users use it!" or "I prefer doing <Structure>.<Property>!", as those are not really valid as arguments.

As a final note, the change originally wasn't very accepted by the team, I was one of those [against the change] too, but the issues of Structures outweight the benefits, and that's why I chose to go forward with it, as other members probably did.

[xhyrom]

Okay, I acknowledge that you're right but I still think it would take something to allow people to just make <Guild>.prefix or <Guild>.language in a different way than if I were to forEach all the guilds and put guild.prefix = "!".

I don't have much of an argument other than the ones you mentioned but I still think structures would be good. I would still be happy for some replacement like adding for example .prefix

I have my own npm package gcommands where I use structures for <Guild> and for <Message>. Any suggestion on how to make it as easy to use as possible for people who have djs v13?

That's all from me. Goodbye, Hyro.

[kyranet]

I don't see the issue with mutating the object and add those properties, it works fine, and I assume you can't even load them automatically because database interactions are usually asynchronous (and constructors synchronous).

Of course, you can always go for not storing that information in every guild and message, as that results on higher memory usage.

[xhyrom]

I don't see the issue with mutating the object and add those properties, it works fine, and I assume you can't even load them automatically because database interactions are usually asynchronous (and constructors synchronous).

Of course, you can always go for not storing that information in every guild and message, as that results on higher memory usage.

Okay, thank you. And if I may ask, how do you plan to handle this in Commando? After all, that's also where you use the structures.

[kyranet]

As of now, there are more plans of replacing Commando with Sapphire, than there are to maintain Commando itself.

[xhyrom]

Okay, I'm going to close the issue because this is more of a discussion and when I do I'll possibly open the discussion. Or is there any way to transform an issue into a discussion that you would do that?

[kyranet]

Not sure what else is there to discuss about, so feel free to close it. 👍🏼

[almostSouji]

Kindly read the original PR https://github.com/discordjs/discord.js/pull/6027 which is linked in the opening post, it explains the reasoning of why this was a bad idea to implement to begin with and why it was subsequently removed in at-dev (which will release as v13)