`parentId` should never be null on `PublicThreadChannel`

MarcusOtter commented 2 years ago

Which package is this bug report for?

discord.js

Issue description

How it should be A PublicThreadChannel (ThreadChannel.type: ChannelType.GuildPublicThread | ChannelType.GuildNewsThread) should never have parentId === null.

How it is in DJS PublicThreadChannel.parentId and PublicThreadChannel.parent can both be null.

The problem It is not possible (to my knowledge) to safely get the parent channel of a PublicThreadChannel. There is no fetchParent(). And we cannot use <client>.channels.fetch(<thread>.parentId) because parentId can be null. How are we supposed to get the parent channel of a public thread?

Code sample

async function getParentChannelId(thread: PublicThreadChannel): Promise<string> {
    return thread.parentId; // Type 'null' is not assignable to type 'string'.
}

Package version

discord.js@14.1.1

Node.js version

v16.13.2

Operating system

Windows

Priority this issue should have

Medium (should be fixed soon)

Which partials do you have configured?

No Partials

Which gateway intents are you subscribing to?

Guilds, GuildMessages

I have tested this issue on a development release

No response

ckohen commented 2 years ago

parent is the same no matter what type of thread it is. because private threads do also belong to channels.

parent is only nullable because of the cache access. If you have the Guilds intent and don't do funky (unsupported) stuff with limiting channel cache, in all likelihood you will never have parent: null. This is a technicality and is not followed everywhere in the lib (notably, messages), but since threads were added after it was noticed that channel cache isn't technically guaranteed, it was decided that the types should actually be accurate.

parentId is always present so you can fetch the parent with <client>.channels.fetch(<thread>.parentId)

MarcusOtter commented 2 years ago

parent is the same no matter what type of thread it is. because private threads do also belong to channels.

That's my misconception, but that makes this even weirder, because ThreadChannel.partial is exposed. I thought this was because private threads are a sort of DM channel. You can definitely not make threads in DMs, so I guess this is a type bug.

parentId is always present so you can fetch the parent with .channels.fetch(.parentId)

If that was the case, that would be great.

ckohen commented 2 years ago

ah.....right, the minor detail of interactions.

not sure if its still the case but there's this bug https://github.com/discordjs/discord.js/issues/5960 which was causing crashes due to the same type of assumption I just made above
This was fixed in JS land with https://github.com/discordjs/discord.js/pull/5961 but the typings were not updated, probably because partial objects don't often have the full backing of types
The types were made technically correct in https://github.com/discordjs/discord.js/pull/6172

So, to get the parent, try <thread>.parent first. If that doesn't exist it's most likely because parentId is missing, so .fetch() the thread and check if parentId exists. If it does, fetch with it as mentioned before, if not, throw an error cause you can't do anything else

MarcusOtter commented 2 years ago

I don't know much about partials so I have no comments there. But I'm sorry, I don't really understand why parentId would ever be null on a public thread. Surely that's a DJS issue?

MarcusOtter commented 2 years ago

Also, re:

If you have the Guilds intent and don't do funky (unsupported) stuff with limiting channel cache, in all likelihood you will never have parent: null.

I have sweepers setup for threads, don't know if that would affect this. Probably?

ckohen commented 2 years ago

Again, all threads behave exactly the same in regards to parentId and parent. It doesn't have to do with the type of thread, its where its received from

Sweepers don't affect the types, but it can be part of a situation where parentId is null.

As for partials, discord sometimes doesn't send full objects, to save network bandwidth for things that most people won't need. In the past, the resolved data for a slash commadn channel option didn't contain parent_id (raw form from discord, same field though), so that field became nullable. Now that is no longer the case, so its probably worth another look to see if parentId can ever be missing.

MarcusOtter commented 2 years ago

Perhaps add typings label and remove need repro?

discordjs / discord.js