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

chore: update to latest discordjs and node. #957

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

laquasicinque
Copy link
Contributor

  • ⬆️ updated all packages to latest except for eslint because that's going to be a chore until @ljosberinn updates his config (nudge nudge 👀)
  • ➖ Removed ts-node, it's dead. removed nodemon too as it's not required anymore
  • ➕ Added tsx (typescript execute, not to be confused with the react kind), does what nodemon and ts-node used to do together
  • ♻️ refactored code to match the latest version of discordjs

also changed from ts-node to tsx
'GuildMembers',
'GuildBans',
'GuildEmojisAndStickers',
'MessageContent',
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Note: This is new and will probably require @ljosberinn to update bot permissions on his end

Copy link
Contributor

Choose a reason for hiding this comment

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

happy to do that but uh where 👀 is this server settings side on discord or bots permissions on the page somewhere?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's on the discord developer portal, under app intents or something.

Copy link
Contributor

Choose a reason for hiding this comment

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

this is already active 🎉
image

src/v2/autorespond/code_parsing/index.ts Outdated Show resolved Hide resolved
src/v2/commands/npm/index.ts Outdated Show resolved Hide resolved
src/v2/modules/mod/commands/roles.ts Outdated Show resolved Hide resolved
src/v2/modules/onboarding/events/handleRoleSelected.ts Outdated Show resolved Hide resolved
const perms = new Permissions(Permissions.DEFAULT);
perms.remove(Permissions.FLAGS.VIEW_CHANNEL);
function addNewRolePermissions(guild: Guild, role: Role) {
const permissionFlags = PermissionsBitField.Default & ~PermissionsBitField.Flags.ViewChannel
Copy link

Choose a reason for hiding this comment

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

question: What's with the ~PermissionsBitField.Flags.ViewChannel particually the ~

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Discord permissions work with a Bit Fields (which are bigints that store permissions encoded as individual bits) and discord.js exposes this now, so doing boolean maths here.

I want to use the default permissions but remove the ability to view the channel.

~ is a bitwise NOT, it flips every bit.
& is a bitwise AND, it ANDs every bit between two numbers

Lets say the default permissions are 0b0111and the view channel permission is 0b0010:

  1. I NOT the view channel perms (turning 0b0010 into 0b1101)
  2. I AND the result with the default perms so 0b0111 & 01101 which results in 0b0101

I'm not sure, although I wouldn't be surprised if there are some helper functions, particularly under PermissionBitField that could allow me to refactor this. Considering that bitwise stuff isn't as common in the mainstream JS world, do you think I should refactor this to potentially use that? Or should I add a comment with the explanation?

Copy link

Choose a reason for hiding this comment

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

Ah nice thanks for explaining it to me! should defo spruce up my discord bot knowledge been a few years

src/v2/modules/onboarding/utils/sneakPin.ts Outdated Show resolved Hide resolved
src/v2/utils/codeBlockCapturer.ts Outdated Show resolved Hide resolved
package.json Outdated
Copy link

@Yofou Yofou Aug 30, 2024

Choose a reason for hiding this comment

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

⬆️ updated all packages to latest except for eslint because that's going to be a chore until @ljosberinn updates his config (nudge nudge 👀)

I'd go further and advice against going to eslint 9 alot of the community (plugins / configs) haven't migrated yet and the compatibility layer for eslint 8 in eslint 9 is almost zero existent, I tried to do this internally at my work a month ago and fell flat on my ass in every corner. so just to save you both time haha I'd wait it out a little longer.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I upgraded my workplace eslint plugin to 9 fairly recently so I'm aware of how painful it is 😅, it's kinda why I'm aiming to wait for when gerrit updates his config so he can do all the hard work, seeing as we're using the config anyway.

There's no rush though. I did it because I wanted to use the new using keyword at work which would break our outdated ESLint, and that we're doing a considerable cleanup of our work codebase anyway.

Copy link
Contributor

Choose a reason for hiding this comment

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

I won't have time for the forseeable future to update my lib either, am not sure whether I'll continue maintaining it as I'd have to practically rewrite it...

Copy link

@Yofou Yofou left a comment

Choose a reason for hiding this comment

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

Looks good!

I'll smoke test this later this week, I'll let you know on discord if you wanna join me.

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

Successfully merging this pull request may close these issues.

3 participants