archive-library-discussion

[outer raven]

I opened an issue, let me know if there's anything to add to it https://github.com/discord/discord-api-docs/issues/3898

[dim steppe]

Checked the site and couldn't find any mention of this yet — have I missed any further discussion of it? Was anything decided upon?

[analog oyster]

@tacit crypt https://github.com/discordjs/discord.js/pull/6710#issuecomment-929528909
you approved the pull request after the fact, so i dont know if i should do it or not?

[tacit crypt]

If its the linting imo I'd say do it but I'd wait for @wild flax too

[analog oyster]

it is the linting

[outer raven]

@remote wasp was testing the role icons PR and I noticed that trying to set a role's icon to another role's icon URL without any options (webp format) gets rid of the transparency of the icon. What do you think of defaulting the role icon endpoint format to png instead of webp?

I would personally like for all endpoints to default to png but that would be something for later

[remote wasp]

makeImageURL defaults it to webp and every other CDN method did too before your PR. If it was a random choice then yeah sure, but if there was a reason why only webp was chosen as default then you might want to ask others too

[earnest topaz]

When is the new api change going to be added?

[outer raven]

my pr only removed redundant defaults, but we could theoretically change the default for the role icon endpoint
As for a reason to use webp, I've never seen any on the discord docs and I can't come up with any myself but I think crawl once said it was better, but I don't know in what world since it's quite an annoying format to work with

[opaque vessel]

Discord tends to use .webp & they're also generally smaller in size for same/better quality... off the top of my head that'd probably be a contributing reason why it's used as the default

[earnest topaz]

Oops sry

[outer raven]

Where does discord use webp? Also if that’s a format that doesn’t even support transparency, even if it has smaller file sizes why would that matter when all you’re doing is generating a url?

[opaque vessel]

All avatars are in a .webp format, a quick Google reveals that it does support transparency, and people may be downloading them

[outer raven]

yeah they can download the image but that doesn't concern the library at all
if the discord client doesn't even show previews of webp images i dont see why it would be a default

[golden mortar]

how can i make the esm portion of the lib manually (ie. a pr)?

[vernal atlas]

there seems to be an error in the latest dev version (13.2.0-dev.1633089797.fe95005) - passing message options w/ an empty string content and an embed gives you an error saying the content can't be empty, but this is valid because the message has an embed

just to confirm, empty content but with an embed, which is valid but obviously the validation checks give a false negative

[copper garden]

Set the content to null instead

[vernal atlas]

ah i see

would it make sense to accept an empty string too though, if there is other content on the message (last time i checked discord did accept this, don't see why that would've changed)

[copper garden]

I see what ur saying, but that would require a breaking change for an edge case that most ppl won’t run into

[slate nacelle]

I think we intentionally are treating an empty string as error since this happens a lot on accident.

[ornate topaz]

it's been erroring like that since at least 13.0.0-dev.b15d825bb3acdf432b94d8413a7a964ccc8734bc, though it didn't complain this way on v12

[copper garden]

v13 added the stricter string checks bc ppl were passing in objects into methods that only accepted string

[outer raven]

you'd get an api error instead when just passing an empty string as the content, so it's not much better

[outer raven]

why would it be breaking though?

[oak quail]

iirc if you pass null, d.js changes it to an empty string before sending

[outer raven]

Nope, it passes null

Because iirc the api doesn’t accept empty strings at all

[oak quail]

the api supports both but doesnt document nullability

[outer raven]

Pretty sure that is not related to sending messages because I just called MessagePayload.create myself and it passes null in the content

[oak quail]

that is called by MessagePayload.resolveData which is called by TextBasedChannel.send

also i tested with the raw api and both work

[outer raven]

Ah I was one step behind then
Well I guess the empty string check could be removed then and let users get Discord’s error instead? Or maybe it would only be allowed when there is an embed

I think embeds are the only other thing that counts as content right

[outer raven]

Oh yeah

That check would be more robust then, but it’s easily doable

And would not be a breaking change

In fact it would probably be minor

[remote wasp]

npx gen-esm-wrapper ./src/index.js ./src/index.mjs

[golden mortar]

thanks, exactly what i needed

[summer beacon]

https://github.com/discordjs/Commando i think there should be a deprecation message or something similar in this repo telling users to not use it

[outer raven]

@ruby terrace "In order to type the response, I had to add @types/node-fetch (since we don't target DOM), which does not have a version matching the version in the library since v3 has built in types." wdym? The last published version of @types/node-fetch supports v2.5 of node-fetch

[ruby terrace]

and the lib has 2.6.1 installed. The types for Response should not have changed, but its a semver minor bump, so some types are wrong

[outer raven]

I find it weird that the maintainers of @types/node-fetch didn't bump it tho
perhaps they did and forgot to bump the version?

[dim steppe]

https://canary.discord.com/developers/docs/interactions/slash-commands#receiving-an-interaction

Regarding <Interaction>.member.permissions — do you think this information should be added to Interaction itself, or should an InteractionGuildMember extend GuildMember? Was gonna get started on it.

[outer raven]

you can't have an InteractionGuildMember because not all interaction.member's extend GuildMember, some are just the raw data from the API

[dim steppe]

that's what I've been doing until something was decided upon:
this.memberPermissions = data.member?.permissions ? new Permissions( data.member.permissions ) : null;

[outer raven]

yeah this sounds like the way to go

[slate nacelle]

What's the use case for that?

[dim steppe]

Discord provides the member's exact permissions, all roles included, of the member which the interaction was created

DJS currently ignores this

[slate nacelle]

You can locally calculate them just fine through the permissions getter, can't you?

Plus they update unlike the number you get through the payload.

[dim steppe]

No. Now, with DJS' caching, if you don't have a channel cached, you have to fetch the channel. If the bot doesn't have VIEW_CHANNEL for that channel, you get an error

[wild flax]

The reason we ignore it is because we already have the ability to easily calculate it if its cached

If you don't have the channel cached you might have other issues with djs

Just saying

[dim steppe]

The only issue I've run into so far is trying to fetch channels that the bot doesn't have VIEW_CHANNEL for. You can still respond to interactions in these channels, but you can't fetch permissions of members/roles for them

[slate nacelle]

You surely can do that.

You get all channels of all guilds your bot is in, no matter the permissions.

[dim steppe]

Right, and if you don't cache them, you have to later fetch them. Manually fetching a channel which you don't have VIEW_CHANNEL for is a missing access error

[outer raven]

yeah and the api docs don't specify any limitations in regards to permissions

[outer raven]

sure? This isn't documented anywhere

[dim steppe]

Yessir

So we need to implement the permissions that Discord sends us with the interaction

Otherwise, if we don't cache channels, we need to fetch the channel (which could result in an error), manually calculate the member's permissions based on their roles, fetch the default/global permissions for each role they have, etc... When Discord just... Sent it to us

[outer raven]

nope that is incorrect, just tested

you can fetch any channel as long as you're in its guild

[vivid field]

it also seems weird to me that you have the guild cached, but not its channels

[dim steppe]

I just tried it, and got Missing access

[outer raven]

you're doing something wrong then

[slate nacelle]

/guilds/:guild_id/channels includes it, it's also part of the guild create payload.

Not sure what the point in discarding channels is however.

[outer raven]

@dim steppe see

[dim steppe]

bot.on( 'interactionCreate', async interaction => {
  await interaction.guild.channels.fetch( interaction.channelId )
} );

DiscordAPIError: Missing Access
    at RequestHandler.execute (C:\Users\Stev\projects\rocket-planet\node_modules\discord.js\src\rest\RequestHandler.js:298:13)
    at processTicksAndRejections (node:internal/process/task_queues:96:5)
    at async RequestHandler.push (C:\Users\Stev\projects\rocket-planet\node_modules\discord.js\src\rest\RequestHandler.js:50:14)
    at async ChannelManager.fetch (C:\Users\Stev\projects\rocket-planet\node_modules\discord.js\src\managers\ChannelManager.js:114:18)
    at async Command.handler (file:///C:/Users/Stev/projects/rocket-planet/commands/reputation/mjs/handler.mjs:30:9)
    at async Command.handleCommand (file:///C:/Users/Stev/projects/rocket-planet/structures/command.mjs:395:30)
    at async Promise.all (index 11)
    at async Client.handler (file:///C:/Users/Stev/projects/rocket-planet/events/command-interaction/command-interaction-handler.mjs:47:9) {
  method: 'get',
  path: '/channels/658894775814062096',
  code: 50001,
  httpStatus: 403,
  requestData: { json: undefined, files: [] }
}

[outer raven]

🤨

[dim steppe]

The only permissions it doesn't have for that channel is VIEW_CHANNEL.

As soon as I give it that permissions, it works

[vivid field]

you probably have the channel cached so it doesn't call the api

but as I said, if you have the guild you should have its channels too

[dim steppe]

I run a couple bot that are in 35K servers, 80K servers, etc. Not cacheing channels was the biggest way to save RAM

[outer raven]

aight yeah you're right, mb

[dim steppe]

So far I've been able to account for that by manually fetching the channels. The only issue I have now, because of the error cause by the VIEW_CHANNEL permission, is determining permissions in channels that the bot cannot fetch, but interactions can still be used in

Still, it seems silly to not implement information that Discord sends, and instead jumping through hoops to attempt to manually fetch and calculate the very same information

It's not an issue for small bots. It's an issue for large bots that can't afford to cache literally everything

[wild flax]

It's not silly at all if you think about how we don't even support overriding the channel manager lol

But I guess we can do it, space just asked for a use case and a reproducible example how it errors

Not that its dumb, sooo

It just needs to nicely work with the current permissions and not be confusing

[dim steppe]

I find it even more silly that you can't fetch channels without view channel. 😕

[wild flax]

Because best case scenario, you wouldn't want to use interaction.member.perms if you can calculate them differently

Except in your use case I guess

[outer raven]

I think a memberPermissions property in the interaction object works just fine, seeing as you cannot overwrite member.permissions

is that present on all interaction types or just some? The api documents it as optional with no extra context so idrk

[wild flax]

well

dms

[dim steppe]

Yeah, global interactions used in DM don't have it.

I wonder if the permissions issue was somehow Discord's way of attempting to address the problem of "hidden" channel names and topics being technically visible to anyone using a self bot or BD or something.

[slate nacelle]

Probably just inconsistency.

The client still has all of them.

[opaque vessel]

Is it me or this didn't happen
#archive-github-djs message

[outer raven]

it didn't but not only can member.permissions never be 0 but if it could that wouldn't be a problem because it's a string

[opaque vessel]

Ah yes, okay

[outer raven]

and it can't be 0 because the member needs permission to send messages and use application commands to send the actual interaction

[outer raven]

I just noticed we can't use ThreadChannel#setLocked on a thread that is archived, could this be documented with an info on the edit and setLocked methods?

[ruby terrace]

archived threads are completely immutable other than unarchiving, which is pretty well documented IMO

[outer raven]

Is it? Where’s that?

[ruby terrace]

https://discord.com/developers/docs/topics/threads#active-archived-threads

[outer raven]

I’m saying there should be sth about it on the djs docs

On another note though, it seems like setting locked to true on a thread doesn't do anything if archived is false, both of them have to be true for the thread to be locked

Am I doing something wrong or is this an actual bug?

[slate nacelle]

That seems to be somehow intended behavior.

[outer raven]

well then the setLocked method should be changed right?

because currently it only edits locked, not archived

[slate nacelle]

It's not called setLockedAndArchived though.

[outer raven]

but setLocked is supposed to, well, lock the thread

and currently it doesn't

[slate nacelle]

It does, it not doing anything until the thread is archived is on Discord however.

[outer raven]

that's very counter-intuitive

[slate nacelle]

Concur

[outer raven]

would it make sense to create a method like this then?

so that the actual behavior on the client can be reproduced

[slate nacelle]

No, at this point you should use the edit method.

[outer raven]

well the setLocked method just seems useless in that case then

[slate nacelle]

I'd say very niche, not useless however, since it's doing what Discord intended for some reason I don't know.

They even added that to the client by now.

[outer raven]

ah I see

well but the setLocked description says this, and the latter part is not true I believe

[slate nacelle]

How is it not?

[outer raven]

because after doing thread.setLocked() everyone can still chat in it, and to me that sentence gives off the impression that the thread would be actually locked so that people couldn't talk in it

[slate nacelle]

Yeah, I agree that locked is a misnomer here, but unfortunately that ship sailed long ago and there isn't anything we can do about that.

[outer raven]

well yeah changing the method now would probably be somewhat breaking so I guess we'd have to wait but it would be nice to change this method to lock and archive the thread

[slate nacelle]

I'd be in favor to explicitly document this "quirk" and suggest to use edit instead when one wants to archive and lock.

[outer raven]

that could be done now then

Wait so if a thread is archived and not locked do I need 2 api calls to make it archived and locked?

[slate nacelle]

I don't know, but could be necessary due to how Discord handles threads.

[outer raven]

seems like it is since you get an error when trying to do anything on an archived thread

well that's extra annoying, but thanks

[outer raven]

Is it just me or does editing threads not support a reason?

the parameter is there on djs but it doesn't show up on the audit log

[opaque vessel]

Loads of methods do that, it's a client bug no one cares about

[outer raven]

Ah I thought it wouldn't be present on the audit log entry but it is so nvm I guess

I thought this was the same scenario as the message.delete method

[lone vector]

I'm gonna post here cause I can't really verify if this is a definite library bug, but I've been noticing that fetching certain invites are throwing errors from InviteStageInstance, which I believe is because in my case, my bot is fetching an invite for a guild it's not in, which has a public stage instance, then the library is trying to set the members cache for that stage instance but it's using this.guild.members where this.guild is a getter from guild cache, so guild ends up being null

Maybe someone else would be able to verify this?

[opaque vessel]

Public stages are going to removed anyway, right?

[outer raven]

well yeah but that doesn't really matter

[outer raven]

without code samples and error stack traces we can't really do much with this info

[lone vector]

I can give you an error stack but that's all I really have

[outer raven]

I suggest submitting a bug report on github, surely you have the code you're running

[lone vector]

I do yes, but it's not really "stock" code

[slate nacelle]

The descriptions seems clear to me. Now the question is: How to address this?

[lone vector]

I had thought for a simple fix, wrapping some of the code in an if (this.guild) check might have worked, but I wasn't sure if there could be better solutions

[outer raven]

well you have the guild in the data from the invite fetch, so it shouldn't be too hard

although you don't have all the members in the guild but you get the members in the stage instance

[lone vector]

Yeah, another option may be to send the guild directly to the stageinstance class instead of just the ID

[outer raven]

yeah

[opaque vessel]

this.members should default to null, and if a guild is detected, it should become a Collection

[slate nacelle]

members is only on the "actual" guild class, not on invite guild.

[outer raven]

yeah but you have the members inside the stage instance object

and those seem to be full guild member objects

[slate nacelle]

That's the problem.

[outer raven]

only issue is that the roles are ids and we dont get any other info abt them

[slate nacelle]

Easiest way would be to just have rawMembers or something, but that's probably not optimal.
And make guild an InviteGuild (assuming the props are the same) - might be breaking

[outer raven]

btw this warning is a bit dangerous isn't it? I feel like it's way too alarming for no real reason since the user should be able to tell when each one of the properties will be present in their case

[outer raven]

guild is InviteGuild or Guild though

[slate nacelle]

It's documented as Guild or null.

[outer raven]

on what class?

[slate nacelle]

The one reported, InviteStageInstance.

[outer raven]

ahh

I was looking at Invite (which has a bug on the guild property that causes it to never be a Guild, will try to fix that)

But yeah it seems like the only solution here would be breaking, which makes it useless since public stage instances won't be a thing when v14 starts to be developed

[slate nacelle]

A non-breaking fix would be to have new props for both things, not really nice, but would work.

[outer raven]

it would take longer for that pr to be merged than it will for stage discovery to be removed soooo

[outer raven]

about this, do yall agree or is this warning still needed?

[copper laurel]

I dont see whats wrong with having that warning?

[outer raven]

like I said, it's scaring users for no real reason. Basically what that warning was meant to mean is that those are the only properties that will be present on the invite object no matter the type of invite, but it gives off the impression that, for example, when fetching a guild channel invite, the guild object won't be guaranteed to be present

[copper laurel]

I dont get that impression

[outer raven]

I'm saying this because I actually saw that when I first started and that was the idea it gave me, but now that I understand the discord API a bit better I know that's not the case

[copper laurel]

It just says they can be missing

[outer raven]

in a way that makes it seem like it's an unreliable behavior

because in a message, the member object can be missing too and that doesn't have a warning, because users can deduce that

[copper laurel]

I think the point of this warning is not to focus on things that are clearly contextual like a guild or member, and more relating to properties like uses

[outer raven]

when can uses be missing?

the api docs don't document that as nullable nor optional

[oak quail]

discovery is being removed, afaik public stages are staying

public stages are very useful beyond discovery

[outer raven]

How so?

[oak quail]

they show in invites and on profiles

[outer raven]

On profiles? 🤔

[outer raven]

I have this on my fork so I assume the upstream repo has this too. When will this be addressed?

[copper laurel]

Did you check what it actually is?

That can show up in some obscure dependency of a dependency of a dependency and its fixed whenever that dependency fixes it

[outer raven]

yeah they are dependencies of dependencies but you should be able to fix them without the main dep fixing it on their side

only 1 of the 4 alerts doesnt have a fix available, all the other ones do

[copper laurel]

I dont see that on my fork

[outer raven]

is it up to date?

you can even see this on npm with npm audit

and they offer you the fixes, however one of them is downgrading dtslint to v3, not sure if that would break anything

[copper laurel]

Yeah I just updated it

Thats kinda my point though - we wait for dtslint to release a v4 version with it fixed

[outer raven]

dtslint is deprecated tho, it won't be updated

[copper laurel]

Probably should replace it then

[outer raven]

it should be on eslint now, lemme look into it

[copper laurel]

dtslint relies on @definitelytypes/utils which relies on tar which is where one issue lies, and npm-registry-client which relies on ssri which is another

tar and ssri are the actual flawed packages

[outer raven]

there is an open PR on the definitelytyped utils repo to bump tar, but ssri comes from another dep of theirs

[tawny warren]

is this a library bug? i used message.guild.stickers.fetch()

    at Sticker._patch (/usr/src/moonlightbot/node_modules/discord.js/src/structures/Sticker.js:81:50)
    at GuildStickerManager._add (/usr/src/moonlightbot/node_modules/discord.js/src/managers/CachedManager.js:48:26)
    at GuildStickerManager._add (/usr/src/moonlightbot/node_modules/discord.js/src/managers/GuildStickerManager.js:31:18)
    at /usr/src/moonlightbot/node_modules/discord.js/src/managers/GuildStickerManager.js:161:65
    at Array.map (<anonymous>)
    at GuildStickerManager.fetch (/usr/src/moonlightbot/node_modules/discord.js/src/managers/GuildStickerManager.js:161:32)
    at processTicksAndRejections (node:internal/process/task_queues:96:5)
    at async Command.exports.run (/usr/src/moonlightbot/src/commands/Owner/eval.js:10:16)
    at async module.exports (/usr/src/moonlightbot/src/events/command.js:127:20)```

[slate nacelle]

Yes, has been fixed in main with #6421.

[wild flax]

If we get some more approvals on stuff I guess we can release 13.2

Nothing stopping us really

[outer raven]

Role icons should be on 13.2 imo

space saw this message

[outer raven]

Hey I'm pretty sure this is false as I just tested and the event didn't fire, can someone else confirm?

[quiet viper]

Doesn't the member has to be cached?

[outer raven]

And it was in my tests

userUpdate fired, not guildMemberUpdate

by looking at the code it does indeed look like guildMemberUpdate should be firing, but idk why it isn't

[opaque vessel]

From glancing at the code, it looks like it happens when the user is first recognised and in a guild

[visual hornet]

that seems like a bit of a stretch, since nicknames aren't really part of the user's info

[opaque vessel]

Look at line 10-15 in the source code for that event

[outer raven]

but those lines don't have a return, so if those get executed, the others should do too

[opaque vessel]

I guess the description isn't entirely inaccurate then

[outer raven]

what I think is that there's a bug here

[opaque vessel]

Yeah, the bug is discord.js emitting user updates manually >_< lol

[outer raven]

yeah that's quite stupid xD but nothing we can change now

it its documented saying the event can fire on username updates, then it should be firing with username updates

[outer raven]

Ok so I found out why guildMemberUpdate isn't firing on user changes, it's because we check if !member.equals(old), which returns false since the member didn't update, only the user did. My question is, do we wanna remove the line saying the event fires on user updates or do we wanna fix the event so that it fires on user updates?

@tacit crypt I see you online so could you give me your opinion on this pls ^^

[tacit crypt]

I have no idea to be honest

[outer raven]

tbh since discord emits guild member update events for user changes I think we should also do the same

[slate nacelle]

Not exactly trivial to do properly.

[outer raven]

it should be easy

[slate nacelle]

Surprise me

[outer raven]

give me a min

@slate nacelle

old is a GuildMemberObject

[slate nacelle]

Aren't those user objects reference equal even?

[opaque vessel]

So you're firing user updates in there now?

[slate nacelle]

That's how we currently do it.

[outer raven]

Sorry, this should be the actual fix

tested and it fires the event

[slate nacelle]

This should only fire for the first guild member update, you should receive one for every shared guild however.

[opaque vessel]

Couldn't a fix also be to simply emit the guild member update in the user update actions as well

[outer raven]

discord emits the guild member update event on user update, so there's no point in doing that

I found some flaws with that fix so lemme do more tests to make sure it works

[outer raven]

Ok so after a lot of head scratching I just keep finding more bugs and I need feedback on these

So the check in https://github.com/discordjs/discord.js/blob/a8c21cd754d634b4d40047f85264528681a61b41/src/client/actions/GuildMemberUpdate.js#L13 is broken and will always return false because the equals function now compares banner and accentColor, which are not present in a user object received from a member object. Should we make a private method to compare the user to the api user? This isn't really a major issue though because in the userupdate action it compares 2 user objects, so that will work as expected
The User#equals method does not compare user flags, causing the event to not be emitted when it should
Due to the first point, we cannot use that same check to emit guildMemberUpdate because we cannot have a user object to compare the cached one with, unless we call the User constructor which I'm not sure we should do ?

So overall the action is sort of working as expected with a lot of things that don't work in the process, which makes it impossible to make guildMemberUpdate fire when the user updates, like Discord intends us to do

[outer raven]

@opaque vessel @slate nacelle btw ^^

[outer raven]

Gonna give my final 2 cents on the matter

Discord sends presence updates where there should be user updates and doesn’t send user updates without the presences intent
Even with that disabled it’s impossible to send a guild member update event with the old data. The only way to do that is to not send a user update event. Either one of them will not have the updated data due to how we store them. I did find an issue in the user equals function though so I’ll submit the PR to fix that and another one to undocument that sentence saying guild member update can fire with username changes

And also due to that I think we should keep the manual userUpdate event emits because otherwise the event will never be emitted except for client user updates which also happens to be a manual call

If anyone wants to attempt a fix for this however be my guest, but I just spent an entire evening banging my head against the wall with this and it didn’t go well

[tacit crypt]

Or we could, you know, drop the equality checks since if we get a GUILD_MEMBER_UPDATE event, something updated.

Just my 2

[outer raven]

But you don’t know if the user updated or if the member updated, and I don’t think we can stop emitting userUpdate events in a semver minor or patch

[tacit crypt]

Does it matter?

It doesn't

[outer raven]

Wait so what exactly are you suggesting

[tacit crypt]

Our fake events for things that don't come from the ws are just a maintenance headache imo

[tacit crypt]

.

[outer raven]

The issue is that you have to patch the existing user and if you do, you’d end up emitting an event with nothing changed

Because the current workflow is
Member update received
User equals check
User gets patched and userUpdate event is sent
Guild member equals check
Guild member update event

If we remove the checks, the guild member update event will fire without any changes since the oldMember.user will already be patched

I think that’s the reason why those checks are in place

[tacit crypt]

Doesn't a clone clone the user too

[outer raven]

It does yeah but that only applies for the user update action

It works correctly on userUpdate, but when you get the old member from cache, it’s user object is already patched because the user update action modified it

And yes this is very painful to work with and would be much simpler if we just didn’t emit fake events, but we can’t do that now

[real jetty]

Sorry to interupt you guys, but I have a question regarding the user.flags. So currently the userUpdate event wont work with flags is what you say? I wanted to know because I am facing issues there.

[outer raven]

it will if you have either one of the privilledged intents

otherwise only for the client user

[real jetty]

I have the Members intent enabled only. And I am listening for the userUpdate event. When it triggers it only has values undefined or 0 but otherwise it does not trigger.

[outer raven]

can you show me the updated data?

[real jetty]

You mean my code?

I only logged a few, like trying to change my Hypesquad house but it didnt emit

[outer raven]

the data you receive

[real jetty]

if (oldUser.flags?.bitfield !== newUser.flags?.bitfield && newUser.flags?.bitfield > 0) {
      console.info(`Flags of ${newUser.tag.red} changed: ${oldUser.flags?.bitfield} => ${newUser.flags?.bitfield}`);
      await User.updateOne({ id: oldUser.id }, { $set: { flags: newUser.flags?.bitfield ?? 0 } });
    }

This is what I am trying to do. Update database entry when the bitfield changes. I know its not the best code. When the event emits oldUser.flags.bitfield equals undefined and newUser.flags.bitfield equals 0. Otherwise it does not emit

I also logged data before the if statement same result

[outer raven]

that's odd, I could not reproduce this in the 3 hours of testing I did on these actions, it always worked perfectly

[unique axle]

friends, this isn't really a support channel
you have to fetch user flags, and if the old ones weren't cached you won't be able to obtain them after they changed

[visual hornet]

isn't uncached stuff supposed to be null though

[outer raven]

and it is

[outer raven]

you really don't have to fetch flags tho

[visual hornet]

oh i brainfarted, excuse me

[real jetty]

I am sorry for asking here, but no one could help me in the help channel. So I need to fetch the users/members on ready event to have the flags cached?

[outer raven]

I don't think you do, they get sent with the member object

[real jetty]

But shouldnt the new flags still be greater than 0?

[unique axle]

do they actually ship them with every user payload now? that was def. not the case before

[outer raven]

seems like they do but yeah I also noticed the fetchFlags method earlier

for some reason there's also flags and public_flags

[real jetty]

Huh

[unique axle]

flags are nullable, they are not sent with every payload, you may need to fetch them, depending on how the user payload arrived
https://discord.com/developers/docs/resources/user#user-object

[outer raven]

yeah but when will they not be sent?

[unique axle]

do you know how many things come with users we cache?

[real jetty]

So the event does not emit when there are no old flags cached because of the discord.js equals checks?

[unique axle]

if our equal check doesn't account for null, possibly so
user updates are a bit of a headache anyways atm, because they don't really exist in this capacity, we forge them from the inner user payload on receiving GUILD_MEMBER_UPDATE event payloads

and iirc the current implementation just only emits the first and drops all others (in case your bot shares more than one guild with the user)

[outer raven]

the equals check didn't check for flags

[loud jay]

#6750 in discordjs/discord.js by ImRodry opened <t:1633292836:R> (review required)
fix(User): compare flags in #equals
📥 npm i ImRodry/discord.js#fix-user-equals

[real jetty]

I just want to fix my problem 😫

[unique axle]

see if the linked PR does

[real jetty]

But I will need to wait for the next version to release or use dev build

[outer raven]

nah, install it

you have the command there

[real jetty]

I am currently not home but I will try

But why not just drop the equals check from discord.js side?

Doesnt it make no sense if the event emits then it is a new user object?

[outer raven]

like we were discussing, the user update event emits with other events

[outer raven]

So I'm gonna remove that phrase from the guildMemberUpdate docs. Should I add an info tag saying this will change in v14 or do we not do that here?

[wild flax]

I'd rather not make predictions for semver versions

[outer raven]

I plan on opening the PR once development on v14 starts but in that case, should I only delete the phrase and not replace it with anything else?

[copper laurel]

Development on v14 starts whenever people start opening semver major / breaking PRs

[outer raven]

there already are semver major PRs tho

[copper laurel]

Then they are for v14

Or might be anyway

[outer raven]

well yeah I know but I'd rather wait for those to start getting merge than clogging up the PR list

[copper laurel]

Theres some technicalities in there

[outer raven]

guess ill just delete it then

[wild flax]

Yeah but theres no guarantee we go this way is what im saying

[outer raven]

ah yeah

[wild flax]

so putting it on the docs now might convey a wrong image

[outer raven]

sure thing then

[dim steppe]

Is it worth updating...
https://github.com/discordjs/discord.js/blob/stable/src/util/Constants.js#L38
...to add the new image sizes?

From what I see, DJS just calculates them...
const AllowedImageSizes = Array.from({ length: 9 }, (e, i) => 2 ** (i + 4));
...but those aren't all of the valid sizes.

[analog oyster]

according to this those are all the valid sizes https://discord.com/developers/docs/reference#image-formatting-image-base-url

Image size can be any power of two between 16 and 4096.

[dim steppe]

The introduction of banners, I believe, added new sizes. The one that comes to my head currently is 300. The sizes can also be used with avatars etc

https://cdn.discordapp.com/avatars/228937760390643713/353889e1eb10ab1e61d7274028a69500.webp?size=300

IIRC, default banners are 300px

[analog oyster]

do you know the full list of valid image sizes that aren't documented?

perhaps the image size validation should just be removed

[dim steppe]

Not off hand — I had the ones I had found when I was at work earlier. Only one I remember off the top of my head was 300

Oh and 600

It's not pretty, but could just:

const AllowedImageSizes = [...Array.from({ length: 9 }, (e, i) => 2 ** (i + 4)), 300, 600].sort((a, b) => a - b);

[copper laurel]

Would they not be two totally separate sets of image sizes

[dim steppe]

For banners and avatars, you mean? The new sizes work for all images, not just banners

[copper laurel]

Fair enough

[real jetty]

I think if you have the message partial enabled then djs should send the messageUpdate event even if the old message isn't cached, instead of just ignoring the newmessage entirely

Sometimes you don't need the old message at all, like for link scanners or swear detection.

[slate nacelle]

That's the idea behind partials.

[copper laurel]

Isn't that exactly what it does? I'm not sure what's been suggested / addressed here

[outer raven]

Could you change the dev build interval to be in the middle of the day or something? The majority of the time the release at midnight is the same as the release at midday because everyone is asleep during the night and barely any prs ever get merged then

[dawn merlin]

So I found a place that tslint is fighting with prettier, TSlint wants this:

export class ...
  extends ...
  implements ... {

But prettier wants this:

export class ...
  extends ...
  implements ... 
{

it seems that setting one-line to false in the tslint.json fixed the issue

[outer raven]

Which one do we want tho

I’d say the first

[ruby terrace]

I don't think prettier should be running in the .d.ts files

[tacit crypt]

I don't think we should be using tslint

[ruby terrace]

well...yeah, eslint needs to support .d.ts files for that though

[outer raven]

does it not?

[outer raven]

I always run it before pushing, it just takes long af because the file is huge lol

but I've never gotten eslint errors on the typings

[dawn merlin]

my vscode runs prettier formats on save

[ruby terrace]

that's because eslint is scoped to the js files only

by using eslint src

[outer raven]

it doesn't tho 💀

[ruby terrace]

vsc can, its a setting

[dawn merlin]

? I set it to do that?

[outer raven]

but I remember when you made the messageReactionRemoveAll PR every PR created after that changed the formatting on that event xd

[dawn merlin]

that might be because prettier isn't supposed to be run on the typings files? idk

[outer raven]

what else would we run then? It has to follow some formatting rules

[dawn merlin]

It looks like it does run prettier in ts files on commits so idk why that in PR caused issues

[copper laurel]

Its meant to do nothing if there are no changes, why is it releasing

[outer raven]

it does because the dev release has the timestamp at which it was released and we don't run any checks to compare the commit hash

which we probably should, but I personally don't know how

[copper laurel]

oh i thought we did

[outer raven]

make a pr pls

[copper laurel]

I mean it doesnt seem that important, whats your concern with it releasing?

[outer raven]

Well it's basically just that multiple releases are published on the same commit and often only the midnight release has changed because from midnight to midday UTC there usually aren't any merges

my initial suggestion was to just change the schedule

[wild flax]

You know timezones are a thing right?

What you describe as midnight only works for the UK

[real jetty]

Will yall ever add a onclick function to interactions?

[outer raven]

well it doesn't because its 1am and 1pm but im talking UTC here but what im saying is still true

[wild flax]

Yeah I don't think thats necessary sorry

its "nightly" for a reason

[outer raven]

Not even a check to compare commit hashes?

[wild flax]

what will that change?

[unique axle]

clicking on a reaction emits "interactionCreate", check for the interaction to be a button with .isButton() and validate it's the right button by it's .customId.
if you want a higher level approach to temporary buttons for confirmations and similar, look into component collectors https://discordjs.guide/interactions/buttons.html#component-collectors

[analog oyster]

clicking on a button emits "interactionCreate"

[real jetty]

yes i know that, its just a very big use of space and switch statements and so on. The onclick function would allow clean easy to read code and support usability, but you gota use what you get

[unique axle]

if you want to target a single specific button, like a onClick, a component collector is what you want

[outer raven]

prevent multiple releases from being published on the same commit, essentially not spamming your npm versions if you care about those

not extremely important, but would be nice thats all

[wild flax]

dont really care about npm versions honestly

[outer raven]

aight

[opaque vessel]

I noticed the change log that gets generated references pull requests as issues o,o

For example

[wild flax]

prs are issues

theres no difference

[opaque vessel]

Welp, alright then (:

[surreal bough]

has v13.2.0 been pushed to npm?

not yet right?

[outer magnet]

since ephemeral messages can now be fetched, does that mean we can create component collectors on em?

nevermind, tested them and it works

[wild flax]

yeah it has been

[surreal bough]

oh okay tysm

[outer raven]

@wild flax github had an outage on actions when you released 13.2, could you try to re-run the docs publish action?

[outer raven]

this is still not fixed

[thorny radish]

hey do you guys think it might be a good idea to export the type of type as a type?

export interface ActivityOptions {
  name?: string;
  url?: string;
  type?: ExcludeEnum<typeof ActivityTypes, 'CUSTOM'>;
  shardId?: number | readonly number[];
}

cause as of the latest d.js update I can't use as ActivityType anymore when setting type to an environment variable and I instead have to add the whole ExcludeEnum to my code since that is not exported either

[outer raven]

why don't you use ActivityTypes? iirc I removed the ActivityType shortcut because it wasn't used anymore

wait no I didn't, it's still there

[thorny radish]

yea but it isn't assignable to the ExcludeEnum thing

[outer raven]

can you show the error?

[thorny radish]

neither is ActivityTypes

when using as ActivityType

[outer raven]

show the code please

[thorny radish]

presence: {
    activities: [
        {
            name: process.env.CLIENT_ACTIVITY_NAME,
            type: process.env.CLIENT_ACTIVITY_TYPE as ActivityType
        }
    ]
}

[outer raven]

hmmm that's a very niche case

what happens if you remove the "as"?

wait you can just use Exclude<ActivityType, 'CUSTOM'> iirc

[thorny radish]

oh that worked, swear i tried that but apparently not. thanks

[outer raven]

uhm bump please this is quite important

this is also erroring out

someone re-run the actions

[copper laurel]

Im sure he will once he sees it

Probably asleep or busy

[remote wasp]

does @discordjs/rest module supposed to be used outside of node too?
It's related to this PR: https://github.com/discordjs/discord.js-modules/pull/55. The issue mentioned in the PR is still there and you still have to opt in for dom types in tsconfig, else it won't compile. I imported the URLSearchParams type from node's built-inurl module, which let's you compile without adding dom types. So, what was the reason that they went with that solution and not this?

[wild flax]

cc @strong hull

[warped crater]

was going over a guide page for my beloved souji and have found out that those (https://github.com/discordjs/discord.js/blob/26340acad946989e9a8825966b76c506d82fc641/src/structures/MessageMentions.js#L217-L233) don't have capture groups for the IDs

what is the stance on this? would there be an issue with having that added?

[strong hull]

Uhm... I'll have to look into that. I could've sworn I tested it back then but I'm starting to doubt now.

[remote wasp]

oops, turns out your fix was released in the @canary tag. I installed that tag and it does fix the issue. Still, why didn't you went for the node type here

[burnt cradle]

They do have capture groups though

[warped crater]

eeerm, my bad

*named capture groups

[burnt cradle]

Is there a reason to make it a named capture group over a non-named one?

[warped crater]

yeah I didn't realize it was grouped in the first place, cus I was looking for named groups

but yeah, it could be beneficial

bit redundant but could be

.groups.id is much nicer to access

[strong hull]

That I don't know. I just added on what was already there.

CC @wild flax
(Why does rest use DOM libs instead of Node libs)

[wild flax]

You’d have to ask the person who implemented it

Git blame it maybe?

[strong hull]

CC @tacit crypt https://github.com/discordjs/discord.js-modules/blob/975aca146d55e3740f76e4a7f2e69c64d2c9590d/packages/rest/src/lib/RequestManager.ts#L51

[tacit crypt]

I didn't add the ///types directive

[strong hull]

No, I did. Thats not the question however. The question is why you made the lib depend on URLSearchParams from DOM insteading of importing it from url (aka node:url)

[tacit crypt]

Because the latter needs an import when its otherwise global anddddd the only reason the node typings don't make it global is bc of the clash

[strong hull]

Well there ya go @wild flax @remote wasp

[remote wasp]

I see. How about using the global URLSearchParams when using it as a value and importing the type when we just need to use it as a type? This way we won't have to do anything related to dom

[tacit crypt]

You.. won't have it in your globals if you don't use the dom typings pretty sure

And TypeScript will yell

[ruby terrace]

that's correct yeah (had this issue with Response typings in the main lib)

[remote wasp]

got it 👍

[analog oyster]

<ClientPresence>.userId is always going to be null
https://github.com/discordjs/discord.js/blob/d32956c6b70a3a03c431d5f761c058072999289a/src/structures/ClientPresence.js#L13

However, <Presence>.userId is both documented and typed as a non-nullable Snowflake
https://github.com/discordjs/discord.js/blob/d32956c6b70a3a03c431d5f761c058072999289a/typings/index.d.ts#L1636

Right now, by the time the ClientPresence class is instantiated client.user is not available so there's no easy way to get the client id

[grand void]

Hey, i want to ask about Error [USER_BANNER_NOT_FETCHED]: You must fetch this user's banner before trying to generate its URL!. I always get this error when json stringify User and fetching User, I not using method <User>.bannerURL(), but why i get that error message?

my bot always getting crashed because that error

[outer raven]

There’s no User#toJSON so what exactly are you doing?

[grand void]

yes i know, i can use JSON.parse and JSON.stringify method

[outer raven]

Ah wait I know the fix

Don’t have the time to open the PR rn but I will later if no one else does before me

Also the toJSON method exists, it’s just not documented so there’s probably a reason for that

[grand void]

i'll test it later

[outer raven]

Me and a friend of mine managed to get the guildMemberUpdate event to fire on userUpdate's like the Discord gateway does. My question is: should we also fire presenceUpdate when we receive it from the API as a userUpdate or should we leave that for guildMemberUpdate only?

[wild flax]

presences don't really exists on users

[outer raven]

Yeah ik despite Discord sending the user with the presence update payload, but we won't fire it then, thanks

[wild flax]

yeah I never really checked how it works when you are only friends and don't share a server

im sure theres something happening for this

but it doesnt really concern bots

[outer raven]

yeah I can't check that either but it shouldn't matter and it's way out of the scope of this PR we're making anyway

oh btw @wild flax I noticed you didn't really review the vscode extensions PR, do you not agree with it or did you just not see it?

[dawn merlin]

if an object has a #toJSON() method, JSON#stringify calls that method instead of using the JSON default serialization.

[outer raven]

yeye I know, I was literally about to create that PR

[dawn merlin]

im out of the loop here, but why does #bannerURL need to throw in the first place?

[outer raven]

starting to question that too now

my idea behind it was to force users to fetch the banner before trying to generate it

but in theory you could throw undefined, but that could be badly interpreted

[dawn merlin]

but doesn't undefined imply it just isn't part of the object at all. And then null just means it could be part of the object but it isnt

if it isn't part of the payload, just returning undefined seems fine, no?

[grand void]

👍

[outer raven]

I'd like to hear from the maintainers now, because while you are right, the error forces users to fetch the banner and actually tells them whats wrong

also unrelated but this doesn't follow commitlint so it won't show up on the changelog, maybe someone should force push to fix

[wild flax]

What do you want to hear, good luck it being a semver:major now

Not like we can change it

[outer raven]

it's not tho

removing an error is patch, adding it is major

[wild flax]

Removing throwing is very much a major

You aren't fixing a bug, you change behaviour

[outer raven]

how is it? I've had prs that removed errors on semver minor

or patch even, can't remember

[wild flax]

Feel free to dig them up

So I can actually see the context

[loud jay]

#6631 in discordjs/discord.js by ImRodry merged <t:1632397615:R>
refactor(VoiceState): use manager edit method to remove error

[outer raven]

it was patch actually

[wild flax]

Thats quite different

Look at what the PR does lol

You made something generally work in all reliable cases

Whereas before it wasn't

[outer raven]

well yeah and here we're making the bannerURL method work in all cases, but output something different in the cases where it throws atm

so going from error thrown to no error + undefined return doesn't seem like a major to me

[analog oyster]

the throwing if not fetched behaviour is documented. it would be semver: major.

[zenith oracle]

You're expected to receive an error when the user isn't force fetched. Removing this error to return a different thing is a major, isn't it?

[outer raven]

Eh I guess so

Both alternatives work

And we should’ve had this discussion before the PR was merged anyway

If anyone wants they can fix it when v14 development starts I guess

[dawn merlin]

Can't anyone submit PR's for semver major now?

[outer raven]

Well you can

But I am saving those for when we’re merging semver major prs so as to not clog up the pr list

[wild flax]

you arent really clogging anything up

[outer raven]

Just a personal preference tho, you can do whatever you want

[wild flax]

we use the new projects boards https://github.com/orgs/discordjs/projects/2

so we have a nicer view of things

[outer raven]

Ohhh I thought you went through the list

Aight thats good to know

[wild flax]

hell naw

[outer raven]

But still, we have TODO comments for some stuff so I’ll wait

[dawn merlin]

also one last question on the bannerURL method, is there a specific reason it isn't a getter?

[zenith oracle]

It's a method as other methods like avatarURL, iconURL etc...

Also it takes an options parameter so it couldn't be a getter

[outer raven]

Only place where that is a getter is on emoji and I plan on making a pr to turn that into a method because it doesn’t make sense to be a getter

It should always be customizable

[dawn merlin]

im literally blind

[outer raven]

What do you guys think of a possible CommandInteraction#toString method that would return the command that was run in a way that can be copied and executed again? It would basically be /commandName {subcommand group} {subcommand} {option1:value} ...

[zenith oracle]

Sounds good to me

[vernal adder]

What would be the use case for that?

You can already do that inside the client.

Not that it works 90 % of the time, but. (parsing the copied command, not copying it)

[opaque vessel]

Logging commands. I do it

[vernal adder]

I see.

[coarse ginkgo]

Do we know if bots can have avatars per guild?

[vernal adder]

They can't.

[coarse ginkgo]

👍

[outer raven]

question for user/channel/role options though, how should we display these? Discord accepts the ID on all and this would be the easiest to implement but it also supports @Username#discrim for users for example

for roles that is just @RoleName tho so maybe we shouldn't go that way as that's quite unreliable

if the point is to just make it executable the ID works fine

[opaque vessel]

You only get the id though, so, there's only one option to use. This also gets opinionated so I wouldn't recommend adding it really

[outer raven]

the other toString methods are all for utility purposes and if the point is to make the command re-executable (if that's a word lol) easily then I don't see why not

but ye I'd def like some more input

[copper laurel]

Does pasting in slash commands even work?
But I'd still go with ID, it would be the only reliable way to actually ensure you're getting the right values that were used the first time

Is it even possible to replicate the command in order though? Is the raw data reliable for that?

[outer raven]

I believe you get options in the way they were sent

I can test if you want tho

[visual hornet]

there was a user in tph that had a problem relating to this, when role mentions were pasted in, they were @name instead of <@&id>
so it works to some extent but not fully, it seems
the helper in that case is present in this server, it was chooks

[copper laurel]

Which is why I'd probably go with ID

Though the new editor is coming soon too, might be worth waiting? idk

[visual hornet]

editor?

[outer raven]

ayo what

[copper laurel]

Pretty sure this was public knowledge from their stage about it
https://auralytical.notion.site/Editor-Upgrades-dee51c93462c44b8a0a53fed3d94c4cd

They released this at the same time as the autocomplete docs and permissions overhaul info

But since its client side rather than dev focused maybe it got less discussion here

[outer raven]

oh I did see that but that's only a visual change for the client and allows us to have multiline strings, which should still be supported by this method and we can always update it if not

[copper laurel]

I mean its not just a visual change

Its an semver major upgrade of the editor in the client

Slate, the library that powers the editor, has undergone a massive rewrite since we originally built this component.

[outer raven]

that rewrite was internal, the editor update shouldn't remove any features, and should just add those boxes and multiline strings from what I understood

[copper laurel]

...of course it wont remove features

But theres a very good chance it will change how it behaves if you were to paste in a slash command input

[outer raven]

still the data passed to the API won't change for sure, and that's what we're using here

it's only a client-side update

I don't see how that update could break this function tbh

[outer raven]

Working as intended btw, will PR

[copper laurel]

That's still not what I meant, at no point did I suggest the API data would change

But the whole use-case for this feature, if I'm not mistaken, was to output a string that could be used to copy and paste into the editor to re-use the command, right?

And the editor input is going to change

[outer raven]

Hmm so you think the option:value format will change?

[copper laurel]

It might still work perfectly fine afterwards, but its just something to keep in mind

[outer raven]

Or could at least

[copper laurel]

Could, yeah. I wouldn't say I think or know it will

[outer raven]

Alright then
I have the branch published and will make the PR once that update is out

[copper laurel]

Since I think one of the issues they did want to address was how you tab through options and stuff

[outer raven]

Yeah I think all options are these by default, not sure how optional options work tho, they didn’t really show those

[copper laurel]

You could probably PR it now if it works now

If it doesnt work with the new editor we can probably patch it? idk

[outer raven]

What if its not patchable? Not likely but its possible

Did they give us any ETA for that update btw?

[copper laurel]

nah

[outer raven]

I’m just worried this isn’t feasible in the new editor but that chance is so small that we might as well just go with it

Anyway quick question: if a property is set to undefined on an object and we change it to not be present in the cases where it is undefined is that breaking?

[copper laurel]

Uhh, probably

Since you're changing what Object.keys/entries would return

[outer raven]

Aight

[ruby terrace]

Currently, it works in the new editor. Only problem is it doesn't let you select which bot, but that's not a djs problem

[outer raven]

Yeah ai actually stumbled across that issue on mobile but we can’t really worry about that

[junior pumice]

Are there any plans om removing client.api access?

[unique axle]

Client#api is an internally used private and unsupported method and is not subject to semver, so it might change at any point without notice

[junior pumice]

but there arent any plans to lock it down like classes

[copper laurel]

There isnt really a way to "lock down" a specific propertyu

We could try JS's new private fields but im not sure what compatibility on those is like

[junior pumice]

good

bc i use it a tone

[unique axle]

once those are reliable, definitely going to do that

[junior pumice]

Awesome guess ill have to switch bc my whole bot is relying on those

[ruby terrace]

If you're relying on client.api you should really be using a package like @discordjs/rest

[unique axle]

if your whole bot relies on raw API access i'm not too sure why you even use discord.js to begin with?

[junior pumice]

For the websocket and the classes

and the client.api

[junior pumice]

Oh what lemme see

is there also a thing like for the classes

[copper laurel]

..yeah.... discord.js

use it properly

[junior pumice]

I use it properly

[copper laurel]

Clearly not or none of this would be an issue

Not really a library discussion anymore

[junior pumice]

Okay then, just sad that everything more advanced people use is getting locked away

[unique axle]

if you were to elaborate on your use case we could maybe understand what you are doing and suggest changes/adapt to make that possible
but since all we get is

Okay then, just sad that everything more advanced people use is getting locked away
over and over (even after explicitly asking for those use cases, yesterday) we really can't do anything productive with this feedback

[junior pumice]

Okay sorry i didnt mean to do that, lemme explain

[copper laurel]

Its not "more advanced" though, if you want to make raw API calls you can just use node-fetch

Using discord.js's internal methods does not make you more advanced

[junior pumice]

So I've made my bot with djs in v12 and back then interactions weren't a thing so I made my own classes and stuff using client.api and to use some useful djs stuff. I used e.g. the message class and invoked a message with the message data given with e.g. message context interaction or the message collector from textchannel class by invoking it with the interaction channel
I use it so frequently that I would need to change 50 of the bot to bot use the classes and I use client.api even more

[junior pumice]

I was talking about classes

[copper laurel]

But now they are classes and you dont need hacky workarounds

[junior pumice]

Changing to those would be even more work than removing the djs class usages

[wild flax]

Which is unsupported and always was

So saying we “take away” things us untrue

It could just break

Or it might not

But that’s your own fault at this point for “I’m too lazy”

[junior pumice]

Why was it documented then

[wild flax]

What was?

[copper laurel]

client.api has never been documented

[wild flax]

The classes? Because of extended structures when we had them

Not to create them yourself

[junior pumice]

I again was talking about classes

Fine about client.api since discorjs/rest exists

[wild flax]

You got your answer

[junior pumice]

Yup

[outer raven]

@tacit crypt in https://github.com/discordjs/discord.js/pull/6790 I changed both ApplicationCommandOption and ApplicationCommandOptionData but I'm not so sure about the first one since all options have all properties (yes, even channelTypes) and some just have them set to undefined. Should I update the Option interfaces to include this or do we just pretend like they aren't there?

[tacit crypt]

You only changed SubCommand(Group) options

I don't understand your question

[outer raven]

well yeah but I changed both the Option and OptionData interfaces

i just said it that way because the actual names are too long lol

[tacit crypt]

I still stand by my I don't understand your question. You fixed the issue in this PR. If there's other issues, make another PR?

[outer raven]

in that PR I removed the required property completely from sub command and sub command group options, but this property is actually present, it's just set to undefined on these types of options

[tacit crypt]

If it's set to undefined it won't even be sent to Discord

So...like...??

[outer raven]

no like

there's 2 different interfaces, one for data we send to discord and the other one for options we receive

and I'm talking about the options we receive

[tacit crypt]

We wouldn't receive required??

[outer raven]

but we create it that's the thing

that's why it gets set to undefined

hope that makes sense

[tacit crypt]

That's an oversight on our part, tho it doesn't affect it dramatically because JSON.stringify skips undefined props

But yes, you can submit a PR to fix that I suppose

[outer raven]

to fix types by adding undefined or to remove those undefined props?

because that would be a semver major right

the second option

[tacit crypt]

If it's strictly internal code that doesn't interact directly with the users, fairly certain it's not major but citation is needed

and it wouldn't be major anyways because its a bug that they're present but undefined

so

Like

[outer raven]

yeah I feel like it wouldn't be since the public methods always return values and not the full option (except for get)

maybe that exception is what makes this semver major

[tacit crypt]

.

[outer raven]

alright then, will do that

thanks!

also gonna address your review in the toString pr in a bit

is there any difference/preference in using Boolean() over !! ?

[tacit crypt]

Other than one being more verbose (which wins me) and the other being syntax sugar, I don't think so. Just be consistent with the rest of the codebase

[outer raven]

yeah seems like !! isn't used anywhere so I'll go with the Boolean constructor

actually, ckohen suggested using only Boolean in the filter but I think this would be more verbose, or does it not make a difference?

vs

[wild flax]

makes no difference here

[outer raven]

Alright, will just use Boolean then

[visual hornet]

if it's just filtering for truthiness then couldn't you just do prop => prop
or is that bad to ts/eslint or against some standard or smth

[outer raven]

well Boolean does exactly that so ¯_(ツ)_/¯

[visual hornet]

it is for readability isn't it

[outer raven]

I think so too but crawl said it makes no difference

[outer raven]

btw about this: if an option can have a property but it's set to undefined should we include it or not?

say for example a STRING option with no choices, should it have choices: undefined or should it not be present at all

[tacit crypt]

Ideally, the former, but even if it's the latter, it still won't be "present" once we send it down the pipe

[outer raven]

aight I'll just default it to an empty array then, since the types and docs never included undefined

actually, can I?

yeah seems like that's fine by the API

[tacit crypt]

No?

Leaving aside the fact I mixed my former and latter this time, if a user doesn't specify choices it shouldn't be sent period

Hell, YOU asked if it should be undefined or not present and now you forcefully make it present??

[outer raven]

My original objective was to remove the property in cases where it absolutely cannot be present (e.g required in sub commands) but keep it everywhere else. On situations when these are not set they could be defaulted to sth depending on the property

[tacit crypt]

if the property is not required, it shouldn't be present PERIOD unless the user set it

[outer raven]

Alright then

[tacit crypt]

Thats how builders and most of our patch methods do it too

Send just thats needed

[outer raven]

But here I’m talking about the received options in ApplicationCommand#options

Well both that and the options sent to the API in this case but I guess it still applies, just making sure we’re on the same page here

[tacit crypt]

Ok, maybe you need to explain a tad better because I don't understand what the issue is still

Code samples would be 🙏

[outer raven]

This is still the best example I can provide
My goal is to remove properties from options that cannot have them (e.g required in subcommands) but keep them if the type of option allows them
If the prop is allowed and not set by the user then it could either default to an empty array or not be present in the object at all, which is what I’m asking you

For context I’m editing the ApplicationCommand#transformOption method, which changes the ApplicationCommand#options property and the options sent to discord

[tacit crypt]

I've already said, if it's not set by the user it shouldn't be present

(I mixed up my former and latter but thats a different issue)

[outer raven]

Alright, will go with that then

Thanks

[outer raven]

Does APIApplicationCommandOption from discord-api-types not support channel_types?

[dawn merlin]

it does, it just hasn't had a new version release since the PR was merged

[outer raven]

ah it should probably be released then, was gonna use that in a PR

it's only a return type for a private function though, nothing urgent

[tacit crypt]

One day I will make a workflow that does a full release every friday

That day is not today

[outer raven]

do you make commits every week though?

[tacit crypt]

Does it matter? Thats not even part of the idea

[outer raven]

oh idk then, was just thinking it could make duplicate releases that way

[zenith oracle]

Why wouldn't it be an array 🤔

[outer raven]

There are a lot of type checks though and when the error isn’t explicit (same when not passing an array to embeds or components in MessageOptions) it should probably throw a custom one

There’s some errors that are fine as it is but when the error references a function in the lib people tend to think its a bug and not an issue in their code

Problem is finding all these instances but I think at least the most common ones would be good enough

[outer raven]

is there any reason why the Channel#delete method doesn't support a reason when the GuildChannel and ThreadChannel methods do?

[tender field]

because DMChannel doesn't support a reason, does it ?

[outer raven]

the endpoint is the same

so it theoretically does

[zenith oracle]

Why should you provide a reason for DM channels though? There isn't an audit log lol

[outer raven]

just to generalize the method so it doesn't need to be duped, but I'll probably do something else about this

[opaque vessel]

Can someone switch https://github.com/discordjs/discord.js/pull/6802's branch to main?

[vernal adder]

Should be done.

[opaque vessel]

It seems there's also a repeated instance of this in the ChannelData type definition >:

[visual hornet]

isn't there like a massive json or something that's for generating the docs
couldn't we run smth like /(\w+) \1/ on that

[opaque vessel]

The error will still be in the source code. I don't think that's appropriate to do

[visual hornet]

well yeah but we could find where it's from from that, no?

[opaque vessel]

Oh you mean for finding duplicate words

Sure, I guess, you can try do that

[visual hornet]

yeah, just needs a human filter

ill try that later ig

[opaque vessel]

Another one in https://github.com/discordjs/discord.js/blob/main/src/structures/CommandInteractionOptionResolver.js#L88

[burnt cradle]

The double for is actually grammatically correct there but if you make it a single for it's still correct just a slightly different meaning

[vernal adder]

"The properties to check `required` for."?

That has a different meaning

How about "The properties for `required` to check for."

[visual hornet]

got 4 matches, both on stable and main

1. WebSocketShard#setHelloTimeout (private method) parameter desc time
   "If set to -1, it will clear the hello timeout timeout"
   https://github.com/discordjs/discord.js/blob/stable/src/client/websocket/WebSocketShard.js#L498
2. seems correct (the one jiralite just mentioned)
3. ReactionCollector#create (event) desc
   "...as opposed to Collector#collect which which will be emitted..."
   https://github.com/discordjs/discord.js/blob/stable/src/structures/ReactionCollector.js#L72
4. "the the" that jiralite initially mentioned

[vernal adder]

https://github.com/discordjs/discord.js/blob/stable/src/client/websocket/WebSocketShard.js#L498 is correct as is?

[visual hornet]

"hello timeout timeout" is correct?

[vernal adder]

That is my question, yes.

[visual hornet]

idk i was debating if it was, but it doesn't seem like it to me
i don't think adding another "timeout" there really helps

in the code for that method it just says "HELLO timeout" as well

[vernal adder]

I think it's more "correct" to have them both, but having just one works as well.

Or, actually...

[opaque vessel]

I don't think we should have double words, correct or not. It's just... ambiguous and seems like a mistake at a first glance. Sentences like that should be rewritten

[vernal adder]

To me it only adds confusion.

But yes, that was changed an hour ago.

[visual hornet]

other methods like the setHeartbeatTimer below it doesn't say "heartbeat timer interval" either, just "heartbeat timer" in the desc and "heartbeat interval"/heartbeatInterval in the code

[visual hornet]

here we can't really rewrite it, since "timeout" isn't a part of grammar so we can't really rearrange it, "heartbeat timeout timeout" would have to stay a single blob i think

[vernal adder]

But it's not a timeout for the timeout.

It doesn't clear the timeout's timeout. It just clears the timeout.

[opaque vessel]

Literally below that is a debug message "Clearing the HELLO timeout.", it's not correct and is already inconsistent

The extra one should be removed

[visual hornet]

well that's 2 we can agree on, and i think we can agree that the 3rd one here is incorrect?
so just the ambiguous "for for" that jiralite found

[vernal adder]

I believe I already included all of those

https://github.com/discordjs/discord.js/pull/6802

[visual hornet]

nice

[opaque vessel]

Pretty sure that's just flat out wrong, it only checks if it's required. The last 'for' should be 'if'

[unique axle]

constructive criticism usually does not involve insults
however, please note that we abide to semantic versioning, the API will stay consistently usable within patch and minor versions.
semver major (11.x -> 12.0, 12.x -> 13.0), however can include breaking changes

you can generally not rely on functionality to stay the same when doing a major update, in no library
if we were to stick to always make our versions backwards compatible we'd throw any chance for innovation and fixing up design mistakes or change the direction the library takes

if you want a consistent API stick to version 12, however, please note that new features will not be ported back to old versions and that discord will eventually (no known date) shut off the version of API version 12 uses

that is precisely why they are introduced in semver major version changes, because they are not minor changes and break the public facing API

now, as to why these changes have been made:
v11 -> v12: optional parameters have been moved into objects for consistency
v12 -> v13: we removed an inconsistency, no other delete method ever had a built in timer + this one was broken beyond repair, with more and more introduced checks against edge cases of when a message may have been deleted already

i can assure you, most of the things we do has a solid reason, albeit it being an inconvenience at time. to remedy that we're trying to list all changes in updating guides https://discordjs.guide/additional-info/changes-in-v13.html, so people can easily find the new usage based on their current code. if you have a suggestion as to what we could add or improve there, feel free to open an issue or PR on the guide repository https://github.com/discordjs/guide

[proven mountain]

I think a good thing to think about is that I would assume none of it is done out of fun, cuz like I don't think any of the devs find entertainment by breaking other peoples code, changing discord API version can break certain stuff which may require for stuff to be changed, it may also be to code new code more efficiently.

I get it, it's really annoying to update a large portion of your code just because of breaking changes, but that's kinda to be expected when upgrading a major version, this is something your gonna find happen more times with both this library and others, but fortunately developers leaves this big changes for major versions so we can prepare more for these changes.

[wild flax]

I dunno, we usually give people months and months to update

If over a year isn't enough, then I don't know what to tell you

[kindred lagoon]

Why in each new version do you make drastic changes to the package and these changes are just a change in functionality and perform the same function as the first version? Why don't you just stick to each version and add features and if there is a modification to the Api from Discord, just implement the modifications and not modify the way the function is created, I know everyone agrees with me on this point
@wild flax@unique axle

[clever spoke]

theres usually good reason to most of the changes to djs, like consistency and whatnot

[visual hornet]

drastic changes
because that’s how semver works?
if it wasn’t drastic, it would be on a minor version

[kindred lagoon]

My friend, the drastic changes are long after the package is released and after 5 years he makes a drastic change if he wants, and he is not obligated to make a drastic change every year.

[clever spoke]

well could you give an example of a "drastic change" because changes that arent backwards compatible go into major versions (https://semver.org/)

[visual hornet]

nor are the contributors required to not make changes
updates to the library are to utilize new feature of js or node, to comply with the discord api, to create consistency between other parts of the library
we’re on major version 13, semver says that this means there are breaking changes, if we didn’t have drastic changes we’d be on 1.45.7 or something

[kindred lagoon]

Software versions
I know it well, I use Google Translate, so it replaces some words

This is the explanation of the program version if you do not understand it well
@clever spoke

Major version numbers change whenever there is some significant change being introduced. For example, a large or potentially backward-incompatible change to a software package.

Minor version numbers change when a new, minor feature is introduced or when a set of smaller features is rolled out.

Patch numbers change when a new build of the software is released to customers. This is normally for small bug-fixes or the like.

[clever spoke]

yeah, thats what djs follows tho

[visual hornet]

we all know that, why do you think we referenced it?
semver major introduces breaking changes, that’s what it means to be semver major

[kindred lagoon]

My friend can make changes and add features, but I noticed in some versions that he changes the name of some of the functions. Why?

[visual hornet]

for consistency, managers were introduced in v12 to manage data structures more easily

[kindred lagoon]

The package has reached version 13 and has not reached consistency yet?

[visual hornet]

well, yes, if you read the update guide, permissionOverwrites were just moved to the manager
stuff is constantly changing, and we have to change to fit those
stuff can be overlooked and noticed and suggested, to try to keep improving the library further
and improving it majorly can’t come without a major change

[kindred lagoon]

I know my friend, but he doesn't have to make a big change every year

[visual hornet]

and they aren’t required to, but they do it to improve the library

[clever spoke]

are you referring to using or developing the lib?

[void rivet]

if you're complaining about major versions having major changes
new major versions are the place to implement the major changes, which is what you mentioned here

[lucid pasture]

Thats why those breaking changes are being deployed in major versions.
1.- No one is forced to update in the same minute as the version is deployed
2.- Djs gives plenty of time for you to update your code
I honestly dont get the issue, thats how semver works

[kindred lagoon]

There are very huge packages. Have you seen it every year that changes the name of the function in each version and changes the internal method of the function and the way it is used?

[visual hornet]

…because that’s what semver major is for? if you don’t want to have to update your code then don’t update to a major version

[vivid field]

it also happens that we find an idea to have a more organized lib design only after a few major versions, like e.g. managers broke a lot of stuff but after implementing them the methods that belong together are actually grouped together

and these breaking changes all get collected first so that there's not a new major version with every change, e.g. v13 would've come later too if it wasn't for discord bumping their api version

[kindred lagoon]

I know what you mean, but what I mean, why does it get me to the point of not updating to the latest version, why doesn't it add features that I can use without modifying the 400 lines in one file, think what I mean my friend and you'll support me

[vernal adder]

Because that is what updating is.

You cannot expect everything to be static.

Things change, and you sometimes need to change what's already there for new changes.

[visual hornet]

if it adds features then why would you expect them to be usable without changing anything?
when you update to a major version, you're making a bargain, you accept new features or better practices in exchange for changed features or removed methods

[kindred lagoon]

Who told you that this is the update Do all the packages in the world make drastic changes and expect the user to change their idea in terms of specific points in one day and modify all projects to use a new feature?

[vivid field]

Other packages get new major versions too, yeah

[vernal adder]

Yes.

[lucid pasture]

As I said earlier no one expects you to update your code in 1 day

[vernal adder]

That is exactly what other packages do as well.

[vivid field]

but here it needs to happen more frequently because discord.js depends on another api

[kindred lagoon]

You don't understand what I mean, why does it make a major release every year, why doesn't it just add features and fix bugs?

[visual hornet]

if you want an example for major drastic changes to other modules, just look at node-fetch, they dropped support for an entire module system with v3

[kindred lagoon]

Why don't you take a look at Mysql

😂😍

[visual hornet]

because we want to move forward with our library, not just stay in one place and fix bugs as they approach
we make major updates that fix major parts or add major things that will fix things in advance, like interaction support coming because the GUILD_MESSAGES intent is going to be restricted

[visual hornet]

they have 2 major versions because they don't have a lot of things to change
look at all the new things discord has brought or changed, we have to change to adapt to the conditions we work in

[kindred lagoon]

Ok I don't want to fix bugs but I want to make changes that are useless and he knows that major version of
node-fetch is 3 every year do they make a major change?"

[copper laurel]

This can stop

You aren't providing any constructive criticism, nor are you accepting the answers given

This channel is not for you to rant and whine about small inconveniences in rewriting code

[copper laurel]

Yes, its been discussed. Read the rest of what I said

[kindred lagoon]

I read it. I said my point, and this would improve the library if the developers of the library understood what I meant and what I said was constructive criticism not sarcastic

[unique axle]

Maybe you mistake some of the breaking changes that happen during development with released versions?
If not, I can just repeat what crawl and I said here (reply and down)

[copper laurel]

We hear this all the time, you aren't saying anything new, nor are you accepting any of the responses as to why it was necessary for us to make major version increases

[kindred lagoon]

I appreciate your efforts, you and the work team, and I hope that the main release will be in two years from now, and that the changes are not in the way of writing the function or changes in the names, but that there are radical changes that deserve this. Thank you.❤️

[copper laurel]

v11 was release 5 years ago. v12 was 2 years ago. 13.0 was two months ago.

Quite large windows and well within the range you seem to be requesting from us

[kindred lagoon]

I appreciate your effort and the efforts of the staff and have a nice day and that you understand what I mean

[unique axle]

The next major version (v14) will very likely re-structure some of the API in a breaking way to address problems we have found during development in the current version (be that actual bugs, unsupported use cases or "just" design decisions). That is, after all, what major versions are for.
The difference to other services (like the quoted node-fetch above) is, that this wraps another API we depend on (the Discord bot API). Meaning, while you can still use very old versions of node-fetch, you will eventually have to update discord.js, as Discord stops supporting older versions of their API. Discord also introduces new features only in new API versions, which forces the user to apply major version jumps and all the changes that come with it

[kindred lagoon]

Thank you for your simple explanation. Good day🤍👌

[wild flax]

What a joke

[wet sail]

hey, with Discord JS, it was always hard to write test with its internal structure and the need for the client. But now with all classes having private constructors, ist much harder. Whats the point of making this even more harder?

[outer raven]

why exactly do you need the private constructors? They were never meant to be used anyway

[visual hornet]

the constructors for most structures were never meant to be used, you should be getting most stuff from cache or from fetching or from other structures

[wet sail]

not for prod code sure but creating mocks for jest test. like here
https://github.com/Yes-Theory-Fam/yesbot-ts/blob/master/__tests__/mocks/message.ts

[outer raven]

Why would you need to test a package that isn't yours?

[wet sail]

we dont want to

we want to test our code

[visual hornet]

can't you just use the actual djs structures then

[outer raven]

well then test your code, not discord.js's?

[tacit crypt]

If you really need to create classes, you can use Reflect.construct but you're really really on your own (and it does not fall under this channel)

[outer magnet]

just a question, I was looking around and in the APIRouter.js file in rest folder and it has target in the parameters of the function but doesn't use it. Is this intentional or?

also in the deleted property in the Message class just defaults to false without any checks. Is this also intentional?

[opaque vessel]

Yes this is intended for internal library calls from client.api

[opaque vessel]

On the message delete event, this is set to true

If you were to remove the target parameter there, clients would never connect to the gateway :P

[outer magnet]

if a message is deleted before the client was ready, it will be false?

[opaque vessel]

If a message is deleted then the client fetches it... well that won't happen, it can't fetch a deleted message. Also it's not a case whether or not you're using the event, the library handles the incoming event

[outer magnet]

well yeah because many people think that it will automatically tell if the message is deleted, and people get confused, I think a note should inform them that messages deleted before client will return false, even if it's deleted

[wild flax]

that makes no sense

you won't even get a message if the client isn't connected

[outer magnet]

oh yea mb, i just realised that

[dawn merlin]

on component collectors can we assume that the component interactions we get back are from a bot that's within a guild?

[opaque vessel]

What about DM channels?

I guess since those methods can only be used from discord.js's structures, you can at least assume that it's not from a guild that the bot is not in (and cached in DM channels)

[distant prism]

hi everyone 🙂 is it possible to mute my music bot for all users in a channel except one ? example --> I want to listen music, but the others users don't necessarily want to hear. can i make my bot play music only for me ?

[visual hornet]

#archive-offtopic this isn't a support channel

[split elm]

A fetchRules function to fetch the guild’s rules ? (Suggestion)

[outer raven]

you mean the welcome screen? That exists

if you mean the rules channel, that exists as well

[split elm]

No, the rules that the member has to accept so as to access to the server

[outer raven]

yeah the welcome screen, it exists

[rustic bough]

Documentation suggestion for @split elm:
<:_:874569335308431382> Guild#fetchWelcomeScreen()
Fetches the welcome screen for this guild.

[split elm]

No, welcome screen = the screen that appears when you join a server

[opaque vessel]

That's membership gating. There isn't anything for it other than a few properties and flags

There is no method to fetch that information from a guild

[split elm]

Rules = the menu that you have to accept if you want to write

Okay so I suggest this function ^^

Because I saw that we can look if the rules are activated, but we can’t look at these rules

[opaque vessel]

That's correct. Discord removed that functionality: https://github.com/discord/discord-api-docs/pull/2547

Long story short, the current API design isn't great
So until they re-add it, nothing we can do

[split elm]

Too bad…

Thx 🙏 !

[gusty carbon]

Why do some some properties in discord.js-modules use the javascript private properties (prefixed with #), while others are just marked as private in TypeScript? Imo it would make more sense to be consistent and use the hash prefix everywhere since the privacy of those fields is also actually enforced by Javascript itself

[dawn merlin]

private in TS is both similar but different than #, one is not objectively worse or better than the other in any arbitrary scenario. This comment from one of the maintainers sums it up pretty well: https://github.com/microsoft/TypeScript/issues/31670#issuecomment-497370201

[gusty carbon]

yeah theres benefits and disadvantages to both, it just feels kind of odd to me that the accessors arent consistent across the project

[outer raven]

I think I remember seeing some discussion about narrowing the type of the value of a collection with the filter or find methods, did that ever go through or is that possible to do?

I'm asking this because I'm trying to find a channel in the channels cache and I'm sure it's a category channel, so I wanna narrow it's type down and there's no method for that. Could we created methods for checking each type of channel like we did for interactions?

[visual hornet]

or just the type property?

[outer raven]

checking for the type property in a filter or find doesn't narrow the type down

I guess, but using the methods like the Interaction ones is more useful to me at least

[wild flax]

How so

[outer raven]

well it's easier to just return if the method returns false than it is to import the class and do an instanceof check

[visual hornet]

oh you mean as typeguards

[outer raven]

yes

these

[icy bronze]

I'm not sure it's normal that Guild.available is sometimes undefined, it leads to an incorrect amount of unavailable guilds (i'll switch from !guild.available to guild.available === false to make sure I have to correct amount but just reporting)

[loud jay]

#6769 in discordjs/discord.js by cycloptux opened <t:1633470939:R> (approved)
fix(Guild): guild.available is never set on new joins
📥 npm i cycloptux/discord.js#fix-guild-available-not-set

[icy bronze]

ohh thank you

[wild flax]

Ok so make your own

If importing another one is such a big problem

[outer raven]

Why can’t we implement them in the library?

[wild flax]

Because it’s not the same as the one you linked above

[outer raven]

How is it not?

[wild flax]

The others aren’t just instanceof checks lol

[outer raven]

Different channel types have different methods and properties, I don’t see how that’s any different from interactions

Yeah I know?

[wild flax]

Yeah

There you go

[outer raven]

So again, why can’t we implement them into the library

[dawn merlin]

As I explained in the PR these type guards are better than instanceof guards because they make all applicable properties on the object the cached type. Instanceof doesn’t do that

[outer raven]

The interaction checks just check for the type

[outer raven]

I’m really not talking about those

[wild flax]

If you don’t want to make your own

Make a pr

You’ll get your feedback on there if it’s something people want or not

[outer raven]

That’s what I was asking, I thought you were against me even making a pr lol

[wild flax]

I’m never. Just know that it could be denied that’s all

[outer raven]

Yeah I’m fine with that

[outer raven]

on second thought, I'd rather do that in a semver major because the current isVoice and isText check for text and voice based channels, and not the text and voice types, so that'd be confusing

unless it was split up and the other methods were added now and those renamed later

[wild flax]

its semver-major later too

you can break either way

whether you do it now, or later

[visual hornet]

isText vs isTextBased would probably confuse some people but i could see that being useful.

[wild flax]

could probably make it isStrictText, jest does that for some things

and then in semver major break it

but then, we might as well do isTextBased and isText

and just have an info block

[outer raven]

ye that was my idea but that'd be semver major
I could do isStrictText for TextChannel checks tho

and if we were to add that we wouldn't need to break it later on

but then again it's a bit inconsistent and it feels weird to have isStage and isVoice which do different things (for example)

so to avoid confusion I think it's best to do this all in one go in a semver major change

[grand eagle]

Is this an error?

I copied this from another bot I had made which was running 13.1. 13.2 seems to have introduced this in the TypeScript typings.

[opaque vessel]

What is ActivityType?

[grand eagle]

From discord-api-types I believe.

[outer raven]

you need to cast the type because we changed it to not accept CUSTOM

so it can't be any string

[grand eagle]

I'm not using CUSTOM. In the screenshot, I am using Game.

[outer raven]

yeah but you still have to cast the type because it's a property in an object and not a string

why don't you just write "PLAYING"? (I think that's what you want)

[grand eagle]

https://github.com/mahyarmirrashed/bot-trout/blob/main/src/events/ReadyEvent.ts

You can see that it worked in 13.1

[outer raven]

yes I know

I'm telling you you need to explicitly say what the type of ActivityType.Game is

[grand eagle]

It's an enum? I am not sure what to say

I am not sure if I can actually cast it to a string.

So basically, we just do not accept from discord-api-types anymore? I found out that there is Constants.ActivityTypes.GAME

[outer raven]

yeah that's probably better. It may be a bug though, since it should know the type of that enum

not sure if it's fixable though due to how enums work in ts

maybe @dawn merlin knows?

[copper laurel]

That does seem a little odd, I'd agree that as a developer even though that field excludes a particular property of the enum, you'd think TypeScript would still let you use the other properties from the base enum

[outer raven]

it should, but TS is weird with number enums

so it doesn't know that ActivityType.Game is 0 but if you type 0 in it works

[vernal adder]

I have a similar issue with my option types, leading me to casting them.
type: ApplicationCommandOptionType.Subcommand as number

[copper laurel]

Isnt this exactly why we provide Constants?

Those are the real values and TS enums kinda dont exist in JS code

[outer raven]

yeah how are those imported? 🤔

[copper laurel]

uhh, import { Constants } from "discord.js"?

[dawn merlin]

The reason that this doesn't work is that TS doesn't let you use 'alternative' enums in place of the enum the that is expected. Basically just because one enum has the same values, doesn't imply it's strictly equivalent to another enum.

[outer raven]

no I mean the enums

[vernal adder]

Mine?

[outer raven]

like how do they translate to js

[vernal adder]

Looks like this in my dist.

Just the number (and a comment)

[dawn merlin]

the fix for this is to have the object accept both the API enum or the djs enum

[outer raven]

I don't really see a point in that tbh

i mean at that point might as well use the djs stuff

[dawn merlin]

I mean I would

like if it's djs, use djs types

[outer raven]

because if we were to do that for the entire lib it would be quite a lot of work for something that can be easily be fixed by using the djs package lol

[outer raven]

yeah saw it in my code too. It's weird that it knows how to compile but still errors

[dawn merlin]

I'm confused, doesn't this just need to be Constants.ApplicationCommandOptionType.Subcommand?

[vernal adder]

Using discord.js' enums works as normal.

[outer raven]

well there's your solution

[vernal adder]

It's not a problem for me. Just thought it'd add to the convo.

[dawn merlin]

wait can enums be used during runtime now? I remember a PR was merged to do that

[opaque vessel]

You mean https://github.com/discordjs/discord.js/pull/6394?

[dawn merlin]

yeah

They said you only need Constants now if you wanna do indexed enum access but accessing an enum directly now works (in runtime) without Constants

[dim steppe]

The values selected, if the component which was interacted with was a select menu
Isn't the last bit kinda redundant, since it's in the SelectMenuInteraction class?

Also, I've been wondering why DJS calls _patch() at the end of some constructors - why does it do this, as opposed to assigning the values in the constructor?

[copper laurel]

_patch() is also called for updates to the structure

From events etc

When we dont want to construct new ones

[dim steppe]

Oh, so anything that could update, gets placed in the patch

[copper laurel]

yeah, which is why it also does checks on the existence of properties

[opaque vessel]

Hi @vagrant holly! For your event removals, I think the typings need to be updated (old event names still in there)? :P

[vagrant holly]

Did I not remove them?

Oh huh I did not

[vagrant holly]

All fixed

[outer raven]

@dawn merlin I think we overlooked the return type of interaction.options.getMember in https://github.com/discordjs/discord.js/pull/6668 because if inCachedGuild() returns true then getMember will always return GuildMember or null, right?

[dawn merlin]

Oh for DM's?

[outer raven]

nah, regular guilds

but if your bot is in the guild then the member options will be GuildMember objects and never API objects

[dawn merlin]

yeah assuming, the guild member in that option is cached, it should be GuildMember | null, I'm kinda busy rn, but if I don't get to it today you can submit a PR

[outer raven]

ill look into it and if it doesn't look too complicated I'll submit a PR

[outer raven]

yeah I'm not quite sure how to do that so I think I'll leave that for someone else

[copper laurel]

You'd have to extend all the generic cached typings right down into the resolver

[opaque vessel]

Can anyone still reproduce https://github.com/discordjs/discord.js/issues/6126? Strict mode needs to be disabled. It looks like pull request #6644 made this behaviour working delightfully again for me

[dawn merlin]

I thought the issue in that ticket was that the person didn’t have strict mode enabled

[opaque vessel]

Yep. With strict mode disabled, the else block from .inGuild() return the interaction as never. It doesn't do that for me anymore

[dawn merlin]

Ah ok

[opaque vessel]

So it seems this issue is now solved :P

[dawn merlin]

Genuinely curious why that PR resolved it

[paper furnace]

I'm encountering a typing issue moving from v12 to v13, when I need to setup the default activity type using env var, I'm making a validator which reflect the type in use which is ExcludeEnum<typeof ActivityTypes, 'CUSTOM'>, but ExcludeEnum type is not exported then I had to duplicate it from my side. Can you consider exporting custom types too ?

[opaque vessel]

ExcludeEnum is already exported, it's just not in a released version

[paper furnace]

so the next release will fix it 🥳

[opaque vessel]

Ideally, yes (:

Pull request that made this happen: https://github.com/discordjs/discord.js/pull/6808

[paper furnace]

there's a fix aboutn const enums too ?

i don't understand why all enums are declared as const, but it's not really useful

currently due to export const enum ActivityTypes... we cannot use those enum to validate values and compare them against data from any type using Object.values like Object.values(ActivityTypes).includes(input). Maybe this choice is motivated, but currently I don't get the point

[outer raven]

Use the constants export instead

But typescript enums are not javascript enums, so you can’t really use them in code like that

They’re exported as consts so that they can be replaced by their values in compiled code

[outer raven]

Can you show a screenshot of the code that’s giving you this error? There’s an alternative to the ExcludeEnum type

[paper furnace]

when exporting a const enum TS don't generate the lookup table, so we cannot iterate on the values, I found the PR with the changes, but frankly I don't understand which errors are solved 😅

I'll check for the constants export

[paper furnace]

bot.user?.setActivity(env.BOT_DEFAULT_ACTIVITY_NAME, {type: env.BOT_DEFAULT_ACTIVITY_TYPE})

options are

export interface ActivityOptions {
  name?: string;
  url?: string;
  type?: ExcludeEnum<typeof ActivityTypes, 'CUSTOM'>;
  shardId?: number | readonly number[];
}

[outer raven]

You can do as ActivityOptions[“type”] after the type and that fixes it

[outer raven]

You aren’t supposed to use TS enums, you can use js enums exported from Constants

[paper furnace]

I see

then now it's fixed, i'll just wait for the next release to remove my ExcludeEnum copy

[dawn merlin]

I wonder what would happen if the user enabled —preserveConstEnums?

[paper furnace]

I'd prefer avoid using flags, they can update/remove them easily 😅

[wild flax]

Just like random strings then

Except that flags usually get updated to the new value, while a raw string doesn’t

So if anything using flags is a good thing

And that one surely won’t disappear anytime soon

It’s an integral compiler feature lol

[dawn merlin]

^

Also tsconfig.json