Discord.js: Remove Partials In Favor Of WS Events

[Skillz4Killz]

Is your feature request related to a problem? Please describe.
Partials make it a nightmare to work with Typescript. Having to put ugly ! everywhere on every file is a pain in the ass. For example, just trying to get message.author.id on any file you need to do message.author!.id now replicate that 100 times and for every single partial value. It is destroying the beauty of the codebase and it is not an optional thing. It is enforced.

Describe the ideal solution
The only benefit that can be derived from partials is basically to have access to raw events. Raw events can be easily handled by using client.ws.on('MESSAGE_DELETE').

So what is left from partials benefits? That when a message is cached you don't need to fetch it?

Fetching a message that was cached has always had a negligible impact on the performance even before partials. So I am wondering where the benefit of partials lies?

Ideally, users should use the ws events to handle the events. Then they can fetch it(negligible impact) and work with it as they would normally.

Describe alternatives you've considered
N/A

Additional context
N/A

[1Computer1]

Alternative solution: better typings. It is quite simple to have a check for the partial property that will narrow the types. This may require a lot of work though.

[monbrey]

You can still use the raw event handler to handle websocket events, so telling what users "should use" is irrelevant.

Typescript wont require the not-null assertion operator if you were to add a type-guard for the property earlier in the the code.

Fetching a message that was cached has always had a negligible impact on the performance even before partials.

Yeah, so don't. Checking if something is in cache is very easy to do, and should be part of the practice in handling raw websocket events also (your supposedly "better" solution)

Partials is nothing more than an option that you are under no obligation to enable and use.

[appellation]

It's relatively trivial to implement correct typings for this. I would recommend defining a Partial interface

interface Partial {
  id: string;
  partial: true;
}

and then emitting all events as Type & Partial. A simple check for the partial property being false will allow Typescript to correctly infer that the value is actually Type. I'm not sure if it will work without Type having a corresponding partial: false property, but one should be added to all the structures for clarity at least.

I'm not a fan of @Monbrey's suggestion to type guard by checking every property since, IMO, that is significantly worse aesthetically than non-null assertions.

[Skillz4Killz]

@Monbrey Just to be clear I am not saying my solution is the only solution or even the right solution. There are a lot of devs on the D.JS team a lot better than me so perhaps they can find a better solution then I can. My goal is to express the trouble and frustration I am having because of the Partials features and suggest a possible solution.

Partials is nothing more than an option that you are under no obligation to enable and use.

Partials as a feature are optional. However, they are forcing(not optional) typescript projects to handle partials EVEN if you don't use partials.

Typescript won't require the not-null assertion operator if you were to add a type-guard for the property earlier in the code.

Appellation said this perfectly, this makes the problem worse.

Yeah, so don't. Checking if something is in the cache is very easy to do, and should be part of the practice in handling raw WebSocket events also (your supposedly "better" solution)

I believe you misunderstood or perhaps I said it wrong. What I am hoping to express is that users should be handling the fetch. Without partials, you need to handle fetching. Even with partials, you have to handle fetching.

if (message.partial) await message.fetch()

You can use raw events and just always fetch them. If it is in cache you get it, otherwise, it will try to fetch it from the API. 0 difference one way or the other.

You can still use the raw event handler to handle WebSocket events, so telling what users "should use" is irrelevant.

The problem is that this doesn't just occur in raw event handlers but every single file in a project. I could be in a command file and I have to handle the typings because of this. In every single file where ever anything could be partial, my code will be made uglier.

---

@appellation I think you are right but I would actually recommend taking it a step further. By default, typings should assume that it is NOT partial and separate interfaces created for partials.

Whenever I use a "partial" event file it could be cast as a separate interface like (message: PartialMessage) or something similar.

For JS users, none of this really matters as they don't get compile-time errors. They are able to use message.author.id in any file partial or not. For TS users are where the problem is and the TS devs will be providing a type to the message objects anyway like (message: Message)

If Message is assumed to not be partial and separate interfaces are created for partials, ONLY the event files you want to use partials in will assume it to be partial. This way every single file is not affected and the code is clean and function the exact same.

[appellation]

By default, typings should assume that it is NOT partial and separate interfaces created for partials.

I'm not entirely onboard with this since you can end up footgunning yourself with incorrect types if you forget to cast, essentially mitigating many of the benefits of TS. It's not much of an ask to do a simple boolean check for partial and you can still do a type cast if you just don't want to.

[ShayBox]

I may not entirely understand the approach being described, so sorry if I got it completely wrong, but from what I understand, your approach would be to have a boolean, check it to see if it's a partial or not, in the end that makes the code look the same as if partials were just enabled, at that rate why not just enable partials and add checks/(fetch or return) for everything, if you're already going to have checks for the boolean.

[1Computer1]

It is a proof that the object won't be partial, because:

• TypeScript can't magically know that partials are turned off
• When you do turn partials back on, you won't get blindsided

[Skillz4Killz]

It's not much of an ask to do a simple boolean check for partial

@appellation Although having 1 boolean in every single file is better than having 100s of non-null assertions in every single file it is still not an ideal solution in my opinion. Then I will need more if checks for channels, users, members and so on for every single partial possibility. Still it is better than using !, but it is not a good solution. This just makes the problem less but does not solve the problem entirely. It is like a band-aid solution instead of fixing it at its core.

Majority of D.JS users will probably never use this and those that do use it could simply be advised to use WS events instead. They have an option to use or not use partials. TS devs are not given that option. We are forced to make the code worse then it should be even if we don't use that feature. A feature that is easily replaced with WS events with none of these downsides as mentioned in the original post. IMO the implementation has a lot of potential for improvement.

you can end up footgunning yourself with incorrect types if you forget to cast

async run(message)

If you forget to give a type for the message like this, one of two possible outcomes will occur.

1. If you enforce no-any then TS compiler will warn you right away so no problem.
2. If you don't enforce the no-any the message will be cast as any. This will lose all the benefits of TS anyway. It will be treated as if all the methods & props DO exist even if it is partial.

Forgetting to cast it isn't exactly an issue here.

I still feel the best solution is to remove partials in favor of WS as you retain the entire benefits with no downsides. On the other hand, you can also set Message and User and Member and DMChannel to by default not be partials. And then create and export PartialMessage, PartialUser, PartialMember, and PartialDMChannel and so on.

---

in the end that makes the code look the same as if partials were just enabled, at that rate why not just enable partials and add checks/(fetch or return) for everything, if you're already going to have checks for the boolean.

@ShayBox This is exactly what I am having frustrations with as well regarding the partials feature. The boolean solution doesn't solve the problem at its core and it will still be forcing TS devs to handle a feature that might not even be enabled at all and shouldn't need to be enabled as there is an alternative way to handle it with WS.

[appellation]

They have an option to use or not use partials. TS devs are not given that option.

from what I understand, your approach would be to have a boolean, check it to see if it's a partial or not

TS users would not have to do this if (object.partial) return; check everywhere. You are, in fact, allowed to type assert to a full object. This is what assertions are for: asserting runtime type conditions that cannot be determined statically. See below for more explanation.

Majority of D.JS users will probably never use this

Majority of D.JS users don't use Typescript, so this is somewhat of a moot point.

you can end up footgunning yourself with incorrect types if you forget to cast

 async run(message)

This scenario I was referring to functions/methods with implicitly typed parameters as in an event listener. For example, imagine listening to the 'messageCreate' event:

client.on('messageCreate', message => {
  // message is Message | Partial
  if (message.partial) return;
  // message is Message
});

// or, with an explicitly typed function parameter
client.on('messageCreate', (message: Message) => {
  // message is Message
});

---

This is a simple approach that maintains type safety by default and can easily be circumvented in cases where you know better than the compiler: ie. you've not enabled partials for that struct.

A hypothetical example from the Typescript playground:

[ShayBox]

Thanks for giving that example appel, that made it clearer, I'm alright with casting like that, if that's the preferred approach.

[Skillz4Killz]

This was closed by #3493