archive-library-discussion

[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]

We have no interest in merging editReply with either reply or update, they do different things

[copper laurel]

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

[dawn merlin]

mistake on my end. For some reason I thought that guildId was the guild ID the bot was in, but that doesn't make sense, and member shouldn't be there

[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]

should there be a Rolemanager#setPositions, or rearrange?
https://discord.com/developers/docs/resources/guild#modify-guild-role-positions
basically a method that takes in an array of roles, and rearranges the roles accordingly
right now if i wanted to rearrange my roles, discord allows it but it's not a thing on discord.js afaik

[burnt cradle]

https://discord.js.org/#/docs/main/stable/class/Guild?scrollTo=setRolePositions this seems to be the method

[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]

The thing is mostly the api changes, and those are semver minor iirc

[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]

So something like this
{Object} CategoryCreateChannelOptions

• (all options from GuildChannelCreateOptions except for parent)
  {CategoryCreateChannelOptions} GuildChannelCreateOptions
• only unique option is parent

does that sound good?

[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]

There's no route for this tho.. that'd be a d.js alias, and I'm not sure how many of those we have nowadays

[outer raven]

we do have quite a lot of those and this would make it easier to set categories and also replicate the behavior on the client (the + next to the category name) so I don't see a problem with this

[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

[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]

The presence of helper methods in other places in the library is not a strong argument to add more. I don't see the point of basically duplicating the channels#create method to set one value for the user

[real jetty]

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

[raven juniper]

Yes

See GuildMember#permissionsIn and hasPermission

[outer raven]

if you pinged me I would've seen this earlier but the PR is made now so I'll leave that up for the reviewers.
Either way no code is being duplicated, this method just calls the method from the manager (just like other trivial things like Message#edit) and the function actually exists on the client, so I don't see why not

[loud jay]

#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]

because there aren't endpoints to do that easily I'm guessing

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 jay]

#100 in discordjs/website by ImRodry opened <t:1630101593:R>
Expand all extended typedefs by default

[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]

Been a few years, still a no-no

[copper laurel]

Seems like a very redundant shortcut method tbh

[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]

there wasn't a better way to name it, since children is only a collection, not a manager
But if a category channel has children, why wouldn't you be able to add children to it directly in the class?

[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]

is it intended that https://discord.js.org/#/docs/main/stable/class/ApplicationCommand doesn't have the options typed?

[real jetty]

if you are looking for the typings its ApplicationCommandData

[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]

I tried for a bit but I have no idea how these types work so I'll leave that for someone else to do in a different PR

[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]

not quite

[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]

will do but testing this on TS playground makes it seem like passing DM is invalid but 1 is valid, which it shouldn't

[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]

https://github.com/discord/discord-api-docs/issues/new?assignees=&labels=bug&template=api-bug-report.md

[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]

Integrations are documented as Snowflakes in some places and strings in others, and they can be both

[outer raven]

So which one should it be?

[outer raven]

Decided to go with Snowflake|string just like reactions

[loud jay]

#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]

is this documented somewhere? if not would be good to

[rustic bough]

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

<:_:874569310025179188> MessageEditOptions
Options that can be passed into Message#edit().

[real jetty]

https://discord.js.org/#/docs/main/stable/typedef/MessageComponentCollectorOptions shouldn't this have some description or are those optional?

[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]

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]

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]

why use enums then? In djs you can use string types pretty much everywhere

[outer raven]

also yeah it doesn't, I tried

[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]

yeah I know I was just experimenting with a type due to an issue someone opened, but it doesn't really matter in this case

[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]

format is for the file format though, not the size
so that screenshot seems unrelated?

from testing, giving 12 and "16" both give just 12 or 16 in the error so yeah i think that might be a bit confusing, i think maybe have a seperate error for being an invalid number vs any other invalid value? like getting a string

[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]

Yeah that’s what I’m saying. I don’t see any reason not to do Number(size) in the includes check though

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]

If discord would change its default (really rare thought) we need to change it as well. Also it's related to that function, not discord

[peak zinc]

that makes sense, thanks so much for explaining to me even tho it's kinda off-topic for this channel

[peak zinc]

does d.js do this? how can you be sure that the api responses will be correctly shaped

or does it just assume everything is correct

tldr does djs do any runtime type checking?

[outer raven]

Can I get feedback from a maintainer on this please? (disregard the wrong screenshot, I meant to write size but the error still applies)

[wild flax]

What about it

Oh allowing strings

No

[outer raven]

why not

[outer raven]

also this

[remote wasp]

Discord API is well documented. No, discord.js doesn't do that because it's not required but we do check for values that may or not be available before accessing them

[peak zinc]

thank you

[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]

https://github.com/discordjs/discord.js/commit/be5efea461660c8fa38a57f21019af4b37746d17

[gusty carbon]

Why does deleting an emoji (GuildEmoji#delete) support passing in a reason as an argument? It doesn't seem like that's a param for the api endpoint (https://discord.com/developers/docs/resources/emoji#delete-guild-emoji) and it also doesn't show up in audit logs or something as far as I can tell

[remote wasp]

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

[gusty carbon]

oh, well yeah that makes sense

[outer raven]

https://github.com/discordjs/discord.js/issues/6627 could someone update the labels on this issue

[solemn oyster]

Done

[outer raven]

Thanks

[winged dust]

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

[wild flax]

Yes

[visual hornet]

where are the accepted default color values for ColorResolvable taken from?
the ones at https://discord.js.org/#/docs/main/stable/typedef/ColorResolvable or at Constants.js:514

[quiet viper]

from this array as you mentioned: https://github.com/discordjs/discord.js/blob/stable/src/util/Constants.js#L514

[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

For reference https://discord.com/branding

[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]

Yeah I can see a clear use for this, would be worth looking into. If you have the knowledge, you could try opening a PR with these interfaces

[tender field]

👍 i'll do that tomorrow or thursday

[outer raven]

the channel being null is not related to the interaction being in a guild or not afaik

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]

yes because target is the user who executed the action

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

[tender field]

not really because targetType is completely made by djs afaik.

[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 CC @wild flax
https://github.com/discordjs/discord.js/pull/6637/files#r708460248 I don't think this would be considered sem major at all

[outer raven]

Because that would mean changing GuildEmoji#url to a function instead of a getter

Since it would need to take format and size parameters

[tacit crypt]

https://cdn.discordapp.com/emojis/264701195573133315.jpg?v=1&size=32

oh cool

that actually works

[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]

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]

Yeah we can always use that, but the idea was also to simplify first and last with that function

[outer raven]

yeah I know but there are situations where it is handy and using .at makes more sense to me than .first

[outer raven]

odd, that should work
lemme try

[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]

I tried multiple things and I cannot get this to work for some reason, only thing I was able to notice is that you need to add back the readonly tag to channel

[outer raven]

bump

[outer raven]

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

[real jetty]

Yes :/
The only way is to use a new type without Exclude like what I did

[outer raven]

Aight, just fix the order then ig

[real jetty]

👍

[tender field]

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]

it uses an enum defined only in -types, so i can use that but it will mean a good chunk of refactoring to create mappings from enum to string (eg AuditLogEvent.RoleUpdate -> ROLE_UDPATE)

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]

My friend, they are not strings.
channel: Exclude<TextBasedChannels, PartialDMChannel | DMChannel> will work

[surreal hollow]

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

[real jetty]

😮 , I'm just dumb thanks 😂

[zenith oracle]

To be consistent with the API

[outer raven]

more like to be inconsistent with the api lmao

[raven juniper]

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

[outer raven]

Ok chill it was 1 message

[tacit crypt]

Also fyi it's consistent. You get interaction.user in dms and interaction.member.user in guilds, we just top level both for users convenience

[outer raven]

yeah ik discord.js is consistent with the api, what I was saying is that the api isn't consistent with itself for providing user for interactions and author for messages

[ruby terrace]

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

[outer raven]

what else?

[real jetty]

webook, system message

[outer raven]

message.author is a user in both of those though 🤔

[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 jay]

#6648 in discordjs/discord.js by acd124 opened <t:1631765512:R>
Bug: Role.toJSON() returns permissions as a BigInt

[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?
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]

this exists under the invite creation methods
https://discord.js.org/#/docs/main/stable/class/VoiceChannel > targetApplication + targetType

you call #createInvite on a voice channel instance and provide the target application and target type as detailed on https://discord.js.org/#/docs/main/stable/typedef/CreateInviteOptions

[outer raven]

not quite sure this is what they meant, have you seen the feature that lets you watch yt and other apps on a voice channel? This doesn't require an invite

[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

[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]

i did ask about this earlier, and it extends beyond these a bit
i got a single reply and they never clarified...

[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

[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]

theres isn't a REST endpoint for fetching individual channels
there is, https://discord.com/developers/docs/resources/channel#get-channel, it seems to return the same channel data as the objects in Get Guild Channels

[copper laurel]

why dont we use that then

is it new by any chance?

[oak quail]

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]

theres no difference though

/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 , 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?

[inland lotus]

v12 tho

[outer raven]

It’s in the typings not docs

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 jay]

#6633 in discordjs/discord.js by iShibi created <t:1631504644:R> (review required)
feat: add support for role icons
📥 npm i iShibi/discord.js#feat-role-icon

[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]

Ik, just I was curios :)

[outer raven]

Except if it’s threads

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

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

[real jetty]

#697138785317814292 message
Does the library already support this modification (PR in progress or not) or not?

[loud jay]

#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]

But you can still commit with eslint errors, nothing will prevent you from doing that

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]

You can not

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]

I don't see anything wrong adding it actually but isn't the .vscode folder ignored in the gitignore

[outer raven]

we can easily overcome that, I have the fix already but I'm hesitant to pr it now cuz of what crawl said

[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]

that's not really a response though, it needs an ephemeral reply

[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]

No

[outer raven]

Would you like to do that? I've never really looked deeply in the docs so I might miss something if I do this

[opaque vessel]

Hint: if you use a ternary in your filter, you can reply to those that do not pass the filter

const filter = something => {
  if (something) {
    return true;
  } else {
    interaction.reply("hi");
    return false;
  }

[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]

https://discord.gg/C6ww3UJH

[outer raven]

yeah I know and that's what I'm saying could be mentioned on the guide, since most users just use a regular filter and don't reply to their interactions

[opaque vessel]

Looks like Crawl already said no

[outer raven]

he said no to the docs, not the guide

[tacit crypt]

Why would you add more dynamic typings exactly?

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 bough]

<:_:874569335308431382> Interaction#inGuild()
Indicates whether this interaction is received from a guild.

[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]

if its not in a guild then interaction.member will be null

[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]

yep, that's what I meant. bad phrasing on my end

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]

So is Snowflake, no?

[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]

It is

[oak quail]

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

[outer raven]

Crawl iirc

Idk why

[real jetty]

yeah because of a ts bug iirc

[oak quail]

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

[outer raven]

What bug?

[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]

It's this, right?
https://github.com/discordjs/discord.js/issues/6322

[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]

https://github.com/discordjs/discord-api-types/pull/171

[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]

Huh, I thought there was an actual bug with it

[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]

wondering the same thing, not like djs to revert a change because developers didn't want to adapt to it
re: .send change

[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]

anyways, yeah I think that'd be an eloquent solution

[copper laurel]

There was a bug too

[dawn merlin]

so a more eloquent way to type narrow GuildMember | APIInteractionGuildMember?

[real jetty]

yep

[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]

TECHNICALLY, if we have the guild in cache, we have a member instance too

[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]

well both

[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]

No, see here: https://github.com/discord/discord-api-docs/issues/3095#issuecomment-868817186

[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]

It has? O_o

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

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

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

[floral hinge]

I had changed the command methods to not modify the cached commands in place, and instead when you received command events these would modify the cache. Only the methods for global commands would modify the cache since these didn't receive command events. So the issue here is a regression from when support for command events was removed

[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]

Oh, so it had been at least mentioned before. :-). Your idea of InteractionGuildMember would be great. My easy-way-out, I just added a ._memberPermissions to Interaction until it gets discussed and/or implemented.

[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]

they were

[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]

I know it's not meant to be a replacement for documentation, that's really not what I'm talking about.
This also applies to printing an object to the console, where you have to scroll a lot to get to the info you actually need and get all sorts of non-important stuff first (like the guild info in a lot of classes)

[outer raven]

By important data I mean data unique to the class you're accessing, which a circular guild property is not

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]

you realize thats so not how it works right? It's also really not worth the headache of rearranging things just because you inspect the object with a big depth. If you don't want the logged object go deep, util.inspect(obj, { depth: 0 })

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]

well the guild gets pushed up by the additional data so you actually see that first without scrolling when you log it

[outer raven]

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

[tacit crypt]

It's a mess for all of us over something that literally doesn't matter and affects nothing

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]

I unterstand your problem. But you can use .toJSON for viewing them in a compact way.
Btw most Debuggers dont show the full Object, when logging nested objects. You have to click on the props to expand it.

[outer raven]

that's what I take away from vlad saying it's a niche case yeah, but if you want a better example, take any GuildChannel, where the guild data is in the middle of other fields. If anything it should be either at the very top or very bottom imo

[outer raven]

Yeah I'll get my own workaround to collapse inner objects since I don't really use debuggers, just run the code like I do normally

[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]

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]

there are a ton of djs libraries in npm and i kinda wanted to make one myself

[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

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

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]

okay, what if I updated my PR with test code that ensures that every class is present in ActionsManager

would you accept it then?

[outer raven]

no because of this

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]

see the issue in my PR

[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]

for certain reasons I want a single .js bundle for entire application and I don't think it's related to d.js at all

[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]

that's why I proposed adding code that checks that to test suite

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]

oh well https://github.com/discordjs/discord.js/tree/main/test

[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]

well then I will close #6682 and open a second PR that introduces export checker and reverts #6102

[copper laurel]

Why close?

You don't have to do my suggestion too lol

[outer raven]

aren't some classes not exported on purpose though?

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]

okay

[outer raven]

well yeah but I think it's a good idea exactly because of that xD

[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]

https://github.com/discordjs/discord.js/pull/6677 space brings up a good point: if user resolvable can be an id, there isn't anything breaking about this PR is there?

@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]

probably because you most likely don't have a channel/message object when fetching these because you could call .fetch() on them instead

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

[remote wasp]

does changing name of a parameter count as breaking? I'm not sure 👀

[outer raven]

nah obviously not, it's only internal

[remote wasp]

updated the PR description

[visual hornet]

just a quick question, sorry if it doesn't fit, but would that be considered a patch or a minor update?

[outer raven]

I would say minor because it adds functionality

[outer raven]

Could this get a priority: high label? I think it's quite a big bug that affects a lot of people https://github.com/discordjs/discord.js/pull/6686

[outer raven]

The description for https://discord.js.org/#/docs/main/stable/class/ClientUser?scrollTo=verified seems misleading but I can't find this on the docs, what exactly does this verified property refer to?

[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]

this is why I think it's misleading, because it leads you to believe that

[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?

[opaque vessel]

In the websocket options of client options
https://discord.js.org/#/docs/main/main/typedef/WebsocketOptions

[outer raven]

ah alright, should probably link to that then right?

[opaque vessel]

I suppose there's no harm :shrugs:

[outer raven]

we can't choose UNKNOWN for channelTypes can we 🤔

[warped crater]

no, the library uses it when a new channel type is introduced and it doesn't have support for it, iirc.

I believe that's a rather new addition (uh, relatively speaking. probs about ~12.x) when people realized that "oh shit, stage (or was it the game thingy channels?) channels cause big bad crash"

[outer raven]

UNKNOWN was here before stage channels but yeah, ill make a PR to remove that type

[opaque vessel]

Are DM channels in there? O_o

[unique axle]

unknown should be any new channel type that may be introduced but isn't yet supported, it should be the default, if the payload type does not match any known type. unsure why you'd want to remove that

[opaque vessel]

I'm getting this strange error where TypeScript won't compile when I'm using a channel type within a subcommand?

<Client>.application!.commands.create({
  name: "test",
  description: "test",
  options: [
    {
      type: Constants.ApplicationCommandOptionTypes.SUB_COMMAND,
      name: "test",
      description: "test",
      options: [
        {
          type: Constants.ApplicationCommandOptionTypes.CHANNEL, // Error here
          name: "channel",
          description: "A channel.",
          required: true
        }
      ]
    }
  ],
  defaultPermission: false
});

Type 'ApplicationCommandOptionTypes.CHANNEL' is not assignable to type 'CommandOptionChoiceResolvableType | CommandOptionNonChoiceResolvableType'.

node_modules/discord.js/typings/index.d.ts:3123:3
    3123   type: CommandOptionNonChoiceResolvableType;
           ~~~~
    The expected type comes from property 'type' which is declared here on type 'ApplicationCommandNonOptionsData | ApplicationCommandChoicesData'

Seems like a bug, this isn't malformed slash command data: https://github.com/discordjs/discord.js/issues/6702

[outer raven]

Passing that as an option is passing a type 0 (?) to the API which doesn’t exist and will error out

[outer raven]

Good point, will add

I few weeks ago we talked about an ExcludeEnum type but I forgot to do that, we really need that type cuz these excludes are getting big

[outer raven]

actually @opaque vessel I have a question
the DM and GROUP_DM types are accepted by the API and throw no errors, however they result in an unusable option since there are no channels to pick from, should they still be excluded?

UNKNOWN does actually send an error so that one should definitely be removed, not sure about those two tho

[opaque vessel]

Ehhhhhhhhhhhh I dunno, the user is most definitely making a mistaking by allowing those types since they're not usable in the first place

Strictly speaking, if the API accepts it, I guess it should also be included...

Might have to defer to others on that :eyes:

[outer raven]

should I ping anyone in specific or..?

[opaque vessel]

Shrug

Hopefully a knight in shining armour will appear and impart their wisdom upon us

[outer raven]

[unique axle]

we usually do

[ruby terrace]

If you used it in a DM, does the DM type work?

[outer raven]

nope, no channel shows up

[ruby terrace]

Weird. Well, removing those from the typings is an opinionated decision from the lib, which we don't do very much since discord could change that at any time

[outer raven]

yeah but I don't see why discord would allow you to set dm channel types, that seems more like an api bug to me

you can't see a dm channel on the client so you cant have it in the dropdown

and neither can you have group dm cuz bots cant join those

[outer raven]

I don’t know if the maintainers have any sorting settings on the pr list but the oldest pr (guild member avatars) is no longer a draft so it can finally be reviewed xd

[slate nacelle]

I have it open already.

[outer raven]

On a different topic, I'd like to discuss something
Since the properties accentColor and banner on a User need to be force fetched to appear, could we remove the else in this condition (and in banner too) so that these properties are not set so as to not lead the person into thinking the user doesn't have an accentColor/banner?

Basically the null from "we didn't get this property" isn't the same as the null from "this user doesn't have this property set" so I think they should be different in some way, and not setting the property would be the way to go imo, but since the library usually doesn't do this I'd like to get feedback first

This also applies for User#bannerURL as that should probably return undefined (or maybe even throw an error ?) when the property is missing, and null when no banner is set

[slate nacelle]

Probably something about "changing the shape of objects is bad" applies here, but I'm not too versed with this problematic.
Maybe we could go with undefined for "this property is unknown until fetched" cases. (No idea if this is a good idea though)

[outer raven]

I wouldn't say an explicit undefined though, just removing that else since that should only really apply when we dont get that data aka when getting it from a member

actually maybe on the user update event but still, it wouldn't change anything I don't think

I see what you mean by "changing the shape of objects is bad" but isn't having a property set to undefined worse? I'm not sure which one you prefer here but I'd personally go with not setting

[slate nacelle]

How would not changing the shape be worse if that's the goal?

[outer raven]

because it would mean explicitly setting a property to undefined I guess
just my opinion though, I'd go with whatever you and the other maintainers prefer

like changing it to this.accentColor ??= undefined doesn't seem like something discord.js would do

[slate nacelle]

Yeah, we generally go for null.

[outer raven]

yeah exactly

so which alternative should we go for

[real jetty]

is this behavior intentional? shouldn't it be TextChannel?

[opaque vessel]

Fine for me

[real jetty]

That's odd, here's my entire snippet:

const channel = interaction.options.getChannel("channel", true);

        if (channel.type !== "GUILD_TEXT") return interaction.reply({content: "You need to use a Text Channel!", ephemeral: true});
        
        

[copper laurel]

Not sure this belongs here, but I'm was pretty sure those strings didnt typeguard unless someone has changed that

[opaque vessel]

Can reproduce in the interactionCreate event. Bit weird

[vivid field]

That only works if you have a union (like TextChannel | DMChannel) because there every channel type is present once, if you have GuildChannel instead there's no guarantee that maybe multiple extended classes have that type

[copper laurel]

Not really a fan of that naming convention, it feels very backwards to read. Plus, although yes they're all channels what they really represent are Guild configuration settings, not the management of channels

[outer raven]

Well how about keeping the property with the id of those channels in the guild object and the getter with the channel object in the manager?

[copper laurel]

....why

That makes even less sense imo

And still ignores the first part of what I said, it's a backwards naming convention

There's no reason at all for those getters to exist on the manager because they are not managing any data

[copper laurel]

Happy for people to disagree but I'm really not seeing any reason for this one

[visual hornet]

i think it's kinda inconsistent with the everyone and premiumSubscriptorRole properties being on the RoleManager, but tbf they aren't settings
that kinda is the "backwards naming" that you mentioned though, and i think premiumSubscriptorRole having the Role at the end is kinda redundant? but that's kinda another topic

[copper laurel]

Hmm, yeah that's a fair comparison

[outer raven]

Yeah I agree the role at the end is a bit redundant so either we change that for v14 or, if we were to move those getters to the managers, we’d keep the names they already have.
The reason why I suggested moving only the getters is because the setting (only the id) belongs to the guild and can be set on the guild itself whereas the full channel is taken from cache which is managed by the manager, just like the premiumSubscriberRole and everyone. Of course those two are different as they cannot be changed but I still think the same logic should apply

[outer raven]

@solemn oyster you approved the PR, any comments on this?

[tender field]

hey can i request some feedback for my PR please? the reason it's draft is because i'm awaiting feedback from the core team for the last touches

[loud jay]

#6647 in discordjs/discord.js by noftaly created <t:1631726793:R> (review required)
types: improve audit logs' typings
📥 npm i noftaly/discord.js#feat/audit-logs-typings

[outer raven]

the best way to get feedback is to not have it as draft
draft prs usually get ignored as long as they're drafts

[tender field]

yeah i figured as much, i'll mark it as ready for review then

[white sapphire]

short question, for further changes to my PR do you prefer single commits or amend+forcepush?

[outer raven]

Single commits so that we can actually see what changed, unless it’s a rebase or fixing an issue that was accidentally committed

[white sapphire]

kk thank you

but duplicating the commit message?

/ the commit message subject

[burnt cradle]

doesn't have to, the commits will get squashed before merging

[white sapphire]

perfect, thanks

[outer raven]

You should give it a unique message

[wild flax]

It doesn’t really matter honestly

Since we do squash in this repo

But somewhat descriptive would be nice

[copper laurel]

docs

docs(Message): corrected description of property for example

fix is code fixes for bugs

Its not a new library feature

And docs is not a scope

https://www.conventionalcommits.org/

[copper laurel]

fix is bug fixes (to code), feat is new library features, docs for anything documentation related

[weary kiln]

shouldnt there be docs for Guild.createWebhook() ?

[slate nacelle]

I don't think there is a relevant endpoint for that.

[visual hornet]

yeah there just isn't, webhooks are channel-specific

[opaque vessel]

Anyone know when uses here turns out to be null? https://discord.js.org/#/docs/main/main/typedef/Vanity
> Guild with vanity URL, uses is a number (0 included)
> Guild with vanity URL but not set, uses is 0
> Guild without vanity URL, uses is 0

Can't seem to find out when this is null, I fear behaviour may have changed when this was done or I'm missing something?

[visual hornet]

might be there for sanity? since Guild#vanityURLUses gives null if it hasn't been fetched

[outer raven]

Yeah that’s it ^^
If a property can be null when partial it’s marked as nullable on the docs

[opaque vessel]

Guild#vanityURLUses does not return Vanity, it returns a number

[outer raven]

Yeah but its null before you run Guild#fetchVanity right

[opaque vessel]

Yes

I don't see how it would be null because of that. We could just remove that property as it's never there initially and is not updated when the vanity URL is even used

Talking about the type definition though, rather than that property

[visual hornet]

yeah i agree, both vanityURLCode and vanityURLUses could just be a single property, as the fetchVanityData we already have

[opaque vessel]

Right now just focused on when the uses property is null, don't think it ever can be, is bugging me in the typings lol

[outer raven]

but it can I don't understand the issue

you don't get that info on startup, so you have to manually fetch it

before fetching its null

I don't see anything wrong with that?

[opaque vessel]

I'm not talking about the property at all

Pretend it doesn't exist

[outer raven]

oh wait

[opaque vessel]

The vanity-url endpoint is documented (by discord.js) that the uses can be null, but it doesn't look like it can be null

[outer raven]

well I don't have guilds with the feature enabled so I can't test, but if you do and what you're saying is true then you can probably change

[opaque vessel]

I do and yeah the results were as posted initially

[analog oyster]

I do too, and the results were as you posted initially. doesn't seem like it can be null

[outer raven]

seems like the uses property isn't even documented at all on Discord's side so it makes it hard to check

[burnt cradle]

https://discord.com/developers/docs/resources/invite#invite-metadata-object-invite-metadata-structure it's documented here as non-nullable

[outer raven]

oooh right I forgot the invite object has that weird documentation style

I'd say you can go ahead and make the pr then @opaque vessel

[outer raven]

If we change a method to throw an error where it wouldn't before, is this considered semver major?

[copper laurel]

yes

[outer raven]

Is it possible to send a warning without throwing an error?

[copper laurel]

Technically yes probably, but thats generally goes against library practices. What is this for?

Or at least, there isnt much precedent for it other than deprecation warnings

[outer raven]

A few days ago I brought up the fact that the accentColor and banner properties in the User object were misleading since they need to be force-fetched to appear and currently they are set to null before being fetched, however they can actually be null after being fetched
My idea was to make them undefined before fetching them and to make User#bannerURL throw an error saying the banner hasn’t been fetched when trying to run it

[oak quail]

dont we typically use null for unknown properties

[copper laurel]

Is that PR even merged yet. I dont see those properties on the docs

[oak quail]

(although a distinction would be useful here)

[outer raven]

It is on main

[outer raven]

Yes but we need a way to tell them apart here

[copper laurel]

Its not breaking if it hasnt been released to stable

So not semver major

[outer raven]

Oh amazing

[oak quail]

was merged over a month ago

[outer raven]

[copper laurel]

Yeah I've been out of the loop with development for a while outside of discussion in this channel

[oak quail]

is a release waiting on something btw

feels weird that its been sitting in main for so long

[outer raven]

Icons prob?

They’re out now

[copper laurel]

No idea

I dont think its anything specific, most likely just maintainer fatigue after releasing v13 and getting bug fixes into 13.1

Im not even a maintainer and I know I switched off a bit once it was out

[outer raven]

Yeah I noticed crawl hasn’t been nearly as active recently

And it was much more fun to contribute prior to the v13 release xd

Either way I’ll make the PR about accentColor and banner tomorrow
Just not sure yet if I should explicitly set them to undefined or if I should just not set them at all, guess I’ll pick one and see what the feedback is

[copper laurel]

I lean towards explicit undefined

[outer raven]

Alright, thanks

[copper laurel]

Theres never a case where its not a property of a user object, just cases where we dont have the value yet

Thats my rationale anyway

[outer raven]

Yeah true, space also seemed to agree from the discussion here the other day

[opaque vessel]

If you were to do that, I'd go with explicitly because of exact-optional-property-types in TypeScript.

Anyways, is it really okay to set them to undefined? If I were making use of these, I would always force-fetch them, as if the user changes these, the client does not update the properties (payload is not sent to us) so the instance you fetch them, they become potentially outdated

[copper laurel]

thats also a good point

Maybe we should be abandoning the concept of them as properties entirely and go with a User#fetchBannerData() route?

[opaque vessel]

Imo, I think that'd be wise. The same goes for others like Guild#vanityURLUses for example, where the property is only set after fetching the vanity and never updated when someone uses the vanity invite (another case of being potentially an out-of-date number right after being fetched). Should just be left as methods

[copper laurel]

Yeah, if we cant reliably maintain the validity of cache I don't see why we'd cache it at all

[oak quail]

well anything in cache can be outdated

[copper laurel]

In a general sense sure, but the event driven nature of discord.js keeps the cache up to date

[outer raven]

I don’t think so because unlike vanity data, there is no specific route for the banner info and it would be hard to have the bannerURL method if we don’t cache the banner. I think that if we set them to undefined at first and users realize that it must be fetched to be present they will try to keep it up to date, or will at least be okay with the fact that it may be outdated at any point and they’ll have to fetch again. Also wouldn’t the userUpdate event fire when these props change?

[copper laurel]

Not based on the discussion here, if I'm reading it right

[outer raven]

The gateway event documentation isn’t very explicit but I guess it would ?

I’ll try to do some more debugging later today but the properties don’t seem to be updating on their own :/

[ruby terrace]

Gateway user update isn't useful at all

you need a privileged intent to get real user updates

also, I think the fact that changing color doesn't fire an update could be considered a bug, though super minor (clients probably don't display correct color background in the vc window after a change)

[outer raven]

That’s only from the guild member update event, but the user update event should fire when a user prop changes so I guess that could be a bug

[ruby terrace]

no, the gateway user update event does not fire for anything but the currently logged in user

discord.js hijacks the GMU / PU events to get user updates

[outer raven]

Oohhh I see

[unique axle]

still think we should not do that

[ruby terrace]

same souji, same

[outer raven]

Yep,same

[tacit crypt]

Pretty sure I've been saying that for a while now

[quiet viper]

Why do we even need the accentcolor property?, 99% of the users will not use it and adding it in the cache, is one property more.... . The fetchBannerUrl() is a better Option.

[unique axle]

it's sent, it exists, API coverage is reason enough

[copper laurel]

You dont get to declare what 99% of users will or wont do

Besides, banners are nitro restricted. Everyone else will have a color, so your "99%" statement isn't even close to correct

[opaque vessel]

What's stopping us from changing how we modify the user update event for version 14? I also don't think we should do that

[outer raven]

Nothing ig, just someone making a PR

But I wouldn’t invest much in v14 atm since those prs aren’t being merged rn

[opaque vessel]

also, I think the fact that changing color doesn't fire an update could be considered a bug
In the event it isn't a bug, I thought of removing those properties and replacing them with a .fetchBanner() method which returns an object of the removed properties via force-fetching the user. You could pass ImageURLOptions to change how the URL is received. This might be weird but I also think it's weird why we store such properties when they're potentially outdated the moment we use it

[copper laurel]

Waiting for Discord to give us a proper user update

[outer raven]

I really don’t agree with this because you would be fetching the user not the banner. There’s nothing wrong with storing them, as long as you tell people it won’t update automatically

[analog oyster]

the Guild#owner property was removed in favour of Guild#fetchOwner() too ¯_(ツ)_/¯

[outer raven]

because that was a getter that relied on cache and become less reliable with intents

[ruby terrace]

People really hated it when I tried to do it

[outer raven]

@solemn oyster this isn't really semver major https://github.com/discordjs/discord.js/pull/6721

[solemn oyster]

It is

[outer raven]

banner and accentColor haven't been released on stable, so if that is merged before the next release it can still be on v13

[solemn oyster]

It is

You don't know how many people, me included, does === null or !== null

By adding undefined as a possible value, you're breaking a lot of code

[outer raven]

well on the dev release, not stable

dev releases are unstable and can change at any time, thats the whole point

[unique axle]

yes, and how exactly does that matter for the semver tag?

that's for releases, in general

[outer raven]

because if the feature hasn't been released to stable, it can take breaking changes and still be on the next minor release

[unique axle]

if something breaks the API compared to current stable that is a semver major, by definition

[tacit crypt]

Wouldn't be the first change they'll hate

[tacit crypt]

That is not how semver works Rodry. If the change is labeled as semver major it means the next release will be a major one - 14 in this case

[outer raven]

yeah I know and that's what I'm saying doesn't make sense
why does this have to wait until v14 to be changed if it has never been released to the general "public"?

[tacit crypt]

If those properties existed before its definitely major
If they were added in your PR it's minor

If those properties existed before its definitely major
If they were added in your PR it's minor

It's simple as that

I think we all get what you meant now, hold on

[opaque vessel]

What he's trying to say is that the features he is making changes to only exist in the main branch. It is not shipped to stable

[outer raven]

that's the thing, they don't exist on stable and neither were they added in my PR

holding on

[tacit crypt]

Yeah the light bulbs just lit up in our brain waves, please hold on for landing

[outer raven]

lmao

[copper laurel]

That seems like exactly what we're doing here too though. A getter/function, which relies on cache, and is less reliable based on intents (GUILD_MEMBERS would be required if it includes the data at all)

[outer raven]

but no intents help you get this info, all you need is a force-fetch

and I think it becomes a bit harder than the fetchOwner method, since you have hexAccentColor and bannerURL

[copper laurel]

Which is what fetchOwner does

Which is what fetchOwner does

[outer raven]

but that has its own endpoint

there is no endpoint for fetching the banner

and why are my messages not sending

but that has its own endpoint

there is no endpoint for fetching the banner

wow

[copper laurel]

Discord API issues

Discord API issues

Discord API issues

[outer raven]

yep

[wild flax]

To clarify this:
As how I work mostly (all the time actually) with semver, I don't treat it as a "semver compard to current release" when I apply those labels. I use those labels more as a "compared to the current source code", regardless of release.
So yes, it is a breaking change, compared to the current code, but also a semver minor in the actual changelog of a release (but thats not how we use the labels, we don't use labels to indicate what a PR means to a release, we use milestones and projects for that).

[outer raven]

The Guild#owner getter relied on cache to find the guild owner and worked somewhat fine since all users had all intents in v12 (mostly), but became unreliable so it was replaced by fetchOwner that has its own endpoint and the method returns exactly what you get from Discord (while creating a GuildMember class obviously, I hope you get the point)
if we were to implement a fetchBanner method, not only would that be a fake shortcut for fetch() but it would also require the addition of a new class - UserBannerData (or something similar) to hold 2 properties, a getter and a method, which doesn't make sense imho
I think we should be good by just doing this initial approach for when the properties haven't been fetched at all, then store them and warn users that the properties will not be updated automatically, which I have done in my PR
This is how I see it, but please let me know if you think any of these points don't make sense

[opaque vessel]

That method doesn't have its own endpoint, it just fetches the owner's id in the guild

[copper laurel]

It's a shortcut for guild.members.fetch(guild.ownerId)

[opaque vessel]

However we spin this it seems a little weird I guess lol

[copper laurel]

My main concern is just caching properties that won't get updated, warnings or otherwise

[opaque vessel]

I'd be open to keeping it as it is with a warning stating it won't be updated tbh

[outer raven]

but it fetches a member and that has its endpoint, this doesn't

damn i just cant seem to send any messages

damn i just cant seem to send any messages

[unique axle]

has anyone filed a bug report/asked/gotten to know if that not updating is an API bug?
if it's not i'm definitely with monbrey, we shouldn't cache things that can become invalid immediately afterwards, but we don't get update events for

[outer raven]

but do you think a class just for those properties makes sense?

[opaque vessel]

has anyone filed a bug report/asked/gotten to know if that not updating is an API bug?
I don't believe so, no. I also think it may be a bug tbh

[outer raven]

unless you were to pass the banner url options in the fetch method and then it would give you the url with those options in just an object, which just seems increasingly stupid

and no i dont think anyone has made a bug report

do you even know if those properties not being sent with the guild member data and others is intentional?

do you even know if those properties not being sent with the guild member data and others is intentional?

[unique axle]

unless.. the client doesn't really update banners, it keeps them as-is and fetches them whenever you open a profile, iirc

[outer raven]

do you even know if those properties not being sent with the guild member data and others is intentional?

[unique axle]

unless.. the client doesn't really update banners, it keeps them as-is and fetches them whenever you open a profile, iirc

[outer raven]

discord please

[unique axle]

unless.. the client doesn't really update banners, it keeps them as-is and fetches them whenever you open a profile, iirc

[outer raven]

discord please

[outer raven]

well yeah seems logical, but how would that translate to bots?