#archive-library-discussion — Page 3

simple veldt Aug 10, 2020, 5:01 PM

#

Right so I'll make a pr

wild flax Aug 10, 2020, 5:01 PM

#

yes this is a bug

simple veldt Aug 10, 2020, 5:19 PM

#

Also I want to add backup support to my discord-backup package. I saw https://github.com/discordjs/discord.js/blob/master/src/managers/GuildChannelManager.js line 65 and I thought we couldn't create news channel x)
Should I add news to the lost?

#

*list

ornate topaz Aug 10, 2020, 7:16 PM

#

What list

vernal atlas Aug 10, 2020, 7:18 PM

#

@param {string} [options.type='text'] The type of the new channel, either text, voice, or category

#

that list

#

maybe create a typedef for ChannelType

waxen swan Aug 10, 2020, 7:19 PM

#

Is there anywhere we can talk about upcoming features or thoughts? :3

ornate topaz Aug 10, 2020, 7:21 PM

#

Whether a new api feature gets added to d.js depends purely if it gets released and documented by discord.

#

If that's what you're asking about

waxen swan Aug 10, 2020, 7:21 PM

#

It was that I was wondering about screen sharing abilities now that they're available on servers. Just a thought 🙂

#

Unless it is and im blind 🤔

ornate topaz Aug 10, 2020, 7:22 PM

#

Bots do not have access to streaming

waxen swan Aug 10, 2020, 7:22 PM

#

Only voice streaming yeah :/

ornate topaz Aug 10, 2020, 7:22 PM

#

And again. We don't decide if they can or not. Discord does.

waxen swan Aug 10, 2020, 7:22 PM

#

Yep

#

Anyway thanks for your time I will go as I'm not wanted here

vivid field Aug 10, 2020, 10:23 PM

#

why does the .id property not exist on Base directly?

vernal atlas Aug 11, 2020, 1:52 AM

#

my guess is it was originally for documentation purposes

simple veldt Aug 11, 2020, 6:38 AM

#

@ornate topaz add news here:

@param {string} [options.type='text'] The type of the new channel, either text, news, voice, or category

real jetty Aug 12, 2020, 2:37 AM

#

Why is message.mentions.members nullable (i assume that's what the ? is albeit im pretty stupid at this sort of thing) when message.mentions.users isn't? they're very similar and I would think that in a message where no one is mentioned both wouldn't exist, correct?

#

raven juniper Aug 12, 2020, 2:46 AM

#

I believe the logic is that users can be mentioned in DMs but members cannot be, as there is no guild thus no GuildMembers

vernal atlas Aug 12, 2020, 3:42 AM

#

yeah nullable in the case of a dm message

#

https://github.com/discordjs/discord.js/blob/03580b23a3a66ea3be773ecd4c8fe82487e3e17c/src/structures/MessageMentions.js#L138

real jetty Aug 12, 2020, 3:47 AM

#

ah, ty

simple veldt Aug 12, 2020, 10:26 AM

#

setExplicitContent filter doesn't accept numbers

#

using ts

#

should this be fixed or it doesn't matter as we can use string levels

vernal atlas Aug 12, 2020, 10:43 AM

#

yes the typings should be updated for that, the method does accept numbers

wild flax Aug 12, 2020, 11:04 AM

#

Using TS, not using numbers here is actually preferred, simply from a type-safe standpoint.

#

But I guess we can loosen the type.

vernal atlas Aug 12, 2020, 2:59 PM

#

strictly speaking couldnt the typings be altered to support only the numbers that match a verification level?

snow crypt Aug 13, 2020, 7:07 AM

#

you could use a numeric enum for convenience with that

oak quail Aug 14, 2020, 2:45 AM

#

https://advaith.is-in-hyper.space/60e73c8f8f.png

#

ah its because d.js drops it if the member is uncached

#

and members arent cached without sending messages if presence is disabled

timber plover Aug 14, 2020, 6:03 PM

#

Would the discord.js maintainers be open to a PR that added optional best-effort pre-emptive enforcement of the global rate limit? That is, add an option to the Client (constructor and/or setter as appropriate) to limit to sending 50 (or a configurable other number) of requests per second?
I say best-effort since obviously the global rate limit is based on timing on Discord's end, so without them providing additional headers, we can't be 100% sure we won't hit the limit, but restricting to 50 outbound requests per second should at least reduce the rate you hit it without really adding any latency above what Discord itself enforces.

#

For context, I have a bot using d.js that does enough things in enough servers that it hits the global limit several times per day, and during our peak usage times, we've gotten hit with the temporary block for 10k invalid requests in 10min. Rather than wrapping every single call to d.js on our end, it seems like it would be a lot easier to add global rate limiting inside d.js. I'd probably also add invalid request tracking at the same time, and have a warning event emitted at some configurable threshold (default 9.5k) so users could have an easy way of noticing the limit coming and avoiding the hour-ish long block it triggers

lofty birch Aug 14, 2020, 6:09 PM

#

there is a ratelimit event that you can use

tacit crypt Aug 14, 2020, 6:11 PM

#

I mean... if you're hitting that limit/s with good reasons, it'd be time to contact support to have your limit raised anyways

#

Also, reminder that the 10k invalid request ban does NOT apply to just 429's, but also 403/401, something you can handle on your end 👍

timber plover Aug 14, 2020, 6:16 PM

#

The ratelimit event now is raised when the responses come back. If there are 10 responses "in-flight" when you hit the rate limit, that means you get 10 429s.
As for contacting support to have our limit raised, our bot is in several hundred guilds, and the docs say to ask about an increase if you are in 200k+ guilds. IMO we are hitting the limit for good reason when we hit it, but I'm guessing Discord doesn't care about my opinion for a "small" bot.

#

And yes, I'm well aware of what the 10k invalid request limit includes - I already instrumented my copy of discord.js to log all the invalid requests, their types, and whatever cause info is available 🙂

tacit crypt Aug 14, 2020, 6:17 PM

#

That's wrong. The ratelimit event we emit means we're currently handling a ratelimit (aka pausing X amount of time before continuing, something we do for any request that 429's). We also handle the global limit when hit by basically blocking all outgoing requests for however long Discord tells us to

timber plover Aug 14, 2020, 6:19 PM

#

right - I want to block when necessary, before discord tells us to. Since the existing headers provided by discord don't provide any info on the global limit until you hit it, they aren't relevant here

#

and as for the ratelimit event - you emit it when you are handling a rate limit. But if I send 60 requests, all 60 might go out before discord responds to any of them, which means the global rate limit will kick in on discord's end when it receives the first 50, and the last 60 are over the limit. Same thing for a non-global limit - if 10 requests go out to an endpoint with a limit of 5, then you're going to get 5 successes and 5 429s coming back

tacit crypt Aug 14, 2020, 6:21 PM

#

That is not true

#

Requests on the same ROUTES are sequentially sent

#

So only after the current one finishes do we go to the next one

#

This is the default (and only option in v12) and the "sequential" in v11 (also default, the other being burst, however you shouldn't use it)

timber plover Aug 14, 2020, 6:22 PM

#

ok fair point, I have been staring at the global part so deeply I forgot how the per-route thing is handled

#

although technically that assumes that the mapping to routes that you have is identical to the buckets that discord uses for the actual rate limit determination on the backend, since

"Per-route" rate limits may be shared across multiple, similar-use routes

tacit crypt Aug 14, 2020, 6:24 PM

#

We do that, except not with their relatively-but-not-that-recently added bucket hashes. We currently group by the 3 major parameters discord has (guild, channel and webhook ID to be exact)

timber plover Aug 14, 2020, 6:25 PM

#

sure, I've looked at the source code 🙂

tacit crypt Aug 14, 2020, 6:25 PM

#

Also, the only cases of there POSSIBLY being 429s is with Discord and their reaction buckets IIRC, there's an issue on their api docs repo

timber plover Aug 14, 2020, 6:26 PM

#

but regardless - the global limit is what I really care about here (well, also the fact that there is an undocumented 2/10 minutes channel rename sublimit on the channel edit route, but that's a whole other can of worms)

tacit crypt Aug 14, 2020, 6:26 PM

#

Back on track tho, I understand what you want, however...I dunno if that should be added (esp since giving a lot of control over rest to users...is one easy way to get api banned)

#

the fact that there is an undocumented 2/10 minutes channel rename sublimit on the channel edit route
It was announced in the ddevs server but sadly cannot be represented in the headers

#

Which also causes issues as it stands; you can still edit overwrites even tho you're ratelimited on name changes but as it stands, we don't have that edge case

timber plover Aug 14, 2020, 6:28 PM

#

yep, I discovered that once I was pointed through several other servers to ddevs 🤦

#

but not before I put a ticket in complaining about it being entirely undocumented

#

which I hold is fair since ddevs is at best quasi official

tacit crypt Aug 14, 2020, 6:28 PM

#

HA, you'd be surprised how much can be undocumented if we don't do it ASmeguFace

timber plover Aug 14, 2020, 6:28 PM

#

oh I'm sure 🙂

tacit crypt Aug 14, 2020, 6:29 PM

#

DDevs IS official, DAPI is the unofficially official or w/e (I think it's the official unofficial one)

timber plover Aug 14, 2020, 6:29 PM

#

since giving a lot of control over rest to users...is one easy way to get api banned
@tacit crypt not really sure what you mean by this. I don't want to give control to my bot end users, I want to give control to users of d.js

lofty beacon Aug 14, 2020, 6:30 PM

#

if blocking the request for invalid ones before it is sent is some how possible, that'd be very useful (if i undersotood corect)

timber plover Aug 14, 2020, 6:30 PM

#

so I don't know who you are concerend about being banned

tacit crypt Aug 14, 2020, 6:32 PM

#

You're an end user of our library, like me, and many here. The more control we give you over the rest dials (like we do with the restTimeoutOffset) the higher the chances someone will turn them wrong and well, suffer the consequences. ~~or someone turns them wayy down and then complain that d.js is sO sLoW~~

timber plover Aug 14, 2020, 6:33 PM

#

so don't make the threshold of 50/second tunable, just let it be off (default, as now) or on (best effort to not global rate limit)

tacit crypt Aug 14, 2020, 6:34 PM

#

What about those whose global limit is higher/lower

timber plover Aug 14, 2020, 6:34 PM

#

right - that's part of why my instinct was to make that configurable

#

but if you want to limit turnable knobs...

tacit crypt Aug 14, 2020, 6:34 PM

#

well, ok you can keep it off

#

Fair point

#

Regardless, I don't see why not but shrug
You should still probably try to contact support for a limit raise (guild limit aside, doesn't hurt to ask) but make sure your use case is actually not considered API abuse and the such

timber plover Aug 14, 2020, 6:35 PM

#

could also set the min global rate limit to 50 - I was under the impression that was the limit for everyone who hadn't gotten it explicitly raised, so if you try to set lower than that, you're probably making a mistake

tacit crypt Aug 14, 2020, 6:36 PM

#

However I do have to ask..what are you doing that causes you to actually hit 10k 429's in 10 minutes

timber plover Aug 14, 2020, 6:43 PM

#

that's a great question, and I don't have a super clear answer now tbh - the bot I work on is for Pokemon Go, and there is a weekly event that sees a pretty substantial uptick in active players and hence in active bot users. The past three weeks, we've gotten blocked sometime around the end of the east coast activity window for the event.
After last week, I added instrumentation to track all the HTTP responses, but all I know now is what happens during lower usage times. So far today, we've had 164 429s from the global rate limit, 94 429s from the channel rename limit (also working on that separately, but that's easy to do from my end), and a handful of other things. My assumption is that those just scale up dramatically during that weekly event window and cause us to hit the 10k in 10 minutes, but I won't know for sure until wednesay evening (or, ideally, never if I can make the necessary changes to not hit the limits before then)

lofty beacon Aug 14, 2020, 8:19 PM

#

Suggestion: Make the discord.js client cache only things you want it to cache. Discord.JS bots can take up loads of memory mostly because of the cache.

timber plover Aug 15, 2020, 2:35 AM

#

Update: on what we were doing to hit 10k 429s in 10 minutes for @tacit crypt:
We made some API calls in our unhandledRejection handler and in response to rateLimit events. Turns out this is a terrible terrible terrible idea. If we hit 50/s global rate limit and then attempted to handle an error, we called the API to attempt to resolve what the name of the channel that the error came from was. Obviously this in turn gets a 429, and the descent into madness begins. So no, we didn't get 10k 429s in 10 minutes, we got 12k 429s in about 40 seconds. Big yikes for us

So while I still want to submit a MR for preemptively handling the global rate limit (especially since I did about 95% of it before discovering what was going on), it is somewhat less important/urgent for my bot 😂

autumn talon Aug 15, 2020, 9:47 AM

#

Why does the typings are in index.d.ts and not directly into the declarations files ?

ornate topaz Aug 15, 2020, 10:01 AM

#

what exactly are you asking about

#

index.d.ts is a declaration file.
also, library is written in javascript, not in typescript

oak quail Aug 15, 2020, 1:04 PM

#

should probably be mentioned in the release notes that v12.3.0 updates the domain

ornate topaz Aug 15, 2020, 1:07 PM

#

what is the difference for the end user though

oak quail Aug 15, 2020, 1:07 PM

#

older versions may stop working soon

ornate topaz Aug 15, 2020, 1:07 PM

#

fair

autumn talon Aug 15, 2020, 1:44 PM

#

Yes, but why do not writing the library in ts ?

ornate topaz Aug 15, 2020, 1:44 PM

#

for the same reason it is not in java or c#

#

because it is javascript library

autumn talon Aug 15, 2020, 1:45 PM

#

typescript is typed js

ornate topaz Aug 15, 2020, 1:45 PM

#

which supports being used in typescript projects via the typings file

autumn talon Aug 15, 2020, 1:45 PM

#

Ok

raven juniper Aug 15, 2020, 1:45 PM

#

A future version of the library may be written in ts

ornate topaz Aug 15, 2020, 1:47 PM

#

also, you cannot just rename all files from .js to .ts and call it a day. typescript code is never ran, it's transpiled to javascript before it runs.

real jetty Aug 16, 2020, 10:33 AM

#

user.bot was changed so it is now always true or false and can't be null in 12.3.0, right?

#

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

vivid field Aug 16, 2020, 10:34 AM

#

yes, null was removed as a possible value in 12.3.1 but the docs didn't get updated accordingly

real jetty Aug 16, 2020, 10:34 AM

#

i would try and change that but im bad at PRs

#

dont even know which repo docs are on omegalul

vivid field Aug 16, 2020, 10:39 AM

#

they're on the default master branch together with the code (https://github.com/discordjs/discord.js/blob/master/src/structures/User.js#L46)

real jetty Aug 16, 2020, 10:40 AM

#

ah thanks

#

I believe this issue is also the case with system, correct? https://github.com/discordjs/discord.js/blob/master/src/structures/User.js#L76

oak quail Aug 16, 2020, 10:43 AM

#

.system hasnt been updated (although i mentioned in the issue that it should be)

#

the code for .system should have been updated but it wasnt

real jetty Aug 16, 2020, 10:43 AM

#

yeah i dont see why it wouldnt?

#

i have a feeling that opening an entirely new pr just to remove a single question mark is a bit over the top so im assuming youre just going to commit to the existing one

#

but its already been merged 🤔 anyway ill prob leave this up to you and vaporox you guys are good at these tings

empty viper Aug 16, 2020, 11:44 AM

#

Also in the Guild class documentation, some of the descriptions of fields speak about de the removed PUBLIC flag

oak quail Aug 16, 2020, 11:50 AM

#

@empty viper where? i dont see any

#

im pretty sure i updated all of them

empty viper Aug 16, 2020, 11:55 AM

#

I supposed that it has been fixed in the meantime, the one that I had noticed have not the mistake anymore

oak quail Aug 16, 2020, 12:09 PM

#

i mean, nothing has changed since you sent that bloblul

empty viper Aug 16, 2020, 12:10 PM

#

I found the errors like 20 minutes after the release of the 12.3.0 and now it's all fixed 🤔

oak quail Aug 16, 2020, 12:11 PM

#

maybe it loaded the wrong docs version? because it was changed in 12.3.0

empty viper Aug 16, 2020, 12:12 PM

#

Yeah it's possible

haughty venture Aug 16, 2020, 5:07 PM

#

Does someone know how do I test the library?

ornate topaz Aug 16, 2020, 5:15 PM

#

what do you mean by "test"

haughty venture Aug 16, 2020, 5:42 PM

#

run the code on test/. all around there's mention to require auth.js, on those files, but there's nothing like this

wild flax Aug 16, 2020, 5:52 PM

#

Just look what it imports from that file and you can reconstruct it.

#

Its not really much used, those files are just to debug certain things, not to completely test everything

haughty venture Aug 16, 2020, 6:31 PM

#

sure, thank you!

indigo oar Aug 16, 2020, 11:23 PM

#

I guess there should be easier ways to get information from a User like If he has nitro

#

without OAuth2

unique axle Aug 17, 2020, 12:41 AM

#

I guess there should be easier ways to get information from a User like If he has nitro
There is not, there shouldn't be. We can only implement what the discord API offers
flags https://discord.com/developers/docs/resources/user#user-object-user-flags do not include that information. premium type https://discord.com/developers/docs/resources/user#user-object-premium-types is only accessible through an authorized user request - oauth2

summary above, discussion around the topic swepped for less clutter sweep

real jetty Aug 17, 2020, 4:24 AM

#

https://github.com/discordjs/discord.js/blob/stable/src/structures/User.js#L74 when checking if a user is system would it make sense to set this.system to null if the data value isnt a boolean?

#

or default to false potentially?

frank turret Aug 17, 2020, 4:30 AM

#

To your second point of defaulting to false, if it’s coerced to a Boolean and is null, wouldn’t it already be turned to false (cause null is falsey)?

real jetty Aug 17, 2020, 4:36 AM

#

it doesnt check if it's falsy, it just checks if it exists in the data, correct?

#

otherwise nothing is set and it remains undefined

copper laurel Aug 17, 2020, 4:36 AM

#

Yeah

#

Is that an issue?

real jetty Aug 17, 2020, 4:36 AM

#

i don't know, you tell me?

#

I just think null would make more sense

copper laurel Aug 17, 2020, 4:36 AM

#

I mean, I dont believe that it is

real jetty Aug 17, 2020, 4:36 AM

#

and be more consistent with other properties

copper laurel Aug 17, 2020, 4:37 AM

#

But it is consistent with the others

#

Nearly everything in _patch has an if in data check

real jetty Aug 17, 2020, 4:38 AM

#

huh - i always thought by default it turned to null

#

my bad if thats not the case

oak quail Aug 17, 2020, 6:15 AM

#

discord sends bot and system the same way: either undefined or true

#

discord.js now makes bot true or false

#

but system hasnt been updates, so its inconsistent

real jetty Aug 17, 2020, 6:27 AM

#

https://ptb.discordapp.com/channels/222078108977594368/682166281826598932/744773648547577966 so this would be valid? @oak quail

#

looking at documentation you're right, they come back identical https://gyazo.com/e96678c3ef53c36bb39ec6a31cb4a5d9

oak quail Aug 17, 2020, 6:32 AM

#

we're defaulting bot to false, so we should also default system to false, for consistency

real jetty Aug 17, 2020, 6:32 AM

#

yeah that makes sense - i'll probably pr it when i can

vernal atlas Aug 17, 2020, 12:34 PM

#

i didnt set system to null in my PR because i assumed it would've been a breaking change

#

although now that i think about it the docs say it should be null

#

typings say undefined docs say ?boolean which is boolean | null

snow crypt Aug 17, 2020, 1:35 PM

#

this seems to be happening on every pr sent

vernal atlas Aug 17, 2020, 1:52 PM

#

yeah i noticed that the other day, i suppose someone can make a PR to ignore it or make a getter that is this.client.options.http.api

lofty beacon Aug 17, 2020, 5:16 PM

#

Suggestion:

When calling the fetch method, you can pass in an argument as false and It won't cache whatevrer you fetch yet it will return it.

raven juniper Aug 17, 2020, 5:16 PM

#

that's already a feature on some, if not all, of the fetch methods

#

?docs client.users.fetch

rich iglooBOT Aug 17, 2020, 5:16 PM

#

raven juniper Aug 17, 2020, 5:17 PM

#

although on other managers (roles, channels, etc) i don't think it's there, but they're usually assumed to be cached anyway

lofty beacon Aug 17, 2020, 8:26 PM

#

thanks! I might work on a PR fo it tomorrow.

ornate topaz Aug 17, 2020, 8:41 PM

#

on what exactly

lofty beacon Aug 17, 2020, 8:44 PM

#

no caching of other collections for the fetch method

ornate topaz Aug 17, 2020, 8:45 PM

#

but why? library assumes that it's always there (and it's actually always there until you forcefully remove it)

#

due to that there is no reason to specifically fetch role or a channel

lofty beacon Aug 17, 2020, 8:54 PM

#

oh ok. nvm

plucky jay Aug 17, 2020, 10:21 PM

#

In the typings, should StringResolvable be string | number | boolean | bigint | symbol | any[] (primitives except undefined and null + any[]) instead of any? If content is an object in TextBasedChannel#send, Message#reply etc., it gets used as the options and won't be sent: https://github.com/discordjs/discord.js/blob/master/src/structures/APIMessage.js#L342-L345

Currently, there are no editor suggestions for MessageOptions in TextBasedChannel#send, Message#reply etc. because anything can be used as content. It also means that typos such as channel.send({embed: {tilte: 'Title'}}) do not get reported because it's still a StringResolvable.

I know this would be a breaking change, which is why I'm discussing it here.

copper laurel Aug 17, 2020, 10:38 PM

#

Typings arent breaking changes, assuming thats all you plan to change

vernal atlas Aug 17, 2020, 11:36 PM

#

i think it would make more sense to change what the first parameter for TextChannel.send is in the typings rather than the actual StringResolvable type

plucky jay Aug 17, 2020, 11:37 PM

#

Yeah true because StringResolvable is also used in Util.resolveString

vernal atlas Aug 17, 2020, 11:37 PM

#

yeah

#

anything with a typeof of object (aside from an array) is treated as MessageOptions | MessageAdditions

#

if no options are provided through the second argument*

real jetty Aug 18, 2020, 10:10 AM

#

https://github.com/discordjs/discord.js/blob/stable/src/structures/BaseGuildEmoji.js#L47 discord states that available is an optional property and the lib doesn't consistently set it as a boolean so would the jsdoc for this not be correct?

#

it appears that available can be undefined from what ive seen

oak quail Aug 18, 2020, 10:50 AM

#

i mean

#

honestly there are probably some mistakes in discord's docs for whether properties are optional or not

real jetty Aug 18, 2020, 10:51 AM

#

theres no harm in making it constantly boolean value

#

even if there is no mistake it doesnt effect anyone

#

but if it is optional it defaults to false

#

i too would not be surprised if there were mistakes in discord's documentation

vernal atlas Aug 19, 2020, 1:11 PM

#

those properties seem to be sent even if they are false

#

https://paste.nomsy.net/unorehalot

vernal atlas Aug 19, 2020, 1:34 PM

#

the docs (typings too probably) would also need updating for Emoji#animated and Emoji#id

#

animated isnt documented as possibly being undefined, and id is marked as ?string which implies null, but its infact undefined

#

should Activity.syncID be documented, its the track ID for a spotify status, except syncID doesnt really give that away so maybe under a different name?

snow crypt Aug 19, 2020, 1:54 PM

#

This appears on every PR

#

This also happens when running npm test: https://hastebin.com/pasageseqi.coffeescript

vernal atlas Aug 19, 2020, 2:25 PM

#

#archive-library-discussion message

snow crypt Aug 19, 2020, 2:47 PM

#

oh i see

#

thing is

#

when i run npm test it doesn't even pop up

#

but that might be because of errors because of prettier/prettier

#

which i have no idea why they don't show on GH

vernal atlas Aug 20, 2020, 1:45 AM

#

there seems to be an issue with Util.mergeDefault (https://github.com/discordjs/discord.js/blob/05c9e301639848121dda27ab1f8a643f465be502/src/util/Util.js#L286) that i can't seem to trace, but it seems to cause a new client with the options { http: { api: { version: 8 } } }; to freak out, and once merged causes client.options.http to be https://paste.nomsy.net/ulediduwot

#

oh wait did i just

#

k i facepalmed there, api is meant to be a string

#

but is there still an issue there? doesnt seem like its meant to iterate over the string

real jetty Aug 20, 2020, 1:48 AM

#

Hey sugden can you read my comment to your PR review when you can, thanks PES_Heart

vernal atlas Aug 20, 2020, 1:49 AM

#

sure

#

regarding that pr review comment ^ are the jsdocs incorrect for properties such as Guild#approximateMemberCount? as its documented as ?type rather than type|undefined, because ?type indicates type|null as per https://jsdoc.app/tags-type.html

real jetty Aug 20, 2020, 1:56 AM

#

if thats the case then a lot of things are wrong in djs

#

such as <Guild>.widgetChannelID

#

and Guild#widgetEnabled
you get the picture, this is the case with a lot of properties

copper laurel Aug 20, 2020, 1:59 AM

#

?type should indicate that something is nullable. We shouldn't have any documented properties be potentially undefined imo

real jetty Aug 20, 2020, 1:59 AM

#

i agree that if something isn't sent by api it should be nulled but that's breaking
at this point the best solution is probably just to go with ?type even for undefined properties

#

unless we change literally every single one to type|undefined

vernal atlas Aug 20, 2020, 2:00 AM

#

?type should indicate that something is nullable. We shouldn't have any documented properties be potentially undefined imo

yeah thats what i think would be the best approach, except it would be a breaking change to fix that accross the board

copper laurel Aug 20, 2020, 2:00 AM

#

v13

vernal atlas Aug 20, 2020, 2:01 AM

#

well for v13 / djs-next obviously yea, but as for v12 at the moment

real jetty Aug 20, 2020, 2:01 AM

#

for now i reckon just making the JSDoc consistently wrong rather than occasionally right and inconsistent is better

#

because this is the case for pretty much every property which can be undefined

vernal atlas Aug 20, 2020, 2:01 AM

#

i think the jsdoc is a bit of a mess

real jetty Aug 20, 2020, 2:02 AM

#

the approach that i take in the pr is the best we can do for now afaik

vernal atlas Aug 20, 2020, 2:02 AM

#

types are correct for the most part i think but its just the little parts like this

copper laurel Aug 20, 2020, 2:02 AM

#

Yeah but I think that like

#

?type will do, it indicates that it might not be there

real jetty Aug 20, 2020, 2:03 AM

#

yeah exactly

#

having 2 properties typed correctly and 100 not is just more confusing

#

i resolved the conversation since its pretty clear we can't make the changes you suggested sugden, at least not for now

vernal atlas Aug 20, 2020, 2:05 AM

#

yeah for now it cant be done, maybe if this is an issue it can be implemented with another PR

paper furnace Aug 20, 2020, 10:28 AM

#

can you explain me the choice which lead to te poorly designed partials feature?

#

why we can't have simply an event like raw previously dedicated to partial instead of dealing with them for each declared listener ? this is completely insane

#

while writing this part of the doc, nobody thinked: oh, finally it's maybe a bad design?

WARNING

Partial structures are enabled globally. You can not make them work for only a certain event or cache and you very likely need to adapt other parts of your code that are accessing data from the relevant caches. All caches holding the respective structure type might return partials as well!

radiant glacier Aug 20, 2020, 10:33 AM

#

you can still use the raw event

#

in v12

paper furnace Aug 20, 2020, 10:33 AM

#

nope, my IDE tell me this event don't exists anymore (using typescript)

#

maybe this event is missing into the typedef

#

the in fact partial would be incredibly more usefull if we can bind then to a custom event instead of using them globally like that

radiant glacier Aug 20, 2020, 10:38 AM

#

well it does @paper furnace

paper furnace Aug 20, 2020, 10:38 AM

#

mhhh really weird

lofty birch Aug 20, 2020, 10:38 AM

#

it's not shown in the typings as you are encouraged not to use it

#

same as v11, wasn't shown there either

#

and partials are very helpful, it's as simple as adding if(<x>.partial) await <x>.fetch(); wherever a partial may occur

vernal atlas Aug 20, 2020, 11:28 AM

#

raw is undocumented, because it emits the raw websocket data

slate nacelle Aug 20, 2020, 11:36 AM

#

https://discord.js.org/#/docs/main/stable/class/WebSocketManager emits raw dispatches, see the info tag

cold basin Aug 20, 2020, 11:41 AM

#

would it be handy to add a guild property to DMchannels

#

which is actually the user

slate nacelle Aug 20, 2020, 11:41 AM

#

There is recipient, isn't there?

cold basin Aug 20, 2020, 11:41 AM

#

so people could just use channel.guild.id

#

and dont keep asking

real jetty Aug 20, 2020, 11:42 AM

#

its easier to tell them guild is null in dms

#

because it actually makes sense

cold basin Aug 20, 2020, 11:42 AM

#

¯_(ツ)_/¯

ornate topaz Aug 20, 2020, 11:42 AM

#

no, because guild would no longer mean guild

cold basin Aug 20, 2020, 11:42 AM

#

thats true

ornate topaz Aug 20, 2020, 11:43 AM

#

so you actually could not do message.guild.roles or anything similiar

real jetty Aug 20, 2020, 11:43 AM

#

that just makes things even more confusing

cold basin Aug 20, 2020, 11:43 AM

#

yeah ok fair enough

oak quail Aug 21, 2020, 3:16 AM

#

honestly, imo its ok (but a bad idea) to not document the raw event, but its really dumb to not put it in typings

unique axle Aug 21, 2020, 8:02 AM

#

in my opinion the typings are there to reflect the public API of the library, "raw" is simply not.
the exact same functionality - properly documented https://discord.js.org/#/docs/main/stable/class/WebSocketManager and typed can be achieved through <Client>.ws.on("evt", cb)

oak quail Aug 21, 2020, 8:54 AM

#

imo the typings should cover the entire library

real jetty Aug 21, 2020, 9:19 AM

#

yes but thats not a public aspect of the library

haughty tide Aug 21, 2020, 10:10 AM

#

Why don't you create a separate typings file and point somewhere that this file exists for those who want to use the whole power of the library?

ornate topaz Aug 21, 2020, 10:15 AM

#

"whole power"? you mean "unrealiable power that can change at any point without any warnings, as its undocumented"?

copper laurel Aug 21, 2020, 2:32 PM

#

Thats... also exactly what it has
https://github.com/discordjs/discord.js/blob/master/typings/index.d.ts

tacit crypt Aug 21, 2020, 2:33 PM

#

I mean, we can type the raw event as "unknown" so it's up to you to either type it or use a typings library for it shrug

raw is this middle-ground between undocumented and documented, it's a slightly more uh.. raw than what Souji linked, and what you should most likely use (cause raw includes things like the sequence and op and stuff, which for most is really NOT needed, however YMMV)

fallen arrow Aug 21, 2020, 10:18 PM

#

i have a question about TextChannel#startTyping, from what i understand it was built so the promise resolves once the bot finishes typing, however it seems a little counter-intuitive considering all other api methods resolve as soon as the request returns a response. Doesnt this prevent the user from properly awaiting it for example at the beginning of a command?

#

for example, you cannot do the following because startTyping will never resolve ```js
await message.channel.startTyping();
await message.channel.send("test");

unique axle Aug 21, 2020, 10:58 PM

#

correct, the promise will resolve once the typing has ended.
correct, awaiting it like this will wait for the typing to end before continuing, consequently never continuing

snow crypt Aug 21, 2020, 11:03 PM

#

can we add an option to not wait for stopping to type?

fallen arrow Aug 21, 2020, 11:06 PM

#

currently the only alternative is not awaiting, which introduces a possible race condition

tacit crypt Aug 21, 2020, 11:07 PM

#

IIrc you can pass a count too

snow crypt Aug 21, 2020, 11:07 PM

#

?docs textchannel.starttyping

rich iglooBOT Aug 21, 2020, 11:07 PM

#

unique axle Aug 21, 2020, 11:07 PM

#

i don't really see the use case tbh, and passing a count does not change the behaviour of the returned promise

tacit crypt Aug 21, 2020, 11:07 PM

#

well, passing 1 WILL make it return

snow crypt Aug 21, 2020, 11:07 PM

#

but also end typing

unique axle Aug 21, 2020, 11:09 PM

#

if you pass 1 the exact same happens, it stars the typing indicator and the promise resolves when the adequate count (1) of stopTyping has been called

snow crypt Aug 21, 2020, 11:10 PM

#

well, what about the option idea?

fallen arrow Aug 21, 2020, 11:10 PM

#

the use case is being able to ensure the typing indicator has started before proceeding with the command, otherwise there is a possible race condition where the command's response will be received by the api before the typing indicator, which makes the bot send the response first and start typing afterwards

tacit crypt Aug 21, 2020, 11:12 PM

#

unless someone PRs a sendTyping (which is just await this.client.api.channels(this.id).typing.post())... Nothing stops you from using the raw API

fallen arrow Aug 21, 2020, 11:13 PM

#

sure, there is always a workaround

vernal atlas Aug 22, 2020, 12:59 AM

#

wouldn't be too difficult to perhaps create a function that listens to the typingStart event

#

for yourself - not in the library

#

but perhaps a wait boolean parameter for the sendTyping function could work?

#

which would of course determine if the promise should wait until.the typing has finished

#

does the count parameter even... work? i don't see it being decremented anywhere in the startTyping function and testing a startTyping(1) doesnt seem to resolve

#

ok yeah im either stupid or the count parameter doesnt even do what its supposed to

real jetty Aug 22, 2020, 6:18 AM

#

Maybe adding an actual error for when you try to ban a guildmember while not providing an object as options would be nice 🤔, for now it gives an Internal Server Error which gives the user no information

#

https://ptb.discordapp.com/channels/222078108977594368/222078374472843266/746602660500602910 - people have asked for help on this on three occasions today

oak quail Aug 22, 2020, 7:54 AM

#

might be a change due to the new update, as djs updated to the new preferred discord behavior which is iirc sending a json object instead of query params

copper laurel Aug 22, 2020, 9:29 AM

#

Yeah, the change mentioned above resulted in the API returning an error when invalid data is passed instead of... whatever it did before, possibly resolving a ban but not passing the reason
The error would be the same as the TypeError for Message#delete
I agree the response isn't meaningful to what they did wrong, but having said that, if they bother to read the docs and use the ban method correctly it won't be encountered

vernal atlas Aug 23, 2020, 2:24 AM

#

should Client#generateInvite have options for other query parameters for the invite other than permissions? eg preselcting a guild, redirect URI etc

#

also bump #archive-library-discussion message 👀

unique axle Aug 23, 2020, 7:49 AM

#

i'm not too sure what you mean there with the count parameter not working?
.startTyping(1) -> .stopTyping()
.startTyping(2) -> .stopTyping() x2
.startTyping(3) -> .stopTyping() x3
and so on. i tested that up to 5 the other day, could not see any misbehaviour in the counts

discord.js heavily diverges from how typing works api wise, we keep the typing indicator alive as long as not all typing "instances" are resolved (the count is decremented through stopTyping to the point where none are left) and sending a message does not resolve the call, but has to be done manually.

and as was pointed out above you can't await the start call as is right now, since it resolves only after the typing is fully resolves (has stopped)

unique axle Aug 23, 2020, 12:13 PM

#

should Client#generateInvite have options for other query parameters for the invite other than permissions? eg preselcting a guild, redirect URI etc
https://discord.com/developers/docs/topics/oauth2#bot-authorization-flow sure, the redirect is really just useful for the non-invite generation, unless i'm missing something?
You'll also notice the absence of response_type and redirect_uri. Bot authorization does not require these parameters because there is no need to retrieve the user's access token.
Since all are optional probably best with PermissionResolvable|object and deprecating the first.

vernal atlas Aug 23, 2020, 2:17 PM

#

sweet

#

so do you have to actually call stopTyping?

#

i was under the impression the route was called count times, and resolved on the final call

snow crypt Aug 23, 2020, 5:22 PM

#

what are the marked ones for?

ornate topaz Aug 23, 2020, 5:26 PM

#

https://sucks-to-b.eu/YcsxQJ.png https://sucks-to-b.eu/iSjJUP.png https://sucks-to-b.eu/4ODuti.png https://sucks-to-b.eu/SsOopj.png https://sucks-to-b.eu/RkjThx.png

wild flax Aug 23, 2020, 6:12 PM

#

Those are all legacy and we’ll prolly not use them anymore except for v12 @snow crypt

snow crypt Aug 23, 2020, 6:19 PM

#

so is the current repo gonna move to a legacy repo when next goes live?

#

next being the typescript one

wild flax Aug 23, 2020, 6:35 PM

#

no

#

next will be merged into the current repo

#

but the tags/labels won't be the same most likely

velvet tide Aug 24, 2020, 7:13 PM

#

so i realized the docs and typings dont have syncID for the Activity class. I decided I will make a PR because it'll be useful. What exactly should I put for syncID's jsdoc comment?
Can anyone provide input on this? i did some further digging and found this PR related to it https://github.com/discordjs/discord.js/pull/2314 with the commit message spotify stuff. So should I put the comment as The ID of the Spotify track?

#

should Activity.syncID be documented, its the track ID for a spotify status, except syncID doesnt really give that away so maybe under a different name?
@vernal atlas my apologies for pinging you, but i'm in the same boat as you are and hoping to get a conversation out of this as well

velvet tide Aug 24, 2020, 9:28 PM

#

so it seems the reason its not documented is because Discord API docs doesn't even document Activity#sync_id because that's their intention? ( https://github.com/discord/discord-api-docs/issues/1239#issuecomment-563425694 ) but I'm also reading this issue comment ( https://github.com/discord/discord-api-docs/issues/545#issuecomment-370604860 ) that sort of backs the previous one, but it's been 2.5 years since that comment has been made and I believe the Spotify integration has matured since then? Someone even questioned the relevancy of the property here ( https://github.com/discord/discord-api-docs/pull/436#issuecomment-349790557 )

So overall, does this go up one level higher to the Discord team to consider adding this? For now, I ended up extending the Activity interface myself and defining that missing property. I'm in limbo on it in general for submitting an issue to discord.js, but potentially the Discord API docs repo instead? Thoughts?

unique axle Aug 24, 2020, 9:34 PM

#

undocumented fields might change name or functionality at any point without further notice. due to that discord.js just implements (and accepts PRs for) upstream (discord-api-docs) documented fields. If you get them to document it it's relatively surely going to be added.

* we can't promise semantic versioning if we introduce a change that might result in breaking functionality, discord mostly doesn't break documented fields

velvet tide Aug 24, 2020, 11:02 PM

#

I see. I think I'll go upstream with this. Thanks for the input @unique axle

glad sparrow Aug 25, 2020, 10:28 AM

#

what will Collector use if there is no time (in collector options)

unique axle Aug 25, 2020, 10:29 AM

#

this is not a support channel, whatever other ending criteria you give it, if none - none

vernal atlas Aug 25, 2020, 2:04 PM

#

https://github.com/NotSugden/yeetcord.js/blob/patch-16/src/structures/NewsChannel.js#L3-L9 NewsChannel doesn't seem to extend any custom TextChannel class, is it intended to be that way or is it a bug? (the same applies for ClientPresence, https://github.com/NotSugden/yeetcord.js/blob/patch-16/src/structures/ClientPresence.js#L3-L8)

dapper leaf Aug 25, 2020, 3:35 PM

#

are there any plans of porting discord js to deno in the future many people have made their own attempts but they lose support in a month or 2

unique axle Aug 25, 2020, 3:38 PM

#

no current plans, has not been discussed - might become easier after the planned typescript rewrite or be considered during it

dapper leaf Aug 25, 2020, 3:39 PM

#

ok

copper laurel Aug 26, 2020, 3:10 AM

#

@vernal atlas not sure what you're pointing out here honestly - what are you suggesting it should extend?

vernal atlas Aug 26, 2020, 3:12 AM

#

you can extend the classes Presence and TextChannel via Structures.extend, however ClientPresnce and NewsChannel extend those classes (respectively), but they don't extend the custom-made extended class

#

a bit like how ClientUser extends User

copper laurel Aug 26, 2020, 3:13 AM

#

Ahh gotcha. So you're suggesting they should extend the extended Structure, if that's even possible

vernal atlas Aug 26, 2020, 3:13 AM

#

the ClientUser export is a getter so it can properly get the User class from Structures.get

#

yea

vernal atlas Aug 26, 2020, 3:27 PM

#

well

#

all channel classes extend Channel

lofty beacon Aug 27, 2020, 8:40 AM

#

I had a look through the events on the client and there isn't one for when messages are swept, so could that be added?

This would also work when calling client.sweepMessages() along with the automated interval system.

real jetty Aug 27, 2020, 4:26 PM

#

the debug event emits whenever messages are swept. it's quite crude, but you could filter through debug messages (i.e. not the WS ones) then emit your own event based on that

vernal atlas Aug 27, 2020, 4:34 PM

#

you could also extend the client class and override the sweepMessages function to emit an event 👀

lofty beacon Aug 28, 2020, 10:29 AM

#

oh yea lol

#

also, there isn't such thing as a message.quote, might be cool.
ex:
if a message had the content of hi and you called the quote method with hello as the content it would resovle this:

> hi
@user hello

ornate topaz Aug 28, 2020, 10:41 AM

#

what would be a use case for that

unique axle Aug 28, 2020, 10:51 AM

#

As discord themselves are planning to remove that feature in favor of proper reply (which may or may not get proper bot API support at some point) I don't personally see much value in this

lapis talon Aug 28, 2020, 11:19 AM

#

Yeah that new quoting thingy is pretty epic

vernal atlas Aug 29, 2020, 5:36 PM

#

the API seems to have stopped sending embed_enabled causing Guild#embedEnabled to always be undefined, rather than a boolean as it should be, should this be left as-is or perhaps changed to use the widget_enabled value?

wild flax Aug 29, 2020, 6:50 PM

#

Considering we can’t just up v13 for this, yes

#

The latter

vernal atlas Aug 29, 2020, 7:47 PM

#

O wait- embed_enabled/widget_enabled doesnt/never come with the initial GUILD_CREATE packet

#

but it perhaps should be cached with the fetchWidget/fetchEmbed method, ill pr that

oak quail Aug 29, 2020, 7:52 PM

#

has that changed, or was it always like that?

vernal atlas Aug 29, 2020, 7:53 PM

#

thats what puzzled me at first because i could've sworn it was sent with the initial packet

oak quail Aug 29, 2020, 7:55 PM

#

i'd make an issue on discord-api-docs

vernal atlas Aug 29, 2020, 8:00 PM

#

did a search in DAPI and found https://github.com/discord/discord-api-docs/issues/1102

tacit crypt Aug 29, 2020, 8:03 PM

#

Guilds do not store this metadata, so it isn't returned in guild creates

GUILD_UPDATE contains them

#

logic 200

vernal atlas Aug 29, 2020, 8:06 PM

#

would it make sense to make fetchEmbed just return this.fetchWidget() to avoid duplicated code?

tacit crypt Aug 29, 2020, 8:09 PM

#

Ye, and mark as deprecated

vernal atlas Aug 29, 2020, 8:13 PM

#

ye, already deprecated, the same for setEmbed too?

#

they both hit a different endpoint, guilds/:id/embed and guilds/:id/widget, but yield the same result, which is why im asking first xd

oak quail Aug 29, 2020, 8:56 PM

#

/embed is removed in api v8

#

which is why the djs embed stuff was deprecated

#

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

vernal atlas Aug 29, 2020, 9:14 PM

#

BlobThumbsUp

snow crypt Aug 29, 2020, 9:25 PM

#

why is https://github.com/discordjs/discord.js/pull/2910 not merged?

vernal atlas Aug 29, 2020, 11:24 PM

#

im tryna implement the changes from https://github.com/discord/discord-api-docs/pull/1886/files into a PR - but the short of it is should i make a new class for IntegrationAppliation or just use the ClientApplication class

making a new class would essentially just be the same as ClientApplication, but without botPublic,botRequireCodeGrant, cover, owner, and rpcOrigins

#

perhaps an Application interface/base class?

#

i think that would actually be a good idea

snow crypt Aug 30, 2020, 5:02 PM

#

@vernal atlas why not have includeApplications just true by default

#

like, not even an option

#

just always true

vernal atlas Aug 30, 2020, 5:08 PM

#

well, i thought it best to add the option & default it to what discord defaults it to

#

oof just made me realise i forgot the typings

snow crypt Aug 30, 2020, 5:18 PM

#

@vernal atlas well they could have done the same thing with webhook ?wait

#

but i really wonder, why not

#

it's not like it will be breaking

#

or cause anything unpredictable

#

i say just have it always true so you always have the application

vernal atlas Aug 30, 2020, 5:22 PM

#

perhaps default to true, but still have the option if you need it

snow crypt Aug 30, 2020, 5:23 PM

#

why would i explicitly NOT need it?

#

yeah i might not really have a use for it

#

but in what case is it gonna be "it must not be sent"?

vernal atlas Aug 30, 2020, 5:26 PM

#

well, it would be easier for a user to filter out bot & webhook integrations

snow crypt Aug 30, 2020, 5:26 PM

#

can you explain?

vernal atlas Aug 30, 2020, 5:27 PM

#

well, if for whatever reason you didn't want to list bot & webhook integrations (eg in a command), simply passing false would be easier than (await guild.fetchApplications()).filter(int => int.application === null)

snow crypt Aug 30, 2020, 5:27 PM

#

ohhh wait my bad

vernal atlas Aug 30, 2020, 5:28 PM

#

FuzzyEyes

#

did you think include_applications did something else?

snow crypt Aug 30, 2020, 5:28 PM

#

yeah

oak quail Aug 31, 2020, 7:52 AM

#

oldeyes PR #4762 was merged before the docs pr was merged

thorny copper Aug 31, 2020, 11:17 AM

#

Why did you make separate methods for avatars (avatarURL and displayAvatarURL)? Why not merging the second one with the first one by making a parameter "callback", which could be an image (URL, buffer or anything else) or "default", which effectively returns default avatar URL?

warped crater Aug 31, 2020, 11:36 AM

#

avatarURL yields the avatar given by Discord as is, null in case the user has none set
defaultAvatarURL gives back an asset from Discord's API with... a default avatar; calculated as they do it in the client
displayAvatarURL is simply a utility that either gets the user's set avatar, or their corresponding default (theoretically, what you should have on screen)

#

this design choice makes sense to me, and either way, it can not be changed as this would be breaking

thorny copper Aug 31, 2020, 11:39 AM

#

(Yes I know this would break existing code, but why not a change for DJS v13)

wild flax Aug 31, 2020, 11:40 AM

#

I don't see a huge benefit in merging them

#

Nor a huge downside in leaving them as is

warped crater Aug 31, 2020, 11:41 AM

#

^ the way it currently is actually allows you to tell if the user has no avatar set

thorny copper Aug 31, 2020, 11:50 AM

#

https://github.com/discordjs/discord.js/issues/4767

snow crypt Aug 31, 2020, 9:30 PM

#

Util.resolveColor returns NaN instead of throwing.

#

I believe this is because of https://github.com/discordjs/discord.js/blob/b0ab37ddc0614910e032ccf423816e106c3804e5/src/util/Util.js#L445

else if (color && isNaN(color)) throw new TypeError('COLOR_CONVERT');

As color is NaN, it is falsey, so it skips the isNaN and just returns color which is NaN

unique axle Aug 31, 2020, 9:51 PM

#

summary:
~~https://github.com/discordjs/discord.js/commit/0d4eab8d241e770cbea065688ccf85aa93fe07d1#diff-fb4312d30636f670d026660847d06636R368~~ https://github.com/discordjs/discord.js/commit/b1473b1e4cdecc782ead80c06ff447d94479e5e4#diff-dd560c495a00593ea828a458e5e13b18R115-R118

has this behaviour since ~~Jan. 2017~~ Dec. 2016
potentially breaks build pipelines for people using it that way
inconsistency is bad (it should probably throw)
still erring towards semver:major which makes this v13 earliest

thanks space for digging shibaHeart

slate nacelle Aug 31, 2020, 9:53 PM

#

Actually nevermind, this bug has been introduced a month before that https://github.com/discordjs/discord.js/commit/b1473b1e4cdecc782ead80c06ff447d94479e5e4#diff-dd560c495a00593ea828a458e5e13b18R115-R118
Either way, way too old to revert the behavior here, imo.

halcyon field Aug 31, 2020, 10:27 PM

#

Add something like .bannable but for ".dmable" (checks if the user can be dm'd)

copper laurel Aug 31, 2020, 10:32 PM

#

Discord has previously said no to this request anyway. They prefer to have the API call to send the message reject than have developers making two API calls for every DM - check + send.

deft holly Aug 31, 2020, 10:41 PM

#

Think this has been asked several times here but with no response 😕 what's the reason https://github.com/discordjs/discord.js/pull/2910 hasn't been merged? Does it count as a breaking change in some way (please excuse my ignorance on this topic :P)

vernal atlas Sep 1, 2020, 12:55 AM

#

its got a semver: minor label on it, and there dont seem to be any breaking changes, so no its not a breaking change

crimson harbor Sep 1, 2020, 3:42 AM

#

Sorry for a dumb question, but I'm not sure how to use util.deprecate on a class method.

#

Nvm, I figured out that you use Class.prototype

copper laurel Sep 1, 2020, 3:52 AM

#

GuildMember.prototype.hasPermissions = util.deprecate(GuildMember.prototype.hasPermissions,
  'GuildMember#hasPermissions is deprecated - use GuildMember#hasPermission, it now takes an array');```

#

Theres an example from v11

patent halo Sep 2, 2020, 2:10 AM

#

How does discord.js handles rate limit internally? PepoHmm

left moat Sep 2, 2020, 2:14 AM

#

check the code

patent halo Sep 2, 2020, 2:15 AM

#

i dont get it, somewhat

patent halo Sep 2, 2020, 3:38 PM

#

Ahh, like a queue

cursive shuttle Sep 2, 2020, 10:33 PM

#

Any idea why guildMemberAdd is marked as receiving GuildMember | PartialGuildMember in the typings? Discord always sends the full member object in the gateway event, it shouldn't ever be partial in that event?
Seen here https://github.com/discordjs/discord.js/blob/b0ab37ddc0614910e032ccf423816e106c3804e5/typings/index.d.ts#L2276

cursive shuttle Sep 2, 2020, 11:05 PM

#

Similarly, Discord always emits GUILD_BAN_ADD with a full user object, meaning guildBanAdd will never emit a PartialUser even though it's marked as such in the typings

#

Again for GUILD_BAN_REMOVE

#

newUser in userUpdate shouldn't be partial either as userUpdate always includes a full user structure

fallen arrow Sep 3, 2020, 2:59 PM

#

currently there is no way to define guildsPerShard when using internal sharding set to "auto". Can i PR a client option for it?

#

well i guess one could always manually call Util.fetchRecommendedShards and feed its result to the client, so nvm

vivid field Sep 4, 2020, 10:57 PM

#

https://github.com/discordjs/discord.js/blob/master/src/structures/GuildChannel.js#L516 what does the ThisType here stand for?

slate nacelle Sep 4, 2020, 10:59 PM

#

I'd say that's a bad autocomplete of my editor and should have been this.

vernal atlas Sep 5, 2020, 12:40 AM

#

'd it

#

would it make sense for ChannelManager and GuildChannelManager to have overloads for the resolve function with a type parameter, so you could use
guild.channels.resolve<TextChannel>(...) or similarly, client.channels.resolve<DMChannel>(...) for the client

copper laurel Sep 5, 2020, 1:03 PM

#

How would that handle the ID not being a text channel ID though

vernal atlas Sep 5, 2020, 2:24 PM

#

well, this would be similar to just casting it yourself, except its just a little cleaner for typescript

snow crypt Sep 5, 2020, 2:25 PM

#

might as well just <TextChannel>client.channels.resolve(id)

vernal atlas Sep 5, 2020, 2:25 PM

#

ig

drifting knot Sep 6, 2020, 3:32 AM

#

so collection.filter doesnt do great with typescript

#

import { A, B, isTypeA } from '../classes';

const collection = new Collection<A | B>();

// ...

const aCollection = collection.filter(item => isTypeA(item));

// aCollection is still typed as Collection<A | B>
// It should be typed as Collection<A>

#

can we use type guards or assertions or some other TS feature to make the return type match whatever the filter predicate is doing?

#

i'm asking because i don't like having type assertions for things like filtering a bunch of channels to just be voice channels

deft holly Sep 6, 2020, 3:40 AM

#

it's worthwhile to note that Array#map has the same behavior. I doubt there's a way to fix the typings for this in a sane way since

you'd probably need TypeScript to infer the return type of a function (the predicate) when called with a specific set of arguments which isn't really possible currently
Array#map would have been updated with proper typings if this were possible easily

the only alternative then is something like collection.filter<string[]>(predicate: ...) which would then result in a string[] but I don't think this a big improvement over just typecasting

drifting knot Sep 6, 2020, 3:44 AM

#

thats a good point

#

i kinda like the idea of passing a type param to Collection#filter

#

but i can just do const a = whatever as unknown as Thing

deft holly Sep 6, 2020, 3:53 AM

#

found a github issue about this. Does seem like it's not possible without explicitly typing the value though which is unfortunate - https://github.com/microsoft/TypeScript/issues/16069

update 2:
seems like supporting type guards can actually be supported through the following overload:
filter<S extends T>(callbackfn: (value: T, index: number, array: T[]) => value is S, thisArg?: any): S[];
(this is for Array but the concept can be applied to Collection as well)

basically, this would allow -
const foo = collection.filter((x): x is undefined => x === undefined);
foo is then typed as undefined[].

long basalt Sep 7, 2020, 7:07 AM

#

I got something that feels like a bug, but cant tell, I also dont know if this is the reason its happening, im not a expert on the request handler. so the bug is, when multple items are in queue and a 400-500 error, it wont continue in the queue, just reject. in result, some stuff will just hang indefinitely until a request gets added to queue and it continues to handle requests until it runs into that error, im not fully sure this is why it happens, its just what I think is whats causing it, im also not sure if this is a bug or not

tacit crypt Sep 7, 2020, 8:57 AM

#

Known, reported and will be fixed 👍

long basalt Sep 7, 2020, 9:29 AM

#

is there a pullrequest or not yet?

#

anyway, I think im going to evade the issue by awaiting before adding another request to queue

random shell Sep 8, 2020, 5:21 PM

#

is there anything special I need to do when importing the webpack version (I'm using 12.3.1) via a <script> tag?

#

it seems to always result in errors, so I feel like i am missing something

vivid field Sep 9, 2020, 1:04 PM

#

about to implement the newschannel following feature, would targetChannel.follow(newsChannel) or newsChannel.follow(targetChannel) fit better into the structure of the library?

vernal atlas Sep 9, 2020, 1:20 PM

#

i would go for NewsChannel#follow

crimson harbor Sep 9, 2020, 1:44 PM

#

I think that NewsChannel#addFollower would be a better name.

ornate topaz Sep 9, 2020, 5:10 PM

#

Imo that points slightly more towards a user than a channel

deft holly Sep 9, 2020, 6:47 PM

#

typings: any plans to use named tuple elements (ts 4.0 feature) for the ClientEvents interface? benefit of this is that client.on('message', ...) - intellisense no longer shows args_0, args_1, etc. as the names for the parameters but rather the labels

For what I mean by this, see the following two screenshots. If you can't see the difference, it's args_0 vs name.

This may be a breaking change for TS users not on 4.0 though

#

oak quail Sep 9, 2020, 6:52 PM

#

API/GW v8 changes:

permissions is now back to the original field, as a string.

game fields are now removed, replaced with the first element in activities. For widget endpoint, game field on users is renamed to activity, since there's still a singular field.

Channel Permission Overwrites have their type field serialized as a number (e.g. 0) instead of a string (e.g. role)

embed_enabled and embed_channel_id fields are removed, replaced with corresponding widget_ fields.

Form error responses now have a format that is better aligned with "regular errors".

Rate limit headers are sending seconds instead of milliseconds now. As such, the X-RateLimit-Precision header is no longer respected.

Bots no longer get CHANNEL_CREATE for DMs on v8.

Many routes have been deprecated. (Consult: https://github.com/discord/discord/pull/29722/files)

Bans delete-message-days is no longer available. Use delete_message_days instead.

Removed roles, premium_since, and nick from Presence Update gateway event.

vernal atlas Sep 9, 2020, 8:13 PM

#

that link is a 404 page ASC_PepeThinking

unique axle Sep 9, 2020, 8:16 PM

#

of course. that's not discord API docs, that's discord/discord it's not public.
that information is a direct copy from the DM of mason with one of their engineers

vernal atlas Sep 9, 2020, 8:25 PM

#

yeah figured the repo wasnt public, just wanted to point that out incase it was meant to be a different link

copper laurel Sep 10, 2020, 11:41 AM

#

@deft holly technically speaking we don't consider the typings to be breaking changes since its a JS lib, however one that major is probably worth holding off on. discordjs-next is a TS rewrite though so we can definitely make use of it there

vernal atlas Sep 11, 2020, 5:42 PM

#

https://github.com/discordjs/discord-api-types/blob/1700bd454edf7e97612b1c3f2f4aa2a9370bec05/v6/payloads/user.ts#L18-L22 @tacit crypt should properties like these, only recieved via oauth2, have a jsdoc comment on them saying that? or is it good how it is

#

~~well- maybe that property isnt the best example but im sure theres others~~
edited to a better example

tacit crypt Sep 11, 2020, 5:50 PM

#

Eh. I just document the types, how they come is not something i should spend time documenting... Plus I link the direct object url so people can read when using it

vernal atlas Sep 11, 2020, 5:50 PM

#

oki

tacit crypt Sep 11, 2020, 6:32 PM

#

Besides.... Think about it this way: properties can come and go at random from different places... And may even be changed randomly, requiring a patch release for it

oak quail Sep 11, 2020, 10:15 PM

#

well

#

owner and permissions are based on the specific context of "get current user guilds"

#

because its "is this user the owner" or "what perms does this user have"

real jetty Sep 12, 2020, 6:09 PM

#

Is it not possible to add the "GuildMember" who triggered an event on events such as channel create/update/delete as a parameter?

unique axle Sep 12, 2020, 6:10 PM

#

we can only implement features around data discord provides, this is not provided by discord

#

"who did something" will always work through audit logs only

real jetty Sep 12, 2020, 6:10 PM

#

Can it not be pulled by the lib through audit?

#

so the user doesn't have to do it for each event?

unique axle Sep 12, 2020, 6:11 PM

#

the library does not auto-fetch data.
additionally audit logs are not guaranteed to have arrived when the websocket event does
(there's also a bunch of other caveats, which i won't get into here)

#

so: no, we can not do that

mint iron Sep 13, 2020, 2:56 PM

#

What the utility for each folder inside packages in discord.js-next?

#

For example where should goes structures like Message, Guild...

tacit crypt Sep 13, 2020, 7:46 PM

#

Those are probably gonna go in packages/discord.js

#

each folder is a submodule

#

for instance rest will host everything rest related

drifting knot Sep 14, 2020, 7:28 PM

#

since new djs will target new node, can snowflakes use bigints please

#

idk if this is a "node doesnt support it" issue or a "we have been using strings for 999999 years" issue

raven juniper Sep 14, 2020, 7:38 PM

#

Discord does provide them as strings on the API, so I suppose one question would be if we'd want to convert it, iirc we don't change any other API types

vernal atlas Sep 14, 2020, 7:44 PM

#

i think there was a discussion about this, i remember it being said that it seemed silly to just convert back and forth between strings and bigints all the time

#

wasn't exactly that but https://canary.discordapp.com/channels/222078108977594368/222197033908436994/741347312583835722

drifting knot Sep 14, 2020, 7:58 PM

#

doesnt erlpack use proper int64s

#

also its like 2 operations

#

if its all written in TS it shouldnt be an issue

#

just call id.toString()

#

it will never fail

drifting knot Sep 14, 2020, 8:30 PM

#

my user id takes up 36 bytes of memory as a string, a bigint would take up 24 bytes

#

for a large bot you could easily be saving 1-2 mb ram lmao

tacit crypt Sep 14, 2020, 9:14 PM

#

Replying to @drifting knot from https://canary.discordapp.com/channels/222078108977594368/682166281826598932/755155411421757451
erlpack still turns bigints to strings

drifting knot Sep 14, 2020, 9:22 PM

#

how

#

surely this is because erlpack is still written as if bigints dont exist in electron or wherever they're running their code

copper laurel Sep 14, 2020, 10:03 PM

#

Just because they exist doesn't make them a particularly good type to use

vagrant holly Sep 14, 2020, 10:47 PM

#

Wait wait wait

#

ShardingManager and Shard cache eval calls?

#

Dang that's why sharding is so fucked

vagrant holly Sep 14, 2020, 11:08 PM

#

Would appreciate some 👀 on https://github.com/discordjs/discord.js/pull/4823 as it's a semi-breaking issue for one of my bots currently (will jump to my branch for now in prod).

frank turret Sep 15, 2020, 10:32 PM

#

I know role#equals just compares properties from both roles, but is it really needed/within the scope of the library to provide a method like that? Even in the methods description it advises a person to just compare the ids for most use cases. I understand it would be a breaking change, but would it be worth opening an issue/making a PR regarding removing it?

oak quail Sep 15, 2020, 10:33 PM

#

its for internal checks

frank turret Sep 15, 2020, 10:36 PM

#

Fair enough, my next topic is out of the list of v8 changes (for discord-api-types)(https://github.com/discordjs/discord-api-types/issues/12#issuecomment-692972229), I made a sub list of which changes are actually related to the issue itself. Could someone look it over and inform me if I'm mistaken about anything?

APPLY:
>permissions is now back to the original field, as a string.
>Channel Permission Overwrites have their type field serialized as a number (e.g. 0) instead of a string (e.g. role)
>Removed roles, premium_since, and nick from Presence Update gateway event.
>embed_enabled and embed_channel_id fields are removed, replaced with corresponding widget_ fields.

DONT APPLY:
>game fields are now removed, replaced with the first element in activities. For widget endpoint, game field on users is renamed to activity, since there's still a singular field.
>Many routes have been deprecated.
>Bots no longer get CHANNEL_CREATE for DMs on v8.
>Bans delete-message-days is no longer available. Use delete_message_days instead.
>Rate limit headers are sending seconds instead of milliseconds now. As such, the X-RateLimit-Precision header is no longer respected.
>Form error responses now have a format that is better aligned with "regular errors".

deft holly Sep 15, 2020, 10:46 PM

#

About the stores -> managers change (yes, I know I’m several months late on this):
Have shortcut methods like XYManager#filter() (which calls XYManager#cache#filter() internally) been considered? From my (admittedly poor) understanding of the issues at hand (https://github.com/discordjs/discord.js/issues/3485):

Poor Inheritance
Since the design doesn’t really change from right now, I don’t see why this would be an issue.

The design is not extensible
As XYManager#filter(), XYManager#find() and so on simply call their respective cache methods internally I don’t see why this would be less extensible than the current approach either.

Naming conflicts
An example of a naming conflict before was MessageStore#delete vs MessageStore#remove (which deletes the message and which removes from the cache?). This would be solved by simply not creating an alias for MessageStore#delete -> MessageStore#cache#delete, i.e MessageStore#delete would make an API call and MessageStore#cache#delete would be used to remove the message from the cache.
I personally believe that the proposed change (shortcut methods) do not remove any of the benefits of managers without any significant drawbacks. About whether this change is needed – from the many, many users who complained about having to add .cache everywhere in their code, I believe that the majority of the user base would appreciate this change. In addition, as these are simply shortcut methods, no existing code has to be modified.

Looking forward to any thoughts about this, perhaps I’m missing something really obvious which prevents this from being a possibility in which case I apologize.

oak quail Sep 15, 2020, 10:48 PM

#

@frank turret im pretty sure this will have to be changed too

game fields are now removed, replaced with the first element in activities. For widget endpoint, game field on users is renamed to activity, since there's still a singular field.

copper laurel Sep 15, 2020, 10:54 PM

#

I both agree and disagree with what you're saying here @deft holly

#

The intention was to achieve a separation of API and cache - for that reason I'm not sure I totally agree that providing methods that call the cache from the Manager is necessarily the right approach.
However Managers themselves really don't act as a good API Manager yet - making API calls still relies on having the cached instances. For example, if you want to pin a Message, you have to get the Message object from cache, and call Message#pin.
A method like MessageManager#delete which lets you delete uncached messages by ID is a much better representation of how it should work imo (and Message#delete actually calls MessageManager#delete now internally, because the Manager is responsible for API calls)

This is something I'm working on literally right now now, providing much more extensive API coverage on the Managers so that they can fulfil their purpose, and better reflect that separation of "I'm making an API call" vs "I'm operating on the cached items" such as you would be with #filter.

deft holly Sep 15, 2020, 11:06 PM

#

The intention was to achieve a separation of API and cache - for that reason I'm not sure I totally agree that providing methods that call the cache from the Manager is necessarily the right approach.
I agree that in an ideal world we would want to have this but the reality is that most Discord.js users do not really care about a separation of API and cache - no one (to my knowledge) brought this up when v12 was still using stores until the beforementioned issue was raised. It is a (admittedly mild) inconvenience to type .cache every time you want to access the cache, and I think having shortcut methods would mitigate that somewhat. I can absolutely understand not adding it though, as I personally don't find .cache to be too much of an issue :P ~~It's just that I'm reasonably sure a lot of people would appreciate not writing it everywhere.~~ It's just that I believe that Discord.js would provide a better experience for users of the library if not chaining .cache everywhere were an option (without causing more issues).

wild flax Sep 15, 2020, 11:12 PM

#

While user experience is an important topic, I simply do not care what the "majority" does, when the majority follows youtube tutorials or does not know JS at a level enough to even write a complete bot by themselves

left moat Sep 17, 2020, 1:03 AM

#

Also I would hope that a clientOption field is eventually introduced to change the cacheType of managers in v13 or in the next big v12 update

#

afaik this isnt a breaking change bc nobodies code will break because of it

unique axle Sep 17, 2020, 7:25 AM

#

Making chaches be exchangeable is definitely on the roadmap, however more expected towards -next.
V13 will focus on adapting the existing js codebase (v12) to the new API versions.

vagrant holly Sep 17, 2020, 12:59 PM

#

Gonna ask here cause I feel y'all might know better than the help channels, is there a good way to completely block a channel and its messages from ever being cached in the library (got a channel where each message has a solid 80MB+ of reactions data on it, so I'd rather just not deal with that channel ever)?

left moat Sep 17, 2020, 1:02 PM

#

well you could just remove the bots perms to see anything in that channel @vagrant holly

vagrant holly Sep 17, 2020, 1:03 PM

#

Not a channel that I have any control over at all, so curious if the lib has any decent internal way to block out a channel from caching at all

unique axle Sep 17, 2020, 1:03 PM

#

please do not use this channel as an upgrade of help channels

#

no, you can not
you can sweep messages on an interval or modify source

vagrant holly Sep 17, 2020, 1:05 PM

#

Is the raw processing of a new message exposed, or would the best bet be to just listen for a new message normally and then purge it from cache at that point?

#

If not, would it be worth looking at a way to add a custom callback/filter to a raw message arriving in the lib, or would that PR never get anywhere if created?

tacit crypt Sep 17, 2020, 1:23 PM

#

Granular cache controls would be nice...but I don't think it'll be a priority for v12/v13? (don't quote me on that)
However, nothing stops you right now from creating a sweeper of sorts

undone ravine Sep 17, 2020, 1:33 PM

#

messageCacheMaxSize is a thing also

vagrant holly Sep 17, 2020, 1:44 PM

#

I have super strict sweeping set up for messages themselves with what the library provides in options, this seems to be coming from a small subset of messages that don't match the main sweeping criteria

vagrant holly Sep 17, 2020, 5:07 PM

#

Is it possible to request an issue be assigned to me on GH?

vernal atlas Sep 17, 2020, 7:32 PM

#

working on a PR to handle channel type changes (text => news, news => text), what would be the best way to handle this? just remove the channel from the cache and re-add it?

#

oh wait- its already handled

tender field Sep 18, 2020, 2:02 PM

#

is there a plan to have better stacktraces in djs-next? I know it would require a lot of changes and i don't expect it to be on v13, but will it be taken into account for -next?

raven juniper Sep 18, 2020, 2:09 PM

#

at some point yes, there will by async stack traces and will include the line of the code

#

DiscordAPIError[50006]: Cannot send an empty message
    at RequestHandler.makeRequest (/home/archid/workspace/alestra/node_modules/@klasa/rest/src/lib/RequestHandler.ts:192:11)
    at processTicksAndRejections (internal/process/task_queues.js:93:5)
    at RequestHandler.push (/home/archid/workspace/alestra/node_modules/@klasa/rest/src/lib/RequestHandler.ts:101:11)
    at async Promise.all (index 0)
    at Map.add (/home/archid/workspace/alestra/node_modules/@klasa/core/src/lib/caching/stores/MessageStore.ts:65:23)
    at default_1.eval (/home/archid/workspace/alestra/node_modules/klasa/src/commands/Admin/eval.ts:66:14)
    at default_1.run (/home/archid/workspace/alestra/node_modules/klasa/src/commands/Admin/eval.ts:24:43)
    at CommandHandler.runCommand (/home/archid/workspace/alestra/node_modules/klasa/src/monitors/commandHandler.ts:58:23)
    at CommandHandler._run (/home/archid/workspace/alestra/node_modules/klasa/src/lib/structures/Monitor.ts:110:4) {
  code: 50006,
  status: 400,
  method: 'post',
  url: 'https://discord.com/api/v7/channels/642137151626018818/messages'
}``` this is an example, you can see where it points to the command file where the occur originates from

tender field Sep 18, 2020, 2:16 PM

#

incredible! did you write this from scratch or is this already available somewhere ? 👀 (like a branch or smthg)

raven juniper Sep 18, 2020, 2:16 PM

#

it's not available yet, i'm not technically proficient enough to tell you where it comes from

#

i'm just a copy-paste monkey. i just know it's planned to be in an upcoming version of d.js, whether that's 13 or next, i don't know

tender field Sep 18, 2020, 2:17 PM

#

ok thanks 😄 still really nice to see we might have better stacktraces at some point 🙂

left moat Sep 18, 2020, 3:24 PM

#

Replying to https://discordapp.com/channels/222078108977594368/682166281826598932/756518942545018921
its from alestra which is kyra's bot(for canvas constructor server) which is built upon the now defunct @klasa/core lib.

#

djs-next/13 will most likely take design ques from klasa/core such as the ws/rest design

oak quail Sep 18, 2020, 4:05 PM

#

well -next will be v14

#

but vlad said that v12.5/v13 might use the updated rest stuff

tacit crypt Sep 18, 2020, 4:06 PM

#

Keyword might

unique axle Sep 18, 2020, 4:07 PM

#

well -next will be v14
-next will be the next major at the point it's completed.

We might need to release another major before -next if discord decides to introduce breaking changes and release other versions of the API, which we might need to reflect to support new features

oak quail Sep 19, 2020, 1:53 AM

#

@frank turret @tacit crypt for api-types, should we maybe keep most things only in v8 and just import&export them in v6 unless they're different

#

so not everything is duplicated

#

because theyre gonna keep updating v6

tacit crypt Sep 19, 2020, 1:58 AM

#

Ehh.. dunno honestly
Personally I'd prefer duplication and once discord marks a version as basically dead, we delete it from the module

#

It'll be annoying sure.. but idk, depends what others suggest too

vivid field Sep 19, 2020, 11:58 AM

#

shouldn't Client#emojis and Guild#emojis be separate managers? for one, the types are different (i.e. guild vs no guild) and also there's methods like create() accessing this.guild, wouldn't that throw an internal error when trying to do client.emojis.create()?

unique axle Sep 19, 2020, 12:00 PM

#

DiscordAPIError: 405: Method Not Allowed does throw, correct

vivid field Sep 19, 2020, 12:10 PM

#

so shouldn't the library handle that instead of sending an invalid request? just throwing an error when calling it from a client would probably be enough imo, but idk if that's seen as patch or major

unique axle Sep 19, 2020, 12:13 PM

#

following the discussions here https://github.com/discordjs/discord.js/pull/4816#pullrequestreview-487356231 it'd be major (i'd also agree with that, it changes the "return type" of a function in the case of failure, which is then no longer a DiscordAPIError.

though as i mentioned earlier v13 will be needed to comply with new api versions and bring new features before the full rewrite anyways, so this might be the right time to propose a change here.
Maybe even on a larger scale, trying to spot and change similar occurences like this (though that could also easily blow out of scale)

solemn oyster Sep 19, 2020, 11:15 PM

#

incredible! did you write this from scratch or is this already available somewhere ? 👀 (like a branch or smthg)
@tender field we'll likely do that for 12.5 (or 12.4 if Crawl doesn't release it before), the trick is that the new REST uses an async lock implementation, versus using stateful deferred promises

#

As such, the queue is done in a natural manner, and by design (unless V8 is bugged, which is not), it cannot deadlock, it cannot halt, errors bubble up naturally, the entire request can be retried entirely (adds support for retried AbortErrors!), and by design, it has proper async stack traces, which is done thanks to the fact that it does await this.queue.wait() before proceeding, it doesn't use middle steps nor makes an object representing the state, it just awaits for all the previous requests and then it jumps directly to the fetch()

#

A try/catch/finally tells the queue to resolve the first promise (wait obtains the last promise, it any, pushes a new deferred promise, and awaits the previous), it may be a little hard to understand, but it's really simple

#

But yeah, before I can use this, I need @tacit crypt to either give me async-queue's code, or upload it so Crawl can publish it, although it might be weird having @discordjs/core as depencency in 12.x

#

Since... that package is for 13.x

#

I still plan on using this ASAP, I have been trying to make d.js have useful stack traces for years, I think my first PR adding them (although it used V8-specific APIs, and broke error messages in some cases yet unknown to me) dated back to 2018, but Lewdcario and a few more people were having issues, so we had to revert it.

My point with telling you all of this is, it's not a new issue, we've been looking for a solution that works for everyone without needing to use non-standard things, and we have now found one :)

real jetty Sep 20, 2020, 1:11 AM

#

how can VoiceChannels have a Channel Topic? Thonk

frank turret Sep 20, 2020, 1:12 AM

#

it inherits from GuildChannel, which I guess has that. Shouldn't that be something more like on a TextChannel/NewsChannel?

real jetty Sep 20, 2020, 1:13 AM

#

Yeah, according to discord api docs, only Text, News can have topics GWbruhThonkNoHands

frank turret Sep 20, 2020, 1:13 AM

#

In addition to that, the setTopic method exists on GuildChannel but not the topic property (at least on the docs), leading me to believe this is a mistake?

copper laurel Sep 20, 2020, 1:13 AM

#

I've raised before that the placement of properties on GuildChannel doesnt really make much sense

frank turret Sep 20, 2020, 1:13 AM

#

worth opening a pr for?

copper laurel Sep 20, 2020, 1:14 AM

#

Like Souji hinted at earlier, now is probably the time to make semver major changes

#

Its actually pretty extensive though

#

createInvite for example

frank turret Sep 20, 2020, 1:15 AM

#

Well, at least for now I'll open a draft pr for removing it and putting it on textchannel

#

and since newschannel extends textchannel, i only have to do it on textchannel

copper laurel Sep 20, 2020, 1:15 AM

#

Yeah make it a collab PR or issue or something

oak quail Sep 20, 2020, 1:28 AM

#

speaking of semver major changes, maybe remove disableMentions (replace with allowedMentions) in v13?

frank turret Sep 20, 2020, 1:42 AM

#

Should i apply the setTopic change to TextChannel or TextBasedChannel, since either way it will end up reaching NewsChannel

oak quail Sep 20, 2020, 2:37 AM

#

@frank turret TextChannel, because DMChannel shouldnt get it

frank turret Sep 20, 2020, 2:37 AM

#

blobthumbsup

#

Updated in the pr, do I have to do any wonkyness for the docs?

oak quail Sep 20, 2020, 2:46 AM

#

Sets a new topic for the Text Channel.

@param {string} topic The new topic for the Text channel

@param {string} [reason] Reason for changing the Text channel's topic

#

this is unnecessary

#

just say channel

frank turret Sep 20, 2020, 2:47 AM

#

Alright, will change

oak quail Sep 20, 2020, 2:47 AM

#

while ur at it i'd change the description for .topic too

#

its inconsistent with the rest and is wrong for news channel

frank turret Sep 20, 2020, 2:47 AM

#

wb the example, should i also make that channel.setTopic? that seems somewhat misleading

oak quail Sep 20, 2020, 2:48 AM

#

well everything else is channel.

#

because it assumes that you have a variable named channel

#

also u gotta fix up the spacing and stuff so eslint stops complaining

tender field Sep 20, 2020, 7:12 AM

#

@solemn oyster thanks for these explanations! I think i understood 😅
Looking forward to these change, then 😄

solemn oyster Sep 20, 2020, 9:00 AM

#

@wild flax one question, are we allowed to use @discordjs/core in v12.x?

#

Or should I yeet AsyncQueue and port it to JS?

tacit crypt Sep 20, 2020, 9:39 AM

#

I..don't see why we wouldn't be allowed to?

solemn oyster Sep 20, 2020, 10:10 AM

#

Voilá!

#

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

#

@tender field in case you want to try it out 😄

tender field Sep 20, 2020, 10:30 AM

#

This is really nice! Thanks 😄

solemn oyster Sep 20, 2020, 10:32 AM

#

The issues https://github.com/discordjs/discord.js/issues/4827 and https://github.com/discordjs/discord.js/issues/3471 can easily be done after this PR lands, give or take 3 lines of code (without counting docs)

#

And I think https://github.com/discordjs/discord.js/issues/3665 is fixed by my PR

old kelp Sep 20, 2020, 11:38 AM

#

am I correct that options.position should be added to GuildChannel#clone? yet it uses GuildChannelManager#create here: https://github.com/discordjs/discord.js/blob/56e8ef2d38633154540748acb64f4c1305fcc6d3/src/structures/GuildChannel.js#L537, which does provide options.position but in the GuildChannel#clone, it doesn't seem to be present.

solemn oyster Sep 20, 2020, 12:09 PM

#

Yeah, it should, although, wouldn't that be rawPosition?

old kelp Sep 20, 2020, 12:32 PM

#

I guess, but wouldn't that create a bit of inconsistency since GuildChannelManager#create and the endpoint both take the position as the param? the user could always just specify it themselves as well.

real jetty Sep 20, 2020, 1:59 PM

#

Hey
What do you think about Message#awaitMessages return a Message object if Collection contains only 1 Message Object ?

old kelp Sep 20, 2020, 2:14 PM

#

imo that's the expected behavior, it would be a breaking change if to do so. there are plenty methods that also do that, if you go at changing that yet - it becomes inconsistent with the rest.
see MessageManager#fetch, with the limit of 1, or GuildMemberManager#fetch with the query option, for instance.

unique axle Sep 20, 2020, 2:17 PM

#

i heavily disagree with this, this makes the API and typings inconsistent and hellish to use. We can only do that with Collection#first because of the distinction no args/ amount

adapting this based on a number that's passed as optional parameter does not seem an improvement in usage

ornate topaz Sep 20, 2020, 6:44 PM

#

that would change from always getting a Collection, with first() method to obtain a Message, to a Collection or a Message, making you need to constantly check how many elements your awaitMessages was supposed to collect.

oak quail Sep 21, 2020, 10:13 PM

#

maybe followTo would be a better name then addFollower?

unique axle Sep 21, 2020, 10:15 PM

#

is there anything speaking against just... #follow ?

oak quail Sep 21, 2020, 10:39 PM

#

I thought it might be confusing as to which channel is following which

snow crypt Sep 22, 2020, 5:15 AM

#

lets say you have channel1 - announcement channel; channel2 - text channel
channel1.follow(channel2) could now refer to either:

channel1 should be followed by channel2
channel1 should follow channel2
it depends whether you write it as follow(follower) or follow(toFollow) which I find very confusing

warped crater Sep 22, 2020, 5:15 AM

#

I... absolutely disagree here.

#

To me channel1.follow(channel2) makes zero sense

#

you have to follow, at the 3rd person, singular, it obviously refers to the channel you're passing in?

snow crypt Sep 22, 2020, 5:18 AM

#

not really
yet i can't really phrase it in a clear way

#

you can call #follow on a channel, and now it is followed
you can call #follow on a channel, and you've just instructed it to follow another channel

ornate topaz Sep 22, 2020, 5:21 AM

#

channel1.follow()
message.pin()

warped crater Sep 22, 2020, 5:21 AM

#

not to mention how the news channel shouldn't have a follow method either way, imo

ornate topaz Sep 22, 2020, 5:21 AM

#

to me it's clear - follow is method used on channel1

warped crater Sep 22, 2020, 5:22 AM

#

which makes it even more clear

snow crypt Sep 22, 2020, 5:22 AM

#

um
what would you use in that context instead of message.pin()

ornate topaz Sep 22, 2020, 5:22 AM

#

channel.addpin(message)?

#

if you're asking about that

snow crypt Sep 22, 2020, 5:23 AM

#

god language is weird

warped crater Sep 22, 2020, 5:23 AM

#

okay

#

I just saw the status of the PR

snow crypt Sep 22, 2020, 5:24 AM

#

i can't really explain it but not naming it follow makes sense

ornate topaz Sep 22, 2020, 5:24 AM

#

imo, just like in the app, you follow an announcement channel, selecting into which channel it posts

snow crypt Sep 22, 2020, 5:24 AM

#

so you want NewsChannel#follow ?

ornate topaz Sep 22, 2020, 5:24 AM

#

so channel1.follow(channel2) would make sense, assuming c1 is news channel

#

yes, that would make it consistent with app, as well as logically with rest of the lib

warped crater Sep 22, 2020, 5:25 AM

#

mmm

#

I feel like TextChannel#follow should also exist

#

to sit in fashion with other helpers

#

such as Member#ban literally pointing to Guild#members#ban

#

as for the naming of NewsChannel#addFollower im starting to see the issue lol

snow crypt Sep 22, 2020, 5:26 AM

#

but naming them both follow just makes it confusing again which is what we tried to avoid in the first place

warped crater Sep 22, 2020, 5:26 AM

#

probs fine keeping NewsChannel's as is, then

snow crypt Sep 22, 2020, 5:26 AM

#

now we have news.follow(follower) and text.follow(news) and without static typings in there you just fuck readability

#

you could accidentally throw by trying to follow a text channel because you got the order wrong

#

you might want it to exist on both but naming them both the same, especially with inheritance, is just wrong

ornate topaz Sep 22, 2020, 5:28 AM

#

followIn() for news?

snow crypt Sep 22, 2020, 5:29 AM

#

i still agree with addFollower

ornate topaz Sep 22, 2020, 5:30 AM

#

imo that sounds odd and everytime i look at that i think of users

snow crypt Sep 22, 2020, 5:30 AM

#

same for folllowIn

ornate topaz Sep 22, 2020, 5:33 AM

#

well, technically, news.addFollower() doesn't really say much on its own. something like "add follower, yeah cool". could argue that news.followIn() asks for a channel to be passed

tender field Sep 22, 2020, 5:56 AM

#

I think followIn() is way clearer than chan.follow(chan) ^^

copper laurel Sep 22, 2020, 6:01 AM

#

This is why documentation exists and has a description

tender field Sep 22, 2020, 6:11 AM

#

that's true, but it doesn't mean that methods should be named anyhow

#

like when you write the code you have the documentation, but when you read someone's code you shouldn't need the doc

copper laurel Sep 22, 2020, 6:12 AM

#

I mean... thats why they should comment their code

#

This is now offtopic though

tender field Sep 22, 2020, 6:14 AM

#

well imo they should comment their code only if their fault that the code is not readable. If the library has confusing methods, it not their fault

oak quail Sep 22, 2020, 6:17 AM

#

so, no to NewsChannel#followTo?

tender field Sep 22, 2020, 6:19 AM

#

i find it clear too 🤷🏻‍♂️ but i think the conversation moved to #archive-offtopic 🤔 they proposed a manager

ornate topaz Sep 22, 2020, 6:19 AM

#

nothing was really proposed

solemn oyster Sep 22, 2020, 7:53 AM

#

@snow crypt as in, adding @private to AsyncQueue class's description?

snow crypt Sep 22, 2020, 7:54 AM

#

yes

solemn oyster Sep 22, 2020, 7:54 AM

#

Added 👍

#

Good call 😄

#

Added the comment, thanks! @snow crypt

snow crypt Sep 22, 2020, 8:00 AM

#

with love 🎉

solemn oyster Sep 22, 2020, 8:00 AM

#

Also @vagrant holly, do you have any progress on the message edit limit?

vagrant holly Sep 22, 2020, 9:35 AM

#

Also @vagrant holly, do you have any progress on the message edit limit?
@solemn oyster I have some stuff locally that I'm testing out in prod, will probably make it into a PR in the next week or so 👍

vagrant holly Sep 22, 2020, 10:15 AM

#

Just noticed this mem creep

#

From a heap dump of the shard, this array stands out as fuckin huge compared to any other shard

#

Any thoughts on figuring out what's causing this, not familiar enough with heap dump or lib internals

left moat Sep 22, 2020, 12:00 PM

#

OR

#

kinda different idea but for follow mayb add it to guildchannelmanager?

#

guild#channels#follow(c1, c2)

raven juniper Sep 22, 2020, 12:13 PM

#

That opens up more opportunities for a screw up, if they put the wrong channel in the news channel spot

#

Although it is more use of the manager, which I know was a discussion lately... Could see it, I suppose

left moat Sep 22, 2020, 12:24 PM

#

also it makes it less bork when u have followTo on newschannel and follow on regular textchannel which will confuse people imo

#

and u can unfollow aswell

#

but imo unfollowing should be on channel but that may be a bit inconsistent

vernal atlas Sep 22, 2020, 5:32 PM

#

should error codes like https://github.com/discordjs/discord.js/blob/1e63f3756e814d6b1a2a9c17af9c2b28ce37e472/src/util/Constants.js#L577 be removed? since they are only obtainable on user accounts, or is it fine because the errors do seem to be documented

analog oyster Sep 22, 2020, 6:19 PM

#

any reason why? https://i.imgur.com/ecczMOE.png

warped crater Sep 22, 2020, 6:25 PM

#

uh.. yes?

#

because its not part of the public api and shouldnt ever be?

#

its an internal property for handling your http requests

#

and should never be touched externally

analog oyster Sep 22, 2020, 6:29 PM

#

what if I want to use it

copper laurel Sep 22, 2020, 6:32 PM

#

Too bad, its private

warped crater Sep 22, 2020, 6:53 PM

#

feel free to use it, but because its private anyone is free to change it to api2 just for the lols in any release and break your code

#

or for any other reason

#

you shouldn't make http calls that way

copper laurel Sep 22, 2020, 6:54 PM

#

TypeScript wont let you use it though

solemn oyster Sep 22, 2020, 8:32 PM

#

Just do Reflect.get(this.client, 'api') to bypass TypeScript's system, @analog oyster

#

Alternatively you can do (this.client as unknown as { api: any })

tender field Sep 22, 2020, 8:45 PM

#

Hey what's the difference between TextChannel#overwritePermissions and TextChannel#createOverwrites ? They both replace the existants overwrites, and it's just that the later takes a userOrRole and overwrites, where the former takes an array of objects with userOrRole and overwrites

vivid field Sep 22, 2020, 8:55 PM

#

overwritePermissions clears all permissionoverwrites and applies the ones provided
createOverwrite clears only the permissionoverwrites of that user/role and applies the ones provided
updateOverwrite clears nothing and updates the permissionoverwrites for that user/role

vagrant holly Sep 22, 2020, 9:14 PM

#

This mem creep/leak continues to grow. Any tips of getting to the root of it? Feels internal somewhere

copper laurel Sep 22, 2020, 9:15 PM

#

Respectfully something "feeling" internal isnt a lot to go by haha, would probably be best to discuss outside this channel until we have something more conclusive to look into the library for

vagrant holly Sep 22, 2020, 9:16 PM

#

https://discordapp.com/channels/222078108977594368/682166281826598932/757907937426407425 the earlier heapdump makes it look very internal, just grabbing a new one now

tender field Sep 22, 2020, 9:51 PM

#

@vivid field ohh ok fair enough, but maybe the doc should explain that better ^^

vagrant holly Sep 22, 2020, 10:06 PM

#

bad shard

#

good shard

oak quail Sep 23, 2020, 6:24 PM

#

https://advaith.is-in-hyper.space/920828d6c3.png

unique axle Sep 23, 2020, 6:25 PM

#

issue link please, i want to track that

oak quail Sep 23, 2020, 6:26 PM

#

https://github.com/discord/discord-api-docs/issues/2078

gaunt flare Sep 23, 2020, 6:29 PM

#

me too because I just tried to start using djs in a browser. djs claims to support browser but i'm not actually sure it does anymore? I'm running into CORS issues which lead me to https://github.com/discordjs/discord.js/issues/4710 (which you also just commented on recently)

vernal atlas Sep 23, 2020, 6:44 PM

#

got buried so bump

should error codes like https://github.com/discordjs/discord.js/blob/1e63f3756e814d6b1a2a9c17af9c2b28ce37e472/src/util/Constants.js#L577 be removed? since they are only obtainable on user accounts, or is it fine because the errors do seem to be documented

vernal atlas Sep 23, 2020, 7:57 PM

#

also, should an error be thrown for GuildChannel#edit if a parentID was passed, but couldn't be resolved? as GuildChannelManager#resolveID returns null in that case - which will have the un-intended effect of removing the category from the channel

oak quail Sep 23, 2020, 10:12 PM

#

@gaunt flare djs supports it but Discord seems to have blocked it

gaunt flare Sep 23, 2020, 11:57 PM

#

🤔 hmm that's unfortunate...

oak quail Sep 24, 2020, 12:10 AM

#

as a temp workaround you can try {http: {api: 'https://discordapp.com/api'}} in your clientoptions

#

that may break in the future tho

oak quail Sep 25, 2020, 1:29 AM

#

https://github.com/discord/discord-api-docs/pull/2097

oak quail Sep 25, 2020, 11:07 PM

#

oh, it merged as addFollower, f

drifting knot Sep 26, 2020, 5:30 AM

#

when djs next gets released can it be under the package discord

#

2 emails to npm and they will give it to you

#

no publishes in 8 years

#

here is the email

to: msteigerwalt@gmail.com
cc: support@npmjs.com

Hi,

I'm writing to see if our organization Discord.js could use the "discord" package you published 8 years ago.

Thanks,
Discord.js Representative

#

the package will be yours within 4 weeks

oak quail Sep 26, 2020, 5:39 AM

#

but why

#

also it might sound too official

ornate topaz Sep 26, 2020, 5:40 AM

#

discord*.js*, discord*.py*, discord*.rb*
those aren't there for show

drifting knot Sep 26, 2020, 5:41 AM

#

npm explicitly requests you dont name package with .js

ornate topaz Sep 26, 2020, 5:41 AM

#

but discord.js is not discord

drifting knot Sep 26, 2020, 5:41 AM

#

keep the github as discord.js

#

make the package discord

ornate topaz Sep 26, 2020, 5:41 AM

#

but why

#

what's the point

drifting knot Sep 26, 2020, 5:41 AM

#

its cool

#

and follows npm convention

ornate topaz Sep 26, 2020, 5:42 AM

#

what convention

drifting knot Sep 26, 2020, 5:42 AM

#

Don’t put “js” or “node” in the name. It’s assumed that it’s js, since you’re writing a package.json file, and you can specify the engine using the “engines” field. (See below.)

ornate topaz Sep 26, 2020, 5:43 AM

#

where is that

drifting knot Sep 26, 2020, 5:44 AM

#

https://docs.npmjs.com/files/package.json#name

#

many other packages do this

#

ex. google is an unofficial lib for using google search

#

google is also heavily active on npm, but they use their scopes of @.google-cloud, @firebase, etc.

ornate topaz Sep 26, 2020, 5:45 AM

#

https://docs.npmjs.com/package-name-guidelines

#

also, it's a tip

#

not a rule

#

it's a tip because google.js sounds worse than google

#

but discord.js is not discord

drifting knot Sep 26, 2020, 5:47 AM

#

i didnt say rule i said convention

ornate topaz Sep 26, 2020, 5:47 AM

#

it's a tip

#

and it's explained why it's a tip in the second sentence

#

also, even if, this will cause literally nothing more than confusion

#

using discord to code bots for discord.

drifting knot Sep 26, 2020, 5:49 AM

#

logical

ornate topaz Sep 26, 2020, 5:49 AM

#

anyway, that went offtopic.

drifting knot Sep 26, 2020, 5:50 AM

#

people already import it as

const Discord = require('discord');

ornate topaz Sep 26, 2020, 5:50 AM

#

what

drifting knot Sep 26, 2020, 5:50 AM

#

module.exports

#

when people import the whole module

#

but you use the cringe discord.js

ornate topaz Sep 26, 2020, 5:51 AM

#

cool, they can also name it lkidfhgkjdfghbkjhdfgekjlihvdfkjbh, should we rename package to that?

drifting knot Sep 26, 2020, 5:51 AM

#

yes

#

simply begin annexing package names

ornate topaz Sep 26, 2020, 5:51 AM

#

ok, case closed

drifting knot Sep 26, 2020, 5:51 AM

#

i am going to take the discord package name and publish malware to steal bot tokens

ornate topaz Sep 26, 2020, 5:51 AM

#

#archive-offtopic for shitposting, thanks

wild flax Sep 26, 2020, 5:55 AM

#

We can double publish packages, that should be no issue

tacit crypt Sep 26, 2020, 11:29 AM

#

For all intents and purposes, it would be better if Discord themselves claimed that name, not us

solemn oyster Sep 26, 2020, 12:51 PM

#

Should AbortError retries be tied to its own option, or share with the 5xx one?

snow crypt Sep 26, 2020, 1:01 PM

#

when would you deploy the new changes to npm?

solemn oyster Sep 26, 2020, 1:24 PM

#

tfw noticed a type bug while trying to fix 3471... I re-arranged the handler to be... easier to read - handle 2xx and 3xx, then handle 4xx, inside it, 429, then handle 5xx

#

Also, this comment was written 2 years ago, it's not really true, at least, not for today's spec:js // NodeFetch error expected for all "operational" errors, such as 500 status code

#

Both of those codes resolve with 500 and 501 respectively:js require('node-fetch')(`http://httpstat.us/500`).then(r => r.status);``````js require('node-fetch')(`http://httpstat.us/501`).then(r => r.status);

#

There are only two cases in which it does, and only one is applicable to discord.js (since credentials are sent in headers) https://developer.mozilla.org/en-US/docs/Web/API/WindowOrWorkerGlobalScope/fetch:

real jetty Sep 26, 2020, 1:58 PM

#

Heya, I was messing around with guildMemberSpeaking on a new project that I forgot to install the voice dependencies for. On the latest version, when I try to play a stream that I've received from a user (so, voice audio), it fails silently if I dont have the dependencies installed. This seems to be different from previous versions, like 12.2.0, where it would throw an error if I was missing deps.

#

Not sure if this is intended and whether or not I should open an issue for this

real jetty Sep 26, 2020, 2:15 PM

#

Actually, scratch that, that may have been a fluke while listening on guildMemberSpeaking. The event doesn't seem to be firing at all, and I'm not the only one with this issue

solemn oyster Sep 26, 2020, 2:50 PM

#

@tacit crypt read above please

tacit crypt Sep 26, 2020, 2:50 PM

#

Replying to @solemn oyster from https://canary.discordapp.com/channels/222078108977594368/682166281826598932/759396659200262186
same imo, we can split later if needed/wanted

#

Replying to @solemn oyster from https://canary.discordapp.com/channels/222078108977594368/682166281826598932/759406927091859476
cool, so cleanup the comment and the error 👍

solemn oyster Sep 26, 2020, 2:51 PM

#

wdym

tacit crypt Sep 26, 2020, 2:53 PM

#

Uh

#

remove the comment if its inaccurate

#

and i guess keep the rest of the code as is?

solemn oyster Sep 26, 2020, 2:54 PM

#

I moved code around because the flow was hard to understand

tacit crypt Sep 26, 2020, 2:54 PM

#

thats fine

solemn oyster Sep 26, 2020, 2:55 PM

#

Because it was handling this way:

2xx/3xx
429
5xx
4xx

#

I made it be:

2xx/3xx
4xx

429
!429
5xx

tacit crypt Sep 26, 2020, 2:55 PM

#

coool

solemn oyster Sep 26, 2020, 2:57 PM

#

I guess by removing the comment, you meant Isa's comment (git blame points to her). I replaced it withjs // Retry the specified number of times for request abortions

tacit crypt Sep 26, 2020, 2:58 PM

#

ya

real jetty Sep 26, 2020, 5:38 PM

#

Actually, scratch that, that may have been a fluke while listening on guildMemberSpeaking. The event doesn't seem to be firing at all, and I'm not the only one with this issue
@real jetty I seem to have found the solution to this problem
https://github.com/discord/discord-api-docs/issues/808#issuecomment-457962359
Now you cannot receive anything unless you first send several packages through the UDP connection.
This issue describes it.
Would it be possible to add this to the library so we don't have to worry about sending empty packets first before listening to user streams?

ornate topaz Sep 26, 2020, 5:42 PM

#

@real jetty tmk library already does that, but you still have to send some data before receiving it.
There's also a fact that voice receiving is not documented, and is changed from time to time

real jetty Sep 26, 2020, 5:43 PM

#

Yeah, as I figured from the issue. I just spoke with someone from #archive-djs-v12-voice-deprecated and they said the solution to listening on guildMemberSpeaking was to send an audio file at volume 0, but then what's the difference between that and creating a readable stream from /dev/null LUL

ornate topaz Sep 26, 2020, 5:44 PM

#

That audio files are not os dependant shrugCat

real jetty Sep 26, 2020, 5:45 PM

#

lol guess so

lofty birch Sep 27, 2020, 9:27 AM

#

When will #4835 be published? I see it's been merged

#

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

copper laurel Sep 27, 2020, 9:28 AM

#

Whenever 12.4 is released, we don't do scheduled release cycles

solemn oyster Sep 27, 2020, 12:50 PM

#

@lofty birch at very minimum, 4852 needs to be merged before releasing, because I introduced a bug in that PR

#

I didn't notice the bug because it's only noticeable when you do an instanceof check, which I didn't during testing

snow crypt Sep 29, 2020, 8:49 PM

#

well

#

both merged now

#

any release soon?

unique axle Sep 29, 2020, 8:55 PM

#

as per usual we do not give an ETA on releases

tacit crypt Sep 30, 2020, 12:50 PM

#

@unique axle how is a major change? In relation to the edit limit

You're still holding edits, and for 99% of people it isn't even a problem we'd now hard limit to 100

Even then, we could just... Default to -1, thus minor

unique axle Sep 30, 2020, 12:50 PM

#

it changes the public facing behaviour of the library, how would it not be major?

#

currently there is simply no limit on it, any form of imposed limit by default is thus changing and major

#

-1 being "no limit" would make it minor, sure

tacit crypt Sep 30, 2020, 1:50 PM

#

it changes the public facing behaviour of the library, how would it not be major?
@unique axle it doesn't change anything public facing. The edits array is the same, and its contents. What it changes is a "limit" (that for all intents and purposes is not breaking at all)

#

An internal limit at that

unique axle Sep 30, 2020, 1:51 PM

#

pardon? before you could access 51+ edits, now you can't, how would that not be breaking?

tacit crypt Sep 30, 2020, 1:52 PM

#

who says you can't

#

message.edits[51] is not different. the only diff is how big that array WILL grow (and come on man you can't find me a person who DEPENDS on that many edits [also i won't comment how personally i dislike the idea of the array but alas])

unique axle Sep 30, 2020, 1:53 PM

#

huh? if you limit an array to a max length of 50 where before there was no max, that's a change in behaviour
or am i completely misunderstanding this pr

tacit crypt Sep 30, 2020, 1:53 PM

#

which, fyi, for all the user knew, it could've been limited at 10 or 100 or Infinity.. shrug

#

Look, idk, I don't agree with setting the limit to 100 being a major change. If it is, thats stupid semver wise, and at that point for every thing we now let you control its cache we.. release a new major??

#

(assuming we don't have 1 big PR with it, but split it into pieces)

#

and fyi

unique axle Sep 30, 2020, 1:56 PM

#

how is that stupid? what? i really don't get it
you set a default limitation where there was none before
adding the ability to let the user manage this is minor, if it behaves the exact way it did before the change by default
as soon as the default behavior changes in any way that's semver major

tacit crypt Sep 30, 2020, 1:56 PM

#

MAJOR version when you make incompatible API changes,
MINOR version when you add functionality in a backwards compatible manner, and
PATCH version when you make backwards compatible bug fixes.

#

its not incompatible at all

unique axle Sep 30, 2020, 1:56 PM

#

introducing a limit where none was is def. incompatible

tacit crypt Sep 30, 2020, 1:56 PM

#

At least thats what semver.org says shrug

#

...you what?

#

Adding a configurable limit is in no way incompatible api change in my eyes

unique axle Sep 30, 2020, 1:57 PM

#

but the default behaviour changes

#

if you have to apply a configuration to get the same behaviour you got before without any that's breaking

tacit crypt Sep 30, 2020, 1:57 PM

#

No, cause its backwards compatible

unique axle Sep 30, 2020, 1:58 PM

#

only if the default is no limit

tacit crypt Sep 30, 2020, 1:58 PM

#

you added functionality (cache size limit), that defaults to 100 ("major") BUT you can set to (-1) => MINOR

unique axle Sep 30, 2020, 1:59 PM

#

if you default it to no limit it's minor, yes
if you default it to 100/50/x it's major

tacit crypt Sep 30, 2020, 1:59 PM

#

HOW

#

Souji. You CAN SET IT TO THE BEFORE LIMIT

unique axle Sep 30, 2020, 1:59 PM

#

you have to apply a setting in a minor release?

#

you have to add code to set it to that

#

you have to make a change to your code base after updating to get the exact same behaviour

tacit crypt Sep 30, 2020, 2:00 PM

#

you have to add between 1 and 3 lines of code MAX. Even then it's still backwards compat imo

unique axle Sep 30, 2020, 2:00 PM

#

the whole idea behind semver is you can update the dependency without anything breaking without you having to apply any change, whatsoever

#

as soon as you have to now apply a setting to get the same behaviour as in the earlier version you made a breaking change that requires someone to maintain their code base

tacit crypt Sep 30, 2020, 2:01 PM

#

doubt

#

I don't see it like that

unique axle Sep 30, 2020, 2:01 PM

#

that is legit what semver is for

#

02Shrug

tacit crypt Sep 30, 2020, 2:01 PM

#

I'm referring to the "requires a setting => major"

#

Not the versioning itself

#

If I delete the prop altogether, thats DEFINITELY major (INCOMPATIBLE API change, no more edits prop)
If I artificially LIMIT the prop to have a length of 50, it still has the prop, its still there shrug

lofty birch Sep 30, 2020, 2:02 PM

#

An update where you have to change anything to get the same behaviour is breaking

unique axle Sep 30, 2020, 2:02 PM

#

you still changed how a public facing part of the library works

#

you have to make a change to keep what you had working before, even if it's just a single letter, that's not minor anymore

tacit crypt Sep 30, 2020, 2:07 PM

#

Yeah no, I disagree, but thats my opinion. The language itself is vague as is (and no, adding an option to limit a cache is not breaking)

What next, we release v13 or w/e, then I make a PR to limit another cache and we jump to v14? that's absurd

#

And from a G search

It sounds like they just mean "mostly backward compatible".

The definition of "backward compatible" pretty much is "not breaking stuff". But if there are some small changes to rarely used things, that only affect a very small portion of users, it may still be accurate to describe a new version as backward compatible overall.

unique axle Sep 30, 2020, 2:09 PM

#

then we'll have to agree to disagree here, the point of semver is so you can specify a range in your dependencies and experience no change in behaviour whatsoever.
after all you guys (maintainers) will have to decide that, i'm clearly not making the decisions in that regard as merely a contributor and triage

tacit crypt Sep 30, 2020, 2:10 PM

#

Technically this PR with a def limit of 100 would be mostly backwards compatible shrug

#

I'm not trying to say you are 100% wrong and I'm right, or w/e. I'm just stating my opinion that this PR, imo, is not major but minor. It's not in my hands when it'll be merged/released with what ver number. But personally, I would've merged it as minor

#

Technically with this one we're both right

vagrant holly Sep 30, 2020, 2:51 PM

#

^ Setting it to 100 makes it a major change per semver, I concur

#

Setting it to -1 means the behaviour matches master by default, thus minor. PR now has that 🙂

solemn oyster Sep 30, 2020, 5:56 PM

#

@tacit crypt JFYI, we can change limit from -1 to 50 (or 100) in v13, if not entirely remove _edits

#

We didn't need _edits when we made KC, and it worked just fine, I don't see what its purpose is, or why it was added in the first place

unique axle Sep 30, 2020, 5:57 PM

#

that was the initial point i was trying to make, seeing that v13 is not going to take very long having the feature be implemented without changes in behaviour now and changing the default/behaviour with v13 seems to be a preferable solution to me

solemn oyster Sep 30, 2020, 5:58 PM

#

Yeah, in v12, any change we make must make everything that has been documented, behave like so

tacit crypt Sep 30, 2020, 5:58 PM

#

We never documented edits having an infinite length 0_EyesMoving

solemn oyster Sep 30, 2020, 5:59 PM

#

It does document "the oldest", anyways

#

https://discord.js.org/#/docs/main/stable/class/Message?scrollTo=edits

tacit crypt Sep 30, 2020, 5:59 PM

#

Considering how message updates work tho..

#

That's wronf

#

And inaccurate of..messages fetched

weak kraken Oct 1, 2020, 2:57 AM

#

Have you heard if they plan on implementing anything like an event emitter/stream for audit logs? or upgrading the guildMemberRemove one to indicate kicked users states?

copper laurel Oct 1, 2020, 3:10 AM

#

Nothing like has come from Discord, no

weak kraken Oct 1, 2020, 3:11 AM

#

kinda shocked, would seem like the audit log would be a natural place to track all manner of events

#

albeit redundant

ornate topaz Oct 1, 2020, 3:11 AM

#

discord doesn't send anything for when audit logs are created

weak kraken Oct 1, 2020, 3:12 AM

#

i know, i was asking if its planned

ornate topaz Oct 1, 2020, 3:12 AM

#

it just reflects changes sent to clients in other events

#

(which already would make audit logs events redundant)

weak kraken Oct 1, 2020, 3:12 AM

#

just feels weird to have to use audit logs to discover if a user was kicked, for example

ornate topaz Oct 1, 2020, 3:12 AM

#

while true, there isn't much we can do about that

snow crypt Oct 1, 2020, 7:13 AM

#

@vagrant holly can't you just remove the -1 for infinity? Infinity exists and works in that case so -1 too seems weird

vagrant holly Oct 1, 2020, 8:41 AM

#

We seem to use -1 in a lot of other settings for no limit, so I reflected that with this PR too @snow crypt

snow crypt Oct 1, 2020, 8:41 AM

#

i see now, my bad

vagrant holly Oct 1, 2020, 11:18 AM

#

👋 This change is being caused by pre-commit hooks on the project automatically: https://github.com/discordjs/discord.js/pull/4867#pullrequestreview-499788677

#

Should I skip hooks to revert the change, or should we just leave it?

vernal atlas Oct 1, 2020, 12:32 PM

#

perhaps you could sneak in a prettier-ignore comment or something?

unique axle Oct 1, 2020, 12:33 PM

#

what even is the actual issue here? eslint conflicting with prettier or something?

#

because i'd much rather fix the underlying issue than monkey-patch it for each PR

vagrant holly Oct 1, 2020, 1:40 PM

#

yeah, eslint & prettier seem to be fighting

lofty beacon Oct 1, 2020, 3:55 PM

#

What are peoples thoughts on User.mutualServers? Just gets the guilds the bot and the user are mutual to.

unique axle Oct 1, 2020, 3:56 PM

#

that information is available

lofty beacon Oct 1, 2020, 3:56 PM

#

I know that, but adding a shortcut on the User class. Do you think that'd be a good idea?

unique axle Oct 1, 2020, 3:57 PM

#

if you know that the information is not available and requires an oauth2 request, how would a shortcut on User be of any use?

lofty beacon Oct 1, 2020, 3:57 PM

#

Can't you just map the guilds and check if the user is present in it?

unique axle Oct 1, 2020, 3:58 PM

#

that information is inaccurate due to just affecting cached member instances

lofty beacon Oct 1, 2020, 3:58 PM

#

You could fetch them, set the cache option to false and see 🤷

unique axle Oct 1, 2020, 3:59 PM

#

no. we don't need a utility function that iterates over n servers, fetches members on all of them and bloats cache into oblivion

lofty beacon Oct 1, 2020, 3:59 PM

#

alright

#

thanks

unique axle Oct 1, 2020, 4:00 PM

#

the official way to retrieve that data (shared servers) is an oauth2 request
we're not going to support some way to prevent api limitations and make N requests for the user in the background, hidden behind a single seemingly harmless function call

lofty beacon Oct 1, 2020, 4:00 PM

#

makes sense

cerulean siren Oct 1, 2020, 5:03 PM

#

Is there a planned version with Deno Support?

raven juniper Oct 1, 2020, 5:03 PM

#

No

cerulean siren Oct 1, 2020, 5:03 PM

#

Ok, thanks for your answer

tender field Oct 1, 2020, 8:28 PM

#

it is a bit weird that AuditLog is not a manager, isn't it ?
Like we have to fetch it to get the actions enums... And i feel like some cache capacity could be added 🤔 Is it planned for v13? Is there good reason to justify the way it is now?

unique axle Oct 1, 2020, 8:28 PM

#

managers manage (hence the name) a cache, neither audit logs nor bans (where this also applies) are received through websocket events (and caches/cached structures) and consequently also don't require managers

tender field Oct 1, 2020, 8:29 PM

#

oh ok

#

but why do we have to fetch it to get the enums ? This also feels weird to me

unique axle Oct 1, 2020, 8:30 PM

#

pardon?

tender field Oct 1, 2020, 8:30 PM

#

(what i call the enums are GuildAuditLog#Actions)

#

oh no i'm dumb

#

i can juste call AuditLogActions

#

then it makes sense the way it is

copper laurel Oct 2, 2020, 5:03 AM

#

So, currently drafting the PR for inline replies
The Discord API property for the message a crosspost references is message_reference, which we implemented as Message#reference
The API property for the message an inline reply references is referenced_message

So I need a naming convention for this property - Message#replyReference?

opaque vessel Oct 2, 2020, 6:08 AM

#

That sounds okay! It feels like in the next major version, .reference should change to .crosspostReference to keep the a clear distinction between that and .replyReference? 😅

copper laurel Oct 2, 2020, 6:12 AM

#

That definitely makes sense - Souji has a PR to replace Message#reply with functionality that would act as a helper for inline replies, and might also include other breaking changes like renaming properties like that

oak quail Oct 2, 2020, 7:52 AM

#

@opaque vessel @copper laurel message_reference is also used for replies, as well as pin messages and follow add messages

#

message_reference is guaranteed on replies while referenced_message may not be present

copper laurel Oct 2, 2020, 8:02 AM

#

Right, that's what some comments suggested but it wasn't clear

#

So referenced_message is only on replies, but message_reference is all of them?

#

Why does the API bother sending both, because a reply needs author data etc for the reference?

oak quail Oct 2, 2020, 8:08 AM

#

So referenced_message is only on replies, but message_reference is all of them?
yeah

#

and it sends both because message_reference just has the IDs which can always be provided; referenced_message has the full message object which is for client rendering (they don't want all clients fetching the message) but in some cases that won't be returned

copper laurel Oct 2, 2020, 9:47 AM

#

So then if its for clients, is there any reason for Discord.js to implement referenced_message as I have as a Message property? Or just cache it using that data, and rely on the properties from message_reference if it needs to be grabbed from cache and/or fetched later?

tacit crypt Oct 2, 2020, 9:56 AM

#

I mean we can still do it, Discord gives us the data, we might as well hand it over as a Message object (so then we also don't need to fetch the reply)

lofty birch Oct 2, 2020, 11:02 AM

#

Would message cache sweeping behaviour not affect this? If it was set to delete all messages older than 10 minutes, for example, and a message younger than that includes a reference to a message that has been removed from cache, how would that work?

ornate topaz Oct 2, 2020, 11:08 AM

#

would probably be removed again in next sweep

#

which would be to be expected, honestly

slate nacelle Oct 2, 2020, 11:09 AM

#

The PR in its current state seems to be referencing the object directly, so it would no longer be in the cache, but still accessible, through the referencing message, after sweeping.

lofty birch Oct 2, 2020, 11:11 AM

#

Is that necessarily good behaviour though? By being accessible through the referencing message, it's still cached, and will only properly be swept once the latter message has been also. Perhaps making it a partial message object once swept would be better?

copper laurel Oct 2, 2020, 11:15 AM

#

Sweeping a Collection doesnt modify any objects that arent being swept though

slate nacelle Oct 2, 2020, 11:15 AM

#

It's also possible to make it a getter, like User#lastMessage and friends. Messing with the object after sweeping is imo a no-no.

copper laurel Oct 2, 2020, 11:15 AM

#

That would actually become quite a complex operation, checking every Message still in the cache

#

Yeah, I did suggest it could be a getter based on the Message#reference property, and we just cache it when it received, not store as a reference

#

Then sweeping would work as normal

lofty birch Oct 2, 2020, 11:17 AM

#

I'm just thinking of the memory usage more than anything. A getter would fix that

#

How would it behave if we then wanted to fetch that other message though?

slate nacelle Oct 2, 2020, 11:18 AM

#

Similarly, by having a field with the id, which you can use to fetch.

copper laurel Oct 2, 2020, 11:18 AM

#

As normal

#

MessageManager.fetch(message.reference.messageID)

lofty birch Oct 2, 2020, 11:21 AM

#

That's a lot better behaviour than the sweeping not working. I'd be totally happy to use that

vernal atlas Oct 2, 2020, 4:48 PM

#

im starting a PR to update to the v8 API, should i bundle it all in one or create multiple PRs

#

im thinking multiple PRs might be better but want to know if it ultimately matters

oak quail Oct 2, 2020, 7:39 PM

#

@vernal atlas you should clearly label that this pr switches to API v8

vernal atlas Oct 2, 2020, 7:39 PM

#

aight will do

oak quail Oct 2, 2020, 7:41 PM

#

or tbh just do all v8 stuff in 1 pr like fyko said

#

did any maintainers say anything

vernal atlas Oct 2, 2020, 7:42 PM

#

got no pings about it

copper laurel Oct 2, 2020, 9:24 PM

#

Note that I already added message type 19 in my inline replies PR, so that's v8-ready

oak quail Oct 2, 2020, 11:02 PM

#

@vernal atlas why cant permissions be null here https://advaith.is-in-hyper.space/3111f6f281.png

#

or a string

#

in RESTPostAPIGuildRoleJSONBody its permissions?: number | string | null;

vernal atlas Oct 2, 2020, 11:04 PM

#

might have been an oversight

oak quail Oct 2, 2020, 11:05 PM

#

should i just remove it so its the original value

#

because afaik only the id is different

vernal atlas Oct 2, 2020, 11:05 PM

#

ye i guess so

oak quail Oct 2, 2020, 11:17 PM

#

also i'll change

export type APIGuildCreateOverwrite = Pick<APIOverwrite, 'type'> & {
    id: number | string;
    allow: number | string;
    deny: number | string;
};
```to
```ts
export interface APIGuildCreateOverwrite extends RESTPutAPIChannelPermissionsJSONBody {
    id: number | string;
}

vernal atlas Oct 2, 2020, 11:19 PM

#

oh wait i think i know why i added them

#

i think it was to do with

#

permissions being deprecated and x_new being a thing

oak quail Oct 2, 2020, 11:20 PM

#

heres RESTPutAPIChannelPermissionsJSONBody in v6:

export interface RESTPutAPIChannelPermissionsJSONBody {
    allow: number | string;
    deny: number | string;
    type: OverwriteType;
}

vernal atlas Oct 2, 2020, 11:20 PM

#

i think that'll be good actually tho since RESTPutAPIChannelPermisionsJSONBody (god, thats a mouthful) doesn't have _new

oak quail Oct 2, 2020, 11:20 PM

#

yeah

vernal atlas Oct 2, 2020, 11:20 PM

#

👍

vernal atlas Oct 3, 2020, 1:58 AM

#

ok, done some work on my V8 PR if anyone wants to give it a test feel free (i have tested it but its always good to have a couple other tests, mine werent too extensive) https://github.com/discordjs/discord.js/pull/4879

#

i think i've covered everything, i went through the diff from the api docs commit, i imagined it'd be bigger but i guess not

#

this PR is mainly just the basics btw, i haven't really got too much into handling uncached data from missing intents, but i think that this was planned for the next major update in discord.js-next

oak quail Oct 3, 2020, 2:20 AM

#

@vernal atlas did you test if messages in DMs work

#

Bots no longer receive Channel Create Gateway Event for DMs
this might break it

#

also did u update ratelimit stuff

The Retry-After header is now based in seconds instead of milliseconds (e.g. 123 means 123 seconds)

The X-RateLimit-Precision header is no longer respected. X-RateLimit-Reset and X-RateLimit-Reset-After are always returned at millisecond precision (e.g. 123.456 instead of 124)

vernal atlas Oct 3, 2020, 2:21 AM

#

hm, i didnt test that, but at a guess you would need the CHANNEL partial type enabled

oak quail Oct 3, 2020, 2:21 AM

#

then should enable it by default i suppose

#

or just not make it an option

vernal atlas Oct 3, 2020, 2:21 AM

#

also have not updated the ratelimits, wasnt that done in another PR or am i mistaken

oak quail Oct 3, 2020, 2:22 AM

#

i dont think it was

vernal atlas Oct 3, 2020, 2:22 AM

#

oh yea that was just the async queue

#

ill update the ratelimits tomorrow

oak quail Oct 3, 2020, 2:22 AM

#

did u account for this

Channel Permission Overwrite types are now numbers (0 and 1) instead of strings ("role" and "member"). However due to a current technical constraint, they are string-serialized numbers in audit log options.

vernal atlas Oct 3, 2020, 2:23 AM

#

oh

#

i dont think i did

#

should be an easy enough fix

oak quail Oct 3, 2020, 2:23 AM

#

that probably wouldn't have been documented if i didn't mention it bloblul

vernal atlas Oct 3, 2020, 2:24 AM

#

FedoraLUL

#

that wasnt on the api docs v8 diff hmm

oak quail Oct 3, 2020, 2:27 AM

#

it was

vernal atlas Oct 3, 2020, 2:28 AM

#

oh i couldnt find it

#

maybe im just blind, happens

lofty beacon Oct 3, 2020, 7:43 PM

#

hey
when fetching members, it allows you to pass in an object when fetching
shouldn't it be like this for fetching roles, channels maybe?
i might make a pr of it just was wondering if well people thought it was a good idea

lofty birch Oct 3, 2020, 7:44 PM

#

You can't fetch only a particular channel or role. The API just doesn't support it

lofty beacon Oct 3, 2020, 7:44 PM

#

oh does it not?

#

sorry then

lofty birch Oct 3, 2020, 7:45 PM

#

The reason you can fetch members with a query is because there could be hundreds of thousands of members

lofty beacon Oct 3, 2020, 7:45 PM

#

yea

lofty birch Oct 3, 2020, 7:45 PM

#

And returning all of them would be a huge waste of resources

#

Unlike channels (500) and roles (250)

lofty beacon Oct 3, 2020, 7:45 PM

#

yea makes sense, thanks.

sullen mortar Oct 4, 2020, 12:10 AM

#

Any solutions to insane memory usage?

#

I have two shards ~1.8k guilds, shards are allocated 1gb of memory and restarts daily due to memory usage

#

message cache set to 50

#

My bot holds no state

unique axle Oct 4, 2020, 12:13 AM

#

you can manually sweep caches on an interval
apart from that currently not much, addressing custom caches is on the roadmap for the planned typescript rewrite, it requires us fundamentally rethinking the inner core structures so it's really nothing we can feasibly do under the current library.

if your bot can completely work without state you are probably better off using a more low level approach to websocket events and a rest API (for now at least)

left moat Oct 4, 2020, 12:13 AM

#

make sure ur using the correct intents

sullen mortar Oct 4, 2020, 12:14 AM

#

my intents are GUILDS, GUILDS_MESSAGES and GUILD_MESSAGE_REACTIONS

#

@unique axle great info, thanks! I'm pleased to see the project making steps to address these memory issues

#

I know you can sweep messages cache on interval, but other caches too?

unique axle Oct 4, 2020, 12:15 AM

#

sure, caches are at the core Collection(s) which are extended from the base JS Map structure, you can #clear #sweep them as documented, providing a predicate function to selectively clear for the latter

#

the emphasis here is on manually, there is no functionality for automatic sweeping at the moment
ref:
https://discord.js.org/#/docs/collection/master/class/Collection?scrollTo=clear
https://discord.js.org/#/docs/collection/master/class/Collection?scrollTo=sweep

sullen mortar Oct 4, 2020, 12:16 AM

#

yeah - gotcha

#

thanks

oak quail Oct 4, 2020, 12:25 AM

#

i don't see X-RateLimit-Precision in the code; does djs use second precision? is ms precision even supported in apiv7?

unique axle Oct 4, 2020, 12:27 AM

#

@tacit crypt (i think that's your thing) this
where thing := area of proficiency

tacit crypt Oct 4, 2020, 12:27 AM

#

I don't think it was ever PR'd to d.js (or it was in that rest PR of mine that is dustier than dust)
And iirc api v8 doesn't support it sooooo

oak quail Oct 4, 2020, 12:41 AM

#

yeah was just wondering if ratelimiting would break if i updated something to v8

#

but v8 sends decimals now, so maybe that could break it? idk

#

it looks like it shouldnt break

#

cc @vernal atlas might not have to update ratelimit stuff

vernal atlas Oct 4, 2020, 12:59 AM

#

ah

copper laurel Oct 4, 2020, 6:46 AM

#

Going back to the referenced_message data, which is a complete Message object to support the message_reference properties of a reply, what do people think:

maintain a complete replyReference property on Message
cache the message, but make replyReference a getter rather than maintaining a direct reference

#

Also to note, that getter needs to know if the properties exist because its a reply and not if its a crosspost or pin notification

warped crater Oct 4, 2020, 6:49 AM

#

And iirc api v8 doesn't support it sooooo
source?

copper laurel Oct 4, 2020, 6:49 AM

#

Which leans more to the first option being easier, as the second would need to define something like isReply

#

@warped crater https://discord.com/developers/docs/change-log

#

The X-RateLimit-Precision header is no longer respected. X-RateLimit-Reset and X-RateLimit-Reset-After are always returned at millisecond precision (e.g. 123.456 instead of 124)

warped crater Oct 4, 2020, 6:51 AM

#

ahhhhhh I see, that's much better then

#

I guess they did it in a backwards-compatible fashion before as a feature on top

#

but now it's just always ms

#

good good

real jetty Oct 5, 2020, 12:36 AM

#

Hm, I'm experiencing a strange bug, when the bot removes its own reaction from a message it sent, it seems to be ratelimited, as in the rateLimit event is fired. This this intentional?

#

Or, wait actually this might be a combination of things, one moment let me see if I can reproduce it

#

Never mind, it's just my bot doing many things in quick succession; what I was doing that would trigger the rateLimit event was I was adding a reaction to a message sent by the bot and then immediately removing the reaction.

#

I guess Discord didn't like that.

surreal geode Oct 5, 2020, 12:40 AM

#

Message reaction ratelimits are 1 per time period so you will always hit a ratelimit when reacting/removing reaction

copper laurel Oct 5, 2020, 6:34 AM

#

Going back to the referenced_message data, which is a complete Message object to support the message_reference properties of a reply, what do people think:

maintain a complete replyReference property on Message

cache the message, but make replyReference a getter rather than maintaining a direct reference
Additional thought - would the getter need to just be for replies? Couldn't it try to get the referenced message no matter if its a reply or a crosspost?

lofty birch Oct 5, 2020, 7:02 AM

#

What if it was crossposted from a different shard (with traditional manager) though?

copper laurel Oct 5, 2020, 7:16 AM

#

Its just a getter, so if it cant get, its null

#

Anyone using sharding generally should expect to have to do fetching of things like that

#

eg

get referencedMessage() {
    return this.reference.messageID && this.client.channels.cache.has(this.reference.channelID)
      ? this.client.channels.resolve(this.reference.channelID).messages.resolve(this.reference.messageID)
      : null;
  }

#

That should have a check for this.reference too but yeah

lofty birch Oct 5, 2020, 7:55 AM

#

That's my point. A reference is different to a crosspost due to which servers they can come from

copper laurel Oct 5, 2020, 7:59 AM

#

Well no, message_reference provides message/channel/guild ID for crossposts, pins, and replies

#

For a reply, we also get a complete referenced_message object which allows us to cache that referenced message also

#

So when it comes to the getter, should it be generic and try to get any sort of reference from cache, which would mostly be replies, or should we restrict it to only replies
And if the latter, we'd need to set a flag like Message#isReply to enforce that restriction

vagrant holly Oct 5, 2020, 10:06 AM

#

👋 Gonna play around with this some more, but can anyone think of any immediate issues to sweeping cached channels? Should the lib cleanly handle a message arriving in an uncached channel etc.?

ornate topaz Oct 5, 2020, 11:12 AM

#

to my knowledge it doesn't emit messages for uncached channels, but i neither kept track of it nor checked on my own

analog oyster Oct 5, 2020, 12:44 PM

#

node v14 will become LTS soon, are you guys planning on supporting it

#

oh ok

vernal atlas Oct 5, 2020, 5:00 PM

#

@raven juniper regarding #4890, doesn't seem out of scope to remove it from the tests too

raven juniper Oct 5, 2020, 5:01 PM

#

i changed the method to fit in the test/random.js file, i'm just saying the whole line seems unwielding