Discord.js: <#Channel>.type does not provide a type guard

[Ayplow]

Please describe the problem you are having in as much detail as possible:

The current type of <#Channel>.type is a unified type of all the potential types that the data can carry. This means that the property cannot be used as a type guard in determining what type a channel is, meaning code is forced to use type assertions or custom type guards

Fixing this issue would involve:

• Removing the type property from Channel's typings
• Adding the appropriate type property for each Channel type
• Using a union type for all channels (ResolvedChannel?)
• Using the union channel type in all relevant locations (Seems to be the channel* events)

And optionally:

• Moving the <#Channel>.type property declaration from the channel class to each individual Channel type to make the typings more 'accurate'.

I'm happy to work on this if there aren't any problem with the implementation

Relevant files:

https://github.com/discordjs/discord.js/blob/54aff3191e3576dce519634fdbf31bbaabfd907c/src/structures/Channel.js#L24

https://github.com/discordjs/discord.js/blob/54aff3191e3576dce519634fdbf31bbaabfd907c/typings/index.d.ts#L126-L132

Include a reproducible code sample here, if possible:

A functioning type guard:

type A = { type: 'a', B: any }
type C = { type: 'c', D: any }

function example(argument:A | C) {
  argument.B // Does not exist on type 'A | C'
  argument.D // Does not exist on type 'A | C'
  if (argument.type === 'a') return argument.B // Exists on type 'A'
  argument.D // Exists on type 'C'
}

Manual Type Guard:

import { VoiceChannel } from 'discord.js'
<#Client>.on('channelCreate', channel => {
  if (!((channel): channel is VoiceChannel => channel.type === 'voice')(channel)) return
  channel.join()
})

Preferable Type Guard:

<#Client>.on('channelCreate', channel => {
  if (channel.type !== 'voice') return
  channel.join()
})

Further details:

• discord.js version: 62e7e26310927d2898eca5c5036d29fd5e79fcc0
• Node.js version: 8.11.4 (N/A)
• Operating system: Windows 10 18272 (N/A)
• Priority this issue should have – please be realistic and elaborate if possible: Medium-High priority. It is a small refactor around some flawed typings that prevent typescript from being used well

• [ ] I found this issue while running code on a __user account__
• [x] I have also tested the issue on latest master, commit hash: 62e7e26310927d2898eca5c5036d29fd5e79fcc0

[1Computer1]

There should not be a need for this beyond a simple union. Simply do

type ChannelUnion = (any possible channel types);
client.on('channelCreate', (channel: ChannelUnion) => {
  if (channel.type !== 'voice') return;
  channel.join();
});

and it should work. The question is now whether or not to include said ChannelUnion in the library. My opinion is no, since the specifics of which channel types are and are not possible should be left to you.

EDIT: My bad, for this to work we would have to add the appropriate string type onto the channels. So we would need to do a bit of work but not as drastic as you are suggesting.

[Ayplow]

I definitely didn't mean to imply it would be hard to implement, I have an example of a working typing here

I don't quite understand what you mean by

which channel types are and are not possible should be left to you.

There are a specific set of classes which can be passed to the channel* events, and Channel is the incorrect typing for them. What option does the user of the library have?

[SpaceEEC]

Since nobody mentioned this yet, this already works fine as a typeguard

const channel: Channel = client.channels.random();
if (channel instanceof VoiceChannel) {
    channel.join();
}

---

But I can see the that people like / are used to compare the channel type however.
Since the linked commit does not seem too drastic, rather straightforward, I don't see anything wrong with implementing that.
Maybe I am not seeing some complexity rolling at us with that though.

[Ayplow]

(Afaik) The changes only make the typings more accurate to the actual code. It would mean having to update the ChannelUnion types whenever new ones are added, but that would only be because the type had actually changed.

I generally think type assertions should be avoided whenever possible. It is just too easy to be wrong, and giving the channel parameter the proper type encourages well made runtime checks.

instanceof Certainly does the job, however I personally like to avoid having many imports, to keep the links between modules clear. The intention of ...type === 'voice' is clear, it will act as a proper runtime check, and is fully in line with typescript conventions. Its totally just opinion at this point, but I prefer the syntax of the property check.

(I have now made a new commit to amend the typings of the collections that you highlighted in your example)

[ghost]

Try not to slow down the runtime with instanceof. Might be a small performance hit, but compile-time checking (TypeScript) has many advantages to runtime checks.

[Ayplow]

Huh, I wouldn't have guessed, but it seems instanceof is actually quite time intensive according to this site:
https://www.dotnetperls.com/instanceof-js

P.S If someone wants to make the PR, go ahead. I'll get to it as soon as I'm up to it, but that could be a while yet

[jkantr]

Not entirely related, but there's a similar issue with using checkJs / jsdoc

/**
 * @param {Discord.Client} client
 * @returns {Discord.Collection<string, Discord.TextChannel>}
 */
function getAllGeneralTextChannels(client) {
    return client.channels
        .filter(channel => channel instanceof Discord.TextChannel)
        .filter(channel => channel.name === 'general');
}

Collection#filter doesn't trigger whatever typescript guard would cause it to understand the contents of the collection. The 2nd filter becomes an error because Discord.Channel ofc doesn't have prop name. But it should recognize the guard and only TextChannels being left in the Collection.

Edit: For future generations... first converting the Collection to a native Array allows use of a return guard:

/**
 * @param {Discord.Collection<string, Discord.Channel>} channels
 * @returns {Array<Discord.TextChannel>}
 */
function getAllTextChannels(channels) {
    return Array.from(channels.values()).filter(
        /** @returns {channel is Discord.TextChannel} */
        channel => channel.type === 'text'
    );
}