V14 update #1
V14 update #1
Conversation
01-discordjs/deploy-commands.js
Outdated
| } | ||
|
|
||
| // Construct and prepare an insance of the REST module | ||
| const rest = new REST().setToken(token); |
There was a problem hiding this comment.
token isn’t defined here from what I can see?
01-discordjs/README.md
Outdated
| $ node index.js | ||
| ``` | ||
| ```javascript | ||
| const dotenv = require("dotenv"); |
There was a problem hiding this comment.
you only required dotenv, never called dotenv.config() before reading the process.env.TOKEN variable
There was a problem hiding this comment.
consolidated everything into one README
01-discordjs/deploy-commands.js
Outdated
| const { REST } = require('@discordjs/rest'); | ||
| const { Routes } = require('discord-api-types/v9'); | ||
| const { REST, Routes } = require("discord.js"); | ||
| require("dotenv").config(); |
There was a problem hiding this comment.
To keep consistent with the other code, wouldn't something like the following:
const dotenv = require('dotenv');
dotenv.config();make more sense here?
01-discordjs/package.json
Outdated
| "discord.js": "^14.13.0", | ||
| "dotenv": "^16.3.1" |
There was a problem hiding this comment.
These would be regular dependencies though? Not dev dependencies?
02-command-handler/README.md
Outdated
| ...with... | ||
|
|
||
| ```javascript | ||
| require("dotenv").config(); |
There was a problem hiding this comment.
As mentioned earlier, should we replace this with the same code from the earlier parts to keep consistent across files?
| } | ||
|
|
||
| // Construct and prepare an insance of the REST module | ||
| const rest = new REST().setToken(process.env.CLIENTSECRET); |
There was a problem hiding this comment.
shouldn't this be process.env.TOKEN?
02-command-handler/index.js
Outdated
|
|
||
| // Setup dotenv and tell it to look for `.env` in the same folder as our main file | ||
| const dotenv = require("dotenv"); | ||
| dotenv.config({ path: "./.env" }); |
There was a problem hiding this comment.
Is the path option necessary here?
02-command-handler/index.js
Outdated
| client.on(Events.InteractionCreate, async (interaction) => { | ||
| // Ignore interaction if it isn't a slash command | ||
| if (!interaction.isChatInputCommand()) return; | ||
| console.log(interaction.client.commands); |
There was a problem hiding this comment.
Same comments here as earlier about the console.logs (same applies to line 84 here)
02-command-handler/package.json
Outdated
| "devDependencies": { | ||
| "discord.js": "^14.13.0", | ||
| "dotenv": "^16.3.1" |
There was a problem hiding this comment.
Starting to become repetitive 😅 same as earlier
package.json
Outdated
| "devDependencies": { | ||
| "discord.js": "^14.13.0", | ||
| "dotenv": "^16.3.1" | ||
| } |
There was a problem hiding this comment.
And finally here the concern about the dev dependencies too
|
I have fixed and updated a bunch of things, I removed the second example, I will likely add more but for now just the "hello world" example with two commands! Thank you for all of the work @klinegareth and for the feedback @supercrafter100! |
| }, | ||
| }; | ||
| // Importing modules using ES6 syntax | ||
| import fetch from 'node-fetch'; |
There was a problem hiding this comment.
I totally forgot to mention but fetch is now natively supported in node. There isn't a need to actually install the package anymore. Since v18.18.0 it is implemented, see https://nodejs.org/dist/latest-v18.x/docs/api/globals.html for detailed information. This removes the need for this dependency.
Updates examples and instructions to discord.js v14