#archive-library-discussion

25085 messages · Page 6 of 26

ornate topaz

discord.js that doesn't fully work on js 🤔

wild flax

so we could either use inversify, or just write our own decorate helper

Its not nice, but i mean thats what you get when you write JS

nvm we dont even have to

we can just use Reflect lmao

exotic flicker

I mean, you dont really need the decorators

require("reflect-metadata");
const { injectable, inject, container } = require('tsyringe');

class CustomChannels {
  test = 'test2';
}

container.register('CUSTOM_CHANNELS', CustomChannels);

class Channels {
  test = 'test';
}

class Client {

  constructor(customStructures) {
    this.customStructures = customStructures;
    // kinda fucky wucky idk
    this.channels = container.resolve(this.customStructures.CHANNELS ? this.customStructures.CHANNELS : Channels);
  }
}

const client = new Client({ CHANNELS: 'CUSTOM_CHANNELS' });
console.log(client);

http://images.chilo.space/rw0x

wild flax

wait so you dont need to mark them as injectable?

I thought that was a requirement

exotic flicker

you do, by using register

warped crater

only if they actually need to.. inject something

oh, am I misunderstanding the decorator too?

wild flax

oh, and since we need users to register it anyway... I guess that makes sense

warped crater

I thought @injectable did implicit resolving so you didn't have to do @inject on every parameter karenconfused

yeah nevermind, it makes sense now

exotic flicker

uhhh yeah dd is right, but I guess as long as we dont do that?
either way you can just fall back to container.resolve in the constructor

wild flax

we need to always resolve anyway for all custom structures

warped crater

am I? I can't even tell anymore.

oh, that's right

@inject all the parameters.

exotic flicker

Class decorator factory that allows the class' dependencies to be injected at runtime. TSyringe relies on several decorators in order to collect metadata about classes to be instantiated.
buut, if I'm not mistaken, you can just do

injectable(CustomChannels)

and get the same result

solemn oyster

@wild flax whatever we use, we'll need something to auto-inject or something

compact spade

is there a reason to use DI over Structures.get/extend?

warped crater

Yes, Structres.get/extend is budget DI

wild flax

Yeah, the current structures are ass

solemn oyster

Client for example is something that needs to be able to resolve in all structures

compact spade

what are the advantages of using "complete DI" over "budget DI"

wild flax

Structures arent DI

compact spade

thats why i said budget DI

wild flax

We just hack together JS to make what they are right now

warped crater

yeah.. it may look "clean" on the outside, but internally the current structures system is very difficult to navigate and understand

compact spade

whatever they are, they seem to work fine rn. are there any major shortcomings with them?

hmm

warped crater

this is similar to a lot of things in the package, for that matter

such as actions

nobody has any idea how they work anymore

wild flax

Yeah, it won't be achievable like this in TypeScript without a lot of Type-hacking

And its certainly just a pain in the ass

solemn oyster

We still need to augment modules, regardless of whether we use DI or not

exotic flicker

what I dont get rn tho, what about the types?
lets say you extended Channels, message.channel would still have the old type, no?

wild flax

thats why you have to augment

compact spade

do you have to augment even with tsyringe

wild flax

yes

solemn oyster

Logically, yes

TypeScript doesn't have anything to do that automatically, types are static, not dynamic

To be honest, I hope we go for resolving the client in all structures on demand, rather than storing a reference to it as a field in all of them

wild flax

that only works well in TS

solemn oyster

We can always make our own solution if you want

So it works well for both JS and TS

wild flax

You make it sounds so easy

solemn oyster

I mean... nothing stops us from doing```ts
import { container } from 'tsyringe';
import { Tokens } from '#lib/util';

export class Base {
public get client() {
return container.resolve(Tokens.CLIENT);
}
}```

compact spade
wild flax

Why would you?

compact spade

big bots who microservice parts of their code would find it very useful, and i don't see a reason not to make that an option

wild flax

?

You dont seem to understand -next very well

You can already use the rest package independent

solemn oyster
compact spade

that's not what im talking about at all...

solemn oyster

And that

compact spade
solemn oyster

REST is just a controller, but the hard work is done in IHandler

wild flax

Yeah its just that your explanation why you want it modifiable didnt make much sense

exotic flicker
compact spade

well handler is just a request handling strategy right? as-in burst or sequential?

solemn oyster

Generally, DI containers are global

And... what even is your use case for more than one client per process?

compact spade

thats not what im talking about kyra, im talking about REST being an interface so people can use their own controllers (maybe one could proxy requests to a server that does ratelimit handling, like spectacles' proxy)

solemn oyster

I'm talking to CHY, sorry

compact spade
exotic flicker
solemn oyster

If you want to make your own REST, just do so, Stitch

wild flax
solemn oyster
wild flax

Except you use the exact same symbols

compact spade

okay cool, that works

solemn oyster

LGTM on the usage of symbols for this

tacit crypt
tacit crypt

IF we want to truly make REST extendable, the interface needs to be VERY strict

compact spade

technically i can already do that extending REST, can't i?

tacit crypt

Unless you overwrite the RequestManager's function that returns a different handler, no

that would mean going down the chain

even then

what you need is to just change api endpoint

to your proxy

and leave the rest to us

think

compact spade

the point of the proxy is it handles ratelimits, not whatever process that is using it

tacit crypt

yeah and?

You forward all requests to the proxy

and return some form of headers that just tell us we can keep requesting (limit 5, remaining 5, boom done)

compact spade

okay fair enough i guess

tacit crypt 
const proxiedRest = new REST({ api: 'http://localhost:1234' });

proxiedRest.get('/gateway/bot');
compact spade

the one proxy that im referring to returns no ratelimit info

so it wouldn't work with your proposal

tacit crypt

we have defaults anyways

compact spade

are they set to zero for everything? XD

tacit crypt 
        // Update the total number of requests that can be made before the rate limit resets
        this.limit = limit ? Number(limit) : Infinity;
        // Update the number of remaining requests that can be made before the rate limit resets
        this.remaining = remaining ? Number(remaining) : 1;
        // Update the time when this rate limit resets (reset-after is in seconds)
        this.reset = reset ? Number(reset) * 1000 + Date.now() + this.manager.rest.options.offset : Date.now();

Infinite limit, 1 remaining, if no headers are present

so basically

compact spade

handling ratelimits in the rest manager would defeat the purpose of a proxy

tacit crypt

it'll work just fine

solemn oyster

Thanks

solemn oyster

I don't recall any place in our code taking an https.Agent instance (which iirc is required for proxies to work)

wild flax

I mean there is no point

its not even how spectacles works

you dont use spectacles REST with spectacles proxy

solemn oyster
wild flax

you use only spectacles proxy and write your own quick rest handler

compact spade

yes

and if i wanted my d.js bot with it, that wouldn't be possible if the d.js rest-handler is enforcing ratelimits itself

wild flax

...?

tacit crypt

Stitch

wild flax

Why would you do that?

tacit crypt

you do understand that, if no headers are present, it'll just...not..deal..with ratelimits right

A limit of Infinity and remaining of always 1 means it'll just pass through

compact spade

ah is that so? nice

wild flax

Thats like me saying, I have the same bot on an eris music bot, but all the other functionality on a d.js bot

Why cant I use the same proxy?

Thats such a weird request

compact spade

it's not as weird as you make it out to be. kyra, for example, wants to microservice everything Moderation-related from skyra. if the new service is doing anything REST related, it's gonna need a proxy for both applications to keep track of ratelimits

wild flax

Yes, but he uses libraries that are meant to work that way

You can't shoehorn this solution

Its the reason I use spectacles, and not a mix of spectacles and djs and eris

I just have a single message broker for example that makes sure all things go into the correct proxy

If we'd support every use case for this, this would mean our "unknown error" ratio would go through the roof because we can not guarantee someone doesnt mess with something we have no control over and suddenly want support for it

compact spade

but wouldn't -next's rest handler being customizable be a good solution to this? it uses the proxy, and we can still use all the convenience methods d.js provides. and if vlad is right, we can do that already

wild flax

I don't think you understand how this works

If you want multiple bots to proxy into a single ratelimit, you would build a server that WRAPS -next' rest handler

And then make requests to that wrapper

Not the other way around

compact spade

yes, and making REST an interface would solve that?

wild flax

Thats exactly how spectacles works

No?

How?

You wouldnt need any of the rest handlers logic

The proxy handles that

You just blindly fire request to the proxy, doesnt matter what it is

compact spade

lets assume REST is an interface with get(), post() and put(). i implement it to send http requests to my proxy server, which then does the actual ratelimit handling.

tacit crypt

No, crawl, what Stitch wants is to forward all requests done through -next's rest to some local endpoint, to a global "proxy" (in go or otherwise) that handles the rest

wild flax

I understand that

tacit crypt

which, as I said, is already doable

wild flax

But thats unneeded

compact spade

that way i can use message.channel.send() and it works with my proxy 😉

tacit crypt

...

wild flax

Yeaaaaah, thats exactly what I meant with shoehorning a solution

tacit crypt

Y'all ignoring me intentionally? Stitch, just override the api endpoint. That's all. If your proxy runs and it catches all requests and works PROPERLY (do note its your responsibility if you fuck ratelimits up and get banned), d.js will just pass requests on

or rather, not d.js but -next's rest

IN FACT, the EXACT SAME is possible right now in v12

Same logic

compact spade

yes vlad i gotcha, i did mention you said it was already possible

tacit crypt

override the api base

wild flax

You talk about microservices but say stuff like message.channel.send when you wouldnt even theoretically have a cache like that in mutliple services because its not feasible

compact spade

that wasn't the point, the point is i can continue using discord.js' REST functions with my own proxy and handler

tacit crypt

you

dont

touch

the fucking

handler

You strictly change 1 option

compact spade

yes vlad i get it, don't worry

wild flax

If it wasn't the point don't bring up the points of microservices

compact spade

now you're just taking whatever i say out of context, im not continuing this, especially since the "issue" is already resolved anyway

wild flax

If you continue to dismiss the claims that proper microservices do not rely on a monolithic library like d.js to properly function and just try to have it your way, go ahead.

You brought examples into this with skyra that have no foundation because its simply not how you would even design such a system

solemn oyster

My opinion on this is that we shouldn't go too much overboard, -next is still very young and while many things are in the design stage, most aren't even in that step, naturally, -next will evolve with time just like any other OSS project

compact spade

¯_(ツ)_/¯

if you say so, sure

solemn oyster

I personally think that having a functional REST package that works for the typical case, is priority

We can enhance it later with things we might need in the future, but as for now, we aren't very sure about everything

wild flax

Nah, its easy, just do it

Don't worry.

solemn oyster

I'd say give the new REST a shot once it's released, and if you can find a solution (Crawl and Vlad proposed some) that works for you, then you don't have anything else to worry about

tacit crypt

Kinda like what I want, a handler that spreads out requests evenly in the reset window (so if we can do 4 requests a second it'd spread them out as evenly as possible), but thats offtopic

real compass

doing some personal testing, exporting a class also exports the built in interface for the class

^ conversation relating to the ts recode and exporting interfaces

solemn oyster

There are chances that we might keep Structures, mostly because it's the polar opposite of DI

The only feasible alternative a friend of mine (who's well versed in DI and many other programming patterns) was the transformer pattern, which is basically the same as Structures but less optimised and bulkier

snow crypt

@tacit crypt @unique axle my point with the PR is to e n f o r c e the consistency, not just change all cases into using the same bracket type (have no idea how to word this).
With the PR in place it means: route building is property access, requests are function calls, no way for it to be confusing like that.
Currently (even with the PR), you can still use .guilds(id), so if the PR gets merged and turns out I missed some changes, the library won't break.
When we're sure that all places use the correct way, we can make it error - e n f o r c ing the new style.

solemn oyster

To be honest, I'm unsure if we'll go for that path, we can very well replace REST with -next's once Smugen's PR lands (since we need the other changes)

tacit crypt

^

wild flax

I'm more in favor of x.y.z.post(), x.y(id).z.post() or x.y(id).z(member).post() than any other variation

snow crypt

unenforced consistency

wild flax

I think thats a pretty consistent pattern

clever crypt

Would it be a good idea to add some sort of error throw on GuildMemberManager#fetch if the correct intent isn't enabled?

ornate topaz

wouldn't that be done on api side already

clever crypt

it'll just timeout

snow crypt

Could Message#authorID be added for partial messages?

clever crypt

hmm .. i mentioned something similar yesterday to be used with Guild#owner, suggesting it should be GuildMember | PartialGuildMember. the only 'valid' property of PartialGuildMember is id and it has a fetch function similar to GuildMember#fetch

perhaps the same thing could be applied here? PartialUser where only id is valid with a fetch function?

unique axle

what's the reasoning for Message? seeing that both user and member data are included in the message payload?

real compass

does running .fetch add to its respective cache?

tacit crypt

yes

unless you pass false to the cache parameter

snow crypt

@unique axle author.id is known in MESSAGE_UPDATE, so I can save an api call by using it on the partial structure and not fetch it.

all i need there is message.id, message.authorID, message.content

which I should not have to fetch

oak quail

so whenever djs has a message it should have an author object

so authorID would never be present if author.id isnt

copper laurel
ornate topaz

Huh

Oh, hm

snow crypt

@oak quail but it is not

when message.partial is true, message.author.id throws cannot access id of undefined

oak quail

well that means that djs is constructing a message object from just an id

snow crypt

why though

solemn oyster

Try enabling user partials, @snow crypt

snow crypt

Still (node:7640) UnhandledPromiseRejectionWarning: TypeError: Cannot read property 'id' of null when message.partial

copper garden
snow crypt

yes, no

I have edited the literally oldest message I could find in a server (which is only 1.5 years ago, but still not very new), and MESSAGE_UPDATE (using client.ws) emitted with author

copper garden

Was it a webhook message?

snow crypt

no

copper garden

Can you show the data object?

snow crypt

well i can redo it in a moment

wild flax

aFeelsLateMan

oak quail

well in that case you... still wouldn't have the author id

nothing djs can do about that

snow crypt

yeah fml

see above the raw event with author.id and then messageUpdate where newMessage.author is non-existent

@copper garden

copper garden

Looks like the method assumes that it'll either get a message object like { message: Message {...} } or just { id: "...", channel_id: "...", guild_id: "..." } (partial message)

copper laurel

We looked into this the other day, getMessage doesnt pass the author to getPayload

snow crypt

and.. is that intentional?

copper laurel

Probably not 😄

wild flax

@ruby terrace I don’t see how that our job to maintain lol

And as I read that, several other libs decided against too

ruby terrace

It's not something that the library should handle automatically, it should be a developer choice, but the developer currently can't make that choice

wild flax

Huh

I would argue it’s discord’s fault and that’s it

We do exactly as it should work

It just so happens that discord fucks up

ruby terrace

I can't think of one of the top of my head, but there is a point to be made for a use case where you do want to change solely allowed_mentions.
I know this is a stretch, but maybe mentioning a user, but disabling the highlight afterwards.

wild flax

That seems very cosmetic at most

Which would fall into the argument of a noop IMO

ruby terrace

Editing with allowed_mentions is always going to be cosmetic, even with content come to think of it

wild flax

I’m not against a fix, but I sure wouldnt put any high priority on it

ruby terrace

I wonder if it affects the inbox

I'll get back to you on ^ but yeah, this falls squarely on discords end as this probably wouldn't need to be used if suppressing embeds didn't reset it. Replies make it more prevalent, so it might get pushed up on their priority.

mighty sequoia
tacit crypt

Once v8 api lands, if interactions pr is done we can review it, and merge it after reviews are done

unique axle

and provided discord doesn't decide to just... annihilate it with changes, because after all it is still a beta

oak quail

those prs dont really have anything to do with each other

unique axle

one won't be happening before the other, that is as far as "having anything to do with" goes here

wild flax

We mentioned several times that interactions won’t land before the v8 upgrade

So yeah, the v8 upgrade is a blocker for a lot of PRs

oak quail

finally, that delay is being removed

i thought that was denied

pearl basin
unique axle

completely out of scope of that PR

pearl basin

aaaight

tacit crypt
snow crypt

what delay?

oak quail
kindred haven

Is there any plans to make partials per-event, not globally?
I remember someone talking about it a few months ago but don't remember what it concluded with

exotic flicker

isnt that already a thing?
I mean, its not per event, but per Structure

copper laurel

I dont believe so, no

pearl basin

whats the rule here? lol

slate nacelle

I think that should have been a reject in both cases.

3 years ago, wew

keen meteor

Is it possible to develop following function: When I am in an audio channel, I can invite easily someone to join my channel. Would be nice to do this by rightclicking on an user of my server in his context menu. Second would be nice, when he gets this invitation like a call, with ringing etc., and then he can join my channel by accepting this "invite-call" by accepting it with one mouseclick.
If this is not possible by rightclicking, it would be ok to use this function by text command.

tacit crypt
ashen summit

I’ve been benefiting from discord.js for a while now, so I’d love to contribute. I’ve looked through some open issues, but I’m not familiar with what’s being prioritized

Would anyone want to point me to an open issue or something you need implemented? If so, I’d be happy to submit a PR

ashen summit

Would it be worth changing the functionality of Guild#owner to fetch the owner if it isn't cached? The current documentation and implementation allows for Guild#owner to be null, but it feels like a nice-to-have for users, especially since it appears to be a common question in this server and on stackoverflow

frank turret

The user can easily just fetch the owner themselves using Guild#ownerID, which isn't nullable

warped crater

.. Guild#owner already yields a partial guild member with the guild member partial enabled, afaik

ashen summit

Yep. You're both right. Do those reasons still justify not doing this?

tacit crypt
warped crater

this

many things are partials for various reasons, it is what it is

ornate topaz

auto under-the-hood fetching could be unwanted for few reasons as well

ashen summit

Seems like you've all thought through this and have some good reasons not to do it.

ornate topaz

well, just like "it appears to be a common question in this server and on stackoverflow", it also happens to be brought up as to why it is that way 🤷‍♀️

ashen summit

Yeah I've seen the pattern of having lazily-loaded, memoized fetches for properties before, but it's possible that's an anti-pattern and I think with your partials param the current functionality is fine, even if it results in some people initially making a mistake because the owner isn't in the cache

In that case, I'm still happy to contribute if someone with a bit more context wants to point me to an open issue that is worth addressing. Otherwise I'll just keep my eyes out.

frank turret

I wouldn’t mind if someone wanted to look over my list of getters to be removed in g#5287, maybe add some more

rich iglooBOT
rain laurel

was djs written in ts then brought over to js?

nice

ashen summit

@frank turret I took a look. Caught a small mistake in the current iteration of removing VoiceChannel#editable from index.d.ts but not removing the actual method from VoiceChannel.

Looks like there is a lot of contention around whether you should actually move forward with removing these, so good luck attaining consensus

frank turret

thonk I haven’t removed voicechannel#editable at all so far

ashen summit
frank turret

oh, good catch, yeah i'll resolve that next commit

oak quail

i dont really agree with removing them

frank turret

yeah there was a whole secondary discussion abt it in hydra land. Debating whether to close the PR as it seems as though the move peeps are into is expanding the amount of getters to cover all bases, instead of removing them

oak quail

why exactly can you set replyTo in Message#reply Thonk

you should probably use __Channel#send if you want to specify replyTo

dense kindle
ruby terrace

I think the point is when you're using Message#reply, you'd be replying to that message, otherwise why reply to that message

oak quail

^

I'm fully aware of what reply does lol

remote wasp

the replyTo is hard-coded to this, as it should be. There can be a new option type in place of MessageOptions, which won't have the replyTo field. As having option to set replyTo doesn't make sense here.

oak quail

ah I see

is it not possible to just hide the option from the docs?

ruby terrace

It was definitely intentionally added, I believe that is because the optional parameter is set to none usually, but for replies it is defaulted (by passing in the extra object) to this

oak quail
ruby terrace

I was just looking at that

I think best option is to add to MessageOptions something like forceReply (that name is not great, I'll think of something better).
Since message_reference isn't anywhere but APIMessage, that should be pretty easy to handle and easily ignorable if there isn't a replyTo specified.

ruby terrace

I'll make a PR later if it hasn't been done already.

real compass

why is User.lastMessageId and User.lastMessageChannelId a thing when User.lastMessage returns a message object?

slate nacelle
real compass

ah

real compass

what's the point of having a cache system for things always cached? like guild roles, guild channels, guild emojis

ornate topaz

what exactly are you asking about

frosty nexus

not discord's cache, they have one too, but discord.js has it's own local cache

we dont need to make requests to discord's api if we already have the resource locally (unless we think it might be outdated, in which the .fetch method exists)

does that make sense?

real compass

yeah ik djs has it's own cache, I was saying what's the point of having fetch method and a cache manager or something that's always cached?

keen cosmos

api coverage I believe is one reason

frank turret

out of the three you specified, only the RoleManager has a fetch method which would be there for what Pyro said. That and there still needs to be a manager because just cause structures are always cached doesn't mean there aren't other api methods that can be called on them.

oak quail

guildemojimanager has a fetch method, and the emoji cache can be outdated so it may be needed

frank turret

can't quite find a fetch method on it disregard, was looking at stable

oak quail

might not be in stable?

ornate topaz

it's always cached because you received it from api, not because you hardcode the information into your bot before you start it
it's just that unlike other data, you receive every channel, role etc upon logging in

oak quail

hm yeah it isnt in stable

oak quail
ornate topaz
oak quail

oh

gray zenith

what does the core(*) mean in commit messages?

remote wasp
grim garden

Is the <GuildMember>.id getter gonna be removed at somepoint? afaik it's just an alias for <GuildMember>.user.id.

copper laurel

no

wild flax

Unlikely

solemn oyster

No reason to, plus, that'd break a lot of internals

clever crypt
unique axle

looked into that the other day, and it can't really be coming from where they claim it does.
if it were an array (doesn't look like it) it'd enter the set branch https://github.com/discordjs/discord.js/blob/stable/src/managers/GuildMemberRoleManager.js#L88 and filter dupes
if it's not an array it doesn't even call GM#edit but goes to a put route directly https://github.com/discordjs/discord.js/blob/stable/src/managers/GuildMemberRoleManager.js#L96 which can't return that error (even if the member already has it)

unique axle

FaffsToday at 05:58
seems to be a library issue, not correctly checking if a value exists before adding it to the set
That is precisely the point of passing it through the set constructor - removing duplicates. The relevant code concatenates the current roles with the roles that should be added (with duplicates), passes it to the set constructor (removes duplicates) and spreads the set values back into an array.
_ _

the only point at which that might theoretically fail is if even after resolving roles (which happens in L.83) for some reason the equality check goes wrong - which i currently don't see as possible to happen, since it should at this point quite definitely be an array of strings, for which the equality op === (which uses the same comparison mechanisms as the set equality check as per es2015) quite definitely works
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Set#value_equality

real compass

is there a reason message references haven't been added in like a minor patch?

real jetty

they're there..?

real compass

are they?

slate nacelle
real compass

I meant as in a method

for inline replies

slate nacelle

Oh, don't know.

cursive spruce
ornate topaz

latest as in latest release, or latest master commit

cursive spruce

master

ornate topaz

master already has some breaking changes, so that's not really a good suggestion

unique axle
real compass

ah gotcha

unique axle

@ruby terrace mind elaborating on

but that poses the problem that the library doesn't expose message_reference whatsoever.
Message#reference is a thing potatodetective
and i'm not too sure what that has to do with:
This means that MessageOpitons#failIfNotExists has no indication that it is reply specific other than the jsdoc string.

ruby terrace

Ah, I mean on the sending side. message_reference is essentially named replyTo when passing options to APIMessage, except that parameter currently only sets the message id obviously.
And for the second part, if failIfNotExists is the parameter name in MessageOptions - instead of say, MessageOptions#reference as would follow Message#reference - then there's no indication that this parameter does absolutely nothing when replyTo isn't specified.

unique axle

might be worth nesting that one further then, grouping together the reply related keys into its own reply options, which then hold failIfNotExists and messageReference/reference keys

ruby terrace

I'd agree with that, that keeps MessageOptions much more similar to the API then

unique axle

.send("...", { reply: { messageReference: <MessageResolvable>, failIfNotExists: <Boolean=True> } }) something along those lines

ruby terrace

I like that, would we want to call reply messageReference though since the API does that? so something like this:
.send("...", { messageReference: { message: <MessageResolvable>, failIfNotExists: <Boolean=True> } })

unique axle

i think "reply" is more verbose for the option, while also keeping the messageReference and fail* fields true to API naming (which are essentially shifted to top level)

ruby terrace

That's fair, I'll go ahead and make those changes.

remote wasp

The MessageReplyOptions also needs to be changed now. I think it will have the new reply field with only failIfNotExists field, because messageReference gets set to this. Will wait a few days until we are okay with the changes that have been made to MessageOptions.

oak quail

so maybe
.send("...", { reply: { message: <MessageResolvable>, failIfNotExists: <Boolean=True> } })
or
.send("...", { reply: { to: <MessageResolvable>, failIfNotExists: <Boolean=True> } })

keen cosmos
loud jayBOT

pr_draft #5132 by NotSugden created at 2020/12/18 13:58 (changes requested)
refactor: new node features and consistency
📥 npm i NotSugden/yeetcord.js#node-features

vivid field

</github:805111231349653544> query: 5132

^ makes react() async so that also gets fixed

keen cosmos

ah, gotcha

tender surge
mighty sequoia

(channel as textchannel)

tender surge

That does not do the same

And also wonder why the ChannelType enum was introduced, what channel property returns a number that maps to the right ChannelType?

tacit crypt
tacit crypt
tender surge

ah okay thanks

I think it would make sense to introduce a channelType property on channel so that the enum can be used

tacit crypt

the enum is more less a misc thing defined in our current types to have a consistent type in options and such

snow crypt

I think it would be better if InviteScopes was an enum like ```js
exports.InviteScopes = {
ApplicationsBuildsRead: 'applications.builds.read',
ApplicationsCommands: 'applications.commands',
ApplicationsEntitlements: 'applications.entitlements',
...
};

also, documenting the uses for all of the scopes seems unnecessary on the lib side, why not just link to the Discord docs like MessageFlags.FLAGS?

vivid field

that enum already exists in discord-api-types, imo it doesnt make sense to copy it, instead it could be added as a dependency (including other places in the lib)

vernal atlas
left moat
keen cosmos

I believe Vlad mentioned that it wouldn't be added until they follow the spec

or maybe that was a different one

exotic flicker

Is that a client? It looks like a server and I failed to spot anything that looks like you can use it as a client (I didn't look very good tho)

tacit crypt

uWs was removed because of it's lack of respecting the ws spec and many issues it brought up

Now, I know why they didn't respect the spec, thats how they're fast fast fast, but...

Personally, something reliable, even if slower is more welcome than something fast and unreliable

wild flax
mighty sequoia
copper laurel

Its faster. But as Vlad pointed out, its faster because it doesn't follow the websocket specifications properly.

oak quail

new permission

Use Slash Commands USE_APPLICATION_COMMANDS 1n << 31n

oak quail

wait was the channel partial never removed in the v8 pr?

so now bots cant get dms by default?

yeah i tested it and cant receive dms unless i enable partials: ['CHANNEL'], that should be fixed

cc @vernal atlas

personally i'd prefer it if it never dropped events due to uncached items

but it at least shouldnt do it in this situation

vernal atlas

yea i guess the best route there is removing channels partial and having channels always be a possible partial

lapis echo

Are partials from the dapi per-event possible? If so, will this ever be implemented?

copper garden

Discord api either gives the full object or partial. No events are ever dropped on their end

So no, there’s no per-event partials option

lapis echo

Damn, that would be a really nice feature if it were possible. Thank you!

copper garden

Could open up a feature request (after making sure it doesn’t already exist ofc)

lapis echo

If I weren't already in bed, I would, but I highly doubt there isn't an issue already opened pertaining this feature.

oak quail
remote wasp

Why does ClientUser#setActivity has name as a separate param? Due to this, the ActivityOptions interface has the name property but the docs for it doesn't have it.

unique axle

My guess would be that someone found .setActivity("name") to default to playing a more comfortable approach, and wanted to support that there

Typing wise that could be solved with omit though, afaik - question is if we want to

paper furnace

on message.mentions.channels the type is Collection<string, TextChannel> but it should be Collection<string, TextChannel | NewsChannel>, announcement rooms are mentionable

unique axle

right, and NewsChannel extends TextChannel

paper furnace

yup, but text channel bind explicitly the channel.type value to text

unique axle

nkoHmm right, also removes the possibility... to use news channel things on it

paper furnace

so we need to being able to check if the channel.type is news

vivid field

actually, since channel mentions get parsed by the lib, it can be any channel type 👀

paper furnace
vivid field

I know

unique axle

oh noes, that's.. hmm

actually... why do we do that?

paper furnace

The current one is

  export class MessageMentions {
    public readonly channels: Collection<Snowflake, TextChannel>;
  }

TextChannel rely statically on channel.type value, which is intended, but due to that the type should be

  export class MessageMentions {
    public readonly channels: Collection<Snowflake, TextChannel | NewsChannel>;
  }
unique axle

oh, i see mention_channels is only included in cross posted messages at the moment... yikes

vivid field

I could fix that with a channel.isText() check in the MessageMentions constructor, if we actually see it as a bug

otherwise the type would be GuildChannel

unique axle

hm, good question, actually
after all the client sort of resolves voice channels, just not clickable
mention_channels will include mentioned text channel for lurkable guilds in cross posted messages

other question being: do we even cache/handle/provide these in any way?

ruby terrace

I would like to mention that channel mentions for voice / category channels are indeed handled strangely by the client right now. (Behave like a channel you don't have access to) However, slash commands do let you select these channels, so it is possible that the client will do something different we these mentions in the future. Therefore I'd be in favor of keeping the API emitted mention_channels and changing the types to reflect it

unique axle

the api provided ones are only present under very very specific conditions and i'm not too sure we do absolutely anything with that at the moment
we resolve mentions from cache right now for messagementions#channels

unique axle
ruby terrace

ah, I have misread the docs. I do see that mention_channels is stored as messagementions#crosspostedChannels

unique axle

oh, so we DO handle them, yays kyuclap

ruby terrace

oh, this is interesting, so the private _channels Collection, which messagementions#channels is a getter for, is typed differently from messagementions#channels for some reason. Also, the jsdoc strings are all GuildChannel

export class MessageMentions {
...
    private _channels: Collection<Snowflake, GuildChannel> | null;
...
    public readonly channels: Collection<Snowflake, TextChannel>;
...
unique axle

that is wrong though, it can also have dm channels

ruby terrace

I guess "correct" isn't the right term

It really should just be Channel everywhere

unique axle

with its current behaviour? yes, def

ruby terrace

the question becomes whether certain channels should be part of that Collection, which is arbitrary since the API doesn't give us anything

silver forge

the version 12.5.0 doesn't exist right ?

nvm

wild flax

great question

silver forge

no it's just that I did npm install discordjs/discord.js and it gave me v12.5.0 but the intents were bug

clever crypt
vivid field
oak quail

tbh the version number should be changed to v13.0.0-dev

oak quail
wild flax
oak quail
loud jayBOT

pr_open #4869 by monbrey opened at 2020/09/30 22:01 (review required)
feat(MessageManager): extend API coverage
📥 npm i monbrey/discord.js#message-manager-additions

oak quail

</github:805111231349653544> query: 4869

copper laurel

Which review was that? Linking to the comment would be way more helpful than a screenshot lmao

oak quail
copper laurel

Might have been caught up with the others in that review

oak quail

tbh im kinda confused why sometimes returning the raw message object was even discussed

instead of just creating a djs msg object

copper laurel

Originally part of the concept was for the manager not to cache things that werent previously cached

Moved away from that

oak quail

well you dont need to cache the message to create a message object

copper laurel

¯_(ツ)_/¯

Not relevant if we're not doing it anymore

oak quail

@trim quartz will you add the new msg type (20, APPLICATION_COMMAND) and permission (1n << 31n, USE_APPLICATION_COMMANDS) in your interactions pr or should they be added in separate prs

trim quartz

¯_(ツ)_/¯

i can add them

ruby terrace

I thought type 20 doesn't exist anymore

pulsar basin

his link to the documentation goes to your website lol

exotic flicker
slate nacelle

The Must sections do not seem fulfilled to me

exotic flicker

true, but still, d.js has allowed this to happen and hasnt done anything about it since a decent amount of time so I dont see whats different with that fork

unique axle

no need for more discussion, really

@fringe temple your decision this

fringe temple

I don't want to waste time on these forks that aren't really getting in the way of anyone

he is in violation of the license yes, but his module seems fairly inactive and hard to mistake for the original discord.js at least

raven juniper

Except that the readme is 100% copied and links to us

fringe temple

i mean, the top of it is sort of clear that this isn't the original discord.js

i mean yes i have contacted npm/repos for less than this

but honestly i am tired of chasing up every fork

tacit crypt

They're also trying to change the LICENSE, which is.. BLELELE

fringe temple

i see

in that case, i'll contact the owner of this before emailing npm

empty viper

Hi, because <Message>#delete({ timeout: number }) will be removed and as is it a lot used, I'm proposing <Message>#deleteIn(timeout: number), what do you think ?

ruby terrace

The whole point of removing that was so that it forces devs to think about how they are deleting messages after a time. e.g. checking if its already deleted etc...

Its also inconsistent with the rest of the lib

hardy plume
oak quail

yeah and then make a union of all the interfaces

copper laurel
tacit crypt

an interface defining the type

and an union of them

hardy plume

pikathumbsup

empty viper

Oh okay

ruby rapids

A weird thing is happening to me, When I start my bot.. It calls guildDelete event on 2 guilds that are already deleted (means bot is kicked)
And when I run same code with token of my test bot... It runs as expected.
someone else in #archive-updating-to-v13 also had same issue
we both are on master branch.. idk much about it but wanted to make sure you saw this

tardy kraken

I was wondering if discord.js is going to support slash commands:

unique axle

</github:805111231349653544> query: 5106

loud jayBOT

pr_draft #5106 by devsnek created at 2020/12/11 19:25 (review required)
interactions
📥 npm i devsnek/discord.js#interactions

tardy kraken

ty

ruby terrace

dependent on discord finishing their breaking changes too

copper laurel

That announcement is only for a Use Slash Commands permission, Slash Commands themselves are still a beta as it says

tardy kraken

Oh yea, mb

oak quail

they said the API will be stable after the announced UI changes ship

copper laurel

Where exactly?

ruby terrace

in DDevs

that's not a final statement though

solemn topaz

Where is the code that manages the queue and waits for rate limits? I just wanted to have a look but I can't see to find it. Thanks!

frank turret
solemn topaz

I see, I looked at it but wasnt too sure, thanks!

In src/rest/RequestHandler.js, why is there a +250? I looked at the issue stated in the comment but it still doesn't say why there is a +250. I have tried removing the +250, and it increases the speed that the bot can react at. Is there any reason that this is here?

copper laurel

Both yes and no

solemn topaz

I think I understand now; the 250 should be the correct timeout for reactions

ruby terrace

the react ratelimit is 1 every 250 ms, by removing that, you are likely to hit a ratelimit whenever you do many reactions at once

solemn topaz

It's like a explicit timeout that is not based on the headers because the header parsing causes 0.25 to be parsed to 1

Is that correct?

copper laurel

That's a hard-coded rate limit, because is 1/0.25s and the rate limit headers used to only be in seconds, so we'd get a 0 I think, but yeah, basically because of header parsing issues
The reason reactions in particular are slowing is the ClientOption#restTimeOffset option, which defaults to 500
This option tells Discord.js how much extra it should wait when it hits a rate limit before continuing with the queue. This works pretty well for every endpoint where rate limits are 5s, 10s, 1m etc because 500ms is nothing.
Unfortunately for the Reaction endpoint, which immediately hits a rate-limit (because its 1-per) it turns a 250ms rate limit into a 750ms delay

solemn topaz

I did set restTimeOffset to 0 for testing

copper laurel

If you do that, then you should definitely leave the +250 there

ruby terrace

Wait Monbrey, are you saying the ratelimit header is in ms now?

solemn topaz

Will the "quicker" reactions just be because of my high API ping or something?

solemn topaz
ruby terrace

rate limit headers used to only be in seconds
I'm talking about this

solemn topaz

It is still in seconds even for reactions tho

ruby terrace

That's why I was confused

solemn topaz

Yeah "used to only be in seconds" suggests that its not in seconds anymore?

ruby terrace

If I understand API v8 properly, then yes actually, it should be in ms.

I might do some testing with that tonight then actually, that special handling for the reactions endpoint should be removable now

solemn topaz

Top: With +250
Bottom: +250 removed

And the pace of reactions with +250 removed is constant, so I don't see any rate limits being hit

ruby terrace

are you on master?

copper laurel

I dont know exactly. It was seconds. Then it could be either and they added a precision header to tell you if it was ms or s

Now I think v8 is ms

solemn topaz

Actually, with the +250 removed, you can see a small choke which is barely noticeable, so I guess its my ping then 🤷‍♂️

But still its a lot faster than with +250

ruby terrace

Theoretically, the time it takes for 10 reactions is 2.5 seconds + your api latency + your event latency

solemn topaz

isnt event latency like a few miliseconds?

copper laurel

If we're moving away from actually discussing library development, can we go #archive-offtopic or support

ruby terrace

I do believe there is no reason to keep the line specifically for the reaction routes though, would you agree with that?

solemn topaz

I think the headers is still in seconds

When I comment out the lines it takes 1 second for every reaction

ruby terrace

if you are not on v8 (either by options or via using master) then its in 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)

that's in the v8 changelog

solemn topaz

I am on discord.js@12.5.1

ruby terrace

I'm talking about in master, which has the v8 PR landed now

so that was more addressed at Monbrey

copper laurel

@ruby terrace yes

V8 reset-after is ms

Well

Its floating point seconds

seconds with ms precision

Is that not already changed in the v8 PR?

ruby terrace

yeah, a little strange considering its present all the time, but smolShrug

I checked out master and It doesn't seem to be removed

copper laurel

But is that PR even merged

solemn topaz

On master it is still this

ruby terrace

oh smolFp I checked out my master, not upstream

and yes, second to last merge

and that line is still present

copper laurel

Remove it and vlad or space or someone can yell at you if youre wrong

ruby terrace

cool cool

I run through my tests anyways so I should notice if somethings wrong

ruby terrace

Ah hah, I've found why this won't work. Server date isn't in ms, and since offset calculations are done to account for...well...offset..., you'll never get ms precision for a ratelimit unfortunately.

For example one request I had had a reset token of 1614330706.302 and my system got 1614330705260 from Date.now() after this request
(for reference the server date was 1614330706000 which would mean I'm roughly 3/4 of a second behind the server, which doesn't matter necessarily. However, given that the reset-after is 0.250, the discrepancy between reset and server date being 302 ms tipped me off)

That's not something we can change, the server sends it like this

lofty birch

oh I misunderstood

ruby terrace

Guess I'll scrap this idea then (until we're all on the same atomic clock or something smolLUL )

frank turret

any reason we don't have support for
https://discord.com/developers/docs/resources/channel#group-dm-add-recipient
https://discord.com/developers/docs/resources/channel#group-dm-remove-recipient
(as far as i looked anyways)?

edit: new addition https://discord.com/developers/docs/resources/user#create-group-dm. Despite the description saying the group dm channels aren't visible to the discord client, advaith says that's outdated and has changed, making the group dm channels visible to the client.

copper laurel

Because the bots cant actually join group DMs iirc

The first two are really OAuth endpoints, since it requires an authorised scope to use

frank turret

yes, but if we have methods for other oauth endpoints like add user to guild, why can't we have support for these?

copper garden
frank turret

please see my edit

copper garden

Is it confirmed somewhere then?

As in they actually have plans for it

frank turret

i can ask for a source, hold on

oak quail
copper garden

So it was a recent change?

kindred haven

that was like last year from the timestamp
not exactly recent if you ask me

oak quail

and I assume it was actually changed long before that

ornate topaz

But in true discord fashion still not documented as such

paper furnace

I'm encountering a weird behavior with permissions, i don't know if DJS is broken on this part or if it's the Discord API who act weirdly. Or maybe I have not understand everything. In any cases, here is my use case to reproduce it:

1. Revoke all permissions to the role bot from the server settings
2. Grant all permissions (green) to the bot role to a category
3. Create a room in this category with synced permissions
4. Try to clone it

Due to the category permissions and the fact that the category is the parent of the room, it should be able to create a new room with same permissions no? Currently it trigger a DiscordAPIError for missing permissions. But when i'm checking those permissions before doing anything, DiscordJS seem ok with those permissions 🤔

copper garden
oak quail

no? you need manage channels to create channels

but to set a permission override you need to have the permission server-wide

and to set a Manage Roles/Permissions override you need admin

however iirc they said there was an exception for cloning 🤔

oak quail

why does packages/voice exist in -next if there's also a voice repo

frank turret

same question for packages/collections i guess too

karmic wharf

because the voice repo didnt exist when next/voice was created meguFace

ornate topaz

collection one did tho

even its TS rewrite happened around half a year earlier than -next had first commit

paper furnace
copper garden It needs Manage Server to create channels

we need manage channels to create a channel, but i think the issue comes from the fact that the bot is allowed to manage the channels from the category permissions, and perms room are sync with the category. Maybe discord consider it's a self assignment, idk. But anyway, if it's the case the library should not return true for the permission check in this case 🤔

It appears, even if i give the permission to the bot from the server settings, if his group is explicitly granted on the category and the room, then I trigger the missing permissions error, which is really really weird. I though there was some sort of hierarchical mechanism like that: Server settings > Category settings > Channel settings

Anyway, DJS don't catch it when i'm checking for permissions while the API reject the query, which is really annoying

copper garden

There isn’t, only role permissions and channel overwrites matter

Pretty sure d.js doesn’t check perms in any case before performing an api call

ruby terrace

The same thing actually happens in the client. You can see the clone / create channel button if you have been explicitly granted manage channel for a single channel / category. However, the API will error. I think this is quite a rare case however, and you can always check your guild permissions.

And this type of permission getter is something that is trying to be avoided IIRC

paper furnace

the bot is granted for the category, so it should be able to perform the clone, if there's a parent, then the parent perms should apply except if there's an explicit deny no? 🤔

ruby terrace

categories are sorta strange, they're actually just channels, and they have no hierarchical inpuit

paper furnace

oh damned 😅

genius idea to put hierarchical rooms on the client but not applying it to the perms 😂

regal panther

idk if this is the place to put it but thought i'd share a little helper code i wrote for making message replies, since i couldn't seem to find anything else online, thought i'd share my findings

Discord.Message.prototype.reply = function({ content, embed }, mention) {
    let data = {
        "content": content,
        "embed": embed,
        "tts": false,
        "message_reference": {
            "message_id": this.id
        },
        "allowed_mentions": (!mention ? {
            "parse": [],
        } : undefined)
    };

    let reply = new Discord.APIMessage(channel);
    reply.data = data;
    reply.options = {};
    return this.channel.send(reply);
};
exotic flicker

pretty sure there is a pr for that (and maybe already merged in master?)

regal panther

for v13 yeah but if u don't wanna have to do any merge bullshit, i didn't want to run an unstable version so i just made the function that i needed

idk how v13 does it but i just tried reverse engineering yalls library to send an api message

copper laurel

You should use Structures.extend, not modify the prototype directly

opaque copper

Are there any plans on making .send() a tad more ts friendly? Currently, I am unable to modify the method directly and the types are atrociously difficult/confusing to any new typescript programmer. It is also not following the types mentioned in the documentation and it is resulting in a lovely handful of errors from ts.

tacit crypt

The short answer is "yes but not right now"

Typings will be improved in the future!

karmic wharf

next ™️

vernal atlas

its worth noting that for the send methods the typings are a bit bugged, you can't do for instance

let x: MessageOptions | string = {};
if (something) x = "hi";
channel.send(x);```

something to do with the fact that there isn't an overload that just accepts both

i was gon do a PR on it but i don't have the patience to mess with those overloads atm

severe sun

will message.reply() be changed to an inline reply in v13?

frank turret

Yes, it's also available in master right now

marble idol

Is there a particular reason for why discord-api-types may export never types? Seems odd since void seems to be more appropriate in those contexts

tacit crypt

technically, both are valid types for it... The never type represents the type of values that never occur vs void is a little like the opposite of any: the absence of having any type at all
In the end, they should be achieving the same thing 👀

marble idol

I don't think the typescript compiler treats them the same

it's really ruining my auto-generator blobsweats

tacit crypt

how so? Can you give me an example of the issue? What's the output with undefined vs never? Which would be the expected output 👀

You can always DM if you don't wanna clog here

marble idol

yea, im not entirely sure if my thing relates to the library itself since I only use the typings for deno :v)

but yeah I'll DM

faint cipher

what's the point of guildMember.permissions.has() if an array of permission resolvables can be used in hasPermission

exotic flicker
unique axle

#hasPermission is removed on master and will accordingly be removed with the release of v13
simply because (see link above) it's not much shorter than <GuildMember>.permissions.has which it calls anyways

molten jungle
tacit crypt

It's "private" as in you shouldn't use it, doesn't mean you can't

If you're on TS and it bitches, either shard['sessionID'] or ts-ignore it

molten jungle

Oh wait really? lmao does js not enforce private properties?

Or is that just a comment and I'm dumb?

unique axle

well, shouldn't should be enough of a: maybe what you are planning to do with it is not that good of an idea to apply in the first place

molten jungle

Yeah I can access it fine lmao I'm just dumb

molten jungle

0 downtime updating

Bad idea ^ ?

wild flax

No, absolutely valid

molten jungle

Okay although maybe best to do with a custom djs fork?

wild flax

As it currently stands, yes

molten jungle

Sounds good cheers 👍

wild flax

though depending on if you use TS or not

you can modify those values anyway

there is no such thing as hard privates in djs

oak quail

🤔 why is it private

wild flax

Do a git blame

molten jungle
molten jungle
unique axle

re: https://github.com/discordjs/discord.js/issues/5277
@opaque vessel but didn't you yourself open the issue on DAPI docs? i'm confused nkoHmm
and from what I understand this would go away if fixed on discords end?
also if you know how to fix it, why not PR it?

oak quail

the discord bug is different

the discord bug is that if you edit the message without setting allowed_mentions, it resets allowed_mentions

the d.js bug is that if you only send allowedMentions it doesn't send anything

tacit crypt

...yes, .send({ allowedMentions: { parse: [] } }) or w/e is never gonna work - or am i missing the issue

OH

editing

faint cipher

Why is Role.comparePositions available while Role.comparePositionTo is pretty much the same

is it gonna be removed on v13?

unique axle

the latter calls the former (static) with this, don't see a reason to

faint cipher

Thonk but they do function the same why would there be 2 methods for it

unique axle

what? the static method is called in the instance method - they're not the same, one depends on the other

faint cipher

oh

unique axle
faint cipher

I thought one of them should be pointless because they both return the same result but looking at the source code and what you said I got it rn

thanks souji!

unique axle

one could argue that we can shove the static functionality into the instance and remove the static, but that's just removing a layer of abstraction vs. pruning duplicate functionality
so not a fan of that, personally

faint cipher

oh

ornate tapir

Yes pretty pointless in that case.

shrewd kelp

Is it intended that when you have fetchAllMembers enabled, but don't have the privileged intent the library will just wait for two minutes until a timeout occurs to start the bot? Im not sure what discord returns however seems odd to fail due to timeout

Also the timeout "error" is logged over debug, rather than the error or warn events.. is that intended? Made debugging my bot more difficult as I was logging warn/errors but not debug

unique axle

yes, fixed in master by removal of the option

opaque copper
old kelp
warped crater

so it makes sense to guarantee it

opaque copper

most of these properties would be changed if that were the case

warped crater

"these properties"?

oh, hm

i see what you mean

i think @unique axle tackled this a while back

unique axle
warped crater

appears so

gray zenith

my memory usage is increasingly indefinitely, it's in around 2.2k servers and I have sharding enabled. it's using around 700mb of memory.

I have minimal intents, messages and guilds. and message sweep is quite strict

messageCacheMaxSize: 10,
            messageCacheLifetime: 60,
            messageSweepInterval: 10 * 60,
            messageEditHistoryMaxSize: 0
ornate topaz

or just move it to where that pr did

to avoid any issues

olive nacelle

just curious, how is discord.js-next doing?

typescript migration seems useful

and seems relatively simple, just adding types to existing javascript code

unless there are some non-obvious difficulties with migration?

unique axle
olive nacelle

ah so like big restructuring of everything?

unique axle

yes

olive nacelle

any plans on supporting deno?

exotic flicker

refer to

olive nacelle

ty ty

real jetty

Would there be any way to have like client.destroy return a promise containing a boolean with like if it successfully deleted the client or not

just bc like rn it doesnt return anything

so you cant really know when the client is destroyed

old hill

it's not asynchronous, it should happen more or less instantly

long mason
real jetty

you don't wanna create a new client till the old one is made

long mason

client.destroy() isnt for restarting bots exactly, you should probably just use a process manager and restart it

real jetty

oof

kindred haven

there's a possibility that there's leftovers which could cause unexpected behavior
destroyed clients should not be reused
I was a bit too late :snail:

real jetty

loll, ty for the info tho!

loud blade

Is there any point on keeping Guild.channels.cache if you have Client.channels.cache that has a property of guild for guild channels?

copper laurel

Well guild.channels is the actual manager with the create etc methods, so yes, it needs a cache

loud blade

the cache could be based from client.channels.cache doesn't it?

copper laurel

But why

You want guild.channels to have to be written as a custom manager that doesn't have it's own cache, and a pseudo-cache getter for a filter on client.channels.cache? Why add that complication?

loud blade

#archive-offtopic message about this, though you said its a reference, but reference to client.channels or other way around?

copper laurel

Neither. Collections with the same Channel objects in them

copper garden

I don’t think you can have one-way object reference in js, it’s always mutual

loud blade

so basically, channels in guild.channels.cache is the same reference on whats its counter part on client.channels.cache?

copper laurel

yes

lavish mural

https://github.com/discordjs/discord.js/blob/f46940228e9f82db4af09ae2f2dad684db0d74ed/typings/index.d.ts#L991 is there any reason to not make the CollectorFilter type generic? so, in this case, you can go from

type CollectorFilter = (...args: any[]) => boolean | Promise<boolean>;

to

type CollectorFilter<T extends unknown[] = any[]> = (...args: T) => boolean | Promise<boolean>;

so that then you can add additional type data for filters like the following for the line I referenced in the link?

public awaitReactions(
      filter: CollectorFilter<[MessageReaction, User]>,
      options?: AwaitReactionsOptions,
): Promise<Collection<Snowflake, MessageReaction>>;

interface ClientOptions<T extends PartialTypes = PartialTypes> {
    // ...
    partials?: T[];
    // ...
  }

type ReactionType<T extends PartialTypes> = T extends 'REACTION' ? User | PartialUser : User;
interface ClientEvents<T extends PartialTypes = PartialTypes> {
    // ...
    messageReactionAdd: [message: MessageReaction, user: ReactionType<T>];
    messageReactionRemove: [message: MessageReaction, user: ReactionType<T>];
    // ...
}
class Client<T extends PartialTypes> extends BaseClient {
    constructor(options: ClientOptions<T>);
    // ...
    public on<K extends keyof ClientEvents<T>>(event: K, listener: (...args: ClientEvents[K]) => void): this;
    // ...
}

Same goes for here

steep swallow

Could someone shed some light on why replies are only being added in v13? It's been 4 months since the feature was introduced, shouldn't it be a priority to add new features as soon as possible?

unique axle

we will not do another minor release on version 12
so we might as well introduce replies as a breaking change in the next major, 13

the usage as Message#reply is much more idiomatic than inventing another method name just for the sake of keeping it minor

steep swallow

Would it be such a breaking change to replace the current reply functionality with the new one that it requires a major release?

In my humble opinion I'd much rather have a new minor release with the new functionality than having to wait months on a major update, but thanks for your answer

unique axle

anything that changes the publicly exposed API is a breaking change, no matter how small it is.
and changing the entire behaviour of a method is definitely a super breaking change

steep swallow

That's true

unique axle

there is nothing about the method that stays the same but the name.
it takes different parameters
it does something else
there is no way replacing this could be framed as a minor release

steep swallow

I can see that, but I still feel like adding new features as soon as possible should be prioritized in some way, but I understand

I'll just use the fork for now

unique axle

that is a priority, indeed, but not as much as to jeopardise semver over it, if we already have moved on with the development branch.

you can of course always install a certain commit and apply the necessary changes
in that case i'd heavily recommend pinning your dependencies to that specific commit, in order to not accidentally introduce more breaking changes when updating.
more on how that procedure works: https://docs.npmjs.com/cli/v7/configuring-npm/package-json

steep swallow

Thanks

elfin wing

I'm sorry if you guys get this asked multiple times, but I am curious and I want to try helping. I tried searching if someone asked about this in this channel before but apparently there wasn't (I could be blind though).

Has anyone got a rough idea about why discord.js is having trouble with piping Readable streams back in node v15? I know the fix is to downgrade back to node v14 but what change in v15 could've caused piping to not work, unless you do it twice.

real jetty

Node.js v15 is not the stable version and has some unstable changes, so it might be having issues with the Readable Streams that are piped through discord.js's voice lib, however, the voice lib is getting a lot of changes apparently, no idea that would do anything in this case but as its not LTS, you gotta dig through Node.js's changelogs to find out what they changed between the two versions and see the difference

exotic flicker

I dont think its worth figuring it out, use the workarounds and wait for discordjs/voice to be production ready

warped crater

frankly there's not much to dig, there's very few changes that could've done this just on a quick scrim through the changelogs

[3f33d0bcda] - stream: fix pipe deadlock when starting with needDrain (Robert Nagy) #36563

being the main culprit (all though I'm not certain of the exact bug or if needDrain is used)

which is actually under the 15.5 changelogs real_eyes

so if this works on 15.4 for instance, it's probably that

elfin wing

it stuck out since the comment said // If the user unpiped during `dest.write()`, it is possible // to get stuck in a permanently paused state if that write // also returned false.

clever crypt
tacit crypt

We have done this..for...ages now

The MAIN reason we were on api v7 is those

vernal atlas

yeah if im understanding right thats already implemented

near fog

the libs are broken atm, there are just random issues after the new stage channel stuff

e.g.

and I can confirm not a single thing has changed on my bots in the past 2 weeks and this is all sudden so its def a lib or api issue with the new change

wild flax

is this with one of the invite events?

near fog

they all look to be related to channel events such as channel create

also in line with the new change

wild flax

can u list me all the events you listen to

near fog

but yes there is also a callback with the invite too

wild flax

aight

yeah that will be fixed momentarily

near fog

nice one ty

prime raft
  1. Does djs support slash commands?
  2. The new stage channels are not the same as vcs it seems so bots will say you arent in a vc how / when will there be a fix for that?
unique axle
  1. not yet pr pending
  2. new channel type
prime raft

Will there need to be a fix for that? Or is there a way to fix it currently

unique axle

we will probably release 12.6 after all which introduces the new channel type and stage functionality

unique axle

We didn't want to release anything else on v12, but things are breaking, so here we go:
• ⚠️ 12.5.2 emergency patch release to stop receiving INVITE_DELETE for stage channels from breaking the library
• 12.6.0 minor release for stage channels (might not happen because of new permission bit)

grave tiger

however commit-msg only lets me use one of [chore, build, ci, docs, feat, fix, perf, refactor, revert, style, test]

which type should I choose? the changes are only to the typings, nothing else

I would choose feat but I'm not sure whether that is reserved for actual features

sugden made a typings type commit 3 days ago but that doesnt work either

vivid field

you can use feat for typings too, yes

grave tiger

thanks!

oak quail

did docs just not get updated?

ah so looks like it just sends the presence directly to Discord, and it changed in v8 but the typedef and interface weren't updated

loud jayBOT

pr_open #5317 by iShibi opened at 2021/02/12 09:50 (review required)
feat: make changes to PresenceData typings and docs
📥 npm i iShibi/discord.js#fix-5316

fallen arrow

why was the before option removed from ReactionUserManager#fetch ?

is it being deprecated by the API?

unique axle

never worked, API docs don't get a response as to if it's a bug or just wrongly documented -> removed it

reasoning being that if it gets fixed and added back it's super easy to re-introduce in a semver minor

fallen arrow

i see, thanks

fallen arrow
vivid field

oh yeah, that was an oversight

olive nacelle

is discord.js-next still being developed?

I'd be interested in contributing

real jetty

Yes, see Github

olive nacelle

last PR was in January

olive nacelle

ran into a typings bug today

GuildMemberRoleManager extends BaseManager, which has the resolve and resolveID methods, even though they aren't implemented

I would do PR but i'm not sure how to

copper laurel

GuildMemberRoleManager shouldnt extend it

covert flint
olive nacelle

^

Interestingly GuildMemberRoleManger is the only class that extends OverridableManager

so either GuildMemberRoleManager should implement the rest of the methods of BaseManager, or not extend from it at all

either way OverridableManager seems rather useless and could probably be removed

oak quail

keeping 7 as unknown is pretty 😬

maybe change to -1 or smth

although it probably wont have a real issue bc 7 was for lfg iirc (which afaik will not return/release), its pretty weird

unique axle
loud blade
patent halo

Client event disconnect exists on the typings ClientEvents but its not documented in the docs Thinkeng

opaque copper
fallen arrow
vernal atlas
loud blade

it rarely happens but if not dealt correctly, it will crash your bot
In my case it happened only this April 3rd

vernal atlas
tacit crypt

how the hell...

What version?

loud blade

latest master

loud jayBOT

pr_open #5312 by NotSugden opened at 2021/02/10 19:31 UTC (approved)
fix(GuildMember): correctly check for premium_since
📥 npm i NotSugden/yeetcord.js#patch-4

wild flax

cc @vernal atlas

fallen arrow

was IntegrationApplication also not added to the Discord object?

real jetty

i just want to ask here for better clarification

wait a sec

real jetty

the fetchInviteInfo method in client couldn't pass some trolling invites
For example, i'll provide 2 working invites for examples: test and 123
Example: https://discord.gg/123/test/
DJS fetchInvite method returns the result of the server with the code 123, but Discord site automatically redirect it to discord.com/invite/test/, which is the latter
(there are actually more cases when i tested the method, will add them here later)

unique axle

i don't understand your request? both are vanity invite links that are given out to two different guilds, which we retrieve correctly from the API via the GET /invites/{invite.code} endpoint and provide the data for after fitting them into discord.js structures

real jetty

I mean, the only problem is that discord.gg invite links only takes the last invite code after the last slash character (if there are no characters like #? beforehand)
Just try the example invite link with by both the djs method and pasting it in the website and you'll see the difference

unique axle

i really don't know what you are referring to, both https://discord.com/invite/test as well as https://discord.com/invite/123 point to different guilds, even if i enter them into the browser directly - we retrieve those guilds accordingly from the API

123/test/ is parsed as 123 by discords redirect system

real jetty

But discord.gg doesn't follow the rule above

unique axle

https://discord.com/invite/123/test/
https://discord.com/invite/123
both lead to the same guild, which is also the guild we return shibaThinking

real jetty

discord.gg/123/test

I mean this one

both discord.com and discordapp.com work well with the method

unique axle

ah, so what you are saying is that discord.com, discordapp.com vs. discord.gg resolve invites differently in complete edge cases, which we don't handle on the library level, see

as i see this this is an inconsistency in discord parsing invite links with excess differently in discord.gg and discord(app).com and i personally don't see reason to specifically add code to handle this edge case

real jetty

I only recognized that when there are some trollers that posted that kind of invite to my server
Fyi, I coded a bot to detect invalid invites and delete them. And it failed in those trolling invites
The trollers then dm me and asked why their valid invites got deleted, which pulls me to a tough situation as I stated there "only post invites that are valid/linked to the discord invite page"

foggy blade

Does Discord.js support stage channel to be created

vivid field

no, not yet

loud jayBOT

pr_open #5456 by amishshah opened at 2021/03/31 19:46 UTC (changes requested)
feat: stage channels
📥 npm i amishshah/discord.js#feat/stage-channels

outer raven

was thinking of suggesting a new property for all guild channels called locked that says whether a channel has the 🔒 icon in it or not (essentially sees if the everyone role has perm to view the channel and connect (for voice channels). Do you guys think this would be a good idea?

unique axle

it's just a permissions getter for CONNECT (voice) and VIEW_CHANNEL (text) permissions after overwrites and depends on the viewing member
not a fan, don't see many use cases for it either

outer raven

well you can say the same thing about manageable which is already a thing right

unique axle

manageable at least factors in ownership apart from just role hierarchy

outer raven

well ownership wouldn't matter in this case since the lock appears to everyone and doesn't depend on anyone's permission but the everyone role

it would be almost the same as this (with different permission flags) without the first line

An example of a use case for this would be if you wanna make a command that locks a voice channel (which is what im working on for my own project) which would make this process easier (checking if the channel is locked already or not)

unique axle

well, it still depends on the viewer. if you don't have connect there is a full lock, otherwise there is a small lock next to the regular voice icon i guess
but i really don't see any use case for this - even less now that you detailed you mean the small lock, which is basically completely useless information

vagrant holly

^ Just write yourself a util that does the permission check -- its literally just a permission check, doesn't make sense to have in the core lib imo

outer raven
unique axle

discord has a built in functionality to do that now, so even as a bot feature that is completely unnecessary

outer raven

but bots don't have that feature and if you wanna list all the channels that are visible to everyone through a bot it would be harder

vagrant holly

You do have that feature, it's called checking permissions? Its very simple to do, and very simple to wrap in a utility function if you don't want to write the permissions check every time?

unique axle

i understand that you found a use case (which you seem to feel rather strongly about) and want to reduce your code complexity

but if we start incorporating permission getters for every small things that may or may not have a use case in someones eyes our API is going to be very, very cluttered. most permission getters that are in the library at this point either have more complicated checks under the hood or at least a broad area of use cases, where they actually make peoples lives easier

channel.permissionsFor(channel.guild.roles.everyone).has("VIEW_CHANNEL")
i really don't see why this single line and single purpose check should get its own method/getter on a library level

outer raven
unique axle

well, the information is available, if you need it but "is icon displayed in desktop chat app" is not really a good criterion to develop a library based on

outer raven

yeah fair enough i guess

opaque copper
loud jayBOT

pr_open #5481 by monbrey opened at 2021/04/05 09:45 UTC (review required)
feat: support for MessageAttachment#contentType
📥 npm i monbrey/discord.js#attachment-content-type

copper laurel
left moat

no

not what djs’s purpose is

ebon radish

I don't know if this is the right place, but I think Util.escapeMarkdown just made a mistake

That name is the same every time, fbz___

raven juniper

What's the input

ebon radish

want it copypasteable?

name: fbz___, displayName: fbz___, id: 29507072, creationDate: 2012-04-04T10:54:40.635Z (over ~9 years ago), type: , broadcasterType: , views: 869, description: :3
channel - name: fbz___, displayName: fbz___, id: 29507072, language: en, title: :), gameName: Path of Exile

turned into```
name: fbz___, displayName: fbz___, id: 29507072, creationDate: 2012-04-04T10:54:40.635Z (over ~9 years ago), type: , broadcasterType: , views: 869, description: :3
channel - name: fbz\_, displayName: fbz___, id: 29507072, language: en, title: :), gameName: Path of Exile

vagrant holly

Hm

ebon radish

but only in 1 of the four times his name is there

it's interesting because it goes \___ and __\_ twice, then that becomes \_\_\_ 3 times and \\_\__ once. well I am sure my commentary isn't helpful at this point, suffice it to say I don't know how it decides what to escape

vernal atlas

a single _ also is italics, so say you have

\_\_message\_\_ (which escapeUnderline is ran on) then run escapeItalic and you get \_\\_message\\_\_

i think thats whats happening right ?

perhaps the soloution is to escape everything else then .replace(/_/g, "\\_");

might also get similar behaviour with asterisks too but idk

ebon radish

I don't think it will mess up on such a common case

but clone and edit on runkit isn't working for some reason

It works fine with __message__

It works in an interesting way with ___message___, then italic really does take care of one and underline the other

Anyway. I kinda expected it to just escape everything that could possibly be markdown with zero analysis tbh

outer raven

deletable, editable, manageable, joinable, speakable, viewable

some like joinable make sense but manageable, deletable and editable seem the same to me

fringe temple

super.deletable and this.manageable also perform different checks

unique axle

they also all have utility for the bot itself to be able and do things

"is a channel accessible for everyone" is completely irrelevant for a bot to do stuff

outer raven
fringe temple

(updated the message to link to a more up-to-date version of that file specifically for the stage channel PR)

outer raven

ah gotcha

thanks

loud blade

seems like the url of null is happening again more frequent this time

tacit crypt

I should really just remove that...

ruby terrace

bringing this up again since https://github.com/discord/discord-api-docs/issues/2702 was fixed: #archive-library-discussion message
I seem to have misread the docs for X-RateLimit-Reset-After.
On v8 it is in floating point seconds, to ms precision. The value in this is not how long the reset is, but how long until the current reset, meaning it is possible to handle ratelimits better.

Basically, I would like to add handling of the x-ratelimit-reset-after header so that, if present, RequestHandler#reset gets set to Date.now() + reset-after, without relying on calculating server offset and manually adding 250 ms for reactions. Falling back to the current handling if that header is not present (in case the referenced issue ever pops up again for example).
Can anyone see a reason not to do this?

faint cipher

recently people are getting errors connecting to discord.com, I am not sure if it's released yet but if not shouldn't it connect to discordapp.com

ornate topaz

no? domain change happened several months ago

faint cipher

Thonk I feel so outdated

oak quail

how should i add msg type 22

should i add 21 to the createEnum too? or just put null for it

since 21 is in the interactions pr

faint cipher

why isn't 'size' documentated as a property of collection

copper laurel

Its a property of Map, which Collection extends

frank turret

so whys that stop it from being in the documentation

strange moat

Because you can view the documentation for it on the MDN docs website

No point on having duplicate docs

frank turret

then how come we do that for the get, set, clear, delete, and has methods

ruby terrace

I believe because they are implemented differently (as in there is code that makes them different from Map's equivalent)

frank turret

not for get, has, and clear. We literally just return the super method

set & delete are extended but provide the caching functionality that collections has

ruby terrace

which is being removed IIRC

I am interested in all these functions that just call super

faint cipher
loud jayBOT

pr_merge #12 by Tenpi merged at 2020/02/24 17:13 UTC
chore(docs): document basic Map methods

oak quail

couldnt joinVoiceChannel in djs/voice just take a channel object?

so instead of js const connection = joinVoiceChannel({ channelId: channel.id, guildId: channel.guild.id, adapterCreator: channel.createVoiceAdapter() }); just js const connection = joinVoiceChannel(channel);

tacit crypt

@discordjs/voice is meant to be lib agnostic iirc

could be wrong

raven juniper

pretty sure that's the stated goal, yes

oak quail

couldnt it work for either

tacit crypt

then it wouldn't be agnostic, advaith

It's meant to be a standalone thing

oak quail

ig blobshrug

tacit crypt

aka you can use with a library or with pure raw data sprinkled from json files

remote wasp
loud jayBOT

pr_open #4154 by izexi opened at 2020/05/03 13:44 UTC (approved)
feat(GuildMemberManager): add 'search' method
📥 npm i izexi/discord.js#GuildMemberManager-search

remote wasp

aight, thanks 👍

oak quail

what do u guys think on interaction.webhook vs interaction.followup(s) ?

i feel like .webhook might be confusing since it isnt really a normal webhook

the library internally uses it for .editReply and .deleteReply but users wont need to use it for anything about the original reply

stone niche
copper laurel

I dont think callback is correct, no

Thats definitely not what it is

ruby terrace

I agree webhook could be a bit connfusing, however, I can't think of a better name, and it is mostly a webhook

oblique prairie

Hey! For v13, do you think it would be possible to keep track of extended classes throughout the library? By that I mean keeping the jsdoc/types. Quick example:

Structures.extend("TextChannel", (Channel) => {
    return class CustomChannel extends Channel {
        constructor(...) {
            super(...);
        }
    };
});
Structures.extend("Message", (Message) => {
    return class CustomMessage extends Message {
        constructor(...) {
            super(...);
        }
        someMethod: () => {
            // Now
            this.channel // jsdoc/hint shows that this.channel is a TextChannel

            // Better I think
            this.channel // jsdoc/hint shows that this.channel is a CustomChannel
        }
    };
});

Maybe what im asking for already exists, in that case please make me aware of how to achieve it :D Thanks!

Btw, i'm asking that because i'm trying to work in typescript so if I want to use a method from an extended class, I have to "force" types (ex: (this.channel as CustomChannel).customChannelMethod() // would not work if type is TextChannel

copper laurel
oblique prairie

Yeah, I don't think this is an important thing rn

copper laurel

JSdocs definitely dont do it, but you can write your own extended typings

declare module "discord.js" {
  interface Message {
    client: MyExtendedClient;
  }
}``` etc
oblique prairie

Okay, thx for answering!

oak quail

Util.cleanContent can be modified to take a channel instead of a message right?

copper laurel

...for what purpose?

Or do you just mean in general because all it accesses is message.client and message.guild

oak quail

well i want to do it so it can be used for interactions

was just making sure i wasnt missing anything

copper laurel

Just make sure you fix the message.channel.guild line lol

oak quail

are the types intentionally not imported?

looks like eslint complains that its not being used, although it helps with intellisense ablobthinking

copper garden

The docsgen recognizes the typings as is since it's compiled into one file anyways

oak quail

ik, but the intellisense helps for developing

copper garden

I don't think the docsgen supportimport("...")

oak quail

well require works

other than eslint

copper garden

Wouldn't that create issues with cyclic dependency?

copper garden
warped crater

I think this approach that isn't event loop bound at all and doesn't cause the tests to take an extra 100ms to exit is much more desirable:

test('Util#delayFor', async () => {
  jest.useFakeTimers();

  const cb = jest.fn();

  const promise = halt(100).then(cb);
  jest.runAllTimers();

  await promise;
  expect(cb).toHaveBeenCalled();
});```

or.. something along the lines of that nevertheless.

but it's kind of redundant to "test" the ms parameter at all when it's just thrown into setTimeout

there's fairly little to test with this method overall so I can't really think of an approach that doesn't look lame

clever crypt
lavish mural

seems like I would need to dig deep into DJS to find out which cache the lib doesn't deeply depend on

ornate topaz

i think the biggest hint would be managers that don't have everything cached from the start

clever crypt

how will main branch deployments on -next be handled?

wild flax

come again?

tacit crypt

how will we do them for -next

build branch? npm publish?

wild flax

not anytime soon

tacit crypt

if so, who can publish or will it be automated for canary?

see: rest module once someone reviews it and it gets merged

wild flax

will be figured out when its in an actual alpha state

clever crypt

i've been told the reason why rest hasn't been merged is because it hasn't been texted in prod but the only way you can at the moment is by vendoring

wild flax

clone, build, npm link

tacit crypt
olive nacelle

what is left to be implemented for v13?

the main features (replies and interactions) seem to be implemented already

raven juniper

Interactions are not implemented

ruby terrace

interactions haven't been merged (although they are not a major change so it can release without and be a .1.0 thing). There are several semver major things that are still left (check pins in #djs-help-v14 ) Some things that aren't noted there: #5490 - Welcome screens, #5298 + #5296 - Reply options, and some other internal things that generally clean up the library

olive nacelle

ah i see, still workin on it

best of luck contributors, thank you for all your hard work

oak quail

wonder what's the API status on screening

I pinged yuugu a while ago and he didn't respond

also it would be nice to get monbrey's manager method prs in

loud jayBOT
ruby terrace

Yeah it'd be nice to get those in before 13, but they are, suprisingly, semver minor

oak quail

oh there are a lot of semver major prs lol

ruby terrace

yeah

a lot are simply internal things, and waiting on conflicts resolved / reviews

speaking of which, for https://github.com/discordjs/discord.js/pull/5161 I never got a response on if I should change it to just not emit presence / GMU if the only thing changed is the user object (still emitting userUpdate, basically the oposite of the current PR). I am aware that the PR will not get merged as is, trying to find a good solution.

oak quail

wait why would it not emit presence/gmu

ruby terrace

because oldMember and newMember are identical in those cases

as they used a referenced user object