#archive-library-discussion

1 messages · Page 10 of 1

sharp wigeon
#

shouldnt locked be optional as well (typings)

strange igloo
# sharp wigeon

I don't remember who exactly but I recall a dev who said that setStatus Status whatever action you want must have their "status" boolean required
So here setLocked needs at least the boolean
Which means JSDoc is probably wrong instead

#

Discussion happened around MessageButton#setDisabled iirc

wild flax
#

@oak quail in fact, both do not work:

unknown.png
#

Also we don't do it anywhere else in the typings lol

oak quail
wild flax
#

TS is complaining about something

#

So I expect it to be this, but even then, we don't use it anywhere else, so we shouldnt here

#

Its introducing inconsistency

oak quail
#

hmm it worked until the latest commit which didn't even edit typings

tacit crypt
#

Ah, this explains

unknown.png
wild flax
#

yes

loud jayBOT
zenith oracle
#

@sharp wigeon ^^

sharp wigeon
#

beat me to the pr 😂

wild flax
#

@remote wasp is 5971 done

remote wasp
#

yeah, I'll make the changes to docs in the typings PR

wild flax
#

Please resolve conversations then

#

We can't merge if they aren't

remote wasp
#

okay

#

done

oak quail
wild flax
#

our CI is on 4.3

tacit crypt
vernal atlas
#

i can't find these conversations to resolve (#5132)

unknown.png
#

🤷‍♂️

unknown.png
solemn oyster
#

Same

outer raven
#

I'm waiting on #5979 to fix an issue with my bot, will it be on this night's dev release?

loud jayBOT
#

pr_open #5979 in discordjs/discord.js by ckohen opened 9 hours ago (approved)
types(ApplicationCommandPermissionsManager): fix types
📥 npm i ckohen/discord.js#permissions-manager-types

solemn oyster
#

Are you going to add type tests? @ruby terrace

ruby terrace
#

want a separate PR for that or in that one?

solemn oyster
#

idm

#

We usually include tests in the same PR that adds the code

#

Because how software is usually tested

ruby terrace
#

okie

outer raven
slate nacelle
#

Nothing is stopping you from using it outside of discordjs DjeetaPeek

outer raven
#

Yeah I know but the name makes it seem like it's not intended to be used outside of it. You can still do it but a better name and README would probably reach a broader audience

copper laurel
#

I don't think anybody cares how wide an audience it reaches, and installing things from a particular organisation is very common

#

@material-ui/core, @typescript-eslint/parser are a couple of examples that come to mind

#

Besides

#

Name is already taken

outer raven
#

hm yeah and it seems to be similar

#

Yeah nvm then, didn't think the name could be taken lol

zenith oracle
#

Wouldn't it be good to have a function for hyperlinks in the builders module?

rough glacier
#

and to take it one step further, it could return a HyperlinkString type acceptable only in embed description and field values not worth it and perhaps impossible

solemn oyster
#

Also, webhooks (and interactions) can send masked urls

loud jayBOT
#

pr_open #3 in discordjs/builders by vladfrangu opened 15 hours ago (changes requested)
feat(SlashCommands): add slash command builders
📥 npm i discordjs/builders#feat/slash-commands

solemn oyster
#

For example ^

solemn oyster
#

Do you want to make the PR? Or do you not mind?

zenith oracle
#

It's the same 👍

loud jayBOT
#

pr_open #4 in discordjs/builders by kyranet opened 46 seconds ago (review required)
feat(Formatters): added hyperlink and maskLink
📥 npm i kyranet/builders#feat/added-escaped-link-and-masked-link

solemn oyster
#

Done, @zenith oracle ^

zenith oracle
#

Nice! 👍

wild flax
#

@outer raven you’ll have to use ~ not > preferably

#

And we usually just have “no longer supported” as a message

#

Also npm deprecate is very prone to failure, so best case you should attach a || true

outer raven
#

Also what’s the difference between > and ~?

wild flax
#

It just fails most of the time that’s what

#

Lol

#

No idea why

#

Npm isn’t giving helpful errors

ornate topaz
#

is > even valid?

outer raven
#

it was in the website I used to test and was in one of the examples

#

Of course I couldn't actually test the command

outer raven
wild flax
#

No

#

I don’t care if it fails

#

And CI being red just because a dev publish failed is kinda dumb

outer raven
#

fair enough, will commit now

outer raven
#

Why is there no getter for the owner of a guild in the guild object?

solemn oyster
#

We replaced it with the more reliable fetchOwner, since intents

loud jayBOT
outer raven
#

ahhh alright

rough glacier
#

right now ApplicationCommandPermissionsManager#fetch() returns ApplicationCommandPermissions[] with the existing registered command permissions but it throws DiscordAPIError: Unknown application command permissions if no permissions are set, probably because that scenario is not handled. Wouldn't an empty array make more sense?

oak quail
#

if discord throws an error shouldn't djs throw an error

rough glacier
#

fair enough

wild flax
#

@oak quail whats with the 2 comments from sugden, are they even still relevant

oak quail
#

honestly idk

wild flax
#

Oh nvm I see

#

He wants you to make a typedef

#

And link it to the discord docs

#

But they arent merged yet

oak quail
#

yeah

wild flax
#

Also can you confirm a thing for me, I saw MANAGE_EMOJI_AND_STICKERS somewhere thrown around

oak quail
#

well just a link I think?

wild flax
#

Emoji not Emojis

#

Which one is correct here

oak quail
#

talked to Jethro for a bit

#

he said use emojis to be consistent with the old name

wild flax
#

aight

wild flax
#

nothing too fancy

oak quail
#

ah I see

#

I mean

ruby terrace
#

isn't emoji the plural of emote though?

oak quail
#

I can see if I can get the docs pr merged today or tomorrow

#

I wanna add the gw event tho, and test the endpoints

wild flax
#

we can't merge stickers without the docs pr anyway, so no rush

oak quail
#

also should I add guild sticker manager and the event to this pr? or a new one later

#

GSM shouldn't be breaking

#

but I think the intent is getting renamed bc of the new event, so that is breaking

wild flax
#

Id say keep it in this PR

#

and just let us know when you added the stuff

#

so we know what to look at

oak quail
zenith oracle
#

About Typings, I think string types can be more specific. For example Channel#toString() returns string for types but it actually returns <#${bigint}>, I think it would be great to have these specific types, especially for TS users...

empty viper
#

Maybe we should have multiple Snowflake types so ?

zenith oracle
#

Why?

empty viper
#

One for users, one for channels, one for roles etc

Oh I'm dumb these are not IDs, but maybe UserString, ChannelString etc ?

zenith oracle
#

Yeah lol

#

Maybe something like ChannelMention would be better

wild flax
#

What

#

Oh their mentions

zenith oracle
#

Yep

empty viper
#

Yes

#

And it could be used for Revolvers types

empty viper
zenith oracle
#

not bad too

#

bur missing a $ lol

#

also a thing about activities

#

This is the activity of my bot logged in the console but there are some difference between this and what docs and typings show. First of all createdTimestamp is NaN and createdAt invalid date, then id is undefined while it should be a Snowflake and also presence is not documented as a property at all.

unknown.png
#

Debugging the data received only this is shown and there are no properties about id, createdAt etc...

unknown.png
#

Is this because the activity is a bot activity?

#

and should we handle the missing properties?

#

Ohh wait is this because of intents?

wild flax
#

Enable them and check again

zenith oracle
#

What's that

unknown.png
wild flax
#

NPM being shit

zenith oracle
outer raven
#

Will guild avatars be on the initial release of v13? I know they're not available to everyone yet but they're on the API so everyone should be able to use them with no problems

zenith oracle
#

Iirc guild avatars are already in the Dev version

#

Oh ig the pr is not merged yet

outer raven
#

yeah it isn't merged

#

I made it a draft because the api docs arent merged either so I assumed it would need to wait for that

#

but then again selects were merged without even a PR on the dapi docs so idrk

zenith oracle
#

Well however it's a semver minor so I don't think they'll wait for that if v13 is ready

outer raven
#

yeah but the PR is ready so it could just be merged even if its not out for everyone the same way it was done with selects and threads i guess

zenith oracle
#

Same as user banners

outer raven
#

well but banners arent on the api yet

wild flax
#

Can’t do

#

If we merge it. We’d have to wait

#

Because things might change

outer raven
#

if they change I think it'll only be on the user's side. The avatar is just a hash similarly to the user avatar

#

and they have been changing the user interface for guild avatars but that's it

wild flax
#

It’s just not how we do things

#

And I don’t want this to be a reference case

#

Selects and Threads were different because we are in direct contact with those respective devs

#

And we wouldn’t release v13 without them either

outer raven
#

Ah so you don't know when guild avatars will come?

wild flax
#

No

outer raven
#

alright that makes more sense

outer raven
#

This MessageActionRowOptions interface is causing a false positive that happens when sending an array with one button/select in a message's components. Is it safe to remove or is this covering some edge case?

unknown.png
copper laurel
#

dont remove it?

#

unless resolvable covers it

#

I'd rather you figured out why [button] is assignable to that

outer raven
#

yeah I tried but I don't really know since that interface is an object with a components property so it doesn't make much sense

copper laurel
#

I mean the answer is definitely not removing the type that supports the object format

outer raven
#

well then in that case I have no clue how to fix it. It seems like it's completely ignoring the array part in MessageActionRowComponentResolvable

unknown.png
copper laurel
#

Maybe because it's optional

#

Can you make components required and see if it still happens?

outer raven
#

but it makes sense that MessageActionRowOptions would require components so I guess that's the fix

zenith oracle
#

MessageActionRowOptions should be like a MessageActionRow without needing to instantiate the class but a MessageActionRow always has a components property, which is an empty array by default.

#

The problem is that MessageActionRowOptions is the parameter to create a MessageActionRow too, that, as I said, doesn't require to set the components property as it's an empty array by default and that's why that components property is optional

outer raven
#

well it's used in the constructor but if you're constructing an action row like that you need to specify some components, otherwise it's a pointless row

zenith oracle
#

You can always add components later with .addComponents

outer raven
#

yeah but if you're doing that then you'd just not pass a data object

#

and that is still fine because data is marked as optional

zenith oracle
#

Making that required means that these are accepted:

new MessageActionRow ();
new MessageActionRow({ components: new MessageButton() });

But not these:

new MessageActionRow({});
message.channel.send({ content, components: [new MessageButton()] })

Which is completely fine in my opinion

zenith oracle
#

Maybe this should be tested a bit more but I think this is the correct solution

outer raven
#

yeah those are the only scenarios where this interface is used so i think its fine

#

great now i cant commit 😐

unknown.png
zenith oracle
#

Oof

outer raven
#

got it in another way no worries

loud jayBOT
#

pr_open #6005 in discordjs/discord.js by ImRodry opened 6 minutes ago (review required)
types(MessageOptions): fix components being optional
📥 npm i ImRodry/discord.js#fix-components-type

outer raven
#

there we go

zenith oracle
#

👍

#

Whoops sorry, this should be an array

Screenshot_20210702-160327_GitHub.png
outer raven
#

oh

#

btw @zenith oracle I meant sorting them alphabetically within the region, not within those 4 types

zenith oracle
#

Uhm yeah makes sense...

#

Should be done. However there are a lot of types without lines and not sorted between the region, but yeah, I think it's better to sort them for consistency

outer raven
#

Why was this removed from README in master?

unknown.png
tacit crypt
#

Probably cause it was either super redundant or super broken

wild flax
#

Took too much space

#

For no reason

outer raven
tacit crypt
#

Done

outer raven
#

ty

oak quail
#

@ruby terrace if a thread is archived when the bot starts, it does not get cached (if it is unarchived) until the bot is restarted

oak quail
#

ThreadChannel#sendable is incorrect: it returns true even if it is a private thread that the bot isn't in and the bot doesn't have manage threads

#

there prob should just be a helper property that returns whether the bot is in the thread

ruby terrace
#

….how do you have that thread in cache then?

ruby terrace
#

Or actually, not, because that’s not how that .add works

#

I’ll look into it in a bit

oak quail
oak quail
#

if the bot wasnt in it after starting then interaction.channel is null

ruby terrace
#

I’ll definitely add a getter

oak quail
#

tbh i might ask them to just add bots to the thread when a slash command is used

#

at least for private threads

#

the bot can just add itself by pinging itself but its annoying to handle

ruby terrace
#

That does kinda make sense, if bot exists in guild add it, don’t know if it’s work for very old bots though (where client is is not the same as application

oak quail
#

im sure they can make it work bloblul

#

they have it working in dms

#

with /applications/bots/:id or whatever

ruby terrace
#

Oh true

#

I’ll still fix that perm check though

oak quail
#

why does Formatters.time use Math.floor instead of Math.round?

solemn oyster
#

But at the end of the day, it matters nothing because Discord's time syntax isn't precise

ruby terrace
ruby terrace
#

All fixed here

loud jayBOT
#

pr_open #6015 in discordjs/discord.js by ckohen opened 19 seconds ago (review required)
fix: correct permissions checks and cache on update
📥 npm i ckohen/discord.js#threads-fixup-2

outer raven
#

Can we get a release since there were many breaking changes in the recent commits?

quiet viper
#

Is Custom Caching planned in Discord-next?

loud jayBOT
outer raven
#

think this is what this PR is trying to achieve

quiet viper
outer raven
#

im not sure I didn't look much into it, you can see the PR yourself

wild flax
#

Not sure what you mean by “custom prop caching”

quiet viper
wild flax
#

Unlikely

#

Not anytime soon

#

This would require a heavy rewrite

quiet viper
wild flax
#

That’s still very far into the future

#

I can’t say anything specific about it yet

real jetty
#

Some properties of the Client class are nullable because instanciated only after the call to the "login" function or there is other reason ?

outer raven
#

there could be multiple reasons, depends on what you're talking about

real jetty
#

For example "user".
If the Client class has no use without the login function (for the moment I haven't seen any without calling the login function) maybe it would be interesting to make a static function mixing the arguments of the constructor and the arguments of the login function so that some properties are no longer indicated as "nullable" (the user property for)

outer raven
#

not sure if that's possible but if you know how to then you can open a PR. The only reason that's nullable is because of the login indeed

real jetty
#

I'm not at home, but if this is the only reason, then creating a static method that does both would surely be the best solution.
I'll see if I can make a PR as soon as I get back from vacations

zenith oracle
#

What should this method do?

#

Like a isLogged()?

#

And if so what should be return value?

real jetty
#

I don't know what to call it but it could be a method named "createInstance(options)". The "options" argument would be the same as the current constructor with the addition of the "token" property which would be mandatory.
Within this function, we would do what the login function does and then we would call the constructor of the class (from now on "private") passing it all the information to create an instance of the Client class which would contain directly the "user" properties (and the other properties depending on the login class).
It is this instance that the static function would return.

To create an instance, users will have to do :
const client = Client.createInstance({ intents: [....], token: "...." })

#

I don't know if I'm clear or even if it's possible (haven't looked at the code of the client class yet) but if it is, it can be a better method. It would avoid some properties to be nullable

strange igloo
#

You may want to instantiate a client, but not to login yet (probably to pass your instance reference to your own structures).

This looks more like unsolvable typings issues as this would require TypeScript/your IDE to detect whether Client#login resolved at that particular point of your code, which is likely not possible due to way of code is analyzed and also because that method can fail (invalid token).

You can still "extend" the typings to force certain properties to exist, make a method that would instantiate + login and return that freshly logged in client as that extended type.

real jetty
#

You may want to instantiate a client, but not to login yet (probably to pass your instance reference to your own structures).

In this case it is necessary to keep a separate login method but in the opposite case (use of the Client class only after a call to the login method) I don't think it is useful and using a static method will resolved this "unsolvable typings issues". In fact, I have not seen anyone using the Client class without having called the login class 🤷

You can still "extend" the typings to force certain properties to exist, make a method that would instantiate + login and return that freshly logged in client as that extended type.
This is what I do actually. but I thought that if everyone called the login method before doing anything with the client instance the static function was better suited for typing

vivid field
#

But the login method is asynchronous, so unless everyone switches to esm to make use of top-level await I think hardly anyone will use that static method

neat bolt
#

you cannot customize properties

real jetty
quiet viper
neat bolt
#

not doable currently

#

and maybe never

vivid field
#

So what do you expect users to do?

Client.createInstance(...).then(client => {
  client.on('interactionCreate', ...)
});
```isn't really good user experience
quiet viper
# neat bolt and maybe never

yeah, I played a little bit around with the Library. Disabled all caches and added custom property caching and setted the properties to null, which should be removed.
But the key uses itself memory, so it did not had so a big effect.

The only solution would be do modify the internal Classes, through deleting some keys, when caching them. But I think this will never happen, bc there will be a lot of breaking changes....

neat bolt
#

that'll save you like 5 megabytes

#

dont bother meguFace

quiet viper
#

you have to delete the key itself

neat bolt
#

you buy ram in multiples of 4gb

#

you know that right

real jetty
vivid field
#

If you're able to use await in that context, sure

#

But you aren't in 99% of examples

quiet viper
neat bolt
#

removing keys will turn that into 795mb

#

not worth

strange igloo
#

node does not support top-level-await yet, will it even do at some point lol
so you'd need to wrap this into an self-called async function

vivid field
#

It does when using ESM

#

And commonjs will never have it

strange igloo
#

Oh right

quiet viper
quiet viper
real jetty
unique axle
#

nkoHmm is this just so you don't have to put a ! after Client#user to assure it's there after login?

real jetty
unique axle
#

if you know they're not null at a certain point, just assert nkoHmm
to keep with the example
Client#user will never be null in any event callback, so you just assert it to not be null and move on with your life

zenith oracle
#

Uh what about making an interface LoggedClient which is just an extension of Client with properties like user not nullable so that we can use a function isX() (like isText() for channels) which returns this is LoggedClient

bitter peak
ruby terrace
# quiet viper yeah, I played a little bit around with the Library. Disabled all caches and add...

With the custom caching you could technically make a collection that auto removed the properties you don’t want. I highly recommend against doing this however, it seems like a lot of extra overhead for not much gain. You’d be much better off using intents to reduce the amount of events received (inherently reducing cache size) and after that PR adding noop caches to the things you still get and don’t want to cache.

quiet viper
ruby terrace
#

That's perfectly fine to do, just know you won't get much support here doing it that way

quiet viper
outer raven
wild flax
#

just making sure they return what they should return

outer raven
#

that wasn't done for TimestampStyles though ?

wild flax
#

But we are losing 6.67% coverage if we don't

outer raven
#

I have no clue how to work with jest, specially when not using functions, are there docs that I can check or something?

copper laurel
#

would it just be expect(Faces.Shrug).toBe('¯\\_(ツ)_/¯')

outer raven
#

but what do I put in the test function?

#

the string that is

oak quail
#

is options supposed to be optional

#

ig it technically works as a fetch

remote wasp
#

why is it marked optional when the options param for MessagePayload#create is required? thinkO___O

oak quail
#

isnt this type (FileOptions[]|BufferResolvable[]|MessageAttachment[]) kinda misleading

#

(in BaseMessageOptions)

#

MessagePayload#resolveFile takes BufferResolvable|Stream|FileOptions|MessageAttachment so shouldn't it be (BufferResolvable|Stream|FileOptions|MessageAttachment)[]

#

each file is resolved individually

copper laurel
#

yeah theres been a few union types like that to be fixed

oak quail
#

in APIRequest.make():

      body = new FormData();
      for (const file of this.options.files) if (file && file.file) body.append(file.name, file.file, file.name);
      if (typeof this.options.data !== 'undefined') body.append('payload_json', JSON.stringify(this.options.data));
#

should i add a dontUsePayloadJSON field to override it and add each data field separately

#

or just abuse files instead of using data

copper laurel
#

Isn't it fine how it is? I thought you were just changing a docstring

oak quail
#

no this is for stickers impl

#

the docs thing is just something i noticed lol

#

anyway ive been working on it and

#

trying to force it into files is way too much trouble ```js
DiscordAPIError: The browser (or proxy) sent a request that this server could not understand.

quiet viper
#

Okay, I tested the new CacheFactory/Cache Manager
and what I can say that they are some Problems, which should be fixed.

When capping the Collections to a specific limit, some Problems are created
In less words: You wont receive Events for the uncached Structures

I capped the cache of the ChannelManager to 10
This caused, that I didnt recieve this Events:

  • MessageCreate
  • ChannelUpdate
    -> Nearly every Channel related Event, which requires the Cache...
    When uncapping the Collection or trying it on a cached Structure, you receive the Events as usual.
raven juniper
#

it's generally not recommended to limit the guild or channel cache because of how d.js works

#

keeping them cached is not heavy on memory usage, that's stuff like messages/presences/etc

stray delta
#

Not receiving events for uncached structures is definitely intended, which is also why partials exist

wild flax
#

@outer raven Uniform Resource Locator (URL)

#

Identifier (Id)

outer raven
#

ahhh

#

right that makes sense

#

Wait but Discord uses ID

unknown.png
wild flax
#

Yeah for Interfaces and user facing text it’s probably better

outer raven
#

Discord's API docs also use a mixture of ID and id but never Id from what i can see

quiet viper
# stray delta Not receiving events for uncached structures is definitely intended, which is al...

Enabled all Partials , but not receiving the Events, when the Collection is capped. When uncapped => works

const {token} = require('./config.json')
const Discord = require('discord.js');
const client = new Discord.Client({
  intents: ["GUILDS","GUILD_MESSAGES", "GUILD_MESSAGE_REACTIONS"],
  partials: ['MESSAGE', 'CHANNEL', 'REACTION', 'USER', 'GUILD_MEMBER'] ,
    makeCache: Discord.Options.cacheWithLimits({
        ChannelManager: 1, // 
    }),
});

client.on('ready', ()=>{
  console.log(client.channels.cache.size)
})

client.on('messageCreate', (message) => {console.log(message.channel)})
client.login(token);
wild flax
#

It’s not like we orientate our code standard based on discord’s :P

outer raven
#

thought you did ¯_(ツ)_/¯

wild flax
#

Hell naw

#

We don’t even know what they use internally

#

And we surely won’t start using snake case just because their API is in python/flask

outer raven
#

yeah thats true but I think the public stuff should serve as reference
And you still based your names based off of theirs for example I asked for ephemeral (which is the worse name they've come up with) to be changed and I was told it wouldn't because that's what Discord uses

solemn oyster
#

(btw Rodry, I updated my comment about URL's casing)

outer raven
#

ah I didn't see it, lemme look

#

I see but why is that the case on that specific scenario?

#

like for example, doesn't displayAvatarURL fall under the "joined with other words" category?

#

or does that only apply to interfaces and classes

#

@solemn oyster ^^

solemn oyster
#

I'm a bit mixed in that regard

#

I agree displayAvatarURL looks good, the problem comes when it's an <Acronym><Word>, like URLIdentifier

outer raven
#

is that used anywhere? Never seen it

solemn oyster
#

Either way it's a different issue, and 6036 will exclusively change the casing of ID, and nothing else

#

It's an example

outer raven
#

ah I see

solemn oyster
#

There's StaticImageURLOptions tho

#

And Guild#vanityURLUses

outer raven
#

tbh im not against nor in favor of this change, it just seems like something so small that's gonna require so many changes that it's barely worth it imo

solemn oyster
#

The URL or the ID one?

outer raven
#

ID

solemn oyster
#

If you ask me, this was the best moment to do it

#

We haven't been below 15 PRs for 3 years or so, and we had plans to change ID to Id with the TypeScript rewrite

#

Tracking the breaking changes in the TS rewrite with that mixed in... ablobsweats

outer raven
#

ah well in that case then yeah sure

#

and this change isn't as controversial as structures (which tbh i was surprised with the backlash, i didn't even know they existed) but yeah if you were planning on doing this anyway, v13 is the best time to do so

#

We're getting closer to the release right? I saw some pinned issues be closed the other day so i thought it could be near 👀

solemn oyster
outer raven
#

christ that's huge, lemme look

stray delta
rough roost
#

imo URL should be changed along with ID, both are always spelled in uppercase, one being an acronym doesn't seem that relevant

#

google style guide for java/js specifies it should be userId, iconUrl

quiet viper
wild flax
#

It's not really

#

Your case is just obscenely obscure

#

Because at this point why do you even use a library at all?

quiet viper
wild flax
#

It could probably be extended to mention caches

stray delta
#

Is there going to be an alternative explanation for what to use in place of structures? Seems like there's going to be a lot more complaints

neat bolt
#

my PR has what's recommended
otherwise, defineProperty

sharp wigeon
#

Is there a reason why to the xID is being changed

#

I mean obviously there is lol

sly ridge
outer raven
#

customId and options shouldn't be optional, any reason why they are marked as such here?

unknown.png
wild flax
sly ridge
sly ridge
# wild flax If anything the first point.

I'm thinking of doing that and making a pr, but I'm not sure whether to add something like <info> This event requires the GUILD_MEMBERS intent.</info> or just add it to the jsdoc description without <info>

zenith oracle
outer raven
#

ah true I guess, but then the docs become misleading on the typedef

copper laurel
#

Docs dont need to perfectly represent the types

#

Those structures will probably move to /builders anyway

bitter peak
#

Would conditional nulling types be beneficial? If so, I could open a pull request...

unknown.png
copper laurel
#

Definitely could be, though that seems like a huge undertaking to apply across the lib

#

I can find docs on the Ternary type modifier, do you have information for it?

bitter peak
# copper laurel I can find docs on the Ternary type modifier, do you have information for it?

Ah, right, sorry. That's custom. Here's the impl:

/**
 * Conditional typing extension helper.
 *
 * 1. If `T extends SUPERTRUTHY`, then `TRUTHY`
 *
 * 2. If `T extends SUPERFALSY`, then `FALSY`
 *
 * 3. Else `FALLBACK`
 */
export type ExtendConditional<T, SUPERTRUTHY, TRUTHY, SUPERFALSY, FALSY, FALLBACK = never> = T extends SUPERTRUTHY
  ? TRUTHY
  : T extends SUPERFALSY
  ? FALSY
  : FALLBACK;

/**
 * Ternary helper function for typing.
 */
export type Ternary<FLAG extends boolean, TRUTHY, FALSY> = ExtendConditional<FLAG, true, TRUTHY, false, FALSY>;
#

Could be changed around, etc. to be easier to include in djs

bitter peak
ornate topaz
#

! solves that as well

copper laurel
#

I mean there's some styling inconsistencies here but that's easily fixed, I think this is more about if we would want to do this in the current version (which is just JS + a typings file) vs leaving it for the TypeScript rewrite

copper laurel
ornate topaz
#

client.user will be there in the events though (obviously assuming that there is client at all, but in such case it would never be there anyway)

copper laurel
#

optional chaining is preferred over not-null assertions

bitter peak
copper laurel
#

Modular, written in TypeScript

bitter peak
#

Ah I see

bitter peak
#

@copper laurel What about something like this?

type Ternary<V extends boolean, T, F> = V extends true ? T : V extends false ? F : never;
type Nullify<T> = { [P in keyof T]: null };
type ConditionallyNull<T, V extends boolean = boolean> = Ternary<V, T, Nullify<T>>;

class BaseClient {
  foo: number;
}

interface LoggedInClient {
  user: object;
}

export type Client<LoggedIn extends boolean = boolean> = BaseClient & ConditionallyNull<LoggedInClient, LoggedIn>;
copper laurel
#

LoggedIn extends boolean would match our casing conventions unless theres a need for that format?

#

Otherwise yeah that looks in line with our style

bitter peak
#

no = boolean?

copper laurel
#

Oh yeah thats fine

#

I'm only picking on styling, not functionality

#

@wild flax @tacit crypt @slate nacelle @solemn oyster is this sort of conditional typing something you want to go for?

#

Probably also @neat bolt because typings wizardry

neat bolt
#

is this the thing where you do the thing

#

with the null stuff after login

#

i'd rather just this if at all

class Client<LoggedIn extends boolean> {
  always: number;
  sometimes: LoggedIn extends true ? number : null;
}
bitter peak
neat bolt
#

seems like theres two different proposals here then

  • a generic that changes the type based on the state of the object in time
  • a generic that chooses which variant of some concept to use
    im ok with the former, though it will require very careful consideration that the typestate is valid
    im against the latter, those should just be two separate declarations, or if applicable, inheritance
    its just error prone due to the type machinery you have to do, bad for documentation for similar reasons, and is just laziness over explicitness for not much gain
#

@bitter peak @copper laurel

tacit crypt
neat bolt
#

above comment yeptune

vernal charm
outer magnet
#

isn't interaction.guildID just a shortcut method for interaction.guild.id?

strange igloo
outer magnet
#

Coz if so, i dont see why it would be needed

vernal charm
strange igloo
#

The two properties exist because you may get a guild interaction from an uncached guild
Leading guildID to hold the correct guild ID... but guild to be null
Should not happen as long as the client user is in the target guild, and you enabled the GUILDS intent, and not messing with cache but you shouldn't do it in anyway

outer magnet
#

ah i see

vernal charm
# vernal charm Ah alr thanks

In OAuth2 that isn't very useful, at least in my opinion. If I was discord I would add it to the api it self not to OAuth2

outer magnet
#

thanks fot the explanation

strange igloo
# vernal charm In OAuth2 that isn't very useful, at least in my opinion. If I was discord I wou...

They purposefully did it so we can't get that information without explicit consent from the user
OAuth2 requires the target user to say "Yes I want to give my profile information to that thing" while regular API doesn't
It won't happen, at least not in a near future, it already took a while for badges & co. to be available to bots

The original reason was to prevent "nitro-only" features that would break equality between regular and Nitro users for non-Discord related stuff

vernal charm
outer raven
#

I know this change was made long ago but why don't ClientUser#setActivity , ClientUser#setAFK, ClientUser#setPresence and ClientUser#setStatus return promises anymore?

strange igloo
#

Because they are gateway commands and I don't think there is anything async in these

wild flax
#

^

outer raven
#

Maybe that should be mentioned in the v13 changes guide?

copper laurel
#

It is

#

Unless you mean the why?

fickle burrow
#

wait i'm confused
would't the custom / commands be a thing at v13 ?
if so why do i already see /commands at the https://discord.js.org/#/ and in the server here aswell ?

not sure if this is the right channel

outer raven
wild flax
#

I don't think anyone benefits from knowing "why" is isnt a promise anymore

outer raven
#

Some people might be interested

outer raven
strange igloo
outer raven
#

Is this still true? (v13 changes guide)

unknown.png
fickle burrow
strange igloo
outer raven
#

yeah so it wouldn't return a type error since the whole point of moving the filter to the options object was to make it optional right?

nocturne glen
#

Hello fellow developers OMEGAlul
If I want to contribute to developing discord.js, where should I start?

zenith oracle
nocturne glen
#

🙏

broken abyss
#

Since Structures are removed from djs v13, how can I get the raw API response data (e.g. guild avatar for GuildMember) from Discord API?

unique axle
#

by waiting for guild avatars to be merged

wild flax
#

or just use client.api or something

broken abyss
wild flax
#

oh, thats not extending anything

#

you just make the raw api call

broken abyss
# wild flax you just make the raw api call

Yeah, I got it. I used client.api before. I forgot about it. I used Structures for all new APIs like buttons/select menus etc in djs v12 so I got used to it. Thanks for the help!

bitter peak
real jetty
zenith oracle
#

Yeah. I think there are still some things about Typings that can improved and this is one of them

upper moss
#

After v13 finishes what do you plan on current library because of discordjs-modules what will be the priority like is major semver prs gonna get accepted in current lib

outer raven
#

@vivid field after the typings rewrite, calling .send on a PartialDMChannel throws an error saying the function can be null on typescript when it shouldn't, do you know how to fix this or why this is happening?

vivid field
#

Oh come on ts

#

Probably an any -> unknown change that broke something somewhere

real jetty
#

Do you think it's possible to change the property partial to function isPartial(): this is Partial... (for example <User>.isPartial(): this is PartialUser) to improve typings ?

outer raven
#

i searched through the entire file and i could not find a reason as to why that is because its all the same as it was before

vivid field
#

Yeah

outer raven
#

also why change from any to unknown?

wild flax
#

because its generally better to use unknown

vivid field
outer raven
#

but you can't reassign a type to an unknown value

wild flax
#

spread function annotations shouldn't be switched though, so that was just a mistake

outer raven
#

so all spread functions should be any?

#

ill open the PR

vivid field
#

I'm going through it already, except if you got the PR ready now

outer raven
#

i think i got it all ready

#

only had to replace all ...args: unknown with ...args: any

#

and hasParams i think too

neat bolt
#

gotta be careful with that unknown/any change

vivid field
#

Single args break that too, so basically any callback parameter

neat bolt
#

always think about subtyping and variance yeptune

outer raven
#

so like this too?

unknown.png
vivid field
#

That no

#

e.g. when you got public method(callback: (arg: unknown) => void): void;

outer raven
#

actually those shouldn't even be unknown, doesn't discord-api-types have those?

neat bolt
#

generally if it produces a value, you want unknown, if it consumes a value you want any

#

and also because nothing extends unknown, any time you do a conditional with unknown itll always be false

neat bolt
outer raven
#

that doesn't actually appear anywhere tho so its just the ...args change

vivid field
#

There's no actual function for that, just an example I tested

neat bolt
#

oh

neat bolt
vivid field
#

Same error as with rest parameters

unknown.png
neat bolt
#

well yea, thats def wrong

#

youre basically assuming an unknown is Message

#

which is kinda against the whole point of it

#

im kinda surprised that works with any tbh

#

they should both fail

vivid field
#

Well yea because any disables practically all type checking

#

So anything is assignable to it and any is assignable to everything

wild flax
#

everything is any in TS until you explicitly type it

neat bolt
#

yep, if you want to be completely correct there, as in cb can be called with anything, then unknown is actually correct

wild flax
#

so it makes sense that it works

neat bolt
#

i should write a blog on subtyping and variance in ts

broken abyss
neat bolt
#

thats actually the same as any

#

or rather, its the same as nothing at all

#

fn can't produce a value of T because it doesnt know what T is

#

so cb can't be called, ever

broken abyss
neat bolt
#

no it doesnt

loud jayBOT
outer raven
#

i think that's all right?

neat bolt
#

the caller specifies that it is string, but at the body of fn, all it knows is T

#

go and try to implement that function yourself and you'll see

#

@outer raven i'll leave some comments

outer raven
#

alright ty

neat bolt
#

bleh, easier here

#

for the setInterval and stuffs, do something like

foo<T extends any[]>(f: (...args: T) => void, ...args: T)
outer raven
#

what difference does it make?

#

i actually had the last args as any[] but changed last minute cuz i thought they were wrong lol

neat bolt
#

the difference is the generic

#

now the stuff you receive in the callback is the same as the stuff you pass to the function

broken abyss
#

that was my initial thought. you can handle that T in your fn body

outer raven
#

true

#

its just these 3 right?

unknown.png
neat bolt
outer raven
#

pushed

broken abyss
neat bolt
broken abyss
neat bolt
#

a generic doesnt work there as i said earlier

#

very simply:

function fn<T>(cb: (args: T) => void): void {
  cb(???);
}```
#

so the type makes no sense

broken abyss
#

yeah

vivid field
#

There's also a last unknown at L4312 that I think also needs to be changed

neat bolt
#

are the first four actually any or should they be unknown, like i said earlier

#

4312 needs to be changed yea

vivid field
#

The first four are what I showed above

neat bolt
#

you want the user to be able to assume the type?

vivid field
#

With the single unknown param in the callback

#

It should be treated the same as unknown rest params in a callback, no?

neat bolt
#

no

#

if its any, any function type works

#

if its unknown, they have to type it as unknown and cast explicitly

vivid field
#

(arg: unknown) => void and (...args: unknown[]) => void should both be any from what I can see

neat bolt
#

not always

vivid field
neat bolt
#

it depends on what theyre used for

#

for example to check if something is a function, T extends (...args: any[]) => void is correct

#

to say that a function can be called with unknown value(s), cb: (...args: unknown[]) => void is correct

vivid field
#

But most of the changed stuff is the latter example

#

Because unknown[] doesn't seem to work for callback parameters

neat bolt
#

how so?

# 
function f(c: (a: unknown) => void): void;
f((x: unknown) => typeof x === 'string' ? ... : ...);
#

if you mean to say that the user knows what the type is and they can assume their types are correct then a: any

vivid field
#

If we were to fix only the extends issue, that would be maybe 3 changed lines, the rest is just so you can do f((x: Message) => ...)

neat bolt
vivid field
#

I.. think so yea

neat bolt
#

depends what yall wanna do theCutest

vivid field
#

I'm not really opposed to keeping that as unknown there, just saw some people having problems with it

neat bolt
#

it looks like those might be served better by generics?

analog oyster
#

there are plenty of breaking changes already, why make the message listener deprecated instead of just removing it altogether PES_Think

neat bolt
#

oh, is there not a type for message collector? @vivid field

vivid field
#

There should be

neat bolt
#

ah

#

there is, but the events arent overloaded

#

theres your actual problem

solemn oyster
vivid field
#

Oh yea

#

There are overloads missing

outer raven
neat bolt
#

i'd say for events and such, you should go with unknown

#

users can augment the interface to add their own events

#

and if they use a non-literal event name string, they should get unknown

#

but depends on what the other guys think

vivid field
#

Sounds good to me

neat bolt
#

@vivid field thats your cue to @ everyone related btw

#

fine i'll do it angeryCat

vivid field
#

I can't decide that tho meguFace

#

It's just my opinion

neat bolt
#

@wild flax what do about unknown/any in callbacks

wild flax
#

just keep any

neat bolt
#

you heard the man yeptune

wild flax
#

less of a headache

#

and with the TS rewrite we can do it better anyway

vivid field
#

Then the PR should be fine as-is I think

neat bolt
#

you gotta make a new pr for messagecollector

vivid field
#

Couldn't that also be handled by the collector generics?

neat bolt
#

i dunno

vivid field
#

Or is there none for that

neat bolt
#

i didnt read the whole thing meguFace

vivid field
#

Same tbh

#

Yeah the params for the collect/dispose events seem to be the same as the ones passed to the filter

neat bolt
#

@zenith oracle read this whole channel meguFace

vivid field
#

So I can probably just do the same thing there

zenith oracle
outer raven
#

does @discordjs/the-big-3 actually do anything on github?

#

also who are the big 3 I thought there were 4 lol

wild flax
#

early responders, which is usually kyra, me and vlad

outer raven
#

but will it tag you?

wild flax
#

yeah

neat bolt
#

space is emergency responder yeptune

outer raven
#

aight

#

cries in not blue text swagcat_cry

bitter peak
#

I'm working on separating Interaction into Interaction and GuildInteraction for typings; where the latter has the guildId and guild properties always set, and the channel is a GuildChannel. Is this pattern okay and how should this be inherited by subclasses of Interaction?

surreal hollow
#

Where/who do I complain to about all the dumb changes v13 has?

neat bolt
#

me

bitter peak
neat bolt
#

oh uh

#

i think we should let typings somewhat reflect the actual code

#

dunno, can you give a sample

real jetty
neat bolt
#

sounds reasonable at a glance

bitter peak
#

hm, maybe in that case would a generic to modify the properties be okay in this case? I know you said you were somewhat against it, but what about this:

type If<T extends boolean, A, B> = T extends true ? A : B;

export class Interaction<InGuild extends boolean = boolean> {
  public readonly channel: If<InGuild, Channel, PartialDMChannel | null>;
  public channelId: If<InGuild, Snowflake, Snowflake | null>;
  public readonly guild: If<InGuild, Guild, null>;
  public guildId: If<InGuild, Snowflake, null>;
  // ...
  public inGuild(): InGuild;
}

export class CommandInteraction<InGuild extends boolean = boolean> extends Interaction<InGuild> {
  public readonly command: If<InGuild, ApplicationCommand<{ guild: GuildResolvable }>, ApplicationCommand | null>;
  public readonly channel: If<InGuild, TextChannel | NewsChannel  | ThreadChannel, DMChannel | PartialDMChannel | null>;
  // ...
}

@neat bolt

neat bolt
#

uhh

#

maybe, maybe not

#

needs more thinking about

#

here's one improvement though, type If<T extends boolean, A, B> = T extends true ? A : T extends false ? B : A | B since the last case is T = true | false

bitter peak
#

ah right ty

neat bolt
#

@tacit crypt we had a whole discussion on why things should be any in certain places beeproll

solemn oyster
#

Can't be unknown*

neat bolt
#

same thing theCutest

tacit crypt
neat bolt
#

see screenshot for why we do any for extends checking

tacit crypt
#

Ok, for those, but for the rest?

neat bolt
#

crawl wants any for listener callbacks so that people can just assume the types

tacit crypt
#

Huh... ok

neat bolt
#

the setInterval stuff is the same

#

not that anyone uses that anyways

tacit crypt
#

Fair fair

neat bolt
#

pr probably needs a better name tbh

outer raven
#

what should it be i didnt know what to call it tbh

neat bolt
#

revert incorrect unknown type changes or something like that

outer raven
#

done

zenith oracle
tired valve
#

The thread update event exists and is documented

outer raven
wild flax
#

eh it doesnt matter really

outer raven
#

aight ill just commit the other change then

neat bolt
#

@remote wasp could you elaborate on that

remote wasp
#

sure

neat bolt
#

i think you're looking for 41 btw

loud jayBOT
remote wasp
#

oh yeah, that seems to be what I was looking for. Collection<Snowflake, TextChannel | ThreadChannel>. The ThreadChannel has isThread, so <col>.filter(e => e.isThread()) should give Collection<Snowflake, ThreadChannel>

neat bolt
#

yea its been fixed for a bit

remote wasp
#

👍

copper laurel
#

djs might not be using that version though?

neat bolt
#

maybe not

copper laurel
#

Not sure if/when we updated djs deps

neat bolt
#

you're in support a lot mon

#

do people 1. extend Collection, 2. rely on toJSON

ornate topaz
#

there wasn't even a collection release with that

#

might be worth to do a release 👀

#

though not sure if removal of array methods wouldn't clash with d.js

#

since that would also be in new release

copper laurel
ornate topaz
#

extend Collection? comp, most people just blindly do .array() no matter what they do or want

copper laurel
#

I think you're forgetting the best class, DataStore

ornate topaz
#

that was extension in the lib, not by people though

neat bolt
#

there still are extensions in the lib

#

mainly the toJSON collection and the limitedcollection

copper laurel
#

Yeah djs Collection is already an extension

#

So does this mean you break LimitedCollection#toJSON() if we dont add typings for that?

neat bolt
#

no

remote wasp
#

just wanna say that some libs that use @discordjs/Collection might/do extend it 👀

neat bolt
#

CollectionDJS#toJSON exists

#

but if you did colldjs.filter() then it wont

copper laurel
#

right okay

neat bolt
#

you need to do one of the alternatives

copper laurel
#

So if you did like, messages.cache.filter(one author).toJSON()

neat bolt
#

itd coincidentally work in JS, but wont typecheck in TS

copper laurel
#

it wont work

#

yeah

raven cloak
#

Hi guys, I'd love to help with development.
Where could I start? Learn TS 😅 ? Is there somewhere I can see a list of things that are currently being done/need doing just to have a browse

copper laurel
#

There isnt anything specific, pretty much everything for v13 is already done

#

The only place things that need doing are logged is really the github issues

remote wasp
slate nacelle
remote wasp
#

righhhht, that's why I didn't find it, cuz I was looking it in that _add PR. Now why is it still omitted tho? 🤔

slate nacelle
#

Oversight

remote wasp
#

I'll leave a comment 👍

zenith oracle
#

Mmh I was trying to apply some changes for structures when they're partials like isPartial(): this is PartialUser but the User is reduced to never for some unknown reason and TS says that there are conflicting types in some constituents...

#

Also are the PartialX types actually correct for partial structures or are they only for parameters passed in events?

wild flax
#

"is this actually correct" is an interesting question, considering we wouldn't know if nothings wrong

zenith oracle
#

Lol

outer raven
#

@solemn oyster why Opcodes and not OpCodes?

solemn oyster
#

It's a single word

outer raven
#

doesn't it stand for something?

oak quail
solemn oyster
#

I have no idea where we discussed opcodes before, but we spend a full day in internal channels deciding which one to use

outer raven
#

lmao

oak quail
#

its short for "operation code" but "Opcode"/"opcode" is the standard

#

nobody says OpCode

outer raven
#

ah I see

vivid field
#

We did that for the dapi-types enums before

solemn oyster
#

Oh, dapi-types, yeah

remote wasp
#

I still don't get why enums in dapi-types are PascalCase when the api has it in SCREAMING_SNAKE_CASE 🤔

wild flax
#

because enums are pascal cased

remote wasp
#

I meant this:

export const enum GuildVerificationLevel {
  None,
  Low,
  Medium,
  ...
}

The api has it like: NONE, LOW, ...

wild flax
#

what api

remote wasp
#

the docs

wild flax
#

yeah those are values

#

what you showed is enum keys

#

either way it doesnt matter much

#

we do not base our code style off of discord lol

#

You don't see use using snake_case do you

#

instead of camelcase

remote wasp
#

no... 😔

outer raven
#

only internally tho i think

#

oh and my PR oruded

wild flax
#

yeah all of that needs to be removed

outer raven
#

ohwait

wild flax
#

we should only use enums internally

#

no strings

outer raven
#

but like these are fine right

unknown.png
wild flax
#

Users using it is fine

#

But us internally not so much

outer raven
#

idk whats used internally, but that is what the users see at least

tacit crypt
remote wasp
#

yeah, I get it now. I got confused because of the enums we create in djs for them

solemn oyster
#

If you insist, we can make all enums in d.js PascalCased

zenith oracle
#

Why is this always false?

unknown.png
#

Same for other possible partials structures

#

Mmh I see that in events a partial dm channel can be passed too and ig that's why

outer raven
#

How do you know that’s a DMChannel type for sure?

zenith oracle
#

wdym?

outer raven
#

like if it's a DMChannel and not a PartialDMChannel then obviously partial will be false

#

my question is how do you know for sure that's a DMChannel type and not a PartialDMChannel

zenith oracle
#

That's what i've asked lol

#

Iiuc it's always a non-partial dm channel and it can be a partial one only in some events properties

ornate topaz
#

afair, on v8 discord doesn't emit channelCreate for DM messages. that's why you need CHANNEL partial to be able to receive DMs at all

zenith oracle
#

Ye

quiet viper
#

I was thinking to add a Client Option, which fires the event, even when the Structure is uncached?
Should I do a pull request or are they any concerns?

solemn oyster
#

@zenith oracle can you test all partial and non-partial structures, please?

#

Also put them at the end, they're grouped in chunks testing 1 thing

solemn oyster
#

Simply make it so the relevant stores use a custom one that emits events in your client

neat bolt
#

oh god pls dont do it like that

quiet viper
solemn oyster
#

declare const messageReaction: MessageReaction; and declare const partialMessageReaction: PartialMessageReaction;

#

Among others

#

Then you use the utilities to assert the types

#

Wait

#

I mixed the two of you, sorry

#

@quiet viper ^

quiet viper
solemn oyster
#

np

neat bolt
#

kyra wait NotLikeAlice

solemn oyster
#

Just curious, @quiet viper, what do you want that for?

neat bolt
#

thats to change caching not to change when events are emitted beeproll

solemn oyster
#

Oh

quiet viper
neat bolt
#

obviously beeproll

quiet viper
#

and the Dm Create Problem (Discord gives you the rawddata)

#

For advanced Devs, there should be a option for getting the Data, which Discord gives....

#

Idk if you understand what I mean

solemn oyster
#

I thought you meant emitting an event when a structure is uncached

#

aka removed from the collection

quiet viper
wild flax
#

just use the raw event then lol

#

since you cant use any of our structures anyway

#

idgi

zenith oracle
zenith oracle
#

Mmh when is Member#joinedAt null? Is it only when is partial?

solemn oyster
#

Presences don't give joinedAt, but they give all others (even roles)

wild flax
#

Also lurkers

zenith oracle
#

👍

real jetty
#

@wild flax srry for mention,
i found a typing error

unknown.png
wild flax
#

fix it

loud jayBOT
zenith oracle
#

@real jetty ^^

wild flax
#

You have to fix the tests in the index.ts too

#

silly you

zenith oracle
#

Done lol

#

Totally forgot about tests

#

Sorry

real jetty
#

@zenith oracle thx for submit a pull for this, but don't pull requests without at least checking that everything is OK

zenith oracle
#

So sorry about it. I always run the npm test command before making a pr but this time I didn't. I updated tests and everything should be ok now. However lmk if something should be changed via comment or review 👍

copper laurel
steady salmon
#

what else is needed to do for v13 release? looking to help out

copper laurel
#

Discord to release features

#

Otherwise feel free to check the Github Issues and fix anything you think you can

steady salmon
copper laurel
#

Mainly threads and stickers actually, banners and avatars are not important features we would delay for

steady salmon
#

makes sense, i was just looking at the pull requests

oak quail
#

why not make each typings "region" a different file?

#

its kinda messy rn, as one huge file

wild flax
#

can't

sacred river
#

add timeout back to message.delete() function ?

#

that would be nice

copper laurel
#

[3:35 PM] KinectTheDots: It didn’t allow for proper error handling

sacred river
#

welp

#

ggs

raven cloak
#

Takes little bit extra code and allows for both a timeout and proper error handling

quiet viper
loud jayBOT
stray delta
#

You can also probably listen to websocket events Thonk

quiet viper
#

why the Event does not get fired...

wild flax
#

@oak quail PR technically done?

oak quail
#

yeah

#

except for the docs link thing sugden? asked for

wild flax
#

yeah

#

Ill wait anyway until discord merges the PR

#

in case they have a fever dream over their break

#

and want to break more

oak quail
#

yeah I'll try to get it merged next week

bitter peak
#

btw does this need to be allowed for my PR?

unknown.png
wild flax
#

Yes, first time contribs need to be approved for a workflow run first

bitter peak
#

Hey @copper laurel, in regards to the conditional typing pr, what do you think about making Client#token readonly? That would prevent arbitrary API calls without logging in first, in a way that is rather unintrusive.

copper laurel
#

I'm not really in favour of restricting the way the library can be used

#

But Im also not responsible for the direction of the lib - my main concern is simply that the types weren't correct. Not all of the time.
I really do like Client#isReady() and TypeScript users should be expecting to typeguard their stuff. I have no sympathy for TS users complaining about having to typeguard - don't use TS then.

bitter peak
#

Alright. It seems to me that the refactor of Base is outside of the scope of the PR then. I'll revert the commits that changed Base.

copper laurel
#

Dont do it on my say so

bitter peak
#

Nah, it seems to be a much bigger issue than I initially assumed it to be

copper laurel
#

Hope i didn't come across too negative, I like the concept you're going for

#

Im just not sure asserting it as true is the right approach, its not truly conditional if there isnt a condition being evaluated

bitter peak
zenith oracle
#

Since Events became a const enum we can't use index access with them.... I mean, doing Constants.Events[eventName], where eventName is a function parameter typed as keyof Events, will now throw the error A const enum member can only be accessed using a string literal. Is this somehow intentional? Why?

#

I use an event handler and obviously this break a lot of things for me...

wild flax
#

Why do you need to access it like that

#

Oh, I see

#

Hmmm

#

That might have been an oversight if anything

#

We didn’t intend to have such a breaking change here

zenith oracle
#

Mmh yeah

oak quail
#

hmm should the library reject this

image0.png
copper laurel
#

Why, the API doesnt

tacit crypt
#

Course it doesn't, because its a valid route. Now if we should handle it...idk.

#

Definitiely unexpected

solemn oyster
#

That's user error if anything, so no

#

Just like passing a non-snowflake to members.fetch can result on a WS restart and a timeout, sanitise your input and you're gold

bitter peak
#

Typings already use ${bigint} to guard against that right?

solemn oyster
#

Mhmm

shrewd kelp
#

When you provide invalid data to the api and get a DiscordAPIError the stack trace is lost, which can make debugging difficult. Could the APIRequest class capture the stack trace when it is created then emit it on error?

I tried to do this myself but I ended up getting a stack trace to TextBasedChannel.js (for a channel.send), the goal would be to get a stack trace back to the users code where they did the actual channel.send call

copper laurel
#

It should work fine on Node v15 or 16, forget which, with async stack traces

shrewd kelp
#

Im on 15.6.0, didn't realize v16 added that ill have to check it out

copper laurel
#

Hmm they should already be in place

#

Yeah it was actually 14

shrewd kelp
#

I don't think its just because its async, its because it gets pushed into a que and then the event is fired later

copper laurel
#

Right, gotcha

shrewd kelp
#

Im currently trying to figure out how to get a stack trace from here, to where the error is created... but I am very lost 😄

unknown.png
wild flax
#

Or do you sometimes just return or smth

shrewd kelp
#

I normally await, and the line that is throwing the error is await'ed

#

Actually, the stack trace may be broken due to how the route builder works

#

Or... its just javascript being funny

#

Put a trace right here and yet the callstack is broken

unknown.png
#

Javascript must be high or something, if I write the code like this, then my stack trace is logged correctly

unknown.png
#

However like this it fails

unknown.png
#

or... maybe not?

#

I have no idea, ill take another look at it tomarrow

warped crater
# solemn oyster Just like passing a non-snowflake to members.fetch can result on a WS restart an...

bit late but I'm not sure if I agree with this - input should indeed be sanitized on the user's end but the return value of such a function call does not match documentation nor typings at all - it is practically UB - which shouldn't happen.

I don't think the library should handle the snowflake passed in and try to validate it, but it should at the very least validate that the data returned from the API is valid to construct a User instance from

zenith oracle
#

I agree with this ^^
I think it can just check if id is typeof string, and, if isn't, throw an error

#

Can prevent unexpected behaviour like that where the returned structure doesn't match any documented one

warped crater
#

uh, no, that's not what this is about.

warped crater
#

i.e. somestring causes the library to do /users/somestring which is an actual 404, the library does throw since it could not find the user

#

however some/string makes it hit an entirely different route than intended (/users/some/string)

zenith oracle
#

So what should the library do to prevent this?

warped crater
#

if the route happens to exist (like @me/guilds) then U.B happens

zenith oracle
#

Ohh I see what you mean

warped crater
#

there's 2 approaches:

  • make sure the string cannot include / (or other HTTP unfriendly characters) <- not a fan of this
  • check the API response so one of those weird User instances cannot be constructed - and instead an error is thrown
#

the 2nd approach makes much more sense as it doesn't do input validation for the user

#

it simply prevents completely unpredictable behavior

#

a User instance should simply never have an undefined id, that's just horrible on all fronts

zenith oracle
#

Mmh do you mean this should be checked by the User constructor?

warped crater
#

sure, it could be done in each structures' constructors, I suppose

#

it could also be done before one is called, it depends how wide spread such checks should become (if at all)

zenith oracle
#

That's really interesting, but how should djs check if the data provided in the constructor is ok?

warped crater
#

it doesnt

#

lol

zenith oracle
#

I meant, how it should 😂

warped crater
#

just how you typically do runtime checks in js. ¯\_(ツ)_/¯

#

typeof - in keyword etc

#

similar to how client options and everything else is validated

zenith oracle
#

Well that's what djs does (using if "something" in data), but it also handle partial Structures so it cannot throw an error if, for example, a property is missing...

warped crater
#

Those checks are there for different reasons and are there for mutable properties and happen in _patch typically.

#

its like

#

for users, yeah I get it

#

a partial user doesnt have much other than its ID as raw data

#

but this is mostly about data coming from GET api calls which is typically complete

#

hence why I was hasitant to say "do it in the constructor"

#

its a very tricky problem to approach, yes

wild flax
#

Lol

#

I don’t think we need to do anything

#

We document very clearly what you can do

#

If you go strictly against that, dont expect anything fancy to happen

warped crater
#

I dunno, I don't think an undefined id on a "valid" User instance is desireable

wild flax
#

Introducing random runtime checks on every construct now seems ridiculous

#

The error is already not passing an id

#

Not the construct you get back

#

Feel free to PR a good solution to this, because I can’t think of one that wouldn’t either impact perf or convolute the code

warped crater
#

👍

real jetty
#

I just saw that interaction includes an "inGuild" function. I think that this could be great if we can add a similar function on Message and maybe on Channel

sharp wigeon
#

what would the inGuild method do?

real jetty
#

i pressume itd check whether the message was sent in a guild or a channel belongs to a guild

sharp wigeon
#

Don't think that would be necessary as the GuildChannel class does exist

#

but I may be wrong

unique axle
#

should be able to guard that with a return on !message.guild

sharp wigeon
#

Yuh

zenith oracle
#

Well it's the same as !interaction.guild_id. Not sure why this was added effectively Thonk

wild flax
#

Does that actually type guard

zenith oracle
#

The inGuild? No, I don't think so

wild flax
#

I mean the others

zenith oracle
#

Others what?

tired valve
#

Jeez, the other structures, message.guild, channel.guild

real jetty
zenith oracle
zenith oracle
real jetty
#

I thought it worked similar to "isText()" to indicate that "guild" is not null and that "channel" is of type GuildChannel

ornate topaz
#

the question wasn't "what should they guard from" but "do they guard"

zenith oracle
ornate topaz
#

does doing !whatever.guild have the same effect as using !inGuild()

zenith oracle
#

No, since the first at least ensure that the .guild exists, while the second just returns a Boolean

wild flax
#

Do you actually look at the typings

#

Hence why it’s called a type guard

#

Returning a bool is how type guards work

zenith oracle
#

Well what I mean is that returning boolean is different from returning this is TextChannel

tired valve
zenith oracle
#

So this returns to my question 😄
The inGuild, what should typeguard?

ornate topaz
#

that the guild exists...

#

and it's notis but in

#

probably also why it cannot be this is whatever

tired valve
zenith oracle
# ornate topaz that the guild exists...

Okok I think I wasn't really clear but I think I'll look into it better tomorrow 😂
However what I meant is that if I use if (interaction.inGuild()), for example, am I sure that .guild or .guild_id (or was it guildId, I don't remember) are defined? That's what I mean for typeguard lol, maybe I did not say it clear enough, sry

ornate topaz
#

did you look at what it actually checks?

#

There's no GuildInteraction you could typeguard with isGuild(): this is GuildInteraction guard that would check if there are guild and member properties.
There's just Interaction, so you have nothing to is against.

#

this typeguard doesn't act as a type narrower

#

but more of a check

zenith oracle
real jetty
#

I think I understood what he meant, he expected to have a this is ...... ||too late XD||

Nvm we come back to my question, why not add this principle to the Message object? Both functions would have the same principle and I think it would homogenize the operation by having this same function in the Message class.

zenith oracle
#

And that's the main question

real jetty
#

Honestly, I thought, like D Trombett, that with this function there was a new class like GuildInteraction that was created and that overloaded some properties like guild by indicating that it was not nullable anymore

zenith oracle
#

If there is a typeguard (which doesn't change the type with an is) that just check for the existence of guild_id, is it there for a specific reason? (And it's not in other structures like message or channel for another?)

ornate topaz
#

why would there be a separate class that has exactly same properties

zenith oracle
vivid field
#

Tbh if there's no GuildInteraction interface that is used for the inGuild() function, I don't see why it was added in the first place

#

The sole reason why I added isText() was to have a type guard

real jetty
real jetty
zenith oracle
#

Yeah!!! That's what I said 😂
Why is it there without a type check... But actually not in any other place (in that case we can say it's for consistency...).

Actually replying to Vaporox but forgot to add the reply woops

wild flax
#

@copper laurel maybe knows 🤔

copper laurel
#

I forget who added it but yeah, it just typeguards a couple properties to rule out it being in a DM. I think I proposed "inGuild" rather than "isGuild"

ornate topaz
#

is it actually supposed to work in this context?

unknown.png
vivid field
#

I would expect it to, but since it's not a type guard it doesn't

real jetty
#

I thought it worked like a type guard to remove the nullable status but it doesn't. So I don't see the point of this function nor the point of adding it as I proposed to the Message and Channel class...

So I have a new question, why not make this function a real typeguard to update interaction typing?

ornate topaz
#

as it was already said, it would require to have a separate interface just to have any way to narrow the type. except such GuildInteraction would be identical in properties to Interaction merely changing one or two property types. Having it would also mean that you would have to have every other interaction class extending Interaction in version that extends GuildInteraction

#

it works like that on channel types because TextChannel extends GuildChannel, adding things related to text channels that are not found on other types of GuildChannels. Similarly, GuildChannel extends Channel adding things that are not on the base Channel. Or even ButtonInteraction adding button related stuff not valid to be placed on the base Interaction class.

rough glacier
#

It's just a fancier way to identify guild interactions than checking guildId. Nothing more really..

#

If it seems like a typeguard because of its name, perhaps we could rename it to fromGuild or something

copper laurel
#

If it doesn't typeguard the guild properties it's useless

ornate topaz
#

Is there a reason why Options.cacheWithLimits({}) takes Record<string, number> instead of being keyed with manager names?

#

Right now i can search up for the PR that adds it since it shows an example, or open v13 changes showing exactly same examples, but to my knowledge there isn't a list of managers to pick from

#

maybe apart from opening the source code since managers are in their own directory

zenith oracle
#

They're just the names of all class that extends another class called like CachedManager (totally unsure about this name). They're really a lot but yeah, a type that includes all of these name would be great

ornate topaz
#

yeah, but if i am a new user, i have to click (or at least scroll) through every single class on the sidebar to see all managers. "and extend? what is that supposed to mean, how is extending relevant"

zenith oracle
copper laurel
#

I don't see why we couldn't make a typedef/constants like ClientEvents

ornate topaz
# zenith oracle <:Thonk:654873234809946132>

I personally, know where to look for managers, or how to tell which managers can be manipulated with the option.
But i was talking from perspective of a new user somewhere down the line. They might not have any idea what they can put there at all, and when they look at the guide (assuming it will land somewhere else than just v12 -> v13 page which might not even apply to them if they start with v13 directly) they might see only those 2 managers shown on the examples. then they can either assume only those 2 can have options, or that they can pass more stuff.
But what other stuff. Every manager? then they need to scroll through every single class on the docs sidebar, since there is no single list of all managers, like for a resolvable or flag list. And while doing so, they will be completely clueless that not every manager extends CachedManager or even have 0 clue what that means

#

having a typedef for it not only makes it appear in the docs on a single page, but also can be made to appear in your IDE as you're typing the option

real jetty
#

I'm sorry I'm pushing a little bit for the inGuild function but I'm convinced that we can improve the typing by making a real typeGuard
I searched for several hours to find a way to make a real typeGuard that can be set up very easily. Be careful it's typescript but I think that transcribing it into javascript should be no problem:

type PropertiesNotNull<T, TProp> = { [P in keyof T]: P extends TProp ? NonNullable<T[P]> : T[P] };
function propertiesAreDefined<T, TKey extends keyof T>(object: T, ...properties: TKey[]): object is PropertiesNotNull<T, TKey> {
    return properties.every( property => object[property] != null);
}
function wasSentInGuild(interaction: Interaction): interaction is PropertiesNotNull<Interaction, 'guild' | 'guildId'> {
    return propertiesAreDefined(interaction, 'guild', 'guildId');
}```
unknown.png
#

||Do not pay attention to the names of the functions they were written in the night XD||

real jetty
strange igloo
#

Shouldn't this just assert whether or not Interaction#guildId is not null?
Let's say we do not have the GUILDS intent for some reasons, Interaction#guild will be null no matter if it came from a guild or not.

ornate topaz
#

that is what the check does right now, along also checking for member

real jetty
#

I shouldn't work so late at night because beyond that interactions can be sent even if the bot is not in the guild...
Maybe this system can be improved but actually right now it would lead to an error...

wild flax
#

No runtime checks apart from inGuild should be needed which is already in place

#

all that is missing are the proper types for it in the index.d.ts

vagrant holly
wild flax
#

There’s a pr for that

vagrant holly
#

Ah sweet

#

Hm, I'm getting a load of PermissionOverwrites#type being an integer on master babyyoda_sad_devcord

wild flax
#

Huh

vagrant holly
#

Should be a string, role or member, but getting a lot of errors thrown that its 0, which is what the API returns directly. Will dig through d.js source in a bit to find where its not being converted

wild flax
#

Uh I can’t link because of mobile

#

But maybe the permission manager PR is a hint

#

Since that changed a bit in that regard

#

Could double check the files it modified

#

Permission Overwrite Manager sorry

vagrant holly
#

nod that would be my suspect as well given that was in my version bump where it broke

zenith oracle
# ornate topaz *I* personally, know where to look for managers, or how to tell which managers c...

I think the same... It's a really good idea to add these cache options, but effectively not really everyone knows all structures (eligibles for this cache limit) or know where to find.

The other day I saw that a property in my clientOptipns (regarding the max count of messages in the cache) was removed and I found this new property called "makeCache", which accepts a function. Sincerely, I totally didn't find anything about what's this and the function it accepts what should do so I tried to check the docs, but they were really generic: Function to create a cache. (-1 or Infinity for unlimited - don't do this without message sweeping, otherwise memory usage will climb indefinitely), which includes a part from the old description (which is not really useful now) and nothing that refers to Options... Nothing neither in the reference of CacheFactory, that actually only confused me more... I found Options.cacheWithLimits for a pure case, because a user asked support about it in #djs-help-v14... So I tried to use it, but saw just a record of string and numbers, so I was totally confused. After checking the pr where it was added I found more informations about it and I went to check all of these managers.

Now, I don't think that everyone can do this, and types and docs are there for a reason, so the first thing I think can be done is to improve the description of this option (replacing the old text with a word about Options) and then adding to types all of the structures that can be used in the Options.cacheWithLimits. Probably, it's an hard work 😂, but is necessary for all people who have no idea about these options and their potential, so, I totally agree with you and I can help with docs or types for this but lemme know What y'all think about this...

I wrote too much, don't read everything because it's useless probably 😂

Screenshot_20210710-192636_Chrome_Beta.png
strange igloo
#

That's some niche feature anyway, I don't think begineers would use it (I hope so at least, scared about what I could see in #djs-help-v14 )
Also except for Message and Presence managers I'm not sure where is the practical use case of restricting guild manager cache (for example)

#

Even if some typedef about accepted inputs should definitely be a thing, of course

zenith oracle
#

I'm limiting the user and members managers iirc

#

To avoid having a lot of users in the cache

#

And Message obviously

vagrant holly
#

Was mentioning this in #archive-offtopic but thought I would surface here -- strings seem to be the worst offender for mem usage (a shock to no one), but it seems d.js currently prefers to store a lot of stuff as strings internally (most enums, etc.). I'm wondering what folks would think of switching things like this to store as numbers, and have getters to retain returning strings when accessed (or just moving to numbers completely and having the user resolve the enum)?