V14 update #1

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

Jump to bottom

Merged

shiffman merged 5 commits into main from v14-update

Oct 5, 2023

Merged

V14 update #1

shiffman merged 5 commits into main from v14-update

Oct 5, 2023

Conversation

@NwE0kmCE

Copy link

Collaborator

@NwE0kmCE NwE0kmCE commented Sep 12, 2023

Updates examples and instructions to discord.js v14

NwE0kmCE added 2 commits

August 7, 2023 11:00

@NwE0kmCE


 Added where to find developer mode toggle, removed uneccessary steps ...

64561db

...for discordjs v14

@NwE0kmCE


 Updated examples to discord.js v14, added documentation

0372ca6

supercrafter100

supercrafter100 reviewed

Sep 20, 2023

View reviewed changes

Copy link

Collaborator

@supercrafter100 supercrafter100 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here are some of my thoughts. You might want to consider switching the imports to es6 ones in the future as they're becoming pretty standard now. Though it might cause some confusion amongst newcomers to the nodejs world.

01-discordjs/deploy-commands.js Outdated

}

// Construct and prepare an insance of the REST module

const rest = new REST().setToken(token);

Copy link

Collaborator

@supercrafter100 supercrafter100 Sep 20, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

token isn’t defined here from what I can see?

Copy link

Member

@shiffman shiffman Oct 5, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated!

01-discordjs/README.md Outdated

$ node index.js

```

```javascript

constdotenv=require("dotenv");

Copy link

Collaborator

@supercrafter100 supercrafter100 Sep 20, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you only required dotenv, never called dotenv.config() before reading the process.env.TOKEN variable

Copy link

Member

@shiffman shiffman Oct 5, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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();

Copy link

Collaborator

@supercrafter100 supercrafter100 Sep 20, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Comment on lines 26 to 27

"discord.js": "^14.13.0",

"dotenv": "^16.3.1"

Copy link

Collaborator

@supercrafter100 supercrafter100 Sep 20, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These would be regular dependencies though? Not dev dependencies?

Copy link

Member

@shiffman shiffman Oct 5, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed!

02-command-handler/README.md Outdated

...with...

```javascript

require("dotenv").config();

Copy link

Collaborator

@supercrafter100 supercrafter100 Sep 20, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As mentioned earlier, should we replace this with the same code from the earlier parts to keep consistent across files?

02-command-handler/deploy-commands.js Outdated

}

// Construct and prepare an insance of the REST module

const rest = new REST().setToken(process.env.CLIENTSECRET);

Copy link

Collaborator

@supercrafter100 supercrafter100 Sep 20, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

shouldn't this be process.env.TOKEN?

Copy link

Member

@shiffman shiffman Oct 5, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed!

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" });

Copy link

Collaborator

@supercrafter100 supercrafter100 Sep 20, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is the path option necessary here?

Copy link

Member

@shiffman shiffman Oct 5, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

removed this exmaple for now

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);

Copy link

Collaborator

@supercrafter100 supercrafter100 Sep 20, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same comments here as earlier about the console.logs (same applies to line 84 here)

02-command-handler/package.json Outdated

Comment on lines 14 to 16

"devDependencies": {

"discord.js": "^14.13.0",

"dotenv": "^16.3.1"

Copy link

Collaborator

@supercrafter100 supercrafter100 Sep 20, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Starting to become repetitive 😅 same as earlier

package.json Outdated

Comment on lines 2 to 5

"devDependencies": {

"discord.js": "^14.13.0",

"dotenv": "^16.3.1"

}

Copy link

Collaborator

@supercrafter100 supercrafter100 Sep 20, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And finally here the concern about the dev dependencies too

shiffman added 3 commits

October 5, 2023 18:29

@shiffman


 finishing the example and expanding README

80d0d57

@shiffman


 es6 update

88e2894

@shiffman


 fix ephemeral

6dede93

@shiffman

Copy link

Member

shiffman commented Oct 5, 2023

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!

@shiffman shiffman merged commit 9fe20ce into main

Oct 5, 2023

supercrafter100

supercrafter100 reviewed

Oct 8, 2023

View reviewed changes

01-discordjs/commands/gif.js

};

// Importing modules using ES6 syntax

import fetch from 'node-fetch';

Copy link

Collaborator

@supercrafter100 supercrafter100 Oct 8, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@dipamsen dipamsen mentioned this pull request

Oct 8, 2023

Use native fetch in node instead of node-fetch library #2

Closed

@supercrafter100 supercrafter100 mentioned this pull request

Oct 9, 2023

Convert code samples to ES6 imports to be consistent with the example code in the repository #5

Closed

@shiffman shiffman deleted the v14-update branch

October 23, 2023 17:27

Labels

None yet

3 participants

@NwE0kmCE @shiffman @supercrafter100

Uh oh!

V14 update #1

V14 update #1

Uh oh!

Conversation

@NwE0kmCE NwE0kmCE commented Sep 12, 2023

Uh oh!

@supercrafter100 supercrafter100 left a comment

Choose a reason for hiding this comment

Uh oh!

@supercrafter100 supercrafter100 Sep 20, 2023

Choose a reason for hiding this comment

Uh oh!

@shiffman shiffman Oct 5, 2023

Choose a reason for hiding this comment

Uh oh!

@supercrafter100 supercrafter100 Sep 20, 2023

Choose a reason for hiding this comment

Uh oh!

@shiffman shiffman Oct 5, 2023

Choose a reason for hiding this comment

Uh oh!

@supercrafter100 supercrafter100 Sep 20, 2023

Choose a reason for hiding this comment

Uh oh!

@supercrafter100 supercrafter100 Sep 20, 2023

Choose a reason for hiding this comment

Uh oh!

@shiffman shiffman Oct 5, 2023

Choose a reason for hiding this comment

Uh oh!

@supercrafter100 supercrafter100 Sep 20, 2023

Choose a reason for hiding this comment

Uh oh!

@supercrafter100 supercrafter100 Sep 20, 2023

Choose a reason for hiding this comment

Uh oh!

@shiffman shiffman Oct 5, 2023

Choose a reason for hiding this comment

Uh oh!

@supercrafter100 supercrafter100 Sep 20, 2023

Choose a reason for hiding this comment

Uh oh!

@shiffman shiffman Oct 5, 2023

Choose a reason for hiding this comment

Uh oh!

@supercrafter100 supercrafter100 Sep 20, 2023

Choose a reason for hiding this comment

Uh oh!

@supercrafter100 supercrafter100 Sep 20, 2023

Choose a reason for hiding this comment

Uh oh!

@supercrafter100 supercrafter100 Sep 20, 2023

Choose a reason for hiding this comment

Uh oh!

shiffman commented Oct 5, 2023

Uh oh!

@supercrafter100 supercrafter100 Oct 8, 2023

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants