Remove *Data types from Discord.js types

[suneettipirneni]

Feature

A big pain point I've noticed recently is that there's a good deal of duplicated work when it comes to both discord-api-types and the discord.js-equivalent types. 95% of the time changes made in one need to be reflected in the other. And it's not always as simple as snake_case to camelCase. As has been evident by the latest updates, our types have become more complex involving tagged unions, intersections and custom utility types. A couple of examples of are command option types, and the autocomplete option types.

When slash commands (or insert any thing with a type prop) were added, their type in d.js was significantly weaker than it's discord-api-types counterpart. It wasn't less strict on purpose, it just wasn't based on the dapi types. And this mistake is completely understandable, because not everyone is going to look at the DAPI types implementation when making a PR for discord.js. However this represents a problem with how we use types: there's no single source of truth, therefore it invites disparity.

Because of this redundancy I'm proposing that we remove *Data types from discord.js and use discord-api-types as the replacement. As for the concerns of doing this, I'll try my best to address them below:

Ideal solution or implementation

We can't use dapi types because they're snake_cased while djs types are camelCase:

As far as I can tell this is the biggest roadblock as to not using DAPI types directly in discord.js. Luckily with the power of mapped types in typescript, this issue can be solved. You can combine template string literals with key remapping to convert any snake cased object into a camel-cased one:

// Types that transform snake_case to camelCase
type CamelCase<S extends string> = S extends `${infer prefix}_${infer suffix}${infer tail}`
  ? `${Lowercase<prefix>}${Uppercase<suffix>}${CamelCase<tail>}`
  : Lowercase<S>

type Camelize<T> = {
    [K in keyof T as CamelCase<string &K>]: T[K] extends {} ? Camelize<T[K]> : T[K]
}

Then in the type definition we simply use this utility type:

// Psuedo code
addCommandOption(data: Camelize<APIApplicationCommandOption>);

All keys from APIApplicationCommandOption are now camel cased even it's sub-objects (yes, Camelize<S> is recursive).

Personally, I like the aesthetics of Camelize<APIApplicationCommandOption> because it signifies to the caller that this is simply a camel-cased version of the DAPI types, nothing more, nothing less.

How would enums be handled:

This may be controversial, but I'm under the opinion that the current enum situation in d.js is annoying to say the least. From typescript enums runtime errors, to inconsistent parameter types from enums, I think this is an area that can be improved on.

Instead, just stop using our enums and use the direct DAPI-types ones. Some cases, the data we send to discord is a number, so using strings ie type: 'STRING' for our json seems misleading and unaligned with the API spec.

In typescript we can use the DAPI types equivalents for readablility:

{
  type: APIApplicationCommandOptionTypes.String
}

However this presents an issue for javascript developers. They won't be able to use APIApplicationCommandOptionTypes because it's an ambient type and it doesn't exist during runtime. This would force JS users to use straight numbers. Unfortunately the only way around this would be to use the Constants object. This is a form a duplication that isn't possible to prevent at this time.

Is this breaking?

very

Alternative solutions or implementations

The only alternative is the current solution we're using currently.

Other context

No response

[ImRodry]

I agree that we should try to remove “duplicated” code whenever possible and I like the idea of “Camelizing” the dapi types, however, I don’t think we should remove support for string types. Yes it’s true that the API only accepts number types, but it’s much more convenient to use a string that is easy to remember, without having to import an object from constants or an enum in typescript. I know many users use string types and I feel like this is just breaking for the sake of breaking, without any real benefit. I’m sure we could find a similar way to turn dapi’s PascalCased types into our CAPITAL_CASE (is that the name?) types, or we could use the CAPITAL_CASE types in discord-api-types as these names are very often used in Discord’s documentation, whereas the current names used in discord-api-types are not mentioned anywhere in the docs. This idea can be further discussed as I’m sure there must’ve been a reason to go with the current names over any other ones.

[monbrey]

I agree that there's an issue of duplication between discord.js and discord-api-types. My possibly also controversional opinion is that the core of the issue is that we only partially implement/rely on discord-api-types for our objects, and as you pointed out, this is because the casing doesn't align.

I kinda hate our overly complex typings and especially the custom utility types. Adding further abstraction layers like a Camelize<S> utility to act as a middleware between these two largely incompatible libraries is not a step in the right direction.

Personally, I think we either need suck it up, stop appealing to traditions, and fully implement snake_case to align with the Discord API and discord-api-types, or acknowledge that the API typings lib isn't compatible with discord.js objects, drop the dependency, and write our own.

The former option would resolve issues where we have to check for the presence of two different naming conventions already, such as Message Components which might be coming in from the API (snake_case) or might be being created by the user (camelCase).

[ImRodry]

I think discord.js should never support snake_case properties because it's a javascript library and the standard is camelCase. Discord API uses snake_case because it's built in python and that's the standard there. discord-api-types is still useful for some types and as long as the Camelize type works flawlessly I don't see an issue with it, but I also don't see an issue in writing our own types and only using dapi for the strictly necessary stuff. Now going back to snake_case at this point in time I think is just not an option seeing as users have already been forced to change in the past, so they wouldn't wanna change it again. Once again, that feels like breaking for the sake of breaking only.
This proposal has good points though so I think it should be worked on, but we need to avoid breaking changes that bring nearly no benefit to development and cause a much MUCH bigger hassle imho.

[monbrey]

I think discord.js should never support snake_case properties because it's a javascript library and the standard is camelCase.

In theory, I agree. So we shouldn't be relying on a library which has typings for a Python API.

It's discord-api-types, not discord.js-types

[ImRodry]

The only reason I don't think we shouldn't drop discord-api-types completely is that some methods do actually return raw API data and, once we move forward with #6567 we will need this even more. I see your point about making types unnecessarily complex and unreadable and that can indeed be an issue, but we need to see if that's worse than having to maintain tons of other interfaces and types where we could simply use that solution

[suneettipirneni]

Just for housekeeping I want to provide updates to what was discussed in library-discussion:

It seems that we're leaning more towards having all public-facing functions accept both the camel-cased and snake_case variants.

ie:

APIApplicationCommand | ApplicationCommandData

However in spirit with the original intentions of removing duplicated types, this was later suggested instead:

APIApplicationCommand | Camelize<APIApplicationCommand>

I'm suggesting we take the above excerpt a step further for readability and do this:

export type APIResolvable<T> = T | Camelize<T>;

Which would result in a function signature looking like this:

addCommand(command: APIResolvable<APIApplicationCommand>);

The main reasoning for the above choice is because djs is already doing this in some areas. Not only is this used as parameter types, it also is returned as values from public getters. For example:

• Interaction<'raw'>#member-> APInteractionGuildMember
• Interaction<'raw'>#guild -> APIGuild

So while I initially disagreed with integrating api types directly into djs itself, I now see why it should support both snake_case and camelCase types. In conclusion APIResolvable<T> seems to be the most compelling solution imo.