#archive-library-discussion

25085 messages · Page 14 of 26

slate nacelle

I doubt there is a reason for it, probably just an oversight.

outer raven

was just looking at the docs folder lmao
I think it used to be used on the old website but now it isn't since the logo is correct on the new one

vernal atlas

because its not a cache manager

(see below, ClientVoiceManager doesn't really do what the description for BaseManager says)

rustic boughBOT
outer raven

@wild flax what's the difference between types and typings and why do you always edit the title of PRs to say typings instead of types? When should types be used?

wild flax

I can’t remember if the changelog gen picks up types correct

So i change it whenever I remember

Lmao

outer raven

ahh ok

so maybe it should be removed from commitlint?

actually typings isn't even allowed now that I look at it

dawn merlin

is Message#createMessageComponentCollector supposed to be able to take InteractionCollectorOptions<T>? Because using the interactionType field and/or the channel and message fields doesn't affect the message component collector?

outer raven
dawn merlin

i closed it just now

outer raven

ah aight

copper laurel

Though isnt that componentType anyway?

dawn merlin
copper laurel

Right, yes

dawn merlin

So wouldn't MessageComponentCollectorOptions<T> be a better fit rather than InteractionCollectorOptions<T>?

copper laurel

Does that typing already exist?

Or are you suggesting making it

dawn merlin

it already exists

copper laurel

Then yeah probably

dawn merlin

ok cool, just wanted to make sure

visual hornet
visual hornet

I'd say it would still fit into BaseManager depending on how you interpret the wording though, and I think something like disconnectAll would be useful if discord would allow that (or if it's already in @discordjs/voice? i can't seem to find the docs for that)

zenith oracle

In the ws property of ClientOptions only large_threshold is documented but the createDefault static method in Options has other properties: compress, properties and version. Also, compress and properties don't seem to be used anywhere and version isn't in the typings...
What are correct the options for this object?

slate nacelle

I think what they currently are.

If anything compress and properties shouldn't be there at all, compressing is not supported by us and properties shouldn't be messed with.

zenith oracle

So only large_threshold should be used? Now I see that neither large_threshold is used 🤔
Only the version property is used in the websocket and it's the only which isn't in the typings 😂

slate nacelle
zenith oracle

Ohh you're right, didn't notice that

Thanks for the clarification

outer raven
vernal atlas

perhaps a more useful change would be adding the id property to Base instead of assigning it in the constructor of every extending class

if you're looking to remove duplicated code

outer raven

id?

vernal atlas

the id of a structure

outer raven
vernal atlas

doesnt really make much sense for ClientVoiceManager to extend BaseManager

it doesnt manage any methods for any data model

outer raven

cache is from DataManager

vernal atlas

its not about the properties it has its about the description of the class

outer raven

that description should probably be changed then because a BaseManager doesn't manage much, that's probably a leftover from when it was moved to DataManager

vernal atlas
outer raven

well yeah that manager does, but BaseManager has no properties or methods other than client, so the description could be updated

vernal atlas

its just not the same type of manager lol

outer raven

hm?

vernal atlas

all the other managers do what the description says

but the ClientVoiceManager simply manages voice state changes

outer raven

it holds the adapters for @discordjs/voice

so in a way it also manages that data

warped crater

one second

vernal atlas

why not tho

warped crater

can't find the conversation I'm thinking of

point is, there's structures that don't have IDs that extend base - which is why its not there

so if you wanna go about doing this you should make another "base" that adds the id and the structures that don't have an id to continue extending this class

vernal atlas
warped crater

this - Yeah that's def been talked about before

uh, "talked about", as in, brought up and not discussed further kek

I 100% remember a conversation about why id isn't on base and the jsdoc being weird, but I can't find it for the life of me no matter what search terms I try

burnt cradle

@tacit crypt when I switch to the enums in the context builder pr should I stick to inRange or should I switch it to oneOf, they would be the same right now but if discord adds more types later, oneOf would be more readable imo

tacit crypt

oneOf maybe?

visual hornet

got a list of classes that extend Base and don't have an id property btw

[
  'GuildBan',
  'GuildTemplate',
  'Invite',
  'InviteStageInstance',
  'Presence',
  'Typing',
  'WelcomeChannel',
  'WelcomeScreen'
]

||code i used to get this list if anyone wants to see actual hell: https://pastebin.com/LatwWFXQ||

so uhh yeah guess there are some classes that do extend base but don't have an id property

sick raft

why are the prerequisite modules for using /commands not installed by default

dusky reef

Because as wizo said, not every bot depends on commands (regular or slash). There are so many bots which just does specific things on ready like that and doesn’t rely on commands. Thus those modules are burden/extra for them.

Isn’t it the best practice to have only what i need? I guess that’s best to install slash modules only if he needs that.

sick raft

I hope you read the rest of what was said, but tl;dr not every bot depends on can be said about almost everything , but realistically, almost every bot uses commands

[and in the near future, /commands is expected to be the only available option for commands for most bots]

copper laurel

I dont really know what you mean

We just tell you to install the additional modules as you reach the concepts that need them

oak quail

hm

fetching invites to unknown channel types also causes an error lol

ig channel is null so its the same issue as fetching friend invites

stable sequoia

this looks like discord's bad but it essentially makes CommandInteractionOptionResolver#getMember unsafe/unusable with the second parameter as true as you can pass in an id into a USER option and if the id is a valid user id (but not a member [not sure if has to be member of the guild the interaction is in/didn't test]) the interaction will go through and the method throws the error ```
node_modules/discord.js/src/structures/CommandInteractionOptionResolver.js:100
throw new TypeError('COMMAND_INTERACTION_OPTION_EMPTY', name, option.type);
^

TypeError [COMMAND_INTERACTION_OPTION_EMPTY]: Required option "user" is of type: USER; expected a non-empty value.
at CommandInteractionOptionResolver._getTypedOption


`CommandInteractionOptionResolver#getUser`  seems to be working fine though

im on djs v13.1.0 btw, not sure if it's been addressed/fixed in master

copper laurel

Not sure what else you would expect it to do

If theres no member of the guild, getMember cant possibly have a value to get

And if you pass true to make it required, it will throw an error

Sounds entirely expected to me

stable sequoia

they why i said "this looks like discord's bad".

copper laurel

I mean... I also don't understand what you expect Discord to do about this

If you provide an ID of someone who is not in the guild, Discord cannot possibly give you a GuildMember

stable sequoia

if you pass in a channel id discord just doesnt send the interaction

copper laurel

To what type of option

stable sequoia

USER

copper laurel

Well duh, that wouldnt be a valid user ID

So no, it shouldnt send the interaction

stable sequoia

i guess yeah, thats true

but doesnt this make getMember unsafe

with the required field true atleast

copper laurel

So dont make it required

Or do, and catch the error, and handle it and tell the person using the command that this user isnt in the guild

This is entirely up to you how to control this behaviour

stable sequoia

mhm

copper laurel

Its unsafe not to handle an error, no matter why it happened

stable sequoia

i expected the error to be not possible because it should always be there if the option is required. but i guess with the nature of the method as its not based off of the option types provided by discord, it makes sense

copper laurel

The getUser will always be there if you make the option required

stable sequoia

yeah, i did test that

copper laurel

But a required option isnt the same thing as making the resolver required

In fact if its a required option, theres no reason to make it required in the resolver at all

The resolver gives you error control flow

stable sequoia

doesnt it provide type narrowing

copper laurel

eh I guess, if TS

stable sequoia

yea yea

i guess this would be a discussion for if this persists when/if discord introduces a member type for options

and still it wouldnt really have anything to do with djs i guess :p

vernal atlas

is it worth adding a note to the docs that it throws if the user isn't in the guild?

@stable sequoia @copper laurel ^^

copper laurel

No, that should be common sense

No guild member for someone who isn't a member of the guild

It also won't throw if you don't make it required

vernal atlas

it would be kind of a wacky workaround to mark a required parameter as optional, but yeah i agree that its obvious, but also might confuse someone seeing the error

which is why i think a warning wouldn't exactly be the worst idea

copper laurel

The option being required is not the same thing as forcing the resolver to resolve something or throw

A required USER option will have a User

I may or may not have a GuildMember

So you can resolve it without required:true and check undefined

Or resolve it with required:true and try/catch the possible exception

Like I said early, the resolver option is an error control flow, it doesn't really have anything to do with if the option is required or not

stable sequoia
vernal atlas

yea

stable sequoia

but i do agree marking a required param as optional feels weird, and i feel like most people wouldn't think of that

vernal atlas

is there really a point in even having the required parameter for getMember (although this isnt a change applicable for v13)

stable sequoia

i don't think so, unless you would prefer for it to error instead of returning a nullish value

i would definitely rather have the 2nd param yeeted and have it nullable so more people don't walk into this but you guys might know better than me

having it nullable just feels right compared to the current behaviour imo

because the required param is kinda irrelevant

copper laurel

You're not marking it as optional

You're simply choosing how the resolver should behave

They are not the same thing

If you want it nullable stop using the second param, there's literally nothing more to this

Why do you want to take freedom of choice away?

If you mark getMember() as required then you're telling the code that this function requires a GuildMember, and will throw if one cannot be resolved

outer raven

You could alternatively use getUser and resolve the member later, this way you can be sure if there is a member and make your own error if not

stable sequoia

that just makes it more complicated than using it with optional false though?

hoary ether
dawn merlin

There is? It’s a context menu command for messages

wild flax

Yeah strictly for context menus

hoary ether

oh. that makes sense. i was too tunnel vision on slash commands.

outer raven

I found another bug with Message but this one is about Message#pinnable, would that fix still be within the scope of #6581?

loud jayBOT
outer raven

the bug is that the getter returns false on replies and other messages that should be pinnable, so it's not related to the channel being missing which is the original scope of the PR

opaque vessel

Meh all these bugs about permission checks makes me think if they should exist in the first place

outer raven

same but they can't be removed now. I can't wait for that permission helpers lib tho

and I think this was missed because you can only crosspost DEFAULT messages, but you can pin types other than DEFAULT

also for ThreadMemberManager#add you can pass @me as the member which results in the function returning @me which isn't a Snowflake. Should the docs and types be updated to say this or should the function be updated to send the client user's ID?

zenith oracle

For types Snowflake is a string so @me is acceptable

copper garden

Snowflake is strictly typed as a numeric string, not as any string anymore

zenith oracle

Is it? Thought the ${bigint} was reverted

ornate topaz

Afair only in discord-api-types, or something like that

copper laurel

Which is where our typing comes from

So ours is also reverted

zenith oracle

Yeah, that's what I meant. Actually if I import the Snowflake type from discordjs I see that it's of type string and I see the discord-api-types description

outer raven

I wasn't aware of that change but the definition of a snowflake is a string with a bigint inside

so @me should not be considered one

ornate topaz
outer raven

Yeah that only makes it easier to use for convenience, but doesn’t change what I said above

Also @unique axle why is the partials part in the issue template required?

unique axle

have you checked PRs?

outer raven

Oh I see it

My bad thumbs

outer raven
tacit crypt

I don't..see a question in that message

meguFace

outer raven

message right above it

that one explains the bug

outer raven
tacit crypt

The answer is simple: Is it related to the this.channel change? If yes, in the same PR, if not different PR (at least imo)

outer raven

yeah it's not, I'll make a separate one

cosmic pagoda

hello, i have a suggestion/request i dont know where to send this. anyway if you have a multiline string like this in embed, the indents are visible on mobile devices (pc discord seems like it parses automatically but not mobiles. it would be good if there was some middleware option which would allow us to remove indents in one place rather than for every message separately. its not big deal but it would be nice and helpful feature to have

visual hornet
unique axle
cosmic pagoda

i know that this is how js works and I know I can use some function but im asking for library implementation where it does this automatically because there is no good for it, or like I mentioned a middleware which gets called each time before a message gets send where we could handle it ourself in one place

unique axle

stripping indents from your strings is not something a discord library should do

cosmic pagoda

then the middleware :/

visual hornet

souji just linked you some

unique axle

then define a function somewhere and pass content to it before sending it?

.send({content: prepareContent(stringHere)})

cosmic pagoda

you dont understand forget it sorry for your time

unique axle

i understand, you want to define a middleware somewhere (client options, maybe) that every message content gets passed to before sending it off to discord

cosmic pagoda

thats not really how middleware should be made

visual hornet

sideone why do you think a library for interacting with a discord api would implement string cleanup

cosmic pagoda

there could also be middleware for kicking, banning deleting messages not just my request

cosmic pagoda
visual hornet

you know we have managers Thonk

cosmic pagoda

what does that mean?

visual hornet

MessageManager for managing messages (ie deletion), GuildMemberManager for managing guild members (ie kicking & banning)

cosmic pagoda

and how does that relate to middleware?

but yes the Manager classes could have have an optional middleware function which could be set (maybe like the makeCache thingy)

unique axle

adding middleware across the library that modifies options before making api calls is not something the library in its current structure supports.
if you want that feature i'd suggest opening a feature request on github, so it can be tracked https://github.com/discordjs/discord.js/issues/new/choose

cosmic pagoda

im banned on the repository 😦 thats why im asking here

ill actually respect the ban and not make new account but please could you reconsider with the staff my unban? I havent done anything terrible really it was stupid joke

swift zealot

I've encountered the following issue when my bot leaves a guild:

Trace Stack: DiscordAPIError: Unknown Guild
    at RequestHandler.execute (C:\goat\node_modules\discord.js\src\rest\RequestHandler.js:298:13)
    at runMicrotasks (<anonymous>)
    at processTicksAndRejections (node:internal/process/task_queues:96:5)
    at async RequestHandler.push (C:\goat\node_modules\discord.js\src\rest\RequestHandler.js:50:14)
    at async Guild.leave (C:\goat\node_modules\discord.js\src\structures\Guild.js:1222:5)
buoyant umbra

is guild.channels.fetch() supposed to force-fetch the channels from the API everytime? The docs state that it does a force fetch unless the channels are cached, therefore returning the cache. I used guild.channels.fetch() in a loop and it got ratelimited after 8 iterations (for doing the force fetch). Providing {cache:true,force:false} to fetch options does nothing too

vivid field

the cache check is only done if you try to fetch a single channel

otherwise you can't tell if a channel is missing

outer raven

@grizzled scaffold wdym the plugin has been uninstalled? (#6587)

placid geyser

is it true that discord will only support slash commands?

raven juniper

Discord has indicated they will not approve bots for the message intent for regular commands if that's why you apply. This does not apply until your bot needs to be verified

placid geyser
pastel magnet

You will be rejected since message content is not required for any of those commands.

grizzled scaffold
outer raven
grizzled scaffold

Yeah I tested it and the file isn't referenced anymore

modest lodge

Is there a channel I can ask about unit tests? Of a bot, not the library. It is proving quite a bit more difficult with slash commands as I pretty much need CommandInteraction objects. Is there a way to create one through the library with particular values? Can I "new" one somehow?

quiet viper
modest lodge

@quiet viper That is what I'm using. I'm trying to unit test my bot and so I need to generate the "responses" that such commands generate without (always) installing the commands and typing in multiple different responses. The unit tests run independent of the bot.

solemn oyster
modest lodge

It definitely looks like there is info there. Is that just a message though? I don't know what that thing is a special channel?

quiet viper
modest lodge

excellent... thank you

ruby terrace
hoary ether

@ruby terrace allow me to grab your attention while you're still here. you mentioned a while back something about discord making an overhaul for slash command permission. would you mind answering this? #archive-interactions message

unique axle
hoary ether

found it. thank you

i know im on the wrong channel but, im currently making a PR for the guide and i was wondering if it would be ok to include a little note in the section about slash command permission that links to that discussion? the note can say something like "Permissions for Slash Commands are getting updated by Discord. You can see the discussion here. For now, you can manage your slash command permission with the different methods explained in this guide"

unique axle

no, guide content is supposed to be long-term
this feature is not developed enough to even mention + the discussion itself is meant to be temporary to inform devs of "things to come potentially, hopefully soon™️"

hoary ether

no, guide content is supposed to be long-term
would you suggest keeping the current section about slash command permission as is then? I already wrote a more extensive explanation for that section, but now im not sure if it's worth including it in the PR because of what you said. I could include my extensive explanation in the PR, and simply omit the "little note that links to the github discussion".

unique axle

if you improved the current explanation sure, go for it
just that temporary notes should not be included
(there have historically been exceptions to that rule with the temporary intents explanation, just because of the sheer amount of people facing the problem)

hoary ether

👍

clever flicker

So I'm just kind of curious, why the hard requirement to bleeding edge NodeJS? What is it about this library that hinders support for LTS?

wild flax

Pretty sure that has been answered numerous times by now

check the issue or search for it on the server

real walrus

Was invite information removed from the guildMemberAdd event? As I know it worked just fine on V12, figured I would ask here as it's more of a library thing. Any thoughts?

I have searched the changelog ^ Nothing touches on it.

unique axle

there never was invite information on the guildMemberAdd event

real walrus

I used to be able to fetch invites when a user joined, and it could log who invited them, what happened to being able to fetch invites?

tacit crypt

You'll need to be more explicit than that, and I'm pretty sure this falls outside of this channel

Methods have changed and you'll need to update to the new ones iirc

spice sierra

Can I ask a question about a discord.js code here ?

copper laurel

This channel is about development of the library, or its source code

Questions about using discord.js go in the help channels

spice sierra

It's about its source code so ig I can

It's about the compression part of the ws client, the chunk size where did the number came from? I'm trying to understand why but no clue

Sorry I'm on phone rn

I'll check out thanks for the answer

outer magnet

imo the messageReactionRemove should emit if uncached and the partials are enabled similar to messageReactionAdd

i checked the code for messageReactionRemove.js and there is no place where it tries to check if partials are enabled

copper laurel
outer magnet

ah, alright then. thanks

visual hornet

why do some managers have fetch options (cache/force) as a combined object with the target (ie GuildMember, Guild, etc) while others have the fetch options as a seperate object as the second parameter? (ie User, Role, Channel, etc)

opaque vessel

I believe the former managers you described have several overloads, and the latter ones you described don't

visual hornet

overloads?

uneven rain

why not merge interaction.update with interaction.editReply? if the interaction is already updated then just use editReply

opaque vessel

Sounds lazy. Those methods are not the same and shouldn't be used as such

vernal adder

Aren't they fundamentally different? update() edits (updates) the message the component is on, whilst editReply() edits the original reply to an interaction?

clever crypt

@tacit crypt @solemn oyster @wild flax
i want to PR to next with doc gen, literally ported from sapphire. thoughts?

copper laurel
copper laurel

@dawn merlin re #6600 I don't understand why you think member and guildId would be non-nullable

dawn merlin
copper laurel

No worries then

outer raven

I hate to be that guy but there’s been a series of bug fixes and new features in the main branch that haven’t been published on a full release yet, discord api changes included, so when can we expect a 13.2 with those?

rough arrow

Bug fixes should be able to be released with a 13.1.1

But it's usually a lot of work to cherry-pick commits that way

wild flax

No cherry picking around here

quiet viper

Is global rate limit handling in plan?
or rate Limit handling across shards?

void rivet
burnt cradle
void rivet

ah didn't notice that was there, thanks 👍🏻

I would think it'd be on the rolemanager 😅

burnt cradle

@tacit crypt were gonna need that -types version fix to bump the builders dep

tacit crypt

I am aware

burnt cradle

👍

outer raven
opaque vessel

Issue Form Feedback

outer raven

Would it make sense to create a CategoryChannel#createChannel() method that creates a channel inside that category?

The only thing I'm struggling with this is the documentation e.g. the name of the typedef and if it should just have all properties from GuildChannelCreateOptions and then make that extend the new typedef. What do you guys think?

vernal atlas

i dont think that method would be a bad idea

as for the documentation/typedef:

just all options except for parent, and of course don't allow the type to be a category channel

outer raven
vernal atlas

pretty much yeah i guess

outer raven

also are we supposed to be able to pass a GUILD_NEWS_THREAD type to GuildChannelManager#create?

GUILD_PUBLIC_THREAD and GUILD_PRIVATE_THREAD are excluded from the typings, but not that one

ruby terrace

no, news threads didn't exist when I did that and forgot to update, feel free to fix

tacit crypt
outer raven
tacit crypt

I didn't say no, I mentioned other things.. what aliases do we even have nowadays

outer raven

ThreadChannel#fetchStarterMessage I'd say is one and there surely are others, I just don't remember how all of them work to know if it's a shortcut or not

now am I missing something here or is TS drunk? The error is saying the type of the type property is not compatible

tacit crypt

And what's the actual error it gives

outer raven
tacit crypt

Pretty sure exclude isn't a type

Isn't it Omit

outer raven

it is a type, but Omit is also a thing, I can try

inland lotus

exclude is a type

outer raven

yeah Omit fixes it

I guess Exclude is for classes

tacit crypt

Oh interesting

I never used Exclude

Just read on it tho

Can see where it's useful

raven juniper
real jetty

Isn't the lib moreso removing helper methods these days anyway?

raven juniper

Yes

See GuildMember#permissionsIn and hasPermission

outer raven
loud jayBOT

pr_open #6614 in discordjs/discord.js by ImRodry opened <t:1631141001:R> (review required)
feat(CategoryChannel): add createChannel shortcut method
📥 npm i ImRodry/discord.js#feat-category-createChannel

raven juniper

Search exists on the client but we don't implement that

outer raven

it would be possible, but unnecessarily hard

raven juniper

There's no route for this either, but you're still proposing to add it

opaque vessel

Feels a bit weird that the GuildChannelCreateOptions extends CategoryCreateChannelOptions. The former shouldn't be relied on the latter as it's just some helper

outer raven

it's just a way to not duplicate the descriptions, but I can keep the old typedef as it is if u want

loud jayBOT
outer raven

also this would really help with that ^^

raven juniper

Don't like moving a manager method off the manager either

opaque vessel

If anything I do feel the reliance should be swapped around

Also bots can't create store channels due to needing to pass a sku id which isn't a field during channel creation

outer raven

I've seen that in an issue, but that's outside of the scope of my PR

opaque vessel

Yea maybe, thought I would mention it since I noticed an overload there

outer raven

so if the manager supports store channels, this method must do too

you can leave that in a review and let the managers say if that should be removed from the manager or not

opaque vessel

Last night I looked to see if store channels explicitly were noted to be unable to be created in Discord's documentation but I couldn't find anything, soooooo... maybe not a strong enough reason to remove it vs. it could be added in the future (big chance no) so idk

outer raven

why would it not be added in the future?

what do you even need in that sku

opaque vessel

sku_id and optionally branch

opaque vessel
copper laurel
outer raven

why

copper laurel

I don't know how to answer "why" - because that's exactly how it seems to me

It saves you passing a parent option directly and... thats it

I also think that createChannel naming convention is a step back towards the pre-managers style of v11, most instances of that sort of interface have been removed

outer raven
copper laurel

children is only a collection, not a manager
This is why

outer raven

there's no reason to make it a manager though is there

raven juniper

No, and there's no reason to have a create method on a single object instead of the relevant manager

mighty sequoia
real jetty
outer raven

@vernal atlas

vernal atlas

yea it prop needs a bit of fine tuning

prob just

keyof Enum

outer raven

that gets rid of the error ye

lemme see if it works

this doesn't seem right

vernal atlas

hm

you might need to fiddle around with it

outer raven
vernal atlas

oki

vivid field

type ExcludeEnum<T, K extends keyof T> = Exclude<keyof T | T[keyof T], K | T[K]> seems to work

outer raven
vivid field

you need to use it as ExcludeEnum<typeof Enum, ...>

outer raven

oh

ok the error is gone but is this right?

vivid field

write some tests in tests.ts

typescript likes to not resolve the type sometimes

outer raven
vivid field

you can't disallow out-of-range numbers using Exclude

that's a limitation of ts number enums

outer raven

ah

so this isn't any better than the regular Exclude

vivid field

for instance, this code is valid too

enum Foo {
    A = 0,
}

const a: Foo = 1;
outer raven

in fact I see

vivid field

it's only validated for string enums

outer raven

eh but I guess it does make it shorter so I'll add this

this shouldn't be exported right?

on second thought, I probably wanna add this type to other things that use Exclude the same way these interfaces did so I'll wait for this to be merged then open a different PR with the new type

grand eagle

I think that it would be great if there was also a list of the intents/permissions that a bot needed for each of the methods.

It is really hard sometimes to know what permissions the bot needs to operate.

unique axle

we deliberately don't document these, as they are documented upstream (discord api docs)
adding DAPI doc links would be ideal, however unfortunately they change so regularly because someone decides to restructure docs, that that's not all that feasible either

buoyant sun

Regarding issue 6602. If that is an API bug, how should it be reported to Discord?

outer raven
buoyant sun

Ok. Thanks!

outer raven

For things that can be a Snowflake or a regular string (like reactions) should the docs include both types or only string?

Cuz I've noticed this behavior is inconsistent in some places

opaque vessel

Can you give an example?

outer raven
outer raven

So which one should it be?

outer raven

Decided to go with Snowflake|string just like reactions

loud jayBOT

pr_open #6623 in discordjs/discord.js by ImRodry opened <t:1631324641:R> (review required)
feat(Integration): add missing props and fix docs/types
📥 npm i ImRodry/discord.js#feat-integration-props-docs

mighty sequoia

Is it a bug that calling .update on an interaction with new files appends the files instead of replacing them?

copper laurel

no

You use files to add, attachments: [] to remove

mighty sequoia
rustic boughBOT

Documentation suggestion for @mighty sequoia:
_ Message#edit()
Edits the content of the message.

real jetty
outer raven

It can have a description, but no description doesn’t hurt either

peak zinc

can someone explain the purpose of the discord-api-types package? what exactly does it do

I'm looking thru the repo on github rn and I can't tell if this actually parses the API responses or just documents them

like are there transformers or anything in this package? or just types

i.e. can this be used to verify json responses at runtime?

glad sparrow

discord-api-types

peak zinc

sorry if this is a dumb question, but if it's only types then why isn't the whole repo just .d.ts files?

glad sparrow

why should it be?

peak zinc

isn't that what declaration files are for

outer raven

it also holds static values to be used with the api iirc

ornate topaz
peak zinc isn't that what declaration files are for

aren't d.ts files supposed to work along .js files? since typescript would be able to work our everything from .ts files, .d.ts would be useless for it
since there isn't any .js code in the -types, and there isn't really anything that would be .js to which you could match a .d.ts, the repo can't really be a bunch of .d.ts files.

for typescript libs d.ts is not useful - .ts is enough for typescript
for javascript libs, .js is ambiguous type-wise, so you use .d.ts to type existing codebase without rewriting it to .ts

remote wasp
peak zinc i.e. can this be used to verify json responses at runtime?

The repo provides static types for Discord API. The compiler generates .d.ts files using the repo which gets published for end-use. TS compiler uses these types to catch errors at compile time, but if you want to validate something at runtime, you can use a lib (https://github.com/YousefED/typescript-json-schema) that generates JSON schema from static TS types and then pass that schema to ajv (https://github.com/ajv-validator/ajv) compiler for validation.

tacit crypt

And there's also JS code emitted from it (enums and util functions as an example)

outer raven

@tacit crypt I think there might be something wrong with dapi types or am I doing something wrong?

vivid field

you need to use EmbedType.Rich instead

tacit crypt

Or cast as const

vivid field

doesn't work either

that's a limitation of ts string enums

tacit crypt

Pretty sure casting as const worked.. hmm

Maybe not

outer raven
outer raven
vivid field

djs usually uses keyof typeof Enum

so then you get a union that a string is assignable to

outer raven

that doesn't work either though

vivid field

wdym?

outer raven

I edited the types file on discord-api-types to say keyof typeof EmbedType and that didn't work either

actually, Rich works, but rich doesn't

vivid field

well because the enum key is called Rich and not rich

outer raven

I see

then is there no way to support rich?

vivid field

also I don't think djs has any string enums anyway

outer raven

yeah true, they're all numbers

tacit crypt

You don't set type anyways

Discord even explicitly tells you that it's ignored

@outer raven ^

outer raven
outer raven

Considering image sizes for the makeImageUrl function in Constants end up being transformed into strings, should we allow string types as well as numbers? I'm saying this because the error can be misleading if a user does use strings for the image size

or maybe we should throw a different TypeError instead

visual hornet
outer raven

Wait bruh I entered the wrong thing
The error is still the same tho

visual hornet

what you said still applies though

RangeError [IMAGE_SIZE]: Invalid image size: 16

this could be pretty confusing

outer raven

In the hypothetical scenario that a user sends the size value from a url they received they’d have to turn it into a number for it to then be turned into a string again

So it would be rather easy and useful to allow both

sage frigate

Shouldn't the style default to f here?

visual hornet

the default is from discord processing the text iirc

sage frigate

Yeah, they default it to f but I just thought that would be displayed in the default column

visual hornet

that would be the defaults for the function though, not the displayed text in discord
don't see any reason to provide a default when the default is provided by discord, not djs

zenith oracle
peak zinc
peak zinc

or does it just assume everything is correct

tldr does djs do any runtime type checking?

outer raven
wild flax

What about it

Oh allowing strings

No

outer raven
outer raven
remote wasp
peak zinc

thank you i_couldnt_finish_the_project

outer raven

Why do methods like VoiceStats#setMute or VoiceState#setDeaf require the member to be cached if GuildMember#edit just calls the manager which only needs the member's id?

could this be improved so that this error could be removed?

outer raven

I found the commit that added this code and it was before managers were introduced it seems like, so this can now be removed

glad sparrow

can you send link to that commit

outer raven
gusty carbon
remote wasp

The reason gets sent in the X-Audit-Log-Reason header

gusty carbon

oh, well yeah that makes sense

outer raven
solemn oyster

Done

outer raven

Thanks thumbs

winged dust

is this change intended => interaction.options._subCommand to interaction.options._subcommand intended? (_subC... to _subc...)

wild flax

Yes

visual hornet
quiet viper
visual hornet

er, that's an object, and im asking where the colors are from, not where the source code is...

outer raven

Some are from discord’s branding colors but most seem random to me

tender field

in Message#equals, why is rawData mandatory? (at least in the typings)? because you check wether it exists or not before using it, so why make it mandatory ?

opaque vessel

It was used to check if two messages were equal on MESSAGE_UPDATE a long time ago. I don't think it's referenced anywhere internally and that code hasn't been touched in years. It probably can be removed

tender field

👍 i realised i don't need it, but thanks for the answer.

**
**btw is it planned to improve audit logs typings? because i always have to do things like this (probably missing property, but those are only the ones that i use), and they're starting to pile up quite a bit.

export interface MessageDeleteAuditLogsEntry extends GuildAuditLogsEntry {
  action: 'MESSAGE_DELETE';
  target: Message;
  targetType: 'MESSAGE';
}
export interface MessageDeleteAuditLogs extends GuildAuditLogs {
  entries: Collection<Snowflake, MessageDeleteAuditLogsEntry>;
}
```You could narrow that if the `type` option has been given in `fetchAuditLogs`, otherwise have a union of all of those (or just the base guildauditlogs, but then it doesn't narrow type when we check for "action").

or is it too much work for a not-so-used feature, and will be improved with -modules?

pale saddle

Is there any reason why the type of the "channel" property is not changed with the inGuild() function for command?

Instead of :

public inGuild(): this is this & { guildId: Snowflake; member: GuildMember | APIInteractionGuildMember };

It should be :

  public inGuild(): this is this & { guildId: Snowflake; member: GuildMember | APIInteractionGuildMember; channel: TextChannel | NewsChannel | ThreadChannel };
outer raven
tender field
outer raven

I mean your suggestion would work as long as you keep null in there

tender field

is it normal that GuildAuditLogsEntry#target's type has nothing to do with GuildAuditLogsEntry#targetType ? because i have a case where targetType is Message, and target is ClientUser (which btw gives me no way to access the message's id but that's another story)

i guess it's normal, but it does seem really counter intuitive

outer raven

it does seem counter-intuitive, but that's how discord named things

tender field
outer raven

It’s based off of actionType and just groups different kinds of actions in one property so that you can know the generic type

But I think the confusing name here is target and that one is given by discord

tender field

true

i admit discord's audit log's api could use a heavy lifting

outer raven

It’s too small for them to care

If they butchered interactions like they did, I doubt they’d care more about audit logs

Actually on second thought, maybe targetType would be better off as actionCategory but I don’t think that changing names just for the sake of it is something we do around here

tender field

Well i have to say it will make things less confusing for sure

outer raven

Up to the maintainers if they wanna do it for v14. You can always open a PR now or at a later development stage to suggest this and collect feedback

tacit crypt
outer raven

Since it would need to take format and size parameters

tacit crypt

Thonk

oh cool

that actually works

sweats

ebon radish

I tried searching first: what are the current thoughts on migrating the whole thing to Typescript?

real jetty

That's what -next is

ebon radish

so it's happening?

real jetty

It's planned and slowly worked on. Though priority is updating current version with new things discord throws in

ebon radish

👍

real jetty

as pointed out by souji elsewhere, actually called -modules now, not -next. #858851268914577428

ebon radish

Does that mean ditching CJS as well?

real jetty

@wild flax and @outer raven I just saw that there was already a type for the textual channel of a guild : GuildTextChannelResolvable. So finally, shouldn't I put this type instead of creating a new one?
And types seems to be ordered alphabetically but not all (e.g. GuildTextChannelResolvable)

wild flax

Hmm

You could probably reuse that

Resolvables are generally a bit different though

outer raven

not all are ordered alphabetically but most are. You should just use

Exclude<TextBasedChannels, 'PartialDMChannel' | 'DMChannel'>

instead of a new type

opaque vessel

That type includes a Snowflake, so maybe exclude that

outer raven

not TextBasedChannels

opaque vessel

The type above your comments

outer raven

ah ye

I also wanted to ask @wild flax if it would be too much of a hassle to require node v16.6 in Collection because I wanted to make a PR to add an at() function similar to Array.at()

vivid field

I would personally say that you shouldn't rely on the index when working with collections, but on the other hand methods like first or last exist

opaque vessel

You could do something like return this.first(<number>)[<number> - 1]) ?? null right? In case the bump won't happen

real jetty
outer raven not all are ordered alphabetically but most are. You should just use ```ts Exclu...

This solution seems to not work :/ With :

public inGuild(): this is this & { guildId: Snowflake; member: GuildMember | APIInteractionGuildMember; channel: Exclude<TextBasedChannels, 'PartialDMChannel' | 'DMChannel'> };

if (interaction.inGuild()) return;
// On this line, my IDE says that interaction.channel can be PartialDMChannel | DMChannel | TextChannel | NewsChannel | ThreadChannel```
outer raven
outer raven
outer raven
opaque vessel

On that note, should the .inGuild() method check for a guild channel type like it checks for the guild id and whether the member data is present?

outer raven
outer raven
outer raven

nvm that, I made the PR without using Array.at()

real jetty
outer raven

Aight, just fix the order then ig

real jetty
tender field
outer raven Yeah I can see a clear use for this, would be worth looking into. If you have th...

I've finish doing this (strong typings for audit logs, i.e. you do guild.fetchAuditLog({ type: 'MEMBER_UPDATE' }) and then you get the typings for .extra and .target that match the member update entry)
BUT it means i can't accept numbers anymore in GuildAuditLogsFetchOptions.type. Only GuildAuditLogsAction. Is it worth the change? Should i do a reversed interface to enable this ? (interface that maps numbers with their GuildAuditLogsAction so we can resolve numbers too) ? wdyt?

Otherwise i can shoot a PR and we can discuss that there

wild flax

It’d def be nice to be able to pass a number

I use the constants in discord-api-types to resolve the type which are raw numbers

tender field

oh yeah right i forgot about -types, i'll look into this. thanks for the feedback

(btw it's a really generic heavy PR, with many ternary expressions. It's def not "pretty" but it's effective. I'll summarize all the pros and cons in the pr)

tender field

so i can do that but "lines-of-code"-wise it will be the same as creating a reverse mappings from numbers to GuildAuditLogsAction

wild flax

Yeah but we can def not allow it not taking a number all of a sudden

And it would be quite the regression imo

Maybe write some safety tests in the typescript file

But other than that it’s fine

tender field

wdym safety tests? and would you prefer mappings from enums to Actions or from numbers to Actions?

i'll make a draft pr, we'll look into that then.

opaque vessel
surreal hollow

Why is it interaction.user and not interaction.author like it is with message?

real jetty
zenith oracle
outer raven
raven juniper

@outer raven this channel is for serious discussion, not shitposting

outer raven

Ok chill it was 1 message

tacit crypt
outer raven
ruby terrace

Probably because the author for a message isn't always a user

outer raven
real jetty

webook, system message

outer raven
ruby terrace

no, its a "user" that you can do none of the normal user stuff with because its a fake user.

outer raven

it's a user class nonetheless, you can do a lot of stuff with it, just not interact with the user directly e.g. sending messages

ruby terrace

in djs it is, not in the api, and since we're matching names, well...we match

opaque vessel 
<Client>.on(Constants.Events.GUILD_MEMBER_REMOVE, (guildMember): void => {
  guildMember.id // No errors
  guildMember.user.id // Error: guildMember.user is possibly 'null'.
});

guildMember.id returns the id through .user.id. Isn't the above error somewhat... strange? Pretty sure this isn't correct behaviour

ruby terrace

👀 it's actually never null, its either undefined or a valid user, though the entire class does assume it is present always....I smell bugs

I'm also not sure when a user wouldn't be included to some degree when constructing a member

warped basin

I'm thinking about making a library dealing with chat messages for a non-discord service and I am wondering how you test features and the library as a whole?

i'm not talking about things like jest, but just using it. do you just have a separate directory where you import the library and test the new feature there?

well how do you do it with djs?

ruby terrace

Most of us test with a dedicated testing bot, using the actual API and using the local instance of the library as a dependency

warped basin

ok cool

copper laurel

Yeah you can use the npm link functionality to link to local packages

loud jayBOT
copper laurel

Regarding this - I mostly agree with Voltrex that its not a bug because BigInt isnt JSON-serialisable, but Role.toJSON() does return permissions as a bigint literal

Shouldnt the object returned from that be JSON-spec compliant?

real jetty

yea, it seems like the only utility class that uses bigint for their flag bitfields is the Permissions class which is what that issue is really pointed towards, I think we should just call <Permissions>.toJSON() when serializing the Role instance into JSON, so it casts the bigint bitfield literals to strings

outer raven

Doesn’t it do it automatically? Thonk
Bot quite library discussion tho

I think chrome has something that lets u download it

Or just hard reload

tacit crypt

If you closed the toast that said it's ready to use online you're fine
Try it out, see what happens

It's automatically downloaded o.o

unique axle
outer raven
unique axle

yes, that is that

youtube together is one of the many embedded applications

outer raven

how would we get the ApplicationResolvable for that?

unique axle

and at the present time it requires

  1. whitelisted server
  2. client modification
  3. an invite

find the application ID, we're not documenting these if they don't get documented on DAPI

outer raven

ah alright, I hope this becomes easier once the feature is available to the public though

also I didn't need a client modification ?

maybe the user who started it did, idk

unique axle

yes

outer raven

alright thumbs

real jetty

I was wondering, why
<GuildMemberManager>.fetch(....) has a completely different management (parameters and on return types) than <GuildChannelManager>.fetch(...) :

First difference :
<GuildMemberManager>.fetch(....) accept only one parameter (BaseFetchOptions + user id)
<GuildChannelManager>.fetch(...) accept two parameters (channel id, BaseFetchOptions)
Second difference :
<GuildMemberManager>.fetch(....) throws an error if the member isn't found whereas <GuildChannelManager>.fetch(...) returns null if the channel isn't found

oak quail

they work totally differently

for members it uses the Request Guild Members gateway command which has a bunch of params so you can find based on many things

for channels it just GETs the specific channel or the guild's channel list

visual hornet
opaque vessel

There's nothing really to clarify. One is just more flexible with methods. advaith explained it above a little

visual hornet

uhm, you mentioned overloading and i asked what that was and you never responded meguFace

copper laurel

advaith is right that those methods function differently, but honestly, theres various inconsistencies across the lib if people really wanted to interrogate them all

The GuildMemberManager#fetch method is a real fetch. It accepts an id or FetchMemberOption (single) or FetchMembersOptions (multiple). Theres a REST endpoint for retrieving individual guild members by id. Otherwise if options are provided it can fetch multiple, but for this it uses gateway opcodes and receives chunks.
GuildChannelManager#fetch actually fakes the individual fetch method - theres isn't a REST endpoint for fetching individual channels. Instead it calls the endpoint which returns all channels for a guild, and then just gets the one you asked for.

That explains the different return values. There isnt really a reason for the different params as input though that I can see, other than unenforced consistency

unique axle

usually, what should be the case is that required parameters are consecutive function arguments with a single options object as last argument, if multiple optional parameters are possible (essentially so you don't have to do .someCall("a", undefined, undefined, 3, undefined, undefined, 5), but can instead do .someCall("a", {d: 3, g: 5}))

  • however, that isn't consistent across the library at present
oak quail
copper laurel

why dont we use that then

is it new by any chance?

oak quail

blobshrug

nope

prob been there since the beginning lol

ruby terrace

can't fetch a single channel in a guild specifically

oak quail

client.channels.fetch prob uses it

oak quail

/guilds/:id/channels/:id would function the same as /channels/:id so theres no reason for it to exist

visual hornet

4 years ago

ruby terrace

there's one functional difference, it ensure the channel is in said guild, but I made a change to GuildChannelManager#fetch that just checks the guild id after fetching using the single endpoint a while back

(although it looks like I forgot to perfrm the same check when fetching from cache dead , there's a free PR for someone) oh no I'm dumb, the cache only has guild channels, I must've thought of this before

why dont we use that then
and to answer that, we do, as of

copper laurel

i must have misread it

thought it was like roles

outer raven

was gonna fix #6655 and found this lonely type that isn't referenced anywhere, what is this for?

ornate topaz
inland lotus

v12 tho

outer raven

And yes thats for v12

ornate topaz

i just showed you what it was

do what you want with that info

outer raven

No, you showed me the docs

ornate topaz

...that say where were MessageAdditions used

outer raven

My question was if that is relevant or if it can be removed

ornate topaz

and since i am showing 12.5.3 instead of 13, it just might be because there is no MessageAdditions mentioned in send on v13

inland lotus

then it's safe to remove from the typings

worn bobcat

Will there be something done with the new role icons?

unique axle

yes

loud jayBOT
outer raven

@worn bobcat
Also discord.js supports all api endpoints afaik, so yes it will support every new feature discord adds, it’s just a matter of time

worn bobcat
outer raven

Except if it’s threads mmLol

They were released before early access iirc

pallid stirrup

I'm a little bit curious, but on the .fetch method to retrieve a Message object. When it hits Discord's endpoint, if the message doesn't exist it errors and gives a 404. Doesn't this make the .deleted on the message object redundant?

Ahh gotcha, makes sense if its .cache, I was using the fetch method and the typing on it said that it promised me a message back. I assumed that the discord API would give me something and I could use .deleted

<TextChannel>.fetch() should promise a <Message | false > than, instead of just 404ing

That is a good point,

you could still throw an exception if the res.error_code = 403

Btw, the GuildChannelManager.fetch when fetching channels can promise a null if a channel wasn't found

it is inconsistent with the message fetcher

KEKW

didn't know, huge discord not enough time to read. gotta bash keyboard

real jetty
loud jayBOT

pr_open #6652 in discordjs/discord.js by iShibi opened <t:1631859222:R> (review required)
feat(MessageAttachment): add support for ephemeral property
📥 npm i iShibi/discord.js#ep-attach

oak quail

yes

ruby terrace

either the guild property needs to be set or the manager can be changed, preferably the former to demonstrate how to set permissions from the global manager

that works yeah

burnt cradle

might be a good idea to distinguish the ids from each other

ornate topaz

yes, mainly so that they don't look like they are the same id that you are supposed to pass twice

outer raven

Can we add vscode extension recommendations? I was thinking of suggesting prettier and eslint so that users can fix errors before committing so the CI doesn't fail

would look something like this

wild flax

Most of those people don’t edit in editors

They do it from the web

Hence it shitting itself. Because git would run the dependencies before committing, you wouldn’t need those things

outer raven

Specially not for non-auto fixable issues

And this won’t fix that either, but at least if people install these extensions they will see the errors

And I’ve actually seen many prs that are made on an editor and have errors

wild flax

only if you skip it on purpose

A normal commit won't go through otherwise if you do it on a local machine (read: not the web editor)

Your IDE has nothing to do with it

the pre-commit hook runs: eslint --fix --ext mjs,js,ts

which means it will fix whatever it can, but throw on whatever lint errors it cant

outer raven

alright yeah you're right, but this would still help them identify their errors and fix them more easily

Having the faulty line underlined with an explanation of the error is easier for people to understand what's wrong

zenith oracle
outer raven
zenith oracle

Btw, what about adding prettier to pre-commit hooks?

wild flax

It already is

We run prettier with eslint

Because of potentially conflicts

zenith oracle

Ohh, didn't know this 👍

outer magnet

Question, since builders utilize markdowns, will Util.escapeMarkdown also be included in the builders as well?

zenith oracle

Builders is a different package from discord.js so if it'll be included it will in a new builders version

outer magnet

oops i mean will it be removed from the djs package itself?

vernal atlas

pretty sure builders already contains the message formatter functions

oh it doesn't contain escapeMarkdown, mb

outer raven

Can we update the message component collector docs with an info block advising people not to use filters? It's bad practice to leave interactions without a reply so they should always let all interactions through and then run their own checks to see if the user is allowed to perform a certain action
Not sure if this should be on the docs or guide though, which is why I'm asking

zenith oracle

The guide already uses deferReply in the filter

outer raven
zenith oracle

Well, yeah, it can be surely improved

outer raven

but do you not think it's worth putting a notice in the docs?

outer magnet

maybe not in the docs but in the guide i guess

zenith oracle

Yeah, I don't see why it should be in the docs

wild flax
outer raven
opaque vessel
clever crypt

@solemn oyster @wild flax there is a way to add more dynamic typing to SlashCommandBuilder#toJSON, are the people opposed to this? #813896878058897458 message

solemn oyster

I don't have access to wherever that channel is from

clever crypt
outer raven
opaque vessel

Looks like Crawl already said no

outer raven

he said no to the docs, not the guide

tacit crypt

Also can't see that message

zenith oracle

^^
The linked message is the last

real jetty

Would it be possible to do a interaction.inGuild() boolean just like we have interaction.isButton()?

unique axle

there.. uh... is?

rustic boughBOT
real jetty

.. oh

Is there a reason why that ts still thinks that interaction.member could be an APIInteractionGuildMember after using interaction.inGuild()?

opaque vessel

Maybe you're not in the guild

oak quail
copper garden

I think he wants to check if the client user is in the guild as a type guard to remove raw api object

real jetty

Basically a shorthand for writing this:

if (!interaction.member || !(interaction.member instanceof GuildMember)) return interaction.reply("I need to be in the guild where this command is run!");
outer raven

That would also be quite useful yeah

real jetty

also I don't know if this is intentional but APIInteractionGuildMember isn't exported from djs, which is why i had to do the messy instanceof

outer raven

It is indeed intentional

oak quail

its in discord-api-types

outer raven

And afaik that’s only an interface so you can’t use it in code

It’s not a class

real jetty
oak quail

Snowflake is in both iirc

opaque vessel

Could probably add a parameter in inGuild() to check if you're in the guild additionally

oak quail

not sure if its the same in both tho

outer raven

Yes that one is the only exported one from discord-api-types

outer raven
oak quail

unfortunately someone made it looser in discord-api-types iirc

outer raven

Crawl iirc

Idk why

real jetty
oak quail

used to be `${bigint}`, now its just string

outer raven
dawn merlin

The infinite recursion bug if you used TS 2.x I believe

with template literal types

outer raven

Why would someone use ts 2.x

Not even microsoft themselves support it anymore

dawn merlin

i dunno, but that was the bug

opaque vessel

I believe that bug's for TypeScript <= 4.2

outer raven

Hmmm

Dunno never faced it

But now that 4.4 is out of beta maybe it could be brought back?

opaque vessel
dawn merlin

oh yeah it is 4.2 not 2.x

but I thought it was removed because of the dev experience, the bug isn't mentioned in the PR?

real jetty

@wild flax can probably answer instead of us just guessing

copper laurel
outer raven

"too hard to use for some developers" what exactly does that mean?

copper laurel

People who shouldnt be using TypeScript at all probably

outer raven

how hard can it be to type Discord.Snowflake

well the library usually doesn't care for people who don't hold the required knowledge to do what they're doing so why did it this time

real jetty
dawn merlin

honestly that would make it more justified imo, but it's w/e, it doesn't make too much difference anyways in the long run

real jetty
wild flax

The send change was reasonable

The type was just annoying to work around

I didn't necessarily make anything stricter

real jetty

fair

real jetty
copper laurel

There was a bug too

dawn merlin
real jetty
dawn merlin

perhaps make the class generic CommandInteraction<InGuild = false> then implement it like this

public isInGuild(): this is CommandInteraction<true>;
get member(): this extends CommandInteraction<true> ? GuildMember : APIGuildMember;
outer raven

Yeah that’d be nicer imo

copper laurel

How is that accurate? If its not in a guild it wont have a guild member

Or is it supposed to be specifically checking a bot user being in the guild?

You'd still have to account for DMs though, it could be null

tacit crypt
copper laurel

yeah, I was just confused if thats what its trying to do

Is it checking if the interaction is in a guild, or if the application has a bot user in the guild

dawn merlin

Ideally it would be checking the latter

outer raven
dawn merlin

@tacit crypt I wanted to mention this but, when committing on the discord-api-types repo the hooks don't run properly, I have to adjust the executable bits on the husky files or I get this:

hint: The '.husky/pre-commit' hook was ignored because it's not set as executable.
hint: You can disable this warning with `git config advice.ignoredHook false`.
hint: The '.husky/commit-msg' hook was ignored because it's not set as executable.
hint: You can disable this warning with `git config advice.ignoredHook false`.
tacit crypt

...I thought I set those already

burnt cradle

@tacit crypt do you think the -types update builders pr could get merged soon so I could finish up the context menu pr?

tacit crypt

thats not in my powers to decide sadly

burnt cradle

arent you the builders maintainer?

tacit crypt

Unlike -types, builders seems to respect the same merging requirements as the main module. I'll need to ask @wild flax about it tho

burnt cradle

What are those requirements?

tacit crypt

Well, in the main module its usually 3 reviewers approving + crawl merging

burnt cradle

I see

real jetty

Reading user's about me section is possible, in the future or not?

slate nacelle
outer raven

@dawn merlin what about this part?

dawn merlin

Oh for some reason I thought that was in your suggested changes, yeah I can change that

outer raven

it wasn't because I didn't wanna spam comments for something that could turn out not to be wanted

opaque vessel

the only issue is I wouldn’t know what to call it
.applicationInGuild() maybe, off the top of my head. Also thought of .clientInGuild()

copper laurel

Client is more accurate imo

The application had to have been added to the guild for slash commands, even if it doesnt have a user present

clientUserInGuild()

Though even then, that implies the existence of specific intents

Technically correct would just be something like hasCachedData()

Or the opposite, isRaw()

But yeah, combined truthy conditions with param, fine I guess, but you've removed the ability to have distinct falsy conditions

dawn merlin

here's some I came up with:

  • isGuildUser
  • isGuildClient
  • inCachedGuild
  • inRawGuild
copper laurel

having both inCachedGuild and inRawGuild could work

opaque vessel

I like raw better, because caching is hell (':

We could separate out the logic to that method then

dawn merlin

it would have to be two methods, since the inverse of inCachedGuild doesn't 100% imply that inRawGuild is true

copper laurel

Well, it depends on what the two methods are

You can have inCachedGuild and inRawGuild, and if both are false its DM

Or a more generic inGuild then one of the two above to determine type of guild-related parameters

dawn merlin

so inGuild({ cached: boolean, raw: boolean }) as another option?

copper laurel

Ehh I still really hate the idea of putting this in one method

inGuild({ cached: true, raw: false }) // Guild, GuildMember, GuildChannel
inGuild({ cached: false, raw: true }) // APIGuild, APIInteractionGuildMember etc....
inGuild({ cached: true, raw: true }) // ???
inGuild({ cached: false, raw: false }) // ???
``` What would the last two permutations be checking here? What are the defaults?

I guess you can throw an error on true/true

And false/false could either error, or simply rule out DM and not care what type of guild?

I still think two explicit methods is a FAR more intuitive, readable interface

dawn merlin

yeah I agree

copper laurel 
if(i.inCachedGuild()) {
  // great, cached
} else if (i.inRawGuild()) {
  // k do some raw handling or whatever
} else {
  // dm
}``` This really just feels like the easiest interface. I'd be somewhat okay with an `inGuild()` helper that did `this.inCachedGuild() || this.inRawGuild()` too

Though its also easy for devs to do that on their own so idc

dawn merlin

I feel like this has been brought up a lot

No like it’s fine to ask I just don’t remember the reasoning against it

opaque vessel

There's Message#resolveComponent that takes a custom id to resolve against

Does that not help?

dawn merlin

oh im thinking of the components thing in the message payload, ignore me

opaque vessel

All good, thought that would indeed clear it up haha

as buttons and select menus now enforce customId
By the way, this has always been enforced - it's just it's recently enforced that custom ids must be unique in a message (previously, there were no checks for uniqueness)

copper laurel

Except a row is actually ordered

Maps/Collections are for storing unordered data

The index of a button is (or at least should be) its position too

copper laurel

And yes - link buttons dont have a customId makes that difficult

dawn merlin

Just wondering, Is there a reason builders isn’t a part of modules?

astral kraken

why does printing out a MessageEmbed reveal that all of its data is stored in field objects rather than in the properties laid out for it?

unique axle

not sure what you mean by this, fields are shown as fields object array - what else would you expect?

astral kraken

those aren't fields though, those are the title and description

the actual embed looked like this

unique axle

your title is "Embed Command Used" and you don't set a description though? I'm confused.

your screenshots can't really relate to the same embed object, one depicts a title/description (or title/value (on a field)) pairing, while the other depicts an embed where both gibberishs are in individual field values

oak quail

editing guild commands doesn't seem to update the cache Thonk

creating and updating does tho

this is really weird:
initial: cmd1, cmd2
renames cmd2 to cmd3
cache: cmd1, cmd2
deletes cmd1
cache: cmd1, cmd3

oh thats prob just because im fetching before renaming/deleting

so renaming/deleting just doesn't update the cache

creating does though

oh.. looks like the code is explicitly set to not update cache for guild commands?? tf

according to the code create also shouldnt cache guild cmds, but looks like it does blobshrug

looks like @ruby terrace changed it from (patched) to (patched, undefined, guildId), and @floral hinge changed it to (patched, !guildId, guildId)

floral hinge
oak quail

hm

dont we typically modify the cache anyway

floral hinge

the code should be updated to modify the cache again

oak quail

yeah but i mean, typically the cache gets modified even for managers with events

so this behavior was inconsistent anyway, just didn't really matter until they removed the events

floral hinge

cache would get modified twice if both managers and events modified it

ruby terrace

You can have guild commands in the global manager! The only problem that could happen is when you have both managers, you could have duplicates where one is an outdated version of cache, because we don't get events to keep cache up to date

copper laurel

Didnt command events get removed from the bot websocket though

So now the manager has to modify it

Should it also modify the guild cache then if its coming from the global one?

...somehow?

ruby terrace

They were never emitted (for bots)

and the guild + global thing is kinda hard

we could update both, but usually people won't have them in both

floral hinge

bots did receive them

copper laurel

So how does a guild command get into the global cache?

oak quail

prob by passing a guildId to client.application.commands.create

feels weird tho

ruby terrace

you can fetch it there too!

think guilds without a bot

copper laurel

eh, I dont see the harm in just caching if the guild exists

dim steppe

I think with interactions, that DJS should take into account the <interaction>.member.permissions that get sent with the interaction, especially now that DJS has such powerful caching capabilities. Currently, it gets completely ignored. Discord sends the guild member's exact permissions in the channel that the interaction occurred in, taking all of their role permissions into account.

To compare, if I'm not caching roles or channels, and need the guild member's permissions in the channel that an interaction occurred, I have to:
• Fetch the channel (for the overwrites)
• Fetch @everyone (for the global permissions)
• Fetch all of the roles for the member (for their global permissions)
• Adjust the global permissions to account for the channel overwrites

Meanwhile... Discord already sent me the info:
https://discord.com/developers/docs/interactions/application-commands#slash-commands-example-interaction

copper laurel

The caching is exactly why we had to ignore this - because we cant put the value on the GuildMember object

I'd probably suggest InteractionGuildMember extends GuildMember or something with its own property

dim steppe
copper laurel

Yeah fair enough, could also go on the interaction classes

unique axle

it needs to be on something that isn't cached, for sure - otherwise that information is held but can be immediately invalid again

tacit crypt
wild radish

i have a question why did a deprecated event (interaction) come out in v13 and it wasnt in v12

unique axle

not too sure what you mean?
"interaction" - as part of the interaction feature came out with v13
after the fact (in 13.1) * mixed that up, correction below
we kept "interaction" because people already used it plenty when switching to v13 when it was still in dev to not introduce a breaking change, where not strictly necssary
a deprecation warning was added to both "interaction" and "message" in favour of the more api-true naming convention xCreate

the "old" event names still operate as one would expect, but will be removed in the next major version (14.0.0)

deprecation notices are used to ease over transitions into breaking changes

copper laurel

People were already using interaction in the dev build so we thought we'd be nice and keep it

opaque vessel

"interaction" - as part of the interaction feature came out with v13
after the fact (in 13.1) it was decided to add a deprecation warning to both "interaction" and "message" in favour of the more api-true naming convention xCreate
Not exactly, it released both in version 13

^^ that'd be why

outer raven

This is just a slight inconvenience in a bunch of classes on the library but I'll be using the example of the Role class here
When printing out a role object, the very first thing you see is the guild data because this is the first thing that is set on the file (since it needs to be called through the constructor) but this makes it so that if you want to, for example, print this data on an embed, you end up not seeing any real data about the role itself since you have size limitations. My question is, could we move "extra" data to the bottom of the file (or at least below the important stuff) and keep important data about the actual class we're accessing at the very top?

unique axle

no

outer raven

😐 why not

unique axle

printing out objects is not supposed to - in any way - be a replacement for documentation or typings, further objects are key-value associations and the order of keys should not play any role

this "problem" also applies to getters, which aren't even listed in printing instances to begin with

visual hornet

also wouldn't "important data" differ depending on use case? so getting data should just be, idk, an array of properties to go through to actually display?

unique axle

this is absolutely not worth the fuzz

outer raven
outer raven

Just saying it would make it much easier for debug purposes even where you have to print a lot of stuff to console and you need to scroll way too much to find what you want which is usually at the bottom of the object

warped crater

if you have size limitations when displaying data, you're probably doing something wrong, such as using an eval command

tacit crypt

We're not gonna rearrange fields just because of extremely unlikely niches like these

outer raven

it's not extremely unlikely, I'm sure people console log objects on a daily basis

tacit crypt

people also log only what they care about

outer raven

well yeah and if that is the entire object then they need to log the entire object to know what they're looking for and if it's correct

tacit crypt

And? Moving the guild up or down will do squat all when you are STILL printing it

Only instead of having to scroll down, you have to scroll up

outer raven

but at least that would be intuitive in the sense that the info owned by the object you're looking at is first, and additional data is last

tacit crypt
outer raven

But I don't understand why like, it wouldn't take any effort at all and it's really not a niche case lol

visual hornet
outer raven

on the role class yeah, but in some the guild is mixed up with other info

tacit crypt

I see literally no point as to why we should randomly move properties around just for something that affects less than .1% of people

outer raven

It doesn't but alright, I guess my example wasn't good enough

unique axle

if that's what you took away from this - not too sure what there is to add

quiet viper
outer raven
outer raven
unique axle

the core issue with this proposal is that it's a lot of unnecessary work for little to (for the absolute surplus of users and developers) absolutely no benefit
there is no inherent order to object fields. people using proper debuggers get folding, people printing objects to console to debug should expect long output and, if applicable, restrict it via inspect - as also already covered by vlad. shifting fields around to make them "more accessible when printed out" is not a concern.

outer raven
unique axle the core issue with this proposal is that it's a lot of unnecessary work for lit...

I understand it's not a concern, but it wouldn't be something impossible to do or that would not be accepted if someone got around to do it would it? I'm not saying I will as I probably won't, but there are so many things in the library that only a few developers use and yet they're still there that I don't see why this improvement would be a bad thing if most people don't care about it

unique axle

it is not an improvement worth considering, i'm fairly certain that this PR would be closed

outer raven

Fair enough ig

unique axle

it's not this one big change someone needs to do once you make it out to be either. if new structures are introduced this needs to either be batched and re-applied or included in the review process, people need to follow this guidelines in further PRs and people need to remember to pay attention to it being properly applied when assessing proposals.

wild flax

If you debug and you need to inspect data, log it to a file

End of discussion

Rearranging source code for a console log

I haven’t heard a more deranged thing by now

outer raven

Jeez aight

dawn merlin

Just use the vscode debugger, it allows you to collapse properties in the UI

novel swift

are there any plans for having a port to deno?

unique axle

not at this time, we might consider it with the planned typescript rewrite (no timeline for that yet, unfortunately)

novel swift

there would still be a split for the codebase tho, because different libs. havent read much about node.js and deno compatible code tho

but thanks :P

i wil be on the lookout for when the rewrite happens, really hyped to make deno more used

astral kraken

im not sure if this should go here, but is there a tutorial or something out there that explains how to go about creating a library for discord.js?

visual hornet

i don't think making custom extensions of djs would fall under developing djs itself?

copper laurel

Yeah I don't quite know what you mean, what would you need a library for a library

astral kraken
copper laurel

They are mostly terrible and definitely not supported or endorsed by us

astral kraken

oki doki

snow dawn

I don't want to spam my github pr with discussion so I'll ask here

why do people seem against that?

I get that you might not support webpack/browser environment

I get that you want a "cleaner" code

but performing filesystem access there feels somewhat stupid

outer raven

why? It's more effective and makes it so we don't have to remember to update that file every time a new action is made, which happened a lot

how else would you suggest doing that while also not having to update the file every damn time

snow dawn

I think you could move that code to tests to ensure that every file from that directory is referenced when you're doing development on the library itself?

I don't really see an issue with updating it manually

outer raven

that still implies that we have to update the file every damn time. It's annoying and not very practical

Also I'm not sure to what extend tests give any warnings at all

and tbh I think the same logic should be used for index.js, I just think it hasn't been done because some classes are intentionally not exported but it would be nice to add that since that's also quite a big source of issues usually

snow dawn

would you accept it then?

outer raven

the test file that actually emits errors is the ts test file AFAIK

I don't think you can really do that sort of thing in javascript but I could be very wrong here

snow dawn

if you really want errors on that you could write a small eslint plugin or something

copper laurel

Why are you reverting it? What are you achieving?

snow dawn
copper laurel

Bundlers like webpack?

snow dawn

basically it breaks bundlers and other tooling that relies on static requires

copper laurel

We don't support webpack or the web environment

snow dawn

I do not want to run d.js in web environment

I use webpack for a node application

and the only thing that it's blocking from working properly is usage of that dynamic filesystem import

copper laurel

That should really be explained better in the PR then

Detail your use case more

snow dawn

webpack itself actually has a functionality to perform dynamic imports as well but implementing it would fix it just for webpack, not for other bundlers

outer raven

But what I don't understand is why you use webpack for a node application

snow dawn
outer raven

my question is exactly what those "certain reasons" are

snow dawn

I'm just pointing out the issue caused by usage of filesystem scanning for require()

copper laurel

Well, I think that's a fair issue, but I still don't like the solution to just revert it to less maintainable code

snow dawn

Running npm test on your local environment isn't really an issue

The CI pipeline seems to run npm test as well so failing test (in this cause caused by missing reference to file in ActionsManager) wouldn't let you accept a PR

copper laurel

Do we even have JS tests though is what Rodry was pointing out

There's typings tests

snow dawn
copper laurel

Hmm yeah fair enough. Might be good to have a similar test that checks for classes exported from index if that's possible, I always fuck that up haha

snow dawn
copper laurel

Why close?

You don't have to do my suggestion too lol

outer raven

I thought that was the reason why we didn't apply this solution to index

copper laurel

Can be expected warnings or exceptions or something

outer raven

do you know what those are?

copper laurel

@snow dawn just keep in mind even if this does change, we still don't really support webpack. Other things might break it in future too

@outer raven it wasn't really a serious suggestion, I don't care. I just frequently forget to export things

snow dawn
outer raven
wild flax

Other things do infact break with webpack too

so good luck fixing that

our web builds were constantly doing some bs

even if you didnt use it in a browser

tacit crypt

I don't get why you'd use webpack over node modules in a server-side app

You could implement a plethora of issues solely because of that

outer raven

@remote wasp

vivid field

yeah it's not breaking

although only accepting snowflakes for fetch methods seems to be the default across the library (see e.g. ChannelManager or MessageManager) so that single change seems out of place

outer raven

although technically the same argument can be used here so idk, but a lot of managers do support resolvables

remote wasp
outer raven
remote wasp

updated the PR description

visual hornet
outer raven

I would say minor because it adds functionality

outer raven
outer raven
opaque vessel

Believe for a verified bot? Unsure

vernal adder

I believe it's for verified email.

opaque vessel

whether the email on this account has been verified
From Discord API docs

vernal adder

👍

outer raven

yeah that's what I found, but a bot's email can't be verified because it doesn't exist, right?

outer raven
opaque vessel

I think it's for the application owner's email or some shenanigans

I dunno

outer raven

what if the owner is a team?

the owner of the team ?

opaque vessel

I'm not really sure

outer raven

either way the description should probably be updated for that

opaque vessel

I see your point yeah, probably should

outer raven

Also for Guild#large, where can large_threshold be found?