Revert some changes of #8027

[DraftProducts]

Which package is this bug report for?

discord.js

Issue description

Some changes were done #8027 witch are not profitable for Djs :

1. The method <Interaction>.isModalSubmit() has been removed, we can't filter anymore to redirect Modal submits interactions. (looks like a error)
   
2. Global types check like <Interaction>.isMessageComponent() and <Interaction>.isCommand() has been removed too, thoose two functions are practical and necessary because in most of the cases chat inputs and context menu commands are managed in a same class, the logic of the 3 types of commands are similar enough.
   

Hi @suneettipirneni, is there a way that I wouldn't have seen who would justify these changes?

Package version

discord.js@14.0.0-dev.1654949087-96053ba

Priority this issue should have

High (immediate attention needed)

I have tested this issue on a development release

96053ba

[suneettipirneni]

All of these have been replaced as so:

if (interaction.isModalSubmit) { ... }

if (interaction.type === InteractionType.ModalSubmit) { ... }

if (interaction.isCommand()) { ... }

if (interaction.type === InteractionType.ApplicationCommand) { ... }

if (interaction.isMessageComponent()) { ... }

if (interaction.type === InteractionType.MessageComponent) { ... }

The interactions with types remains the same and they still narrow properly.

Alternatively you can use instanceof

The rationale was that because the type guards only aliased a single condition check, they were more of a shortcut method that a user can easily replicate, rather than an instance method of any actual value.

[DraftProducts]

I understand that it's optional or even considered a shortcut @suneettipirneni, but we have plenty of shortcuts everywhere in Djs and in terms of DX, it's a good thing and keeps the code clean, having methods for some advanced checks and not having them for simple ones seems to me to be a change that doesn't add anything to Djs.
I don't see why it would be annoying to leave them.

Here is an example where it would not be aesthetic at all, if I only deal with slash-commands and modals, it is impossible to keep a uniform code:

client.on('interactionCreate', (interaction) => {
  if (interaction.type === InteractionType.ModalSubmit) {
    // return this.functionToHandleModals(interaction);
  }
  
  if (interaction.isChatInputCommand()) {
    // return this.functionToHandleSlashCommands(interaction);
  }
})

[suneettipirneni]

I understand that it's optional or even considered a shortcut, but we have plenty of shortcuts everywhere in Djs and in terms of DX, it's a good thing and keeps the code clean, having methods for some advanced checks and not having them for simple ones seems to me to be a change that doesn't add anything to Djs.

The other shortcuts were kept since they involved more than one check to the same interaction. As for djs having trivial shortcuts still in its library, it's a long term goal to remove them all.

If your method's only job is to check the value of a single property on a class, that doesn't seem like it needs it's own method, unless it has type implications that can't be expressed with ifs.

That's the equivalent of having a hasComponent method on messages like so:

message.hasComponent(component);
message.components.includes(component);

Sure the first is shorter and less code, but it's rather trivial so why does it need to be provided?

In this specific case, it's also common practice to use duck-typing (.type === toSomeValue) and/or instanceof to narrow wide types in other js/ts libraries.

If your end goal is uniformity then shouldn't this suffice?

client.on('interactionCreate', (interaction) => {
  if (interaction instanceof ModalSubmitInteraction) {
    // return this.functionToHandleModals(interaction);
  }
  
  if (interaction instanceof ChatInputCommandInteraction) {
    // return this.functionToHandleSlashCommands(interaction);
  }
})

[DraftProducts]

Okay, thanks for your complete answer @suneettipirneni, nevertheless I remain convinced that keeping this kind of methods can only be a bonus that has no negative points.
Leaving them in would not prevent the use of (.type === toSomeValue) or instanceof while allowing developers who wish to use the other methods and want consistency to do so.

I'd love to hear from other maintainers and developers about these changes, other perspectives would be welcome.

Unfortunately, and this doesn't question all the work you do on the library, I find that, like a great majority of changes in this version 14, it only brings breaking-changes that are mostly useless and that will either kill projects because the migration will be so painful, or will make all the developers who give their all to their projects lose an incredible amount of time.

[suneettipirneni]

There can be a discussion but I'd rather it be in a GitHub discussion or another issue ticket. Since this current issue ticket is created as a bug report, but it's currently intended functionality.

[imranbarbhuiya]

Those typeguard's typings were wrong. If you write

if (interaction.isModalSubmit()) {
		if (interaction.isCommand()) {
			console.log(interaction);
		}
	}

It'll make the typing to ModalSubmitInteraction<CacheType> & CommandInteraction<CacheType>
but when u use

if (interaction.type === InteractionType.ModalSubmit) {
		if (interaction.type === InteractionType.ApplicationCommand) {
			console.log(interaction);
		}
	}

It'll become never also ts gives us error This condition will always return 'false' since the types 'InteractionType.ModalSubmit' and 'InteractionType.ApplicationCommand' have no overlap.. Imo we shouldn't revert them as checking for .type is much typesafe.

[skick1234]

This is due to the older version is not type-safe.
#8027 fixed this issue by adding type property to ModalSubmitInteraction, CommandInteraction, e.t.c.
Not related to the author's opinion.

[Jiralite]

This'll be coming via #8328.