#archive-library-discussion

25085 messages · Page 8 of 26

wild flax

Where you can handle those errors a lot more gracefully yourself or even increase the timeout (in case it really takes longer to connect)

There is technically speaking nothing stopping you from waiting 30s or however long you want, we just assume 15s as a default currently

ornate topaz

but doesn't that still not touch the topic of intent at all?

does the new voice know that there is no intent and throws due to that?

or at least mentions that in timeout error

wild flax

Yeah it does not.

Mentioning it would be an appropriate step I could get behind. Maybe something generic as "Check if you have the correct intents enabled" or something.

So we don't lock ourselves into a specific name (though I am not sure how careful we need to be with this. Realistically speaking discord hasn't changed any names on us so far that are problematic, apart from MANAGE_ROLES I think, that used to be named differently)

ornate topaz

additionally, how far is new voice from being added? if there is code that has to be removed from library anyway, i don't see many reasons to not add such mention in current error (unless of course that would count as semver major)

wild flax

Adding /voice to d.js is a minor (even while keeping old code), so its debatable

Only the removal of the old code would be a major

Thats why I am unsure if v13 or v14

Since /voice can be plugged into v13 easily already

Even as 13.1

real jetty

Why is CommandInteraction#isCommand only function?

ornate topaz

?

remote wasp
keen cosmos

He may be wondering why it's not a getter when it easily could be, but I believe that's due to TS assertions can't be getters

sharp wigeon

Are there any plans of -next yet?

Submitted a pr a while ago, but was blatantly copying some of the types from the current version of discord. I understand that it would be a rewrite of discord js (if I’m not wrong), but how?

copper laurel

The benefits of rewriting it fully in TS is to leverage much more than just types, especially copied ones. We can do better inheritance, application of interfaces, dependency injection, all sorts of stuff.

keen cosmos

it can also better allow for division of packages for both microservicing and monolithic packages, though I'm not sure this is an idea d.js maintainers want to embrace

copper laurel

We are, if you look at the structure of the repo

mighty sequoia

how / where are tests run?

i see the test/ folder, but past that I can’t find anything

wild flax

There isn't much testing since all we do is pretty much running against the discord API and mocking that is quite the nightmare

So we are pretty much on-par with other API wrappers in other languages with that

oak quail

oh options is a collection now nice

subtle oxide

do you guys think discord will ever add video-in-embed support?

like an inserted video

wild flax

It doesn't really matter what we think lol

But considering they didn't for the last 3-4 years, unlikely.

kindred yoke

i want to help with library dev, but idk where to begin with understanding how the library works (ik how the underlying discord api works, but i want to understand how the library implements this api)

where should i start?

copper laurel
kindred yoke

alr cool ty

wild flax
copper laurel

Hmm yeah I guess. Project is fully open source, but the internals are not well explained lol

outer raven

why don't you guys use CODEOWNERS instead of having someone request reviews from others when a pr is made?

copper laurel

Because the same four people who need to review would be the code owners anyway

They generally review anyway, the requests are more like "hey I want to merge this soon, you need to review too"

outer raven

It usually doesn’t happen that soon tho and you could just set those 4 ppl as codeowners for the entire repo

amber ridge

someone can say me why types/ws is on dependecies and not in devDependencies? ||sorry for the mention, discord goes crazy with the @ ||

vivid field

Because people were complaining that they had to install it manually when using typescript

amber ridge

:weird:

Okay

solemn oyster
outer raven
solemn oyster

It's fine

ornate topaz

except this isn't really project like typescript where you have tens if not hundreds of people looking at the code

solemn oyster

And IMO using codeowners for that feels like using a tool for it's unintended purpose, to gain one of its intended features

outer raven

unintended purpose? why

solemn oyster

Because even if we're the same 4 reviewing (almost) all PRs, we're not the code owners

outer raven

well no one's gonna come after you for that xD

wild flax

Yeah but it’s not reflective of the “owner”

outer raven

eh fair I guess

wild flax

@remote wasp your PR actually seems to work but breaks in the IDE 🤔

actually no, its not your PR, all good

outer raven

Can someone run the publish action so that the latest commits are uploaded?

wild flax

yeah will do

outer raven

Thanks :D

remote wasp
zenith oracle

If you want to set a param as optional you can add square brackets around the param name but if you want to use the type null you need to add it manually (e.g. string | null)

remote wasp

yeah, that's what I was going for

@vivid field We have a problem here blobStare

vivid field

Seems to be a docgen bug then ¯_(ツ)_/¯

{?type} should be universal

But in the meantime you can change it i guess

zenith oracle

Btw there aren't any params null in discord.js other than this Message#edit 😂

ornate topaz

fairly sure that's how you remove embeds in edit, at least on stable

zenith oracle

Ye I saw

I think there should be something else to remove embeds

late iron

Is there a library to implement buttons? Some devs made a library for buttons but it's only for python (discord.py) so I was wondering if there was anything similar for discord.js

clever spoke

theres a pr

rich iglooBOT
late iron
clever spoke

pull request

late iron

oh ok

I will take a look at it

outer raven

Would you guys like to change the default options for image URLs in the future so that the format defaults to png and dynamic to true? I feel like it makes more sense for it to be this way as png is a more easily readable format, and people usually want their images to be animated if they are. There might be a reason it's this way though so I'm not sure

solemn oyster

No, we try to stay as close to Discord as we can

wild flax

Our default of webp already allows that

webp supports static and animated

solemn oyster

Discord uses webp mostly because it's more optimised than png

outer raven
outer raven
solemn oyster

It is, to both of the affirmations

webp is something Google created for web, why would it not be supported in Google's browsers?

ornate topaz
oak quail

imo the defaults should be format: webp and dynamic: true

ornate topaz

isn't webp problematic on "certain platforms"

copper laurel
oak quail imo the defaults should be format: webp and dynamic: true

This was discussed before - if those were the defaults, what should specifying format: "png" do to the dynamic param?
If it forces png format, then dynamic isnt always the same default - undesired
If it is still dynamic, then the format you passed isnt guaranteed - a little better because you'd have to override both defaults, but still undesired
Since neither of these was ideal, was decided to keep the existing defaults

That way - you always get the format you ask for, unless you choose for it to resolve a gif if possible

solemn oyster
ornate topaz

do .webps finally display in discord app? woah

remote wasp

Shouldn't the gen-esm-wrapper be run on commit? People who install from master and use import/export won't be able to run djs as there is no index.mjs in the repo now. If doing so might create a big diff then is there a way to run it when the user installs from git. Is that what prepare is for?

vivid field

That was a concern at first but it was resolved by the dev publishes to npm we have now

remote wasp

I see, it won't be a problem for most people then but still does the dev get published on every commit or after some interval?

vivid field

It's published every 12 hours

wild flax

Or manually if need be

remote wasp

It might become an issue when we want to test something really quick after a commit tho. Why is the interval, some kind of npm ratelimit or what?

rn I'm running the gen-esm-wrapper inside the installed djs folder blobStare

wild flax

Yeah it’s not worthwhile to “test something real quick” for the majority of people

If you need to do it, obv you can just run it manually

Which is a fair trade I would assume

wild flax
remote wasp

right, makes sense neptuneWink

mighty sequoia

How long on average does it take for me to get a review on a pr after someone requests it from someone else?

mint flint
tacit crypt
sleek matrix

they've already talked about what's new in the new version of "discord.js v13" or what we'll need to change in our apps so we can run on lib v13?

zenith oracle

?v13

rich iglooBOT

Version 13 is actively being developed on the #master branch. To test new features and see what's being worked on, check the pins in #djs-help-v14

zenith oracle
sleek matrix

Ok

copper laurel

That guide is missing things

devout relic

@zenith oracle look damn nice

are they using docz

for discord.js

copper laurel

I don't understand your question

mighty sequoia
copper laurel

Just put part of it on a new line?

mighty sequoia
copper laurel

I think so yeah

mighty sequoia

Ah cool, all tests passed, ty

copper laurel

They run automatically

wild flax

Not for new contributors

mighty sequoia
zenith oracle
remote wasp

It's Vuepress, if you're talking about guide

ruby terrace

For PR https://github.com/discordjs/discord.js/pull/5695 if it is merged, I think we are at the point where permissions should get there own manager, accessible via ApplicationCommandManager#permissions (GuildApplicationCommandManager#permissions for that PR ) and ApplicationCommand#permissions although this manager would be without a cache. I think it would fit better within the rest of the library structure. As a side benefit, it would help 5729 out immensely as well. Any input on this?

radiant glacier

do you think they would make a small change to v11? Since stage channels were added, it now throws an error which can be easily solved by doing an if check inside one of the djs files

node_modules/discord.js/src/client/ClientDataManager.js:81:9

Before:

guild.channels.set(channel.id, channel);

After:

if (guild) guild.channels.set(channel.id, channel);

it's not that important, just a bit annoying

remote wasp

Just to be clear, you want bug fix for djs v11 here?

wild flax

v11 has been unsupported for almost over a year now.

Good luck.

radiant glacier

I know I'm pushing my luck, but thought I'd ask anyway. Don't know if I don't ask

remote wasp

can't happen, only fix is to upgrade

radiant glacier

yeah unfortunately v11 is the best lib 😦

wild flax

v11 will stop working any day anyway.

Once Discord decides to shut down the v6 API.

You refusing to upgrade only contributes to your bot being offline forever.

radiant glacier

v12+ doesn't have what v11 has

if you get what I mean

wild flax

Yeah it does and more.

radiant glacier

I do prefer v12

solemn oyster

Then why are you on v11

radiant glacier

but it's not possible to use for certain things

wild flax

You can prefer whatever you want, it will stop working lol. Is that so hard to understand.

It's literally out of our control and your bot will stop working.

radiant glacier

yeah I'm aware of that

and prepared

solemn oyster

v11's codebase is ancient, and many of us contributors just don't want to maintain it any longer

wild flax

You should have been prepared over a year ago, because v12 will be deprecated soon too and v13 will be released.

radiant glacier

I heard it was getting shut down last year but its stayed open for a long time

wild flax

So your v12 update will be out of date too.

solemn oyster

v12's and v13's internals are a lot cleaner and superior (async stack traces ftw), as well as pretty similar

That being said, what are those things you claim v12 can't do, that v11 can?

radiant glacier

it should be pretty obvious

but like

if I need to spell it out for you

manic zinc

It's not obvious for anyone.

wild flax

We maintain this library, so we would know.

Thats why we ask.

radiant glacier

👨 🤖

manic zinc

Oh you mean you want to keep breaking the terms of service??? Good luck with that.

solemn oyster

This is precisely one of the reasons why we can't and don't want to maintain v11, we can't support code that breaks Discord's Terms of Service

radiant glacier

that's what I meant when I said I prefer v12

manic zinc

You will not get support for breaking the ToS with any library that is allowed to exist on Discord's API.

radiant glacier

I never said I was using it

manic zinc

You're implying it extremely clearly.

radiant glacier
wild flax

So why do you want us to patch v11?

I don't follow

manic zinc

Do you have any legitimate example?

wild flax

If you don't use it, it should not be of your concern what errors it throws.

manic zinc

when your only justification for using a year old unsupported version is "I wanna break the ToS". blobguns

gloomy bluff

Would it make sense for embed setters to have optional params

ornate topaz

wouldn't that mean that they default to something?

gloomy bluff

Default to null/empty as if they weren't used
Right now they are marked as mandatory so if you have a function that returns string? you have to do something like
embed.setThumbnail(user.displayAvatarURL() ?? '')

in TS

ornate topaz

well, not really

solemn oyster

They absolutely allow null and undefined, iirc

gloomy bluff

I looked at the code and the jsdoc params are mandatory

So TS compiler complains

ornate topaz

param being mandatory and param allowing undefined to be explicitly passed are 2 different things

gloomy bluff

if you don't OR it with an empty string

vivid field

You can also do .stuff()! instead of .stuff() ?? ''

But shouldn't it allow null anyway to remove a value?

strange igloo

I see the point, but that's against the logic
You say setX on some embed, it means you want to set something
If you want to setX only when some condition are met, it's up to you to put your code inside some conditional statements in order to let it run only when it can

For user avatar as an example, it's as simple as

if (user.avatar !== null) embed.setThumbnail(user.avatarURL());```
gloomy bluff

Alright

outer raven

Would it be possible to update the typings for ApplicationCommandData#options to specify that, if a SUB_COMMAND or a SUB_COMMAND_GROUP type is specified, all other types are invalid?

So basically this would throw an error

[
  {
    name: "test",
    description:"A sub command",
    type: "SUB_COMMAND"
  },
  {
    name: "parameter",
    description: "This is wrong",
    type: "STRING"
  }
]```

Either that or something in the documentation could be added to warn users about this, since discord will throw this error

ornate topaz

if discord throws the error, and the error is descriptive, why would we mention that

outer raven
ornate topaz

i wasn't talking about typescript

outer raven
ornate topaz

you were talking about 2 things

typescript, and docs

outer raven

either update the typings or add a warn in the docs

those were my suggestions

bc idk if updating the types to say that is possible

sharp wigeon
copper laurel

I don't quite understand your question - the PR shows you exactly what properties they'll have and where

outer raven

it's waiting for Discord to deploy this to all users and then it'll be merged

loud jayBOT
smoky thunder

is there a contributing guide for @djs/collection or does it follow the same one the main lib uses?

wild flax

Yup

fathom aurora
copper laurel

already have

fathom aurora

as in, in djs v12?

copper laurel

no, in v13

the in-dev v13

fathom aurora

ah awesome

can't wait to utilize it

copper laurel

?master

rich iglooBOT
copper laurel

You can install it using npm i discord.js@dev

fathom aurora

ah nice, will take a look then

copper laurel

subject to change though

outer raven

Would it be possible to add a parameter inside ApplicationCommandData for permissions and make setting and fetching permissions easier?

By making it easier I mean having a permissions parameter in ApplicationCommand instead of having to fetchPermissions()

ruby terrace
keen bone

it would be great if had a function to check if the bot can send message on a channel before try to send it. ( An boolean type, like message.channel.sendable ) i know it's a terrible name, just example haha

clever spoke

channel.permissionsFor or something exists already

zenith oracle

Yeah you can just check if you have the send messages permission

remote wasp

The getPayload method in Actions.js calls ChannelManager#add with cache as second param, which is fine with most of the other managers but for ChannelManager#add the second param is guild. Now this doesn't break anything since channels are always cached hence the method never get called but I thought I should let you know about this. Also, what was the reason to put guild before cache in #add here?
https://i.imgur.com/kgqThNr.png

I'm asking this because I have a similar situation in the new StageInstanceManager#add

slate nacelle

I suppose the reason was because the parameter is optional, but we probably want to keep it consistent with the others?
Although I may not be aware of all edge-cases we may have somewhere.

remote wasp

It would have been consistent with other managers if it was like add(data, cache = true, guild)

slate nacelle

Something like that

remote wasp

yeah, the guild can go inside the extras of the last optional object instead

strange igloo

Is a PR only modifying typing falling into these cases or not Thonk

- This PR changes the library's interface (methods or parameters added)
- This PR includes breaking changes (methods removed or renamed, parameters moved or removed)

Or the non-code change category

wild flax

non-code

hallow ocean

Does the non-code case related with the "I know how to update typings and have done so" sentence?

wild flax

I would assume so if you update the typings that you know what you are doing

copper laurel

Half of that is true for my PRs

tired valve

I just changed that to 'pretend'

outer raven

Would it be possible to add a VoiceState.user property that represents the User object of the member that state is attached to?

I know it's possible but would it make sense to do so?

slate nacelle

No

outer raven

And why not

slate nacelle

Because users can be connected to multiple voice channels across guilds.

outer raven

Yeah but almost every class has both the user and member properties, I think it makes sense for consistency because you often need properties from the user object

wild flax

And how would you solve the aforementioned problem

outer raven

yeah I know I can use voicestate.member.user but it could also be added imo

slate nacelle

Oh, I misread that, I though the other way, whoops.

wild flax

Where do we put users where we also have members consistently?

If you are referring to msg.author and msg.member, those are a bit of a different case

outer raven

same thing goes for interactions, I can't think of a class that has member and doesn't have user besides voicestate but i could be wrong

wild flax

iirc thats the case because it comes with the actual payload

slate nacelle

Also that there might be no member (i.e. interaction in dms)

So far there are no non-guild voice states, I think even for instances you get "member", although I could be wrong here

outer raven

yeah true but it's just as a shortcut, the same way that client is present in every class as a shortcut

wild flax

we don't really do shortcuts anymore

we used to, but at one point you just ask yourself "but why doesnt this have one, or this, or this"

outer raven

fair enough

wild flax

Thatd prolly fall into the same category as why we dont have permission getters for every single permission you might want to check

outer raven

btw crawl can you explain exactly what is gonna change in regards to the .send and .edit PR? (enforce the options object and when exactly we're gonna be able to not use an object)

outer raven
wild flax

Yeah but thered be a shit ton more if we fully comitted

Uh, not much really

You can just check the issue thats linked to it

the 3rd codeblock

outer raven

ye but I saw you comment about .send(embed) and how that would work but it would be undocumented? Maybe I misread that

ruby terrace

I said that it would be undoucmented, that was unintended per crawl

outer raven

but that means it would throw an error in TS right

ruby terrace

yes, its been removed already anyways

outer raven

can't you just keep that 1 exception though? I believe a lot of people send their messages as embeds whenever they can and thus they use that method

wild flax

I explained why not

outer raven

is it because of interactions and webhooks?

wild flax

yes

outer raven

you could keep that for messages though, they don't exactly need to be consistent

wild flax

I explained why they should be

Are you sure you read the issue?

outer raven

Yeah I did, I just don't understand how "too much flexibility" is an issue

I understand it could be confusing from the library's side, but it's much easier for the end-user

wild flax

its quite confusing

outer raven

yeah when converting from sending messages to interactions it can be confusing and I totally agree with the change for interactions, but as long as the docs mention specifically that you can't use .send(embed) it should be fine right?

although some people like to not read the docs but that's something else

strange igloo

Imo the more consistent you are, the least confusing you are (thought hard on this one)

Issue with .send method & co. is that people (especially new programmers) might think they can pass absolutely anything they want inside since most of things they will usually pass (content, embeds and for some attachments) will produce the expected result

Now, is it too late? No idea but if this change gets pushed I think this server will need a bit more of 2 general help channels to absorb the flow of people coming for the same errors

ornate topaz

we have tags for that

tired valve

It's better then people just having no idea what overloads are valid and which aren't, the amount of (embed, options) I'm greeted with in support is insane

mighty sequoia
wintry karma

Is it intentional that Interaction. fetchReply (Webhook.fetchMessage) returns raw webhook data in when the channel is a DM (on master)?

copper laurel

I'll have a look

Hmm, its because DM channels arent cached

wintry karma

Is there a reason to not use channels.fetch here instead of a get?

copper laurel

The library generally never does any internal fetching

remote wasp

Would it be a good idea to add typedefs for methods that take an object as option? For example the method for fetching audit logs: https://discord.js.org/#/docs/main/master/class/Guild?scrollTo=fetchAuditLogs. options.<something> doesn't look that good imo. I have seen more of these in the docs. It wouldn't be a big change because we will only be making change to the documentation.

wintry karma
zenith oracle

This will delay the Promise, even if it's ready, only without an element

outer raven

Can we change MessageActionRow#components to a collection of buttons mapped by their customIds or url in the case of link buttons?

wild flax

I don't think that would be very beneficial

outer raven

would be easier to find a specific button since (I believe) all buttons must have either a customId or a URL

wild flax

For most useful commands you save custom data inside the buttons custom id, so it would not really be easier retrieving them

tired valve

The key being a URL also seems like a bad idea

wild flax

Let me just retrieve the button for ban:237812371237878:12938712932819389

seems like a terrible idea

outer raven

oh I didn't know that was something that was done, I just name mine after the action they represent

usually a word or two

tired valve

Now make that a key of 100 chars

Which is possible

That'd be awful

wild flax

How do you know who to ban with that, for example

In case you make some confirmation action

outer raven

I guess you'd have a variable with the member object that you pass initially through the interaction

wild flax

So what if 2 mods issue 2 bans

Which button is for which interaction?

If they are both named the same

outer raven

you'd have button collectors and each one only fires for a specific message

wild flax

That works if you exclusively use collectors

outer raven

yeah which seems to be the easiest and most accurate way

wild flax

That doesn't matter in this case

You can't just make one use case the "correct", and the others barely possible anymore

outer raven

eh fair enough

wild flax

@outer raven there is no point in asking for merges

outer raven

Vlad said he would wait for the feature to be released, I was just asking if now was a good time or not

wild flax

vlad doesn't speak for all of us

generally speaking we only merge when general availability hits and all reviews have been through

tacit crypt

What did I do

Oh, I see what I "did"

outer raven
tame whale

I assume the new thing with the buttons is going to come in the next update?

copper laurel

yes, it already merged into the dev release

tame whale

Epic.

worthy coral

The code returns this, so I would assume it's not intentional and the class was renamed

copper laurel

ahh, yeah thats a copy and paste mistake in the JS docs

easily fixed

fossil prairie

hello awesome community ;). So i have some concerns about discord.js interface for Message Embeds. I am developing a bot and when i wrap a text into a code block, i get a different appearance on desktop and mobile as seen below.

My question is: Is there any way to develop something like "media queries" and render different messages in mobile (with different formatting), cause as it seems, code blocks arent working as intented in mobile.

wild flax

No

None of this concerns us as a library

It’s all discord’s doing how things are displayed

fossil prairie

hmm i see, thats what i thought. Thanks anyways.

grizzled field

for djs devs: are you planning on adding a further layer of abstraction for the discord buttons api? It seems a lot less organized than I think people would like it to be, with buttons being added on messages, but implementations of the button functions being done in a completely separate place.

wild flax

Can you further elaborate on this

tired valve

You can stash them in an array of arrays if the actionrow bothers you

grizzled field

currently, we would add buttons to an actionrow and set a customID for it. however, we would only implement what the button would do on the interaction event, which i think is not as organized as it should be

tired valve

I don't understand the problem you are proposing, what exactly is bothering you?

grizzled field

I guess, the separation in declaration and implementation is bothering me

wild flax

You can create the buttons anywhere you want really. It doesn't have to happen in a specific "event".

Or any specific place inside of your code.

If thats what you are saying

grizzled field

yes, but implementing the function for the button needs to be done on event

tired valve

You can just create a componentcollector for that

strange igloo

Have a kind of array of buttons with a function associated to them
When a button gets pressed, see which item of that array it corresponds to and execute its corresponding function
You can use message component collectors too, which is suitable for a lot of situations, and would not require you to listen to the interaction event

tired valve

the collector does listen to the event and forwards it so to speak

wild flax

You can also have a function that listens to "componentType 3" and off-load everything there.

And split that up further however you want.

We don't make any assumption as to how you write your code.

But if your question is to have something like other modules, an event named clickButton which you can listen to, I do not think thats the right approach at all.

grizzled field
worthy coral

in ClientApplication#flags, are the flags supposed to be Readonly<ApplicationFlags>, because they're never frozen in the ClientApplication._patch

It seems to break consistency with other Structure.flags in terms of typings and functionality since everywhere else is Readonly<*Flags>;

slate nacelle

Sounds like an oversight, it should be frozen.

faint cipher
lone vector

I noticed that discussion and put in a pr for it since it seems like an unintentional byproduct of another change

faint cipher

cool to know, can you send its link?

lone vector
faint cipher

ty

worthy coral
copper laurel

Im not quite sure what you mean

oh nvm I see

Yeah I think that typedef should say id

ruby terrace

it probably should, but that reduce operation is occuring on fetched data from discord, where it is perm.id, so nothing wrong there

copper laurel

Yeah the typedef is still wrong

worthy coral
copper laurel

The object users pass in expects id

yes

worthy coral
worthy coral
ruby terrace

oh, its being done twice, the one you linked is returned from discord, but it happens before with passed data smolFp

worthy coral

oh whoops. My bad

I see now

ruby terrace

I can actually handle that in #5729...done

wide sand

I tried it on djs and found this ->
when i disconnect the bot from vc it still shows channel id which should be null but when i disconnect other user or bot from channel it shows channel id as null

I am not sure if this is djs thing or api thing

okay so it's fine from api side. It is happening when i am using erela.js for voice when i connect using erela then on manully disconnecting via discord interface it shows channelID. But if i connect the bot to channel and restart the bot then disconnect from vc manually it shows channelId as null

also to note when i use erela.js player.disconnect() function it shows channelID as null

However if i have one more instance of bot running the other instance shows null which this one still shows the channel ID

ornate topaz

does it happen without erela?

if no, seems like erela is messing stuff up.

wide sand
sly forge

Got redirected here from #djs-help-v14, so it seems like the PR https://github.com/discordjs/discord.js/pull/5565 introduced a bug that wont let you add permission overwrites to channels/etc. without supplying a valid PermissionResolvable to both the allow and deny parameters (which should be optional by default).

I tried to modify the resolve method of PermissionOverwrites structure shown below and it works but i dont know if this is a worthy PR fix.

/**
   * Resolves an overwrite into {@link RawOverwriteData}.
   * @param {OverwriteResolvable} overwrite The overwrite-like data to resolve
   * @param {Guild} [guild] The guild to resolve from
   * @returns {RawOverwriteData}
   */
  static resolve(overwrite, guild) {
    if (overwrite instanceof this) return overwrite.toJSON();
    if (typeof overwrite.id === 'string' && overwrite.type in OverwriteTypes) {
      return {
        id: overwrite.id,
        type: OverwriteTypes[overwrite.type],
-        allow: Permissions.resolve(overwrite.allow).toString(),
-        deny: Permissions.resolve(overwrite.deny).toString(),
+        allow: Permissions.resolve(overwrite.allow ?? Permissions.defaultBit).toString(),
+        deny: Permissions.resolve(overwrite.deny ?? Permissions.defaultBit).toString(),
      };
    }

    const userOrRole = guild.roles.resolve(overwrite.id) || guild.client.users.resolve(overwrite.id);
    if (!userOrRole) throw new TypeError('INVALID_TYPE', 'parameter', 'User nor a Role');
    const type = userOrRole instanceof Role ? OverwriteTypes.role : OverwriteTypes.member;

    return {
      id: userOrRole.id,
      type,
-      allow: Permissions.resolve(overwrite.allow).toString(),
-      deny: Permissions.resolve(overwrite.deny).toString(),
+      allow: Permissions.resolve(overwrite.allow ?? Permissions.defaultBit).toString(),
+      deny: Permissions.resolve(overwrite.deny ?? Permissions.defaultBit).toString(),
    };
  }
velvet tundra

we can trigger the same actions on webhook_events as well as schedule right?

because i was wondering why djs uses 2 different workflow

ornate topaz

wdym

strange igloo
velvet tundra

one that runs every 12 hours

another which runs on push and pr

outer raven
clever summit

when updating from a master commit before 2f5424bdac7c97d0a371dd72084ac02d5e774f1a to this one or later I noticed that all GuildChannel#permissionOverwrites are empty
I think the issue is that in https://github.com/discordjs/discord.js/pull/5796 the permissionOverwrites get set to an empty Collection in the constructor (https://github.com/discordjs/discord.js/blob/93b0a4e005b5b1d371f7936238556db2e36cc982/src/structures/GuildChannel.js#L40) after _patch from within the super call already populated them with the data from the API, removing this line restored the previous behaviour

vagrant holly

Ohhhhh

I was just digging into what was causing this, and yes, that'll be it I think

vagrant holly
real jetty

Will v13 support changing a guild’s vanity code? I’m pretty sure that still exists in the API but is just undocumented

wild flax

We generally don't implement non-documented features

real jetty

aww okay

ornate topaz
velvet tundra one that runs every 12 hours

If you're talking about publishing dev which runs every 12h, then it's supposed to be like that. Doesn't make much sense to release 4 dev versions whenever you merge 4 PRs in a span of 2 minutes

velvet tundra

@ornate topaz I understand that. But what about the test-cron.yml

ornate topaz

what about it

worthy coral
copper laurel

Yeah, because as far as I know we don't directly support passing flags. The only two end user usecases are supressing embeds, and ephemerals, both of which have their own options/methods

analog oyster

@fringe temple any reason why you didn't make the second argument just BroadcastEvalOptions<P>? because as is the context is always unknown

I guess because of the shard option, but take could just be another generic type

fringe temple

it felt unnecessary to introduce another generic type there

analog oyster

right, however, the type is incorrect

fringe temple

How so

analog oyster

the context is always unknown

because you're not passing the P generic type to it

fringe temple

Oops!

feel free to PR a fix for that, otherwise I'll make one later today or tomorrow

analog oyster

will do

outer raven

If there isn't a reason for that I could remove them in #5696

unique axle

default user avatars

outer raven

ye thats for the default user but the others need snowflakes right?

in fact ill change the id param to discriminator since thats less misleading

and the type for that can never be a snowflake

unique axle

nkoHmm that makes no sense though

wild flax

I don't see whats wrong with the current approach

outer raven

so the typings are technically incorrect

wild flax

Stop putting stuff into your PR that has nothing to do with your PR

make a new one if it bothers you that much

outer raven

lol ok

unique axle

it's not used

outer raven

but it will be right?

wild flax

We don’t know

unique axle

so far nothing indicates that it will

outer raven

hm alright

solemn oyster

omg, I love that one

tender field

Hey i just needed to sync the permissions of a channel with its parent channel, and it took me a long time to find it because i was looking for "sync" (which is the word discord uses), but it turns out it is "lock".
Is there any reason why it is called like that? What about renaming it to "sync", to make it perhaps more clear? because afaik it does not really lock them, then can still be changed right ? afaict it just overwrites the existing ones with the parent's ones

remote wasp

can't because discord uses "lock" for syncing in api

ornate topaz

technically yes, and due to how discord handles that, that will sync permissions with parent

ornate topaz
wild flax

the naming of the api uses lock

remote wasp

yeah, it's just like saying change "guild" to "server"

wild flax

also technically speaking, discord can take the liberty and change names like that on the client all the time

ornate topaz

well, there is no "lock" param or anything like that for editing a channel. only lock there is is for moving to a parent

wild flax

if we would constantly switch around naming as they do on the client, wed be in for a bad time

tender field

Yea i see, but i did not mean « use the same naming that in the client », but rather maybe a clearer and more explicit name, so you can understand and find it more easily

Not a big deal anyway ig

btw this is just about make it clearer, so ig it could also be added to the doc, because i really find "lock" to not be explicit at all : i had to check the code to be sure that it indeed synced the perms 😅

outer raven

Since the required value for ApplicationCommandOption is set to undefined by Discord when it's false, can I make a PR so that it defaults to false?

velvet tundra

a lot of bots are gonna break due to this new change to the old channel.send(<MessageEmbed>)

wild flax

Yes

A lot of bots are also going to break because of hasPermissions to permissions.has

remote stream
remote wasp

It did, also v13 as itself is a major breaking change. So, it is expected that users will have to update their code in order to not break it

worthy coral
wild flax

@outer raven yeah I honestly don't care enough if its used both ways

I was just thinking it might be for only sending

which you could argue you might be sending a useless prop because the default on discords side, if the key is not provided, is false

I guess you get my point

But if its also for saving it on our side, it might be more beneficial to have it an explicit value

outer raven

Yeye I know. For my bot I prefer having it always set explicitly so that we know what is required and what isn’t

But I just thought of an edge case that I’ll need to fix when I get home, which I commented on the PR about

wild flax

yeah I saw your comment

oak quail

afaik there are currently no plans to use it again

outer raven

oh

that's sad, I liked that one

but makes sense

outer raven
empty viper

Hi, I have a question, why does the Client class has setTimeout/setInterval and setImmediate (with their clear version) methods ? 🤔

copper garden

It will kill the timers when the client is destroyed

empty viper

Oh yes, logic, thanks

oak quail

would be very useful if RangeError [EMBED_FIELD_VALUE]: MessageEmbed field values must be non-empty strings. said which field had the invalid value

fallow crater

^would be useful for all errors where possible 😅

oak quail

also would be good to not log full image data when its logging the request body lol

ruby terrace
real jetty
empty viper
outer raven

doesn't really matter how small it is

outer raven

What's the progress on #5692? Is it going to be merged soon-ish?

loud jayBOT
zenith oracle

Aren't select menus still in private beta?

unique axle

yes

no need to ask status on pr's either

they are merged when released and ready

outer raven

oh sorry, I saw a linked api docs pr that was merged and thought that was it

copper laurel

Unsure what you're referring to, I haven't linked any API docs to that one

outer raven

Why is the patch function in Message.js called patch instead of _patch like in most other structures?

solemn oyster

@remote wasp you missed Guild

remote wasp

oh, I'll fix it. 👍

solemn oyster

Everything else looks fine to me thumbsup

remote wasp

neptuneWink

umm, I just realized since we are building the _data object here, we do have to check it this way. In the earlier case we were passing the param directly.

solemn oyster

Oh

Approved thumbsup

outer raven

shouldn't these error codes be in Constants.js? A lot of them are missing so idk what the criteria is really

solemn oyster

No criteria, just nobody PR'd them

outer raven

so they should all be added?

solemn oyster

Yes

outer raven

well that's gonna be a fun job

solemn oyster

An option, though, is that we re-exports discord-api-types's enum

Since that's always updated, and sometimes faster than d.js itself

outer raven

wouldn't it be better to just import them from there and not have this export?

cuz this is clearly heavily outdated

I'm willing to make the PR adding all of these btw, just wondering if it's worth it or not

unique axle
remote wasp

yeah, I saw it too. The current PR is already done tho. Should I add this to it or make a new one? ThinkO_O

outer raven

Can someone run the publish action once this round of merges is done pls?

slate nacelle

Is that what the invite_code property on the instance is for? Thinkeng

solemn oyster

@remote wasp I unresolved the comment about update because upstream PR in API docs, btw

remote wasp

yeah, saw it. Should I change the update to edit now?

solemn oyster

I'd wait for the docs to be reviewed, tpcstld is on it

remote wasp

okay

solemn oyster

He approved and merged it

So no wait needed

remote wasp

lmao, that was fast neptuneWink

solemn oyster

Yeah, I asked them before making the PR, and told me to ping them back once the PR was created

remote wasp

aight, I'm on it prettythumbsup

solemn oyster

thumbsup

I'll approve as soon as that's updated

Everything else looks good to me

solemn oyster

@remote wasp just 2 docs things and I'll approve

(Also, #5843 needs a rebase)

remote wasp

okay 👍

solemn oyster

Shibi~

I just noticed a small thing, sorry 😓

StageInstanceManager#24, missing final dot

@remote wasp ^

Wait, it's a property

remote wasp

but that's same for all mangers, I checked and none of the manager have dot at the end. ThinkO_O

solemn oyster

Yeah, properties don't have final dots, my bad

remote wasp

lol, it's okay neptuneWink

solemn oyster

I thought it was a typedef because they're blocks of docs without any code below them

remote wasp

oh

solemn oyster

Approved thumbsup

remote wasp

thankuwu

real jetty

Is there a pull request (and if not is it useful?) to compare the local command with the registered one (name, description, arguments, required field, etc...) in order to update or register the command only if necessary

strange igloo
real jetty
copper laurel

I think generally the consensus is that's what belongs in a framework, not the core lib

graceful steeple

Hello everyone! i have tried to run discord.js in chrome extension. this is the boilerplate i've created and im using right now https://github.com/fxnoob/browser-extension-boilerplate. i'm using webpack 5 so its asking for the polyfills. when i am requiring web build of discord.js, its giving me 403 on console.
I've read in the github issues that due to cors web build is not working but its not the case for browser extension. they can do cors fetch from background pages.
Is it possible to use the Discord.js in chrome extensions background pages? is there webpack config for webpack 5 for this project so that polyfill error can go away?

strange igloo

I don't see any use case that would require discord.js to be used in a Chrome extension (or in a web browser in general)
If you only need some API requests I think it'd be better to directly interact with the Discord API

raven juniper

discord has dropped support for webpacks

graceful steeple
wild flax
strange igloo
graceful steeple
wild flax

Would this be an extension for you alone?

graceful steeple
wild flax

Because you are not allowed to request a users account or their token to send the message from their user

This would fall under the "user bots" category which is not allowed by ToS

graceful steeple
wild flax

It would not be

remote wasp
wild flax

But its surely not possible with our library

How would you make sure the token you distribute is secure?

What shibi suggested would be the proper way to handle this 👍

graceful steeple
remote wasp

You can look into CF workers

graceful steeple

what does cf stands for ?

remote wasp
graceful steeple
wild flax

Whenever you feel like it.

But then again, you might as well switch to the best discord lib: discord.py.

Oh, I was assuming you were memeing.

There is no reason to switch to Eris to be "worth" anything really.

Yeah people have been saying that for years now. Its a dumb circlejerk.

Benchmarks have shown that there is no significant difference between those 2 if used properly.

And big bots run their own forks of eris too, don't be swindled.

Big means >50k here btw.

Yes, 50k+ servers.

What about it? Are you looking for some numbers or something else?

Depending on what your bot does, @rich igloo runs on this server only, which sits at around 50k people. With a lot of cache, uses around 130MB of RAM.

Which is a joke really.

No.

Eris is not matter. It doesn't matter what they will tell you.

The only difference I know for a fact by now is that Eris is behind when it comes to features quite a bit compared to us.

So if you need interactions, buttons, select menus, threads. You will get them faster with us.

vagrant holly
solemn oyster
quiet viper

nah, there isnt a big difference.
see the orange and blue one

wild flax

Eris is not in TS, and it has worse typings.

lol

Also that graph isn't saying anything about our library

This is why I hate graphs like this.

The following test is a ~3 hour run (each tick is 10 minutes), with ~3700 guilds total (4 internal shards), with all intents enabled (including presences), and with message caching disabled (messageCacheMaxSize:0).

No one would enable all intents on their bot.

The introduction of intents also made that light fork completely obsolete.

icy trout

Hey Crawl,
I accidentally pushed one commit to my pull request that I meant to have in a separate pull request...
Do you know how I can undo the most recent commit?

wild flax

Might as well rename the PR and description

Instead of making 1 line changes in so many separate PRs

also less overhead for us to review

icy trout

Thanks for the help 👍

quiet viper
wild flax

Which is useless to compare.

You will never use everything

quiet viper

"Big Bots" such as Dyno would use nearly every Intent

wild flax

No they wouldn't.

And they use a modified version of Eris too

quiet viper
wild flax

They wouldn't use all the intents, no.

quiet viper
wild flax

They don't :D

tacit crypt

Realistically, at the scale they operate on, they would have custom almost everything, from cache to who knows what else

solemn oyster

@icy trout I requested a change, just an easy copy+paste fix

unique axle

more intents
-> more websocket events
-> more data that gets cached + bandwidth used
with intents you control which ws events you want to receive

rustic boughBOT
solemn oyster

I hope you know we don't endorse discord.js-light as it's not official

wild flax

The introduction of intents also made that light fork completely obsolete.

lol

solemn oyster

Pretty much the only benefit of light is customisable caching, although I have no idea how it works and how might it be more memoy efficient than what we have

However, by using it, you also lose the benefit the official discord.js ecosystem gives you, such as more active support, up-to-date features, etc

wild flax

I tried reading their source in the past one time

Its very convoluted lol

solemn oyster

Well, it's a bit to be expected, discord.js's review and maintenance process is pretty good

And is something forks and alternatives don't get. In most of the libraries at least, I see 1 or 2 people reviewing, of course that's going to slip in more messy code, here we have 4 people, so even if it takes longer than 1 or 2 people, the overall code quality is just a lot higher

icy trout
solemn oyster
left moat
solemn oyster

I made it a suggested change so you can commit it from the browser

left moat

u dont have to circle jerk a lib just bc ur in their server

grow up

wild flax

You don't have to continue in this channel do you?

quiet viper
left moat shut up

Thx for the constructive critic, when you think that eris does something better than discord.js, then you can provide me the info on my dms.....
I just made a git compare and must structure and file names were similar. "looks like", that was my opinion.
But let us move this in an another channel.

raven juniper

that's because, if i remember right, eris started as a fork of d.js v8 or something

manic zinc

Eris actually started when Abal left the discord.js development team back in v6, about 5 years ago, because he did not like the class-based/OOP direction hydra was heading towards.

unique axle

thanks, and with that, back to topic and away from djs history lesson, thanks catThumbsUp

vagrant holly

So I have an issue with GuildChannel#permissionsLocked -- if a channel parent permissions update, this then causes a permissions update to the channel itself, and it also appears as if permissionsLocked has changed because the parent has changed at the same time? Not sure what the best solution to this is really

I was thinking maybe having permissionsLocked be an actual method instead of a getter, so you can pass an updated parent into it for the calculation if needed?

worthy coral

I noticed BaseMessageOptions now includes the files field, but does not include the embeds field. Is there a *Options T that doesn't accept embeds?
I looked through and only found ReplyMessageOptions which should be incorrect as it routes through TextableChannel.send which accepts embeds and not embed

ruby terrace

Not anymore I believe, PR 5860 seems like a great candidate to fix that, feel free to make a comment on that.

smoky thunder

Can't the event listeners' return types be Awaited<unknown> instead of Awaited<void> so we can do things like return message.reply({ content: 'Unknown command' })?

ornate topaz

there was a PR to do that, but it was decided against that

however, as far as i'm aware, that's going to change to accept void | Promise<void> after we switch to node 15 or later

since uncaught promise rejections are a throw on there

smoky thunder

seems like a pretty harmless change, there's no way to access the return values anyways

ornate topaz

while true, consensus was that such type would indicate that library handles whatever you return for you, in some way

while the library takes nothing

wild flax

Because they can not be caught

smoky thunder

Oh, didn't know that

wild flax

Yeah, thats mainly why we did it

A lot of libraries allow it, but in this ecosystem I think a lot of people also don't understand the fundamentals of this stuff

Not saying we are clearly superior, but if we find something could definitely backfire on the user, we try to fix it and make sure they don't make that mistake

smoky thunder

Alright, thanks

real jetty
ornate topaz

Message already has one

though not sure what would be the purpose for the channel one, as well as why add it; you can't really get one in the client, as far as i'm aware, without taking message one and removing message id

url gives no benefits over mentioning the channel, both inter-server and cross-server

real jetty

I use channel links when my bot sends a DM to a user so that the user can click on the link and get to the channel quickly

ornate topaz

you can mention the channel instead

as i said, there is exactly 0 benefit of using an url over using a channel mention

real jetty

Thx ❤️

drifting knot

for voicelink: fastify is good but keeping the types on each route in sync with the JSON schema is very annoying to maintain
i strongly recommend you find a schema builder library in typescript which can output valid JSON schema (which fastify consumes) and also a TS type (which the RouteOptions generic accepts)

and it really should be a schema builder library, don't try writing .d.ts files and compiling those to JSON schemas, the community tooling is very lacking in this area

wild flax

The schema here is mainly for swagger honestly

I don't do any real validation with it, because its just a perf decrease

drifting knot

that's still the same issue

documentation is getting duplicated in every route and still needs to be manually kept up to date with the types

wild flax

Yeah but I wouldn't need any library for this really. VoiceLink is rather a done deal after the initial footwork is there, since if you look at the LavaLink repository, there isn't really a lot of code.

It's really just a wrapper around discords voice sending capabilities as a server

So the chances of us updating it a lot is very minimal

And my experiences with that kind of stuff aren't really on-par either.

I have used joi a lot, but I don't think joi is applicable here

Since thats more of a validator than json shema

drifting knot

yeah if this is never getting changed again it's reasonable to leave it as-is

wild flax

I'd argue probably 80% of the routing code will stay the same after the initial MVP

Though I wish fastify had some good 1st party fs-routing capabilities PepeHands

drifting knot

nextjs api routes win again

wild flax

I like doing my routes in the file system as /[id].ts etc

drifting knot

yeees

drifting knot
wild flax

mongo though

blobugh

rough arrow

How does the ratelimiting work in discord.js?

I understand that we have the parent class, RESTManager. It has a global count that gets scheduled calls to reset it (this seems to be handled by RequestHandler right?), this makes sure we don't hit the global rate limit. RESTManager also has a collection of routes (for example /channels/id/ right, including methods?) to RequestHandlers.

That gets us to RequestHandler, it has a queue of promises (so requests that want to be made) and ratelimit information. That is, each RequestHandler represents a Discord API bucket correct?

When the request is to be made, it first makes sure we're not ratelimited by checking the global info (the count is 0 and if a reset has been scheduled) and local info (remaining is 0 and if a reset is scheduled). If any of those 4 are true, it will sleep until that is to be safe. Then actually make the request, and use the ratelimit information retrieved to update local values.

Does this mean discord.js does not use X-RateLimit-Bucket to for example differentiate the endpoints/routes that share (or don't) buckets across the methods they use? Additionally, am I right in that this implementation disallows concurrent requests (which is an acceptable drawback)?

oak quail

Does this mean discord.js does not use X-RateLimit-Bucket
afaik yes that is correct

copper laurel

djs-next's handler uses buckets

solemn oyster

^, we also need to update REST to catch up with the new features from v13's

iron pier

I have a request about the buttons, please use the npm package discord-buttons as a guide so that the code that was written with this package still works
I hope it's the right place for this^^

solemn oyster

discord-buttons has never had, and will never have official endorsement from us

ornate topaz

i think that was more of a "write in in the way that package is done so i dont' have to rewrite"

copper laurel

Buttons is already complete, it will not change to match an external package

solemn oyster

Not happening

iron pier

Ok, I didn't know this

solemn oyster

Kinda your fault for using a third-party package knowing there was an official one on the way, tbh

(And if you didn't know... I'm pretty sure support members have been pretty persistent about it, so it's a bit hard to believe)

rough arrow
copper laurel

No specific version but yes, a TS rewrite

tacit crypt

Additionally, am I right in that this implementation disallows concurrent requests (which is an acceptable drawback)?
Yes? Concurrent for different buckets, sequential for the bucket itself

if that makes sense

rough arrow

Yes I get what you mean. You cannot make concurrent requests that use the same handler

hushed sedge

Is v13 changing alot to break v12 code?

solemn oyster

Yes

hushed sedge

like, for example i believe message.channel.send() will break

:/

thats like - everywhere!

solemn oyster

For the better

hushed sedge

What is it getting changed to?

solemn oyster

Something more consistent

hushed sedge

okay.

:/

oak quail

its still staying message.channel.send, but it might stop working for you depending on how you're using it

hushed sedge

Ohhhh, i see.

oak quail

so is it gonna be user or author? (for emojis and stickers)

hushed sedge

what

oak quail

different conversation from earlier

hushed sedge

Ah.

wild flax

User def

outer raven
wild flax User def

I know the user object from messages is called author on the API's side but wouldn't it be better to call it user to make it consistent with everything else?

sharp wigeon

does the lib have some kind of utility function to check the language of a codeblock?

for instance

const num = 2;

the codeblock has js highlighting to it, so I was wondering if there was a function that would return the language of a codeblock that has syntax highlighting to it?

ofc I could just do this with some simple string manipulation, but was wondering

covert flint

No

sharp wigeon

ok was thinking of making a pr that would add something like a Util.containsCodeBlock() function, idk if it would be necessary tho

copper laurel
winged dust

Can d.js v12 no longer delete other people’s link previews?
I’m getting this error: DiscordAPIError: cannot edit a message authored by another user
msg.suppressEmbeds()
It’s been working okay for 10+ months but about a week or two ago it started giving errors and not deleting the previews

copper laurel

I've just opened a PR

Though thats a v13 issue, not v12

I'll try again on v12

Okay yes, I do get that

Fixed in master then, I guess

copper laurel

PR already open for that one

winged dust

Thanks!

sharp wigeon

Would a Util.codeblocks(text) be a necessary method? Essentially, the method would be used to get all the codeblocks of a message, which syntax it was highlighted with, and then the content inside it. It would return an Array<CodeBlockData> | null

Where CodeBlockData would be an interface like

interface CodeBlockData { 
content: string
highlightedWith: string: //I would add better types for this
}

And null would be returned if the text contains no codeblocks?

rough arrow

To be honest, that sounds more like something you would do yourself in your code.

There's no CodeBlockData from the Discord API at all

zenith oracle

Also why should it return null instead of an empty array if there are no code block

However I don't see any implementation of this. Why would djs use this?

clever spoke

couldnt people just use regex?

kindred yoke

yeah probably

copper laurel

Yeah, util methods don't really exist just for end users - they all have a specific purpose that the lib uses I think

sharp wigeon

Ah ok

rough glacier

Is there still a discussion about a deep comparison between registered and current ApplicationCommandData for automatically updating if there are spotted changes? Last i heard of it was #archive-library-discussion message or did it conclude that it should be left for the user to implement?

wild flax

The latter

We don’t do automatic stuff in this lib

copper laurel

The automatic updating would be up to the user to implement, but would an ApplicationCommand#isEqual(data: ApplicationCommandData) method make any sense to have?

ruby terrace

It would make sense to have, it would be hard to implement

outer raven
slate nacelle
outer raven

ok ty
btw does the name MessageFile for the typedef sound good?

outer raven

Shouldn't this say discord.js@dev on the master version? There's also links to the WIP guide so I think it would make sense

wild flax

We generally don’t modify the readme for dev

More changes to make when we release

outer raven
wild flax

Yeah but that will generally stay

The “this is still for stable” I can’t remember anymore

But probably shouldn’t be like that

Not that it matters rn, since it’s already like that

outer raven

I think at least that should be removed cuz stable will be v13 once it's out

but yeah fair enough

outer raven

@solemn oyster there aren't that many types to update but I can import them from discord-api-types and add them, want me to do that?

solemn oyster

Yeah

We should always use discord-api-types for raw objects

real jetty

Is there a reason why embeds field aren't in BaseMessageOptions ?

ornate topaz

it seems more that it's an oversight

since bots weren't allowed to send multiple embeds before, so those wouldn't be in MessageOptions, but would be in WebhookMessageOptions

real jetty

Oh a little PR to make 🙂

real jetty

But I don't know how to do it if someone want to "correct" it :/

sharp wigeon

aren't errors of type DiscordAPIError?

ornate topaz

?

loud jayBOT
remote wasp
real jetty

I was also wondering, why there is no BaseMessageOptions type in the index.d.ts file?
P.S. thanks shibi for the PR 🙂

zenith oracle

Because it's not needed probably

outer raven

can we change all collections that hold ApplicationCommand's to be mapped by their names instead of ids? Most of the time we know the name of a command and not it's id or at least this is the case whenever i need to access an ApplicationCommand

I know it's a sort of rule to map everything by it's id when it exists but I feel like this scenario would make sense

rough glacier

its not a great idea, especially when you can have a sharing name between guild and global ones, and although they're in different CommandManagers if someone decided to merge them somewhere, there would be a conflict

cobalt comet
outer raven
outer raven
cobalt comet

Plus, I would be assuming that you'd be storing some kind of per-server config if you're going to be deploying commands per guild instead of globally, otherwise there's gonna be a lot of commands getting overwritten repeatedly (on the assumption servers can effect the functionality of a bot based on slash command input).

zenith oracle

So when setting the status of the bot we can use the setStatus method (or the status property with setPresence) and the string passed must be online | idle | invisible | dnd (PresenceStatusData), but when accessing the status of a user, with <User>.presence.status, we can receive online | idle | offline | dnd (PresenceStatus). Why are these types different for the offline option? Why is called invisible in the first but offline in the second. Wouldn't it be better to use the same word for both?

unique axle

there is a semantic difference in offline and invisible
bots can not receive invisible for other users (so you can't discern it from offline)
bots can also not set offline, but just invisible

zenith oracle

Thonk

real jetty

like when someone says "how can I set the bot's status to invisible?" people supporting could trip up and say client.user.setStatus('offline')

bots can not receive invisible for other users (so you can't discern it from offline)
meaning if someone is offline but still on Discord, the client receives it?

unique axle

you receive both invisible and offline as offline, you can not receive invisible

bots known when you are offline and when you are "pretending" to be offline would be a terrible idea privacy wise, you only ever receive "offline".

and adapting the setStatus to accept offline would be a bad idea as well, semantically offline is - offline, not able to participate or read conversations, where as invisible means the offline indicator but still being able to interact with the app

naming two fundamentally different states the same would be a terrible design decision

real jetty

but invisible in PresenceStatusData?

oh wait I see now

real jetty
unique axle

i don't understand how this is so hard to discern
offline: you close discord
invisible: you set your status to offline
as a user you don't want people to know when you are offline vs. when you want to be shown as offline, so discord sends out "offline" even if you are "invisible"

outer raven

For some reason, User#system is always null is the user isn't part of the urgent message system instead of being false. This is due to the fact that system doesn't come in the User object from Discord and d.js checks for that. Should we change this so that it's declared as false instead of null?
A similar thing goes for User#bot where it's always false for partial users when it should be null because it's checked if typeof this.bot !== "boolean" and then a boolean is created from that value. Since this.bot is undefined, it becomes false. Should this be changed as well?

oak quail

didn't user#bot get a lot of changes to work properly? does it still not work?

outer raven

partials are the only scenario that I found where it doesn't work as expected

I have a fix for this so just let me know if I should open the PR for it or not

wild flax

iirc jake said any null/undefined/non-existent-key value that is supposed to be a boolean should default to false on those cases

if true isn't being sent

cc @unique axle

unique axle

yeptune

outer raven

yeah but I'd assume not for partials right?

wild flax

always

it being null has no meaning really

outer raven

because you don't know who the user is in a partial, so saying they're not a bot could be incorrect

outer raven
copper laurel

We don't know if it wasn't sent because it's partial or if it wasn't sent because it wasn't sent by Discord

We've have to start adding isPartial to all our constructors to know where/why the instance was being created and stuff.... which is eww

Thats the logic I couldnt remember hence directing the convo here

outer raven
copper laurel

exactly

So we set it to null, because if its missing, we dont know if its a partial for a system user or not

zenith oracle

What will happen after v13 is released? like, the master branch will work for a new version? Will be there to fix bugs and add feat to the actual v13? And what about djs next?

copper laurel

Nothing really changes, yeah. The stable discord.js version on npm just gets updated to v13. Bugs will get merged into stable in the form of minor or patch releases. Major breaking changes will wait for a v14.
djs-next has nothing to do directly with the version numbers, its a separate TypeScript rewrite.

zenith oracle

Okk, thanks!

outer raven

but not this

copper laurel

I believe so

oak quail

643945264868098049 and 669627189624307712 are the system users

oak quail
outer raven

Yeah so just by fetching their messages I can see that system is true for the first and not present for the other

outer raven
oak quail

ik what it is, that feature is powered by channel following

outer raven

ah alright

then when does the first user send messages?

i only got one when i enabled community on my server

oak quail

the discord system user only sends dms

the community updates system user is for whenever there's something for just one server

tacit crypt

I mean you can see in your screenshots that one has a SYSTEM badge, the other has a SERVER badge

outer raven

ye ye ik

oak quail

should be system without a checkmark

tacit crypt

The checkmark doesn't hurt

oak quail

looks like they broke it lol

yeah I mean it was that before

outer raven

but yeah so I don't think system being present or not depends on whether it's a partial or not so it would be safe to set it to false when it's not there

unless it's a partial

oak quail

so vlad wants me to add sticker.fetch but we disagree on how to implement it

so theres a /stickers/:id route (implemented in the djs pr as client.fetchSticker) and a /guilds/:id/stickers/:id route (not currently implemented as I didn't add a GuildStickerManager in the pr)

unique axle

i don't see how this is significant for discord.js library devlopment
(re: system)

oak quail

I assume that to get sticker.user, you need to use a guild id route, and you need the manage emojis permission (edit: has been confirmed)

vlad says that if the sticker is a guild sticker, djs should always try to fetch it with the guild id, and just error if the bot isn't in the server

imo it should just call client.fetchSticker(this.id), or the user can optionally choose to fetch with the guild id if they want

tacit crypt
oak quail

yeah

oak quail
outer raven

cuz if so, the typings need updating I think

zenith oracle

Iirc it's null when the interaction is in a DM, with the DM Channel not cached

outer raven
zenith oracle

I saw it

👍

copper laurel
outer raven

Which is actually how I found the bug

copper laurel

But it's a getter so it should have always been nullable

Yeah weird

outer raven

Yeah exactly but you can add it to that PR

vivid field

But it does include null?

Or am I missing something

slate nacelle
wicked rain

Whats the new activity type competing ?

clever spoke

competing in (whatever)

wicked rain

Nice that, ok bro

outer raven

btw why is it readonly?

ornate topaz

getter

copper laurel

Was there a reason we didnt make ApplicationCommand an extendable structure?

oak quail
winged dust

or is this feature discontinued

ornate topaz
loud jayBOT
copper laurel

This one also

Is the v13 fix

winged dust

will the fix come to v12 master?

clever spoke

v12 master?

copper laurel

v12 isnt master

It will come to master

Its up to the maintainers if we continue patching v12 or not

winged dust

ignore the "master" part of the question xd

copper laurel

lol, Crawl just merged the v13 fix

winged dust

niceee. is it also a v12 fix?

ornate topaz
winged dust

oh 😔

copper laurel

Theres different bugs and different reasons they happen in each version

The suppressEmbeds one is not related to editing out embeds

sharp wigeon

For discord js -next

Since I know it’s supposed to be vastly different from discord js

What are some ways it could be different? Like how could we create a new instance of client and stuff? I know kyra had some good ideas

ornate topaz

afaik -next doesn't even have any shape or form yet

copper laurel
kindred yoke

its going to be completely written in ts right?

nvm pins

sharp wigeon
copper laurel

But its not going to be "vastly different"

outer raven
wild flax

but we all missed it, and it pretty much broke all bots

lol

outer raven

oh shit

wild flax

No unrelated changes in PRs please, even if they seem like a no-brainer next time

👀

outer raven

yeah noted, sorry

I thought that was transforming the type into a string, not the other way around

copper laurel

That method is for getting the data to send to the API, so needs to be an int

outer raven

yeye got it now

On a completely unrelated note though: what do you guys think of merging sub command options into the main interaction's options? Let me explain:
When you run a command with a subcommand or subcommand group, the only option in the main collection will be the sub command that you ran, so if we manage to merge them all into the first there should be no issue (unless an option has the same name as one of the sub commands, which i don't know if it's possible)

copper laurel

merge how?

you still need to know which subcommand was run, are you proposing moving that property up to CommandInteraction#subcommandName or something

outer raven
copper laurel

I dont like the idea of breaking the hierarchy, no

outer raven

imo it made sense since you'll never have more than 1 option with sub commands and groups but sure then

copper laurel

hmmm actually yeah maybe

I think the part I'd find awkward is that if the subcommand is kept, you'd be getting it essentially like a boolean option I guess? Just checking that its... there, and then you know what the other options are?

outer raven

When you have multiple subcommands you'd be checking which one the user ran. I usually do this with interaction.options.find(o => o.type === "SUB_COMMAND")

honest barn

why is code option in #5918 being removed? there's no other clean way to do it without manually writing it into the string

outer raven

this seems clean enough to me

although you need a bunch of \ there

honest barn

yeah thats why code was very useful for me

outer raven

but why is adding \ an issue?

honest barn

its an issue when you're using the `` type of string

too many \ affects readability

outer raven

not that you would need to read a lot from a bunch of slashes

you can even put that in multiple lines if you prefer

ornate topaz

for the codeblock to be formatted properly, you need a single newline after language. no matter if you use ` or ', it has to be there

however, template strings (`) have a bonus of being able to be multiline themselves

wild flax

It just wont be a message option anymore

honest barn

oh then it's a good change

that would be allowed for any string right?

wild flax

yeah it will just be a function you wrap your string with

outer raven

will that be included in that PR?

wild flax

no

solemn oyster

We talked about adding that in /builders and then re-exporting in d.js

outer raven
wild flax

just open it if you think its useful

doesnt mean well accept it

outer raven

alright

copper laurel

If there's only one possible sub-command at a time, why make people find it as an option when it could be bumped up to a property though

I feel like you could have
interaction#commandName
interaction#subcommand
interaction#subcommandGroup
and then just the options or something

outer raven

so maybe remove it all together would be a good idea and much easier

actually nah even then I have no clue how to do this in a smart way since I'm not familiar with the way djs patches stuff, could you try to do this?

winged dust
frank turret

AFAIK, there are no additional releases for v12, so you'd have to install from dev. npm i discord.js@​​dev

copper laurel

Not fixed in v12, no

I dont think that issue relates to v12

oak quail

thats due to sending embeds

copper laurel

The suppressEmbeds bug is different to the empty embeds bug

sort of

It probably shouldnt be closed though

wild flax

Yeah we have no more releases for v12 planned.

solemn oyster

Are you gonna do the largest rebase ever before continuing a lot further? Or split the changes in smaller PRs?

vernal atlas

well i was at first going to rebase and continue, but now im thinking that splitting the changes is a good idea

wild flax

Yeah, smaller ones are also easier to review and merge

outer raven
slate nacelle

I wouldn't call the bot property being invalid expected behaivor.
Or at least hope it's not.

outer raven
slate nacelle

Patching with partial data should not invalidate the state of the object.

If you are not sure a change has to be made, make none.

outer raven
copper laurel

I think I'm missing exactly what you think is incorrect here. You kinda lost me right from the start when you told me you were re-assigning properties to things they aren't meant to be. The identifier on a custom emoji will be its name:id format, while on a unicode emoji, it will be the unicode. It's not ever meant to be the id, which yes, will be null on unicode emojis.

The ReactionEmoji class is kinda like a partial guild emoji, eg when the reaction came from a guild the bot isn't in so it can't get full data I think

yes, but it wont have an id if its unicode

If you need to store them in a database for whatever reason, const unique = emoji.id ?? emoji.identifier

Because it is the identifer at the time

Youre just misusing the property

Its not supposed to be a primary key for a database

loud jayBOT
wintry karma

Is there a reason for skipLibCheck being set to true in the tsconfig.json?
Would it make more sense to set that to false to catch errors in the typings?

solemn oyster

I have never tried but... I suppose we could add a stub augment

Something likets module '@discordjs/voice' { interface TheInterfaceWeRequireFromIt {} }

If the module is installed, then it'd merge an empty interface with the full one

That way we can use that interface from a module that's otherwise missing, without getting compiler errors

That must be inside declare module 'discord.js' {}, btw

cc: @tacit crypt @fringe temple

vivid field

Why do we even have @types/ws and discord-api-types as dependencies now, but not @discordjs/voice?

wintry karma

Typings are currently broken on master because there are interfaces imported, that don't exist in the used discord-api-types version.
And with skipLibCheck: true that could easily be missed.

vivid field

What interfaces exactly?

wintry karma 
typings/index.d.ts(142,5): error TS2305: Module '"discord-api-types/v8"' has no exported member 'APIActionRowComponent'.
typings/index.d.ts(147,5): error TS2305: Module '"discord-api-types/v8"' has no exported member 'APIMessageComponent'.
typings/index.d.ts(1634,12): error TS2416: Property 'options' in type 'ReactionCollector' is not assignable to the same property in base type 'Collector<string | `${bigint}`, MessageReaction>'.
  Type 'ReactionCollectorOptions' is not assignable to type 'CollectorOptions<[MessageReaction]>'.
    Types of property 'filter' are incompatible.
      Type 'CollectorFilter<[MessageReaction, User]> | undefined' is not assignable to type 'CollectorFilter<[MessageReaction]> | undefined'.
        Type 'CollectorFilter<[MessageReaction, User]>' is not assignable to type 'CollectorFilter<[MessageReaction]>'.
typings/index.d.ts(1640,12): error TS2416: Property 'collect' in type 'ReactionCollector' is not assignable to the same property in base type 'Collector<string | `${bigint}`, MessageReaction>'.
  Type '(reaction: MessageReaction) => Promise<string | `${bigint}`>' is not assignable to type '(...args: any[]) => string | `${bigint}`'.
    Type 'Promise<string | `${bigint}`>' is not assignable to type 'string | `${bigint}`'.
      Type 'Promise<string | `${bigint}`>' is not assignable to type '`${bigint}`'.