Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

creating slash commands: REST + Routes vs. application.commands.create() #1098

Closed
intcreator opened this issue May 1, 2022 · 4 comments
Closed

Comments

@intcreator
Copy link

Part of the guide or code sample the question is about

https://discordjs.guide/creating-your-bot/creating-commands.html#command-deployment-script

Question

this is one of the very first pages of the guide for newcomers. it teaches them to register slash commands using three additional libraries that are not in fact Discord.js but Discord REST, Discord Builders, and Discord API types. I'm sure most newcomers are already overwhelmed by the complexity of the Discord.js, I know I was when I first started bot development. why are we asking them to learn about three new libraries when Discord.js can register commands much more simply? here is what I ended up using for my bot as my deploy-commands.js:

require('dotenv').config();
const { Client, Intents } = require('discord.js');

const client = new Client({
	intents: [Intents.FLAGS.GUILDS]
});

client.login(process.env.TOKEN);

client.once('ready', async () => {
    try {
        await client.application.commands.create({
            name: 'ping',
            description: 'Replies with pong!',
        }, process.env.GUILD_ID)
    } catch (e) {
        console.error(e);
    }
    console.log(`Successfully registered application commands for guild with id ${process.env.GUILD_ID}`);

    client.destroy();
})

I don't think this is a duplicate of #859 as I think that issue is referring to an older version of the documentation

@BenjammingKirby
Copy link
Contributor

The ready event would be a horrible place to make this call since you're going to be making an unnecessary call every time you turn on the bot pretty much
I don't think we can just tell the users to use it once and remove it because they'd need to reuse it
That said the guide's simplifying the raw api request part and on a separate script it's imo the best approach

I agree users may overwhelmed but that doesn't mean we should straight up switch the whole procedure
Instead add comments and make it easier for the users to comprehend, perhaps link the api docs to show we're making a put request to this endpoint

@intcreator
Copy link
Author

intcreator commented May 1, 2022

I think you misunderstand the code a bit, this is still deploy-commands.js which would only be modified and run when commands need to be updated and kept in the project unused but saved otherwise. this is not in the same file as the rest of the bot code

it's funny that you say we shouldn't switch up the whole procedure because I first implemented slash commands using some of the unsupported commands in v12 and even that didn't require installing new packages. the rest/routes method of the guide felt disorienting, almost as if Discord.js has an incomplete API. in theory registering bot commands should be built right into Discord.js, right? and there is a way to do it with just Discord.js. so why not do it that way?

if it doesn't bother anyone else I guess it's fine, but it bothered me so I wanted to flag it

@BenjammingKirby
Copy link
Contributor

There's a limit to how many times you can login, I'd say that this is still a bit more wasteful for one call. The separate raw api call approach is the safest judging limits. And i don't agree with the whole we're making them learn three - packages point. All it's doing is getting the route from api-types and as the method says, it's making a put request to that route with rest. None you really need to learn. I think in this cases a more descriptive comment does the trick.
Builders however the guide uses for the command handler and I'd say even though i don't like builders, that it's easy to understand.

@almostSouji
Copy link
Member

The use case for the main lib method is runtime changes. Logging in just to deploy commands is not necessary and depletes limits.

Teaching users best practices is something we aim to achieve with the guide, even if they are uncomfortable at first.

Builders however the guide uses for the command handler and I'd say even though i don't like builders, that it's easy to understand.

This has been brought up internally as well, and we'll see if we can feasibly cover raw structures without negatively affecting the flow of the guide, a separate issue #1122 tracks this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants