#archive-library-discussion

25085 messages · Page 9 of 26

vivid field

Oh that's intentional I think, it's going to be released with the next version

wintry karma

They exist on discord-api-types main so probably.
But I personally think that they shouldn't be ignored in the CI, like they are now, so they don't get missed.

copper laurel

Not on main, on next

I think

flint zephyr

Please add better documentation for FriendlyError and CommandFormatError in Commando 😦
I really want to use something that lets me customize the command format error but there is no clear documentation

Like add use cases and examples

wild flax

Commando is currently unmaintained

rough roost

why are snowflakes not just bigints?

cloud kraken

Because that's how the api sends them

wild flax

you can't serialize bigints

rough roost

that's unfortunate

ebon radish

How does Discord.js achieve seamless ESM-CommonJS interop?

vivid field
ebon radish
oak quail

pretty much everyone uses commonjs though?

ebon radish

but why? I guess that's my real question. why have we not fully embraced ESM?

we meaning the node.js ecosystem

wild flax

Because requiring cjs inside of mjs is problematic

Or rather the other way around

So our lib couldnt be used outside of mjs anymore

Which is rather problematic

We would have to supply cjs builds

And lets just assume we have a big typescript userbase, the standard tsconfig.json tsc generates does not set the compilation target to ESNext by default either

So we would get daily questions about that 100%

Thats not to mention all glitch/replit users

ebon radish

is it problematic for reasons other than that?

oak quail

node esm doesn't support json

wild flax

No, but I would argue this is a big problem indeed

ebon radish

other than users not knowing how to switch, I mean

oak quail

which is why I've tried esm multiple times and have had to go back to cjs

vivid field

Yeah, some things are still experimental

ebon radish

hmm can't you read the json the normal way

vivid field

JSON modules are stage 3 but node doesnt have them yet

wild flax

Its a lot of work honestly that is not very clear

ebon radish
ebon radish
copper laurel

I still really dont see any reason to just drop support for something that is still in such widespread use

ebon radish

thanks for your answers, my questions were not rhetorical

oak quail

since discord deprecated Message.stickers and is switching to Message.sticker_items (presumably to not break .stickers), should d.js keep using Message#stickers (with the .sticker_items data) or use Message#stickerItems or similar? (sticker_items contains partial sticker objects)

(personally I like Message#stickers and think Message#stickerItems sounds dumb)

copper laurel

Agreed, we can still make breaking changes to Message#stickers so go for it

oak quail

vlad says that if it knows that the sticker is in a guild, it should try to fetch with the guild route (which will only work if the bot is in the guild) to try to get sticker.user
I say that it should just run Client#fetchSticker, and people can use Sticker#fetchUser (or GuildStickerManager when that's added) if they want the user

copper laurel

Is it possible for the bot to have/receive a sticker without being in the guild

oak quail

yes

in messages

or by fetching with the non-guild route (which works for all existing stickers)

itll probably not be in the sticker's guild most of the time

and I think Sticker#fetch should work in that situation

copper laurel

what message would it receive if its not in the guild? or are they like emojis and can be used globally?

oak quail

latter, if the user has nitro

copper laurel

right

oak quail

u can already use them if u have android alpha

copper laurel

yeah while it might be nice to be able to have it know where to fetch from I think that would introduce inconsistency in the expected behaviour

Since people will need to call fetchUser anyway?

oak quail

well if the bot is in the guild and it did the check, you could just do (await sticker.fetch()).user and it would work

but the sticker.fetch() would error if the bot isnt in the server

copper laurel

but it should fallback to the other fetch right?

oak quail

vlad seemed to want it to let it just throw the DiscordAPIError

but ig we could fallback

outer raven

Is there a difference between using the @readonly tag or making a type Readonly<> in the docs?

zenith oracle

I think only with the tag @readonly this can be showed

copper laurel

Readonly is a typescript type, not something jsdocs would understand

vernal atlas

Readonly<T> means that all the properties in T are read-only, but the object can still be 'overwritten' to put it simply

surreal geode
wild flax

No

Why?

Also non-standard headers

surreal geode

I thought for type safety/intellisense. What additional headers are used other than these three?

wild flax

anything can be a header

as long as the server on the other end handles/implements it

surreal geode

Makes sense, thanks

outer raven
vivid field

As sugden pointed out, @readonly means that the property on the class can't be overwritten, Readonly<T> means that the returned object itself is frozen

So they're not the same

outer raven

and how can we tell those two apart in the code?

vivid field

Tell apart where exactly?

outer raven

Which ones should have @readonly, Readonly<T> or both

vivid field

Readonly<T> when you use Object.freeze() or a similar method
@readonly usually when you have a getter without a setter

outer raven

Alright. I just found this odd because Readonly seems to be the only thing the docs don't link

is it worth linking this to ts docs maybe?

flint zephyr
sharp wigeon

Why don’t I get the proper intellisense for ColorResolvable


type ColorResolvable =
    | 'DEFAULT'
    | 'WHITE'
    | 'AQUA'
    | 'GREEN'
    | 'BLUE'
    | 'YELLOW'
    | 'PURPLE'
    | 'LUMINOUS_VIVID_PINK'
    | 'FUCHSIA'
    | 'GOLD'
    | 'ORANGE'
    | 'RED'
    | 'GREY'
    | 'DARKER_GREY'
    | 'NAVY'
    | 'DARK_AQUA'
    | 'DARK_GREEN'
    | 'DARK_BLUE'
    | 'DARK_PURPLE'
    | 'DARK_VIVID_PINK'
    | 'DARK_GOLD'
    | 'DARK_ORANGE'
    | 'DARK_RED'
    | 'DARK_GREY'
    | 'LIGHT_GREY'
    | 'DARK_NAVY'
    | 'BLURPLE'
    | 'GREYPLE'
    | 'DARK_BUT_NOT_BLACK'
    | 'NOT_QUITE_BLACK'
    | 'RANDOM'
    | [number, number, number]
    | number
    | string;
``` it doesn’t show these color names with intellisense, maybe it’s just a problem on my end
fallow crater

had a similar issue before, I think it's bc of the | string. Not sure tho

solemn oyster

It is

sharp wigeon

Yeah that’s what I was thinking too

solemn oyster

Fixing that rn

loud jayBOT

pr_open #5950 in discordjs/discord.js by kyranet opened 19 seconds ago (review required)
types(ColorResolvable): change string to #${string}
📥 npm i kyranet/discord.js#typings/strict-type-string-color

solemn oyster

@sharp wigeon @fallow crater ^

sharp wigeon

thank you

solemn oyster

np meowthumbsup

vernal atlas
outer raven

Why can’t we just link it like a promise or an object?

vernal atlas

rafShrug i guess so

outer raven

although im not sure because most people don't use typescript (mostly beginners) so it might confuse them

and readonly is pretty self-explanatory by itself

so ill let that be for now

glass cypress

there should be a way to view the old design of the website

I find it easier to navigate

wild flax

Won’t be a thing

oak quail

for threads

Error [GUILD_CHANNEL_RESOLVE]: Could not resolve channel to a guild channel.
at GuildMember.permissionsIn
i sent this to ckohen over a month ago, looks like its still unfixed in master?

copper laurel

Assuming this happens when you pass a ThreadChannel?

wild flax

Yeah this needs a bit more info

ruby terrace
outer raven

Why was it decided to create the thread channel types with _ instead of the camel casing that is used basically all across the library?

Talking about public_thread, private_thread and news_thread which could be publicThread, privateThread and newsThread

the only places I see _ being used are in full caps names that usually come from Discord's docs

solemn oyster

cc: @ruby terrace ^

outer raven

Also it would be relatively easy to switch to a date-based version number for dev releases. Can I open a PR to change this so that dependabot starts working again and people stop getting confused when they install an old version with npm up? @wild flax

this is my proposal (YYYYmmddHHMM)

solemn oyster

No

The hash is handy because it prevents duplicated releases (aka releasing the same commit twice)

And it also helps identifying the commit in GitHub as well

outer raven
solemn oyster

It's extra steps, and also refer to the first point

outer raven

Ye I know but that doesn’t happen very often since a PR is usually merged every x hours

And even if it does it’ll be once and doesn’t hurt that much

It’s just that I’ve seen way too many people in #djs-help-v14 saying they have the latest version when they don’t because they used npm up

vivid field

That could also be fixed by deprecating outdated dev versions automatically

wild flax

Which they are

vivid field

Oh they are now?

I thought you just did it manually once

outer raven

Are they?

wild flax

I still do but it should only have 1 not deprecated

vivid field

At the moment yes

solemn oyster
outer raven

Well yeah but that doesn’t seem to fix the npm up issue unfortunately

Nor dependabot

vivid field

It did work for me in a test environment Nvm, installing an outdated version and then running npm update won't update to the latest commit

outer raven

ah ok cuz I was testing and it wasn't working for me

is it really that big of an issue if a "duplicated" version is released? Imo the pros outweigh the cons here

Its also easier for the user to know when the version they have is from

tacit crypt
outer raven

They have to go on GitHub and find the commit

solemn oyster

No, just paste the hash

outer raven

Way more complicated than looking at a number and getting the day from it

solemn oyster

e.g. https://github.com/discordjs/discord.js/commit/5ad83a6a65e5944ceb3a41fee2df40ba1f5b03e4

outer raven

Yeah it’s still more complicated than looking at a date

solemn oyster

Looking at a date is unreliable because GH shows local time while GHA uses another when releasing

obtuse hazel

There can be several commits on the same date so it won't be unique too

outer raven

Only 2 unless it’s manually ran

And the date is the least relevant, the day is enough and accurate enough as well

solemn oyster

We do multiple releases a day sometimes, tho

outer raven

The CI runs twice and Crawl sometimes runs it manually. I’ve never seen more than 3 releases in a day

wild flax

make it 7 SHA and a unix timestamp and I accept it

solemn oyster

But that doesn't offer de-duplication, Crawl

wild flax

13.0.0-dev.7_SHA.UNIX

outer raven

Fine by me

wild flax

thats fine, don't care enough, as long as this whining stops

solemn oyster

And not like the UNIX timestamp is readable at all

I could find the date a commit's from faster than getting the full date from the timestamp

outer raven

Wait yeah why not a date

wild flax

because dates suck

honestly

solemn oyster

I prefer keeping the system as-is, maybe reduce sha to 7, but that's all

wild flax

Dot seperation is nice

- separation is not

outer raven

But you can have both

Or none

My proposal was YYYYmmddHHDD but you can add dots in between

wild flax

yeah but if you do 20200628 you might as well do a unix timestamp

its not for the user anyway

outer raven

Why not?

wild flax

its to fix npm

I really don't want to discuss this, its a temporary thing anyway until we have proper pre-releases

outer raven

Will there be dev releases after v13?

wild flax

I'm not going to heavily invest into a system thats gone in less than 6 months

outer raven

Possibly for v14

outer raven
wild flax

I just don't like it

As much as you don't like the current system

outer raven

Fine then, I’ll try to find a way to get the unix timestamp and PR

So this is good right

wild flax

sure

outer raven

aight, will open rn

wild flax

Actually, now that I think about it, --short might be better

but it doesn't matter

outer raven

with the 8 chars?

sure

wild flax

it won't be staying "alive" for too long

its mainly to guard against collisions

outer raven

so should I change it

wild flax

but we arent a repo like the unix kernel lol

nah just keep things as is

outer raven

aight

wild flax

ah, vaporox brings up a good point

so I guess you might as well commit his change

outer raven

what does --verify do?

wild flax
outer raven

alright, will commit then

real jetty

wouldn't it be a good idea to create some guild webhook manager in master, just like how <Guild>.fetchBans() and BanInfo were removed and replaced with a GuildBanManager in v13?
maybe same for invites, integrations, templates, etc?

wild flax

We don't put managers everywhere on purpose

Only if it adds a real benefit

wild flax

A new dev version has been released with the changes.

outer raven

hmm npm up doesn't seem to be picking up on it but it doesn't downgrade either, lemme try dependabot

alright can confirm dependabot is working again, thank you!

remote wasp
ruby terrace
ruby terrace

only docs for startMessage need to be changed

outer raven

there are not many places that I'm aware of where lower_casing is used

ruby terrace

We're fighting two different library conventions, parameters are typically camelCased, whereas Constants are UPPER_CASED. I think the lowercaseing of channel types may have been an interesting (unsuccessful) attempt at ensuring camelCase for the properties. Not opposed to changing it, its just a very large whole library refactor

ornate topaz

Wait, where are we using lower_case instead of camelCase?

outer raven

more specifically news_thread, public_thread and private_thread

ruby terrace

channel types get .toLowerCased so I used that for all _thread types everywhere

outer raven

Yeah I don't blame you and I know it's a large change but so was changing to camelCasing in v10 or whatever it was so, for the sake of consistency, I think we should go for it

ruby terrace

just so you know, I'm going to do something about this, don't know what yet though, should have a PR within a day though

lone vector
ruby terrace

Looks like it got removed accidentally in the last rebase

lone vector

Seems like the changes in that PR weren't applied to that function too, unless that function wouldn't need them, I could potentially PR a fix though it may be easier for someone who already has an environment set up

copper laurel

I just fucked up a rebase there and deleted an extra line. Add the type back

That function doesnt need the changes

lone vector

Alright, will do

tender field
solemn oyster

Not really, because we're exporting ButtonBuilder, EmbedBuilder, MessageBuilder, and a few more, in later PRs to /builders

Which we'd like to expose separately (and outside Formatters, as they're different things)

An alternative, though, is to namespace them in /builders

But that might be bothersome if you use it directly

remote wasp

I was testing thread creation to decide how to document the params and believe me I had to bring out my notebook to write down all the possible ways in which startMessage and type param can be combined. I would share it but it looks horrible. This mess can lead to a lot of potential issues in the support. The solution I want to propose is to have two seperate methods for thread creation just like there are two seperate endpoints for them. For eg: ThreadManager#create and ThreadManager#createFromMessage or something along that line. This will remove all the issues that arise due to possible combinations of startMessage and type param.

ruby terrace

It could work, I highly anticipate people to never start a thread from a message using that function, instead using Message#startThread(). That paremeter exists so that can work.

all the possible ways in which startMessage and type param can be combined.
What do you mean by this though? They can't?

remote wasp

Yes correct, they don't as passing one specific param will do nothing in a particular case as it acts like a noop because something else will get passed no matter what you pass as value for that param. Which is where the issue is. The behaviour is weird because that params will matter sometimes and won't sometimes. We are mixing two endpoints into a single method and hence why this behaviour. I know I'm not making much sense here so I'll make a table to clear what I'm trying to say.

remote wasp

https://i.imgur.com/J4mfNr5.png
As you can see in the table the type param only matters when startMessage isn't provided. This means that type affects nothing in the other three cases. As you pointed out Message#startThread doesn't even talk about type because it takes the type from the message there and that's why it is much more clean. I was thinking about doing the same for manager. A method that creates threads from message and another that creates using type (just like the endpoints). This will seperate these two params into two different methods where their presence/absence does affect the result.

ruby terrace

The news _ + type column is wrong, its always news thread

remote wasp

right, my bad

ruby terrace

you see the problem here? type really only matters if provided on a text channels thread manager, and technically news_thread is an invalid parameter.
If you create a new function, createWithMessaage for example (cause I think it would be the less used one, called internally mostly) then you still have much of the same issue with create, always creates news in a news channel, but can have errors etc in text

Technically speaking doing this could make it much more type safe for ts users though since news doesn't extend text in that case. You just can't separate it in the actual code

With that, I say do it, adding something like

export class ThreadManager<AllowedThreadType> ... {
  public create(options: { ... type?: AllowedThreadType };
}
export class NewsChannel {
  public threads: ThreadManager<'news_thread' | 10>;
}
export class TextChannel {
  public threads: ThreadManager<'public_thread' | 'private_thread' | 11 | 12>;
}

to make it very type safe

it'll be hard to do for docs, but for ts users? worth it

remote wasp

So, just to be clear you want to:
Restrict values for type depending on type of channel the ThreadManager is on ✅
Want to divide the thread creation on manager into two methods redcross
Is that correct?

wild flax

I’m against 2 methods too

It kinda of goes against libraries design

We tried so hard going in a direction where we have a nice single method style

Throwing that overboard seems like a bad thing here

remote wasp

All right then restricting the values for type is what we are going to do

ruby terrace

By all means, give a shot to making the docs more clear too, I definitely understand it being confusing but I couldn't think of a better way to document it, the warnings being my best attempt.

remote wasp

meowthumbsup

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:

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

wild flax

yes

loud jayBOT

pr_open #5980 in discordjs/discord.js by DTrombett opened 54 seconds ago (review required)
types(ThreadChannel): make locked and archived param optional
📥 npm i DTrombett/discord.js#locked-patch

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)

🤷‍♂️

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

Thonkang

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

BlobThumbsUp

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.

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

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

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?

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

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 😐

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

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?

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

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

OMEGALUL

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?

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...

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)

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

OMEGALUL

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?

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

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?

neat bolt
outer raven

pushed

broken abyss
neat bolt
broken abyss
neat bolt

a generic doesnt work there as i said earlier

yeptune

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

Thonk

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

theCutest

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

meguFace

outer raven

meguFace

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

ReimuHammer

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

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?

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