Skip to content

Conversation

suneettipirneni
Copy link
Member

@suneettipirneni suneettipirneni commented Sep 20, 2021

Please describe the changes this PR makes and why it should be merged:

Allows you to narrow out a GuildMember or a APIGuildMember from the GuildMember | APIGuildMember union.

if (!interaction.inCachedGuild()) {
  return;
}

// Member is narrowed to `GuildMember`
const { member } = interaction;

Alternatively...

if (!interaction.inRawGuild() {
  return;
}

// Member is narrowed to `APIGuildMember`
const { member } = interaction;

Also look at changes here: #6668 (comment)

Some have brought up "Why not use instanceof interaction.member"

The answer is because it's not as elegant and it doesn't mutate any other types on the interaction such as guild and the removal and/or assertion of properties being null. In some circumstances it can be misleading, the methods are more explicit in their purpose.

Status and versioning classification:

  • Code changes have been tested against the Discord API, or there are no code changes
  • I know how to update typings and have done so, or typings don't need updating
  • This PR changes the library's interface (methods or parameters added)

suneettipirneni and others added 2 commits September 20, 2021 17:00
Co-authored-by: Rodry <38259440+ImRodry@users.noreply.github.com>
@monbrey
Copy link
Member

monbrey commented Sep 20, 2021

Although I like the concept of what this checks I can't help but feel the interface / parameter is really awkward to use in practice. Combining the truthy conditions to achieve the assertions, although confusing, would work from what I can see. However this method returning false doesn't seem to have been considered.

CommandInteraction#inGuild(true); // false - does this assert that its not a GuildMember, but it could still be APIInteractionGuildMember | null? Used in this way it doesn't actually help you determine if this is a DM or not, at least not that I can see? I don't even know if you can have falsy assertions.

I think combining these into one method with a boolean parameter is where I'm finding it awkward. I feel like inGuild() should clearly assert the difference between a DM and a Guild, then a separate method should assert the existence of a bot user.

@ImRodry
Copy link
Contributor

ImRodry commented Sep 20, 2021

When this was brought up on the discord server I also thought of it as a separate method, the only issue is I wouldn’t know what to call it but if someone does then I think it would be a preferred scenario

@suneettipirneni suneettipirneni force-pushed the types/interaction-member-narrowing branch from 8e99dff to a331029 Compare September 21, 2021 02:18
@suneettipirneni suneettipirneni changed the title types(Interaction): allow Interaction#member to be type narrowed types(Interaction): allow Interaction cached properties to be type narrowed Sep 27, 2021
@suneettipirneni
Copy link
Member Author

suneettipirneni commented Sep 27, 2021

Substantial changes have been made since this PR originally was opened, rather than just being able to narrow regular Interactions it now allows interaction sub types to be narrowed as well using conditional typings. This allows a much more robust type checking system for interactions. For example let's say I want to know if a command interaction is in a cached guild, given a base Interaction:

// Narrow two levels
if (interaction.isCommand() && interaction.isInCachedGuild()) {
  // interaction is now of type GuildCommandInteraction<'cached'>;
  ...
  // Because we are in a cached guild, we can intuitively narrow `Message | APIMessage` to `Message`
  const message: Message = await interaction.fetchReply();
}

It now can not only infer guild specific properties are of a certain type, but can also infer typings for methods specific to CommandInteraction.

Alternatively you can use the types to give expectations as to what your function recieves:

{
  ...
  // The command handler run method only accepts command interactions where the bot is in the same guild.
  execute(interaction: GuildCommandInteraction<'cached'>) {
    // No need to verify cached or not at this point, the parameter type guarantees it's in a cached guild.
  }
}

This eliminates redundant type checking/casts in areas where the developer knows the guild will be cached.

@suneettipirneni suneettipirneni force-pushed the types/interaction-member-narrowing branch 2 times, most recently from 48aaa04 to e7f7e56 Compare September 28, 2021 03:53
@suneettipirneni suneettipirneni force-pushed the types/interaction-member-narrowing branch 2 times, most recently from 131f0bd to dcad948 Compare September 28, 2021 04:02
@suneettipirneni suneettipirneni force-pushed the types/interaction-member-narrowing branch 2 times, most recently from d169f07 to b4c353a Compare September 28, 2021 04:08
@suneettipirneni suneettipirneni force-pushed the types/interaction-member-narrowing branch from b4c353a to 6ba34b8 Compare September 28, 2021 04:10
kyranet
kyranet previously requested changes Oct 3, 2021
Co-authored-by: Antonio Román <kyradiscord@gmail.com>
@iCrawl iCrawl merged commit d873a19 into discordjs:main Oct 4, 2021
@suneettipirneni suneettipirneni deleted the types/interaction-member-narrowing branch October 10, 2021 18:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants