archive-library-discussion

[outer raven]

Which part

[ruby terrace]

the upstream one

[river linden]

Thanks all. Shame its a year later and still no update from discord

[outer raven]

Is it intentional that the EnumResolvers class has no jsdoc? It would be nice to have it show up on the docs

[solemn oyster]

Possibly not

Everything should ideally have jsdocs

[raven vigil]

Why errors thrown by functions are not documented? i.e. MessageManager.delete(id) throws a DiscordAPIError if no message with that id is found, but how am I supposed to know as it is not mentioned in the documentation for that function?

I'm trying to look on the documentation for the official Discord API, but there are no mentions about errors that can occur there

So it is just trial and error?

[zenith oracle]

That error can be actually thrown by every function that calls the API so I think it's useless to add a warning in every function that says "Hey, this function may throw a DiscordAPIError as it calls the API"

[wild flax]

Pretty much all promises can reject and you should handle that

Which I would categorize as a general understanding when it comes to using JavaScript

[raven vigil]

I am coming from Java, so that was not obvious, thank you

[dawn merlin]

Oh yeah I forgot you have to handle all possible exceptions in java

[raven vigil]

Yes, it is good practice to declare all possible exception in method signature so that the compiler won't let you not handle it

[outer raven]

The functions on DJSError.js all have JSDocs but they're not inside a class so they're not displayed on the docs. Should they be removed?

[copper laurel]

No? Surely if you want to solve something here the better option would be if they are displayed. Even then at the very least having documented source code is better than undocumented

[analog oyster]

the docs could have another section for standalone functions

[outer raven]

the only way to do that would be to make DJSError a class (could do, lmk if that's a good idea), otherwise the docs don't support lonely functions atm

[copper laurel]

I don't know what the best/right solution is but I'm against removing the jsdoc just because it isn't parsed - documented source code is still good

Intellisense probably still picks it up

[outer raven]

meh there doesn't seem to be an easy way to document this so I'll leave it be

CI has been failing on every commit now, I keep having to force push for it to run again and work, did something break?

[spice thicket]

Why do we manually have to call .toString() on numbers for message embed fields

[visual hornet]

input validation, you can also put it in a template string

[spice thicket]

But I feel like if u passa number it should automatically parce it to a string

[dawn merlin]

You should always be explicit with your types. If it’s a number and a method expects a string you should explicitly convert it. Implicit conversion of function arguments is just asking for unexpected behaviors and takes control away from the user

[idle cypress]

Congrats on passing the 6000 commits on main branch

[void rivet]

https://discord.com/developers/docs/resources/channel#delete-message

This endpoint supports the X-Audit-Log-Reason header.
curious as to why
https://discord.js.org/#/docs/discord.js/main/class/Message?scrollTo=delete doesn't have a reason parameter

[slate nacelle]

Doesn't appear in audit logs, at least didn't the last time I chcked.

So I'd say that box is wrong. (Or the behavior changed)

[void rivet]

okay then sure, i'll test again to make sure

[slate nacelle]

do it

[opaque vessel]

I'm pretty sure bots deleting a message doesn't show up in audit logs lol

[visual hornet]

a few other actions are labelled as supporting the reason but don't show it as well iirc

[void rivet]

yeah i couldn't get it on the audit log
thanks

[outer raven]

That can be true for any other type but numbers could be allowed since those will never have unexpected behavior

[wild flax]

Won't happen

[wild flax]

Doesn’t belong here

That’s what’s wrong

[velvet egret]

mb

[strange crane]

When shards are spawning and you try to perform an operation (broadcastEval or fetchClientValues), the manager throws the error SHARDING_IN_PROCESS
In my opinion, the ShardingManager should either perform the eval on the shards that are spawned and ready or we should have an event that emits when all shards are spawned are ready so users can perform the operations without any errors
Just my thoughts 🤷‍♂️

[warped crater]

perform the eval on the shards that are spawned and ready
👎 this is just outright unpredictable behavior - it isn't guaranteed to be the same on every start up

an error is the correct behavior even if an event were to be added for this usecase

[strange crane]

Ah true

[outer raven]

Why?

[dawn merlin]

https://kentcdodds.com/blog/inversion-of-control

[outer raven]

I see your point. I mean we’ve had full string conversion and now we have no conversion at all. I think a balance between those two could be to support numbers and strings since those are the only two types that can be safely converted to strings. In this case it would just be an if statement since all the others have the possibility of breaking

[warped crater]

not a fan

the argument of "cannot have unexpected behavior" doesn't really bring much when the idea is that implicit conversions shouldn't be made period

[outer raven]

But a number converted into a string will always be that exact same number

No matter what

[zenith oracle]

If a string is expected you cannot pass a number. If an object should be rejected so a number should be too... If you really want to pass just a number then it's quite easy to stringify it

[outer raven]

I know it is but there’s a difference between displaying [object Object] and 5

[dawn merlin]

So let’s assume this is added, someone comes up and says “hey how come the method accepts numbers but not bigints, I mean if it supports numbers it might as well!” Ok we add support for bigints since numbers are in fact supported. Then someone else comes and says “hey your library accepts numbers bigints and strings, why not accept a Boolean as well, it’s a primitive too!” Ok makes sense we add support for booleans to be passed in. Finally someone comes and says “you support all primitive types in this method except objects, why not just add that in for completeness” we support for it completeness. Now we’ve ended up with a function that’s a black box in terms of input and has to do five different checks for its input. TLDR; You need to stick to an parameter type and keep it.

[outer raven]

Well it makes sense for numbers and bigints because, when stringified, they keep their exact value, which does not happen with objects like I said since no one wants to display [object Object] just like no one wants to display true or false without any context, even though those would be stringified cleanly too

[dawn merlin]

So why can’t people just do .toString() then? Why does it need to be done for them? Furthermore why is it our responsibility?

[visual hornet]

just like no one wants to display true or false without any context, even though those would be stringified cleanly too
field values for yes/no states

[outer raven]

It would result in cleaner code on the user’s end because what you put in the field is what gets put in the embed on Discord, no messing around

[outer raven]

That’s not user-friendly

If you want yes or no, you pass yes or no

[real jetty]

As someone who doesn't see much of this from internals, this just sounds like the usual "I want djs to friendly guessthings for me" which djs has moved away from in the last year or two

[dawn merlin]

Yeah, if someone wants a string pass a string

[real jetty]

the goal for djs has been to give specific things to get specific outs

[outer raven]

What do you have to guess when converting a number to a string?

[visual hornet]

but it's not an [object Object], so an argument could be made that it's useful. not saying it is, but someone could say that with a similar argument to the "number to string is the same thing" argument

[outer raven]

There’s literally no risk with that

[outer raven]

It’s not the same thing. A number is a number and can be displayed, true and false are technical terms that should not be displayed without context to the end user

[visual hornet]

name: "Is this statement true or false"
value: true | false

yes it's arbitrary, but still, that argument could be made

[wild flax]

Not doing String(…) or x.toString() or using template literals does not make your code cleaner

What is “clean” even at this point

[outer raven]

to me it's code as straightforward as possible, without unneeded visible modifications, so imo that would make it slightly less clean, although it's def not the end of the world

[wild flax]

That makes no sense sorry

Again, it won’t happen

[wild flax]

And if we go into detail here, “1” is not the same as 1

[outer raven]

when you put it in a field in discord it will look exactly the same

because the quotes are removed

[dawn merlin]

But it’s always a string in the embed that’s the point

[wild flax]

It doesn’t change that this would be one of the only places in the lib where we start to transform and coerce user input

Just because someone thinks it’s “clean”, whatever that means

Please don’t impose some weird concepts that don’t exist

Read some actual language concept books

[outer raven]

I'm not imposing anything, it's a simple suggestion being discussed on a discussion channel. After all, isn't that what this is for? I told you what looks clean to me, that's all. It may make no difference for you, and I'm not extremely bothered by it either, just a nice QOL thing, just like string types were. But thanks, you're super nice as usual

[wild flax]

Idk, I don’t care mostly

It’s like we say reasons why and all that comes back is “but X and Y and Z”

And then we say some more reasons and it’s “but A and B and C” lol

No offense but we already answered the “why” I’m unsure why we need to explain further and further and looks like it’s not something that gets accepted as a reason, while we should accept whatever comes in as feedback lol

[outer raven]

to every argument there's a counter-argument, and that's how discussions work. I understand you may not want that for X Y and Z, but some people might and in most of these topics we're just discussing QOL, which has been getting reduced over time in djs but I don't wanna get into that again

[wild flax]

For every QoL change so far we’ve introduced some alternative

That is almost as equal

Minus the Embeds

We even ended up keeping the permission stuff

[copper laurel]

The clean argument is really dumb either way

If it makes your code "cleaner" by not having to include .toString(), then it has made discord.js's code "dirtier" because we do have to include toString()

Its not like the conversion doesn't still need to happen, the so-called "dirty" code has just moved

Which means it isn't actually clean or dirty at all, its a necessary conversion step. And as per the inversion of control principles, that step belongs in your code

[outer raven]

Did the image URL methods change to the rest syntax?

so format is now extension

[dawn merlin]

think so

[outer raven]

and now we can't pass camelCase properties to the builder constructors

could we make classes extending the builder classes that convert camelCase to snake_case and send it over?

along with other stuff that might be needed

[dawn merlin]

you could

[wild flax]

It seems like a really odd pattern to even pass a manually created object to the embed constructor

Why get an instance if you already have the object?

[dawn merlin]

I think it's mainly used for serialization when djs receives api data

[outer raven]

I construct my embeds like this because I feel like it's unnecessary to construct an empty embed and call subsequent functions on it when I can simply save some performance and do it all at once

[wild flax]

🤔

[outer raven]

since, with the setAuthor and setFooter deprecations, the functions no longer have any advantage when compared to the raw object, so might as well just pass it like that

[wild flax]

At this point why even use a builder

[outer raven]

because of the functions

[wild flax]

It’s just taking up memory

But you don’t use them like you said lol

[outer raven]

I don't use them on declaration

I use them later on in the code

when actually modifying a value

I find that to be cleaner than an assignment

and it also allows me to do this (different builder, same issue)

[dawn merlin]

I had this conversation with rodry already 🙃

[outer raven]

yes we did

[wild flax]

Ah, I guess we just need a toSnakeCase function then

And we should be fine

[outer raven]

how about the types?

[wild flax]

Generics I guess

[outer raven]

cuz I did try that, but some types were a mess because of unions

like the button type is a union of a bunch of other types and you can't simply replace a property

[wild flax]

I mean allowing both will mess up your expectations instead of types

What takes priority

Snake or camel? And why?

Arbitrarily decided?

[outer raven]

just like it was already happening with djs embeds, it's the type that comes first until you add a property exclusive to one of the types

then it starts expecting all properties to be either snake or camel cased

[wild flax]

So what exactly wouldn’t work with a toSnakeCase function

[outer raven]

the function works, but button types are a union of link and customId button interfaces, so we have to re-type them all

I tried to avoid that with a type generic that replaced props but it didn't quite work due to that

and also ended up finding this issue which was another pain for the same reason https://github.com/discordjs/discord.js/issues/7342

at least to me, if anyone knows how to fix that please be my guest

but I tried with Omit and it didn't work

[copper laurel]

I dont see why the types need to be changed. If there are utility functions to convert casing, the constructors can stay as they are and you just convert your object to suit the accepted input

new Embed(toSnakeCase(rawCamelCaseEmbedObject))

[outer raven]

I was thinking of a class in djs that would extend builders and run that there

and either way if djs or builders provides the function, you need the types for that function

[copper laurel]

As in MessageEmbed.toSnakeCase() with types for that output?

A standalone utility function would have generic types

[solemn oyster]

Going to be fun when we'll have to pass iconUrl because it snake-cases to icon_url, since iconURL would snake-case to icon_u_r_l, which of course leads to inconsistencies since we use URL (avatarURL, displayAvatarURL...)

[copper laurel]

Though I do find that amusing wouldn't it be relatively simple to ensure the capital letter is followed by a lowercase one before inserting an underscore?

In the case of consecutive capitals, underscore then lowercase until an already lowercase is following

Of course that'll probably break some other parsing instead

[dawn merlin]

The issue is that you’d have to make exceptions for anything that’s considered an acronym.

[copper laurel]

Sort of?

If you target capitals specifically yes, if you target changes in casing its sort of manageable

I havent done any technical feasibility of course lol

Just a rough unfinished thought

[solemn oyster]

Well, I'm surely not writing the type hell for that

[copper laurel]

yes

fair

[solemn oyster]

We used a package for that, and the type hell it had to strict type the conversions was already enough bad as it was

[copper laurel]

customURLFormat has changes at mU and again at Fo

And at those spots specifically, an underscore is required before the capital letter

Then toLowerCase the lot

I know that phrase isnt in djs I just wanted one where it was in the middle

[vernal rose]

I just found a bug in MessageEmbed#length in djs v13 but it's not present in Embed#length in builder so should i create a pr to fix it in MessageEmbed as people will still use v13 after v14 release?

[dawn merlin]

what's the bug?

[vernal rose]

If use

Embed.setFooter(null)

It sets this.footer={}

So here footer is defined but footer.text isn't so when using MessageEmbed#length it checks

this.footer?.text.length here text is undefined so it'll throw error

In builder it sets footer to undefined so it's not throwing err

[dawn merlin]

ok so it's fine now?

[vernal rose]

In v14 it'll be fine but it'll throw error in v13

[dawn merlin]

Oh I mean we don’t usually take prs to stable releases soo it’ll probably have to stay

[copper laurel]

v14 will be a lot less breaking than previous majors

[vernal rose]

Yeah that's why asking

[copper laurel]

Probably not necessary, people will be able to upgrade easily

I don't think there will be much need to maintain a 13.6.1 alongside it, but there's no harm in opening a simple PR like that I guess

[vernal rose]

Yeah ig ppl rarely uses MessageEmbed#length

[vernal rose]

Ok I'll do it then

[outer raven]

So this should be Flags, right? https://github.com/discordjs/discord.js/blob/main/packages/discord.js/typings/index.d.ts#L1269

[dawn merlin]

yeah

[outer raven]

aight that's a 1 character PR

[outer raven]

Shouldn't the first one work? How are we supposed to add intents now?

[dawn merlin]

GatewayIntentBits.Guilds

[outer raven]

but that should work too, right?

[dawn merlin]

doesn't look like it'll work, but we'll probably make flags private anyways for the TS rewrite so you should use the enum directly

[outer raven]

why was it kept then?

could just be removed

[dawn merlin]

because the classes use it internally via inheritance

[outer raven]

aight ig

then maybe deprecate it?

[dawn merlin]

I'll just mark it as private in the typings

or can you just append that to your PR?

[outer raven]

oh yeah sure

then maybe it should be marked as private in the docs too?

[dawn merlin]

yeah

also I figured out why it doesn't work, I have to use typeof Enum not just Enum

go ahead an add that in as well, since it doesn't hurt

[outer raven]

will do but I don't think private works

[dawn merlin]

you have to make the base class prop private

[outer raven]

I did

ill add typeof for now and keep it public, I dunno how to fix that error

[dawn merlin]

i see the issue, mark them as protected

[outer raven]

right

ye that fixes it, thanks

[outer raven]

@wild flax do you know if @protected tags render correctly on the docs (with the private toggle too) or should we use @private instead? re: https://github.com/discordjs/discord.js/pull/7363#discussion_r794959391

[wild flax]

Won’t render

Use private

[slate nacelle]

Why make it private?

[wild flax]

Discussion above

[slate nacelle]

I don't see any, just some assumption that I have no idea where it came from.

[outer raven]

@slate nacelle I just made it private because of this, and also because people can use the enums directly

[slate nacelle]

That's the assumption I meant.

How are people supposed to know which flags belong to this bitfield now?
Read the source?

[outer raven]

the names are pretty similar but I see your point yeah

should I revert?

[slate nacelle]

I don't know, I seem to be the only one with any concerns here.

[outer raven]

that is a valid concern tho, I didn't think of that

[outer raven]

maybe @dawn merlin

[dawn merlin]

My reasoning for wanting them private was that BitFields used to serve both the purpose of an enum and a BitField class. Now that we’re using -types it no longer needs to act as an enum. The enum values only need to be used internally now to make sure certain methods work. As for knowing what enums to use, I think it’s relatively intuitive to find the analogous -types enum, and if you’re using typescript the enum types are defined via generics.

[outer raven]

I'm honestly fine with both approaches, although I'm more tempted to make them public again, but I'll go with whatever the maintainers prefer

[solemn oyster]

I'm ok with making them public so they can use it over importing a package they most likely did not install

[outer raven]

they don't have to import from -types, they're re-exported from djs

[sweet galleon]

Does anyone know the release cycle of discord-api-types & discord.js - waiting for a merged bug fix to be released

[dawn merlin]

which bug fix was it?

[sweet galleon]

when I say bug I mean change not necessarily bug but:
discord-api-types: https://github.com/discordjs/discord-api-types/pull/306
discord.js - https://github.com/discordjs/discord.js/issues/7270#event-5949161400

[dawn merlin]

well since it's not a bug you'll have to wait for whenever v14 comes out, but if you're just using -types they're probably be a release sooner

but no schedule

[outer raven]

Why is the ApplicationCommandPermissionsManager#set method called set and not edit? The endpoints it calls are either edit or batch edit so I feel like that would make more sense?

[wild flax]

Is it a PUT

because then set is accurate

[analog oyster]

yes it is

[wild flax]

There we go then

[outer raven]

so if we call batch edit and only provide some command ids, do the commands that get left out get their permissions reset?

[wild flax]

Same with commands yes

[graceful ravine]

More of a technical question rather than a discussion, but when is manager cache actually used internally? Every event sends along all the user data necessary to construct a djs structure (right?) and from what I've seen in the codebase I think it just overwrites the cache when that happens. (Might be wrong there). So when is it being used, apart from explicitly getting a value from a manager?

[real jetty]

Every event sends along all the user data necessary to construct a djs structure (right?)

No. If this was true partials would be irrelevant, off the top of my head messageDelete doesn’t send the full message, message edit doesn’t send the before message, some things can be missing from guildmemberleave, and there’s more im definitely forgetting

Oh yeah also guilds, discord doesn’t send that again after startup (obviously except for guildCreate)

[graceful ravine]

Yeah forgot about partials, however I'm not currently listening to any events that send partials, in that case cache would be minimally used right? (apart from guilds like you said) Asking mostly because we currently have a sponsored vps with minimal memory, so as long as most of the events send along the necessary data to create a djs structure we can make a relatively small limited cache.

[solemn oyster]

That's a dangerous proposal, Darkerink

[wild flax]

If I can forward the death threats to you

¯_(ツ)_/¯

[copper laurel]

The cache is used way more often than you might think

[copper laurel]

The most common examples I can think of are Interaction.channel / Message.channel - both of those are getters that retrieve the channel from the cache

Manager#fetch will check cache first

[void rivet]

i'm super late to this but okay what about this now? the issue was that the library sets default values which are already api default values

[wild flax]

Yeah sure

[void rivet]

well i haven't kept up with any of the changes
i'll go look for some now

limit_defaulted_to_max
https://discord.js.org/#/docs/discord.js/main/typedef/FetchReactionUsersOptions's limit

redundant_default_values

https://discord.js.org/#/docs/discord.js/main/class/GuildManager?scrollTo=fetch's limit
https://discord.js.org/#/docs/discord.js/main/typedef/GuildListMembersOptions's limit
https://discord.js.org/#/docs/discord.js/main/typedef/GuildSearchMembersOptions's limit
https://discord.js.org/#/docs/discord.js/main/typedef/GuildPruneMembersOptions's days and count

https://discord.com/developers/docs/resources/channel#create-channel-invite
https://discord.js.org/#/docs/discord.js/main/typedef/CreateInviteOptions's maxAge maxUses unique and temporary
https://discord.js.org/#/docs/discord.js/main/typedef/ChannelLogsQueryOptions's limit
https://discord.js.org/#/docs/discord.js/main/typedef/ReplyOptions's failIfNotExists

[vast fiber]

I've had a message that's edited every 10 seconds to update an embed in it for a couple years now I think, and recently the embed started to not update after a while, even though the message itself gets "edited". any insight on this?

[analog oyster]

#djs-help-v14

[wild flax]

There’s a rate limit now for messages older than 1h

It might be 3/15 I think

So 3 edits every 15 seconds max

[void rivet]

these are all i could find

[wild flax]

Feel free to open a PR

Someone will likely chime in if there’s more and tell you

[void rivet]

also curious as to why MessageManager.fetch fetches 50 messages and not 100 like some other methods like <Client>.guilds.fetch() and <MessageReaction>.users.fetch()
I was under the impression they'd fetch all they could by default

[opaque vessel]

If you don't supply a message limit, the API defaults to 50 for messages

[void rivet]

I am aware
but as i've mentioned, in cases like <MessageReaction>.users.fetch() the limit is by default 100, which is the max
whereas the api default is 25 https://discord.com/developers/docs/resources/channel#get-reactions

[opaque vessel]

I am aware
Then... that's why

I don't think we should be setting default values if the API has a default value though

Imo if we set default values when the API has defaults already, we should remove them and leave that to the API

I don't think we should get the max just cause

[void rivet]

I don't think we should be setting default values if the API has a default value though
that's what this whole thing is
a list for the ones that set the default value even when the api does it

[opaque vessel]

Yea they should all change

All of them should go poof

[void rivet]

that is the plan
but just to confirm what you're saying, even the methods like <MessageReaction>.users.fetch() shouldn't by default fetch with the limit set to max?

[opaque vessel]

Yea, I would say leave that to Discord

If a user wants max, they can specify it, it's no big deal

[void rivet]

trying to find more default values i came across something elsejs const newData = await this.client.rest.patch(Routes.channel(this.id), { body: { ... lock_permissions: data.lockPermissions, rate_limit_per_user: data.rateLimitPerUser, default_auto_archive_duration: data.defaultAutoArchiveDuration, permission_overwrites, }, how come https://discord.com/developers/docs/resources/channel#modify-channel-json-params-guild-channel doesn't mention a lock_permissions field?
has it been removed?

[opaque vessel]

Not looked into it but changing a channel's position has a lock_permissions JSON parameter to sync with the new parent. Maybe it works but just isn't documented?

[void rivet]

these are all i could find, surprisingly, the only method where the limit by default was fetching the max (and wasn't already the api default) was the one listed, some existing methods like <Guild>.fetchAuditLogs never set a default value
https://discord.js.org/#/docs/discord.js/main/class/GuildManager?scrollTo=fetch some of the methods just lists the api default on the documentation even though it's not hardcoded

[foggy blaze]

Hey, can anyone fill me up with the future of bot prefixs? I have a bot that only runs on my server, should I be worried and rewrite my bot in order to support slash commands? Or is this something that noone knows until time comes (april)?

[real jetty]

⚠️If your Bot is operating in 100 servers or less, this change doesn't impact you.
https://support.discord.com/hc/en-us/articles/4410940809111-Message-Content-Intent-Review-Policy

[foggy blaze]

Thanks for the info and link 🙂

[copper laurel]

Slash Commands are generally better anyway, and you should probably be rewriting to make benefit of the ongoing new features that will be released for them

[ornate topaz]

Wait but, doesn't the reaction.users.fetch fetch all users who reacted?

Cc @opaque vessel

[foggy blaze]

I agree, but I’m still not really in fond with having to press tab everytime i fill in a parameter, felt like I could execute command way faster using bot prefixes.. or am i missing something?

[copper laurel]

The command being correct?

Sure typing messages might be slightly quicker for power users, but having an integrated UI, tab completion of options, choices, autocomplete suggestions, user/channel/role resolution handled by Discord

All far more powerful

[foggy blaze]

Yeah i agree, any idea if slash command scale and run faster than bot prefixes? Or is it the same.

[copper laurel]

I wouldnt expect there to be any difference though newer versions of discord.js have becoming increasingly lighter in terms of memory footprint and performance

[foggy blaze]

Alright, ill take that into consideration. Cheers for the info!

[opaque vessel]

Iirc no, it fetches the first 100? Then if you want to fetch another 100, you gotta use the after parameter to get the next batch and so on

[ornate topaz]

well, then description for it seems to be incorrect?

since it implies similar behavior to passing no parameters to other fetch() methods, that do end up fetching everything, more so as the options are optional

[opaque vessel]

Oh yeah, that's incorrect

[raven juniper]

I could have sworn I made a PR that clarified that

[analog oyster]

why does the -types UserFlags have a None = 0? I know its listed in https://discord.com/developers/docs/resources/user#user-object-user-flags but i fail to understand why its there too
its causing this https://github.com/discordjs/discord.js/issues/7374

[outer raven]

yeah I think it's safe to assume that 0 is no flags, that doesn't need to be in -types

[tacit crypt]

...Thats an interesting find

And yeah makes no sense for it to be in the enum

although it doesn't hurt to have

[analog oyster]

it does hurt because otherwise that will have to be accounted for in discord.js

[outer raven]

yeah bc it gets put in the permissions array

I can PR

oh I guess not

[alpine bough]

sorry for interrupt the discussion, but if https://github.com/discord/discord-api-docs/pull/4405 get merged, will it be necessary change the default parameter in OverwriteData? as well in RawOverwriteData and PartialOverwriteData

or it has nothing to do? lol

[slate nacelle]

I think our intention is to not document api defaults (although I think that slipped in on some occurences), but our own defaults.

[alpine bough]

alright, thanks :)

[outer raven]

uhm @wild flax i was about to push something to that PR that jiralite mentioned

oops

[wild flax]

if you still want changes, make sure to put it in draft mode then

and they were most likely unrelated anyway

[outer raven]

sorry it was a small thing, I was gonna get it done when I got home

they weren't actually

[vernal rose]

what about adding them in optionalDependencies in package.json?

atleast butterutil and utf-8-validate as they don't need visual studio installed

[copper laurel]

That still gets installed automatically I believe

[wild flax]

It does

[outer raven]

why do errors look like this now?

[opaque vessel]

Source maps or something

[analog oyster]

that's what happens when code is minified. [variable] names are also minified
source maps have nothing to do with this

[outer raven]

I didn't change my code tho, so something changed on the lib

[analog oyster]

yes, /rest is being minified

[outer raven]

why tho

why can't we have DiscordAPIError anymore lol

[analog oyster]

https://github.com/discordjs/discord.js/blob/0803665183f62e5aeb3b708c1bf9cad598cb2087/packages/rest/tsup.config.ts#L8 ¯_(ツ)_/¯

[outer raven]

Can we remove that?

turning minify off does fix it

minify saves a total of 15kb, from 34kb with minify off to 19kb with minify on. I mean it is a reduction but if these errors are supposed to be shown I think we should have better names and 15kb never made a difference anyway

managed to get the same size while keeping the names, the only thing that keeps weird names is the error stack, is this preferred or can I PR with a slightly bigger file size that keeps all function and class names

[rough roost]

It makes no sense for djs to care about size, it should be turned off completely

[outer raven]

I mean there must be a reason why they added minify

[zenith oracle]

Tsup supports a preserveNames options, which is also used in /collection

Maybe it should be used here too?

[outer raven]

@zenith oracle thats what this is

I don't think that's enough

because if we wanna trace back an error, the source code is an absolute pain to look at

[analog oyster]

i dont understand why theres a sudden urge to minify stuff 🤔

[zenith oracle]

I mean, there's literally a stack trace with a .ts file, what's the problem?

[analog oyster]

at C.runRequest
is the problem, i guess

[ruby terrace]

Tbh, if you want to look at the source code for that, you probably should already be familiar with the lib and know exactly where those functions are. Unless you are In the unlikely scenario it’s a lib issue, that part of the stack trace is practically irrelevant.

[outer raven]

Being familiar with the lib doesn’t make it any less difficult to read the source code with that option

[ruby terrace]

Not in every library, for /rest it does though, there aren’t that many functions. I can read that error just fine

[tacit crypt]

this is getting fixed by keeping the names of the classes

[ruby terrace]

Fwiw I don’t care about minification, I think some people will though

[tacit crypt]

also i'm 90% sure the sourcemaps fix the errors in your console so you can go on github and see it

[ruby terrace]

Yeah that’s kinda the point of sourcemaps

[wild flax]

Just enable source maps in node

js errors will always be a pain to look at when the code base is in ts

It won’t really match up either way

But if you use source maps the problem is easily solved

[outer raven]

How can you read it if it gets compressed to 8 lines?

[dawn merlin]

Just enable source maps in node

[outer raven]

also I do use source-map-support and I still get this

[outer raven]

I have it enabled

otherwise that error wouldn't be pointing to a ts file

[dawn merlin]

all I can say is that it should work, otherwise everyone creating ts libraries wouldn't be able to see the source when they debug

[outer raven]

when I click the linked file and line yes it does, but it still says C.runRequest 🤔

[solemn oyster]

No, just use https://nodejs.org/dist/latest-v17.x/docs/api/cli.html#--enable-source-maps

node --enable-source-maps . or similarly set the NODE_OPTIONS env to --enable-source-maps

[outer raven]

do I have to include that in the start command every time I start my bot

ill try the env

[solemn oyster]

If you use Docker, you can also add ENV NODE_OPTIONS="--enable-source-maps" before your CMD and you're all set, although I'd argue this solution is less useful for most

[outer raven]

alright thanks! Does that mean I don't need source-map-support?

[solemn oyster]

That's right ^^

I have been using that flag myself for almost as long as it's been out (of experimental) in v15, works pretty well

[outer raven]

alright, thank you

[solemn oyster]

@outer raven people are whining about the performance impact of validation, if we add extra properties, it'd cause an even bigger performance impact.

Also, we'd need to hide that property somehow because it'd get serialised if it's left enumerable

[outer raven]

1 single property is less performant?

didnt know that then sorry

the name unsafe sounds scary tho lol

[solemn oyster]

It's meant to be scary

[dawn merlin]

I'm open to names that are synonyms to unsafe

[solemn oyster]

Rust uses unsafe to give you raw power and performance, while it also discourages developers from using it

[copper laurel]

If you want it less scary but accurate you could use Unvalidated

But I think unsafe is fine

[golden mortar]

or it could have the normal naming and validated components could be prefixed with safe

either way good job siris 😄

[copper laurel]

That would encourage people to use the unsafe ones by default, probably not what's intended

[golden mortar]

that's what i'm suggesting more or less

[copper laurel]

While I personally would probably use the unsafe ones (or not builders at all tbh) they're intentionally presented as validation being normal, unsafe being the alternative. I don't believe anyone wants to flip that

[woeful rain]

Is there any possibility for the PR https://github.com/discordjs/discord.js/pull/7200 to be merged into discord.js stable, so still up to v13 version? A lot of people would surely appreciate it, since it's coming out any minute now and some people don't want to wait a year for news (djs major version comes out mostly every year) and don't want to use the unstable dev version either.

[vernal rose]

(djs major version comes out mostly every year)

v14 is coming v soon

[vernal rose]

If I use inGuild() in a partial message it makes it Message<true> but acc to src it doesn't checks for cached message it just checks for in guildId. So ig this typeguerd is wrong. Is it's intentional or a mistake?

[ornate topaz]

Yeah, because the name if it is misleading

It's called "cached" but it is more of a "inGuild"

[icy bronze]

Hey, would be nice to have a djs-rest channel for futur help and suggestions

I was wondering if it would be possible to add a "default format" option for the CDN, I don't like the webp one and having the format option in every cdn function of my code is meh. (I know format was renamed but I forgot the new name)

[wild flax]

Not a huge fan of global settings

[icy bronze]

well it could be added to the rest options ?

[neon flower]

While debug turned on my bot stopped sending heartbeats after 15 - 20 mins and went offline while the process is still online. Anyone know what can be a reason for that

[wild flax]

Then it’s global on rest

Doesn’t change my view

[icy bronze]

Ill do it on my fork then but im having issues compiling the rest package, can I share it here to get some help?

[wild flax]

Unsure in what issues youd run

but feel free

[icy bronze]

will do when I get home ty

[scenic meadow]

is it possible to keep the packages updated whenever i start the bot?
like for example something like in package.json

"dependencies": {
  "packageName": "latest"
}

[analog oyster]

read the channel topic

[quiet viper]

Thats a bad way handling this. On breaking changes, you can recieve type errors and unexpected behaviour.

You can do it with the npm start script.
Btw wrong channel

[icy bronze]

here is my issue, seems like it get the tsup config right but the tsconfig isn't being read in the correct folder

[scenic meadow]

Thanks, btw where is the right channel for this question?

[quiet viper]

#resources since its a general question

[wild flax]

How are you building this?

When installing or something?

Guess youll have to also change this command here

And supply -p with the correct location of the tsconfig

[icy bronze]

yeah, I do npm run build on postinstall

[wild flax]

then youll have to do what I said

otherwise this wont work

[icy bronze]

aight lemme try

[icy bronze]

i'm having difficulties using the -p flag, it always return an error with Cannot read file '/home/koyamie/koya.gg/binteractions/node_modules/tsconfig.json'.

i've even tried to specify the full path from root

[wild flax]

How do you use it

[icy bronze]

I'm installing the package and with my postinstall script I do npm run build (which I changed to "build": "tsup && tsc --emitDeclarationOnly --incremental --project tsconfig.json",)

but the project option doesn't work, I have tested changing path or providing the full path it always return this path: node_modules/tsconfig.json

[analog oyster]

because /rests tsconfig.json uses the root tsconfig.json in the monorepo

[icy bronze]

oh

make sense

damn yeah the extends props

thank you, sorry for the mess that was my issue

[dawn merlin]

If you want to do this create a higher order function for the cdn functions

So like my myprofile = applyOptions(rest.cdn.profile)

The you can reuse that function anywhere

[wild flax]

^

But

They prolly want djs to return it

[icy bronze]

I'm not sure to understand sorry

[outer raven]

So with the recent switch to /builders, embeds and components no longer get transformed from snake_case to camelCase when receiving them through the API and this is the only place in the library where this seems to happen. Can we make a class on discord.js that extends /builders and implements that? I'd be willing to open a PR for this with the types as well

[void rivet]

on builders
<Embed>.setFields
takes a rest parameter

    /**
     * Sets the embed's fields (max 25).
     * @param fields The fields to set
     */
    public setFields(...fields: APIEmbedField[]) {
        this.spliceFields(0, this.fields.length, ...fields);
        return this;
    }

while for some reason <ActionRow>.setComponents takes an array

    /**
     * Sets the components in this action row
     * @param components The components to set this row to
     */
    public setComponents(components: T[]) {
        Reflect.set(this, 'components', [...components]);
        return this;
    }
``` shouldn't these be consistent?

[outer raven]

should be yeah

[void rivet]

setChoices is a weird one

public setChoices<Input extends [name: string, value: T][]>(
        choices: Input,
    ): Input extends []
``` it takes an array of arrays
instead of objects like setFields

[outer raven]

yeah that should probably be an object too

[void rivet]

okay but a rest parameter too, right?
we're trying to make these methods all have arrays or rest parameters?

[outer raven]

I think so yeah

you probably dont need the type generic either

[void rivet]

so what, just make a new type for this?
i don't know if APIApplicationCommandOptionChoice can be used since APIApplicationCommandOptionChoice["value"] is typed as string | number

[outer raven]

that is fine since it can be a string or number

[void rivet]

these methods are called on string/integer/number options specifically

[outer raven]

yeah which is why value can be a string or a number

[void rivet]

the point i was making was that why would SlashCommandStringOption.setChoices take a number as it's value?
other methods like addChoice lets you use the type based on what type of option it is
it uses the generic

[outer raven]

then follow that ig

[void rivet]

okay so..
make a pr which changes all these methods to now take rest parameters and make setChoices take objects to be consistent with setFields, sounds good?

[outer raven]

I don't see why not

[void rivet]

okay then, i'll make that shortly 🤠

[outer raven]

meanwhile let's bump this back

[real jetty]

Perhaps the ability to use color strings in Embed.setColor() could also be readded with this? 🤔

[rocky latch]

@wild flax anything regarding this? Would still love to see this get merged xDDD

[void rivet]

okay i am done
i have tested it and they seem fine, before i push, what do i name this object? i'm calling it ChoiceData for now

[outer raven]

yes!

[dawn merlin]

no that won't be added, if you want colors you'll need to use the Colors enum or a hex literal ie 0xffffff

[outer raven]

😕

was this related to the recent turbo update?

[solemn oyster]

The latest turbo update works fine to me on Windows 11, have you tried reinstalling node_modules?

[outer raven]

yeah did that and it seems to work now, weird

[void rivet]

@outer raven did you want me to actually get rid of the generic or will it stay?

[outer raven]

I found it to be redundant but if it solves an edge case in TS it can stay

[void rivet]

okay then

[dawn merlin]

you actually can remove the generic, we found out recently that the cases where setChoices couldn't be used are still show up in TS regardless of input

just have it return this as well

[void rivet]

wait what cases?

ahhh

    public setAutocomplete<U extends boolean>(
        autocomplete: U,
    ): U extends true
        ? Omit<this, 'addChoice' | 'addChoices'>
        : this & Pick<ApplicationCommandOptionWithChoicesAndAutocompleteMixin<T>, 'addChoice' | 'addChoices'> {
``` that's because `setChoices` was never used on Omit

[dawn merlin]

no, like for example your current code won't allow setAutocomplete after setChoices, but if you do something like setName after setChoices, it still allows setAutocomplete

[void rivet]

oh yeah that makes sense

[dawn merlin]

probably gonna PR those conditional types out since they aren't reliable anyways

[void rivet]

so it's a separate pr or do i do it here?

[dawn merlin]

you can do that one method if you want

[void rivet]

hmm, then i think it'd be better if it were a separate pr where both are removed at once

[dawn merlin]

ok

[void rivet]

thanks again both of you 🙂

also this is unrelated to this particular pr but i had a few questions

about the setEmoji methods, following up on this conversation now only taking an object instead of an EmojiResolvable, is that intended behavior?

should the documentation for the methods list the API defaults as it's own default value?
like https://discord.js.org/#/docs/discord.js/stable/typedef/FetchGuildsOptions this lists limit's default as 200, in the code it doesn't set a default value, it's actually the api default that's mentioned here

and aren't the builder methods like addFields, addComponents dropping support for arrays to be passed and are only supporting rest parameters to be passed?

[woeful rain]

Discord.js v15 will already be in TypeScript?

[unique axle]

we do not give version numbers to milestones that are on the roadmap

[copper laurel]

Can you clarify the benefit of this?

I'm not sure why this is necessary

Just for property consistency?

[copper laurel]

Everyone assumed v14 would be TypeScript rewrite too - TS rewrite will be whatever the next major version number happens to be when its ready to release

[outer raven]

Consistency with property names (everything in djs is camelCased except for stuff imported from builders)
Ability to use types and utility function from djs in these classes (e.g. Util.resolveColor)
Ability to pass an object to the constructor of these classes with its properties in camelCase, just like we could in v13

This would all be easier if we just didn’t use builders at all, but since that change was made we’re gonna need dirty fixes for these issues

[copper laurel]

I tend to agree, I personally think the way builders have replaced discord.js components is a mess. Property consistency would be nice, I'm assuming the child class constructor would just convert the incoming object to snake_case and pass to super()?

The color thing, couldnt care less either way tbh. Hex literal numbers exist

[dawn merlin]

I thought we were sticking to snake case json payloads, I closed my pr bc it was suggested that decamelization would be handled by the user

It would also allow us to use -types directly

[copper laurel]

This is where I get very confused. If:

• the API sends snake case
• the constructor accepts a snake payload
• and the builder properties are all snake case
• and the toJSON method on the builder produces snake case

When is there ever a camel case object to be decamelised?

[dawn merlin]

Anywhere snake case is accepted

[copper laurel]

But it isnt camel case in those situations

Where is the camelCase object coming from - are we suggesting the user is going to write it in the wrong case just to pass it through a decamelize function for the constructor?

Theres no reason to do that

Rodry is making the opposite point, and I agree with him, that builders are completely inconsistent with the rest of discord.js because there is never any camelCase involved

[woeful rain]

Well, it made me happy to rewrite discord.js to TS. Just one more thing. Are you planning to use minify the same way as with the other modules? It doesn't make any sense, at most it will increase the package size by a bit.

[copper laurel]

Dont know, minify wasn't my decision nor was I involved in any discussion around it, and I also think its dumb

[outer raven]

That and the properties on the class would be camelCased too

[outer raven]

Yeah but constant colors are nice, specially random and official discord colors like blurple

[copper laurel]

So... make it a constant hex literal?

Unless I'm missing something here

[outer raven]

Well I personally don’t know what hex blurple is and it changed recently so having just “BLURPLE” in the code is much more helpful as we can be sure we’re sticking to Discord’s branding

[ornate topaz]

https://discord.com/branding

youy can't get closer to source than this

[outer raven]

Also about this, since there is a toJSON method already we could simply store data in builders as camelCase and then convert it, since people have to run the method anyway to send it to the API

[copper laurel]

I disagree that embeds should accept strings to address that, but I'd agree with keeping the colors constant or enum, whatever that was

[outer raven]

Would you rather check that every time or literally just type the name of the color? Cuz I’d pick the second option

[ornate topaz]

i'd rather open it once and make myself a constants file

and reuse it

[copper laurel]

See this is the problem, /builders were originally designed to be for the /rest package. Having camelCase properties in /builders would have made absolutely no sense since in that ecosystem its a waste of time converting them

[ornate topaz]

making 0 difference in usage apart from where it comes from

[outer raven]

And here we go back to the good old “let’s remove every single utility from DJS since the users should be doing that themselves anyway”
It just doesn’t make sense. It’s a library, it’s meant to help

[copper laurel]

But now they're in discord.js too, where their casing doesnt align

[dawn merlin]

yes, that's suggestion because the alternative is performing a transformation regardless if it was snake case or camel cased input. There's no good way to tell them apart in a function so they'll both be transformed

[outer raven]

Every other method in djs does camelCase to snake_case convertion, why is that a problem in builders?

[dawn merlin]

not anymore

[copper laurel]

This is why

Or at least this is the reason it originally made no sense for them to be doing that conversion

[outer raven]

Well yeah but now that they’re in djs, it should follow the same rules

Originally it was fine because it was even mentioned that builders should not be used with djs

[outer raven]

Huh?

[dawn merlin]

which methods are you referring to?

[wild flax]

@copper laurel this is about a quite nice use case anyway

[outer raven]

Literally every method that calls the API

Every property you pass is in camelCase, every property in a structure is in camelCase

[wild flax]

It’s about supplying a camelcase object in the constructor of a builder, to then use the methods on it

Instead of constructing it solely via an object, or use the builder methods solely

[copper laurel]

Why would you write that object in camelCase knowing that the constructor doesn't accept camelCase

[dawn merlin]

but we're not supporting camelCase payload either right?

[wild flax]

*nice = niche

[outer raven]

By using a raw object you lose the utility functions, by using the class with subsequent methods you lose some performance, even when using the unsafe classes

[copper laurel]

I agree that's a niche, but also agree that the design of builders is inconsistent with the rest of discord.js

[outer raven]

Not incoming, because that’s not a real word scenario. Every private constructor accepts snake case, every public constructor should accept camel case

[copper laurel]

I think the monorepo and cross-package dependencies is overall a far worse experience for the user

[outer raven]

Yeah that’s for sure

[wild flax]

Not sure how the mono repo is related to that now

[copper laurel]

I think the docs link issue has been resolved

[wild flax]

I mean yes, it wasn’t meant to be a permanent issue

[copper laurel]

Installing individual dependencies from GitHub is still difficult. Is there a plan to have an @amber obsidian release for every package?

ahh damnit sorry

@dev

[wild flax]

Unlikely, maybe

[outer raven]

Monorepo has been sorta tied to all the changes to builders and all, but to me the only issue is the CI not reporting errors for subpackage dependencies, as it makes them go unnoticed

[wild flax]

Our package is pretty unique in people even wanting to install via GitHub

Like you never see that anywhere else really lol

[outer raven]

That’s only true when there aren’t prereleases

[ornate topaz]

also kinda sad that there was no mention at all about stuff moving to monorepo on separate repos

[outer raven]

[copper laurel]

:thonk: curious actually, does npm link work if you run it from the correct package subdirectory?

Thinking of forks

[wild flax]

No, not really

Lol

[outer raven]

It does, I’ve used it

[ornate topaz]

seems weird to also not mention the fact that it's a monorepo in the monorepo readme as well, but i suppose there is only a single dir that isn't dotdir now, so eh

[outer raven]

If people care about the development of a package they will. Heck some people even install node from their master branch and it appears on their release cycle graph

[wild flax]

No, really not. It rarely happens

You can die on that hill if you want, but 99% of TS written packages out there simply don’t support that use case at all

Neither do they dev releases or pre-releases

[outer raven]

TS itself does dev releases

[wild flax]

What has that to do with libraries

tsc is a compiler

[dawn merlin]

ts isn't a monorepo?

[wild flax]

You can clone it, build it yourself, and run the binary

We aren’t a binary

[outer raven]

Not the point anymore

[outer raven]

And also a dependency

[wild flax]

No

[outer raven]

There’s been prs on djs to fix bugs from ts pre-releases

[wild flax]

It’s a cli tool

It’s not a library

Need a rust tool to run on a dev version? Clone it, build it, run it

You can’t easily do that with libraries, that’s just a fact

Sure you could do the same approach and npm link it, but then you might as well just do that

It has nothing to do with dev or pre-releases

[dawn merlin]

even if ts wasn't a cli tool, they do specific manual beta releases, something we've never done

like it's feature complete, it just is getting tested by the community

[outer raven]

nom link only works locally, you can’t have your app run off of an npm link on a VPS

[outer raven]

Ye thats true, but it could be done

[dawn merlin]

? anything could be done, that's not much of a point

[outer raven]

We’ve kinda drifted away from the main point which was the /builders integration with djs and how that’s caused a hot mess of issues and inconsistencies

[wild flax]

So… you clone the repo on your vps

🤨

[outer raven]

Do you realize how complicated that is when we could just install stuff directly before?

[wild flax]

And you realize that we already do things that others wouldn’t even attempt to?

And just because our setup isn’t 100% ready yet, it’s like everyone acts like it is lol

I’m the only one that does organizational stuff here, esp repo setups and all

And I don’t care about v14 to be stable, so yes CI doesn’t “throw” errors, because it doesn’t need to

The packages “in development” work with each other.

v14 being the only one with constant dev releases, not working with not released changes of the other packages is a given here

[outer raven]

A good way to have people not complain about a product in development would be to have a public roadmap of features that you want to add to v14 before releasing, instead of just discussing that internally. People also wanna help so having that would help them pick a feature to work on and PR, and you could direct them to that list if they complain about something that is there

[wild flax]

The main complaint here is purely organizational + builders discrepancy

And no one can really help with dev releases because of npm access restricted to maintainers

[tacit crypt]

I swear there's so many topics going on, please thread them because I see concerns about the whole camelCase vs snake_case and now @dev releases and something else about them

[outer raven]

Everyone can always make a PR

[outer raven]

Yeah sorry, lemme make a thread

[wild flax]

How’s that help if it can’t even be properly tested and would potentially need a lot of follow up prs instead of just someone doing it that has actual control over it to actually fix it

[tacit crypt]

I didn't say you do it, but if that topic is planned to be discussed further pls thread it 🙏

[outer raven]

Builders discrepancies with DJS

[outer raven]

Someone that has control can review the PR and tell the person to add whatever they feel is missing. If that roadmap also has a bunch of goals that must be met for each feature that would make the review process faster too

[wild flax]

Yeah and that’s nonsense

There’s things that I don’t want to teach people and them doing it wrong or me having to correct them over and over, or worse them not responding for days

I might as well do it myself

[outer raven]

You have push access to every PR too

Most at least, I doubt anyone disables that

[tacit crypt]

That's.. irrelevant? By that point Crawl can just do it himself without needing a middleman for it

[outer raven]

I’ve never seen someone call a roadmap “nonsense”, that’s def one I wasn’t expecting lol

[wild flax]

I wasn’t responding to a roadmap at all

[tacit crypt]

Crawl never called the roadmap ^

[outer raven]

That’s assuming everyone is stupid and will make incomplete PRs, which isn’t true and there are many smart contributors that don’t have the shiny green role and I’m sure would love to work on such a thing

[wild flax]

But letting others do prs that are of organizational nature, or generally PRing things we rather handle ourselves

The first interaction PR set us back MONTHS

For having support for them

I’d rather not run into this again

[outer raven]

I’m not saying the community should be doing prs like the monorepo one, but minor features are harmless

[outer raven]

And since then I personally haven’t seen any situation like that

It was a bad experience, sure, but those are the quirks of open source

Most of the time people making PRs in djs know what they’re doing

[solemn oyster]

You're missing the experience factor, besides, I think I said this already, but we communicate a lot in internal channels, and those who aren't part of the proficient+ team just don't have access to those, so there isn't much of a coordination either

[wild flax]

🤨

[wild flax]

🤨

Sorry but

I’m not sure if you make it sound like you know better or… undermine however we are involved in OSS

[outer raven]

I know you do, and not having an official way for people to try to get the role also prevents a lot of good contributors from reaching that and being able to contribute in a more positive way.

[outer raven]

notice how I never said my name and I'm not even thinking about my case in particular. I could name a few people who have contributed to djs recently who are actual good contributors and would take a lot of advantage from a public roadmap. I haven't talked to any of them privately, but I believe they would

[solemn oyster]

It's also not like our roadmap is private or something, it's public, and I also did not want to imply that most/all communication is internal, in fact most of the time it just mirrors the discussions that take place here to get to a better conclusion or just give a better answer

[outer raven]

the only roadmap we have is made off of PRs that are already open, us outside collaborators have no idea what's planned to happen in the future before the release

[solemn oyster]

Then check issues

I don't know why are you looking for stuff to do while checking stuff that's basically done

I have a few issues open that have a lot of thumbs up but so far almost no feedback in them, even though they improve performance and memory usage *drastically*

[outer raven]

some of your issues have already been set back to v15

[solemn oyster]

You can still provide design ideas

[outer raven]

there was never an issue about converting everything to discord-api-types enums

[outer raven]

they're good as-is IMO, that's probably why they don't have replies

[quiet viper]

Thats often not a good idea.
New Features should be correctly designed, inorder to prevent future breaking changes, bc the code has to be refactored/changed

[outer raven]

New features should also not get merged until they’re done properly

[wild flax]

Says who

[outer raven]

Common sense idk

[copper laurel]

It's not like things get merged intentionally unfinished

[wild flax]

New features shouldn’t get released until done properly

They can get merged whenever

But then again, discord releases unfinished and not thought out stuff all the time

[solemn oyster]

@analog oyster don't we also need to define an external?

Like those: https://github.com/discordjs/discord.js/blob/main/packages/discord.js/src/client/Client.js#L537-L550

[analog oyster]

yeah, about that..

[solemn oyster]

... oh right

No website, I forgot lmao

[analog oyster]

[alpine bough]

there is many "errors" like this one in the code
should it be fixed or no need?

import/order eslint error

[analog oyster]

probably just a version mismatch between your editors eslint and the actual eslint version on the repo, because there are no such errors in the workflows / on my end

[alpine bough]

alright, thanks :)

[dawn merlin]

I believe you can fix this by setting your “eslint.runtime” to node in your settings.json

[vernal rose]

It's because of the node version mismatch. Vscode node version isn't >=16.x so it doesn't support node protocol thats why it'll show this error only in vscode and not when running eslint scripts

[void rivet]

regarding #7395, shouldn't we update the tests?

[opaque vessel]

Yes

[void rivet]

okay then, i missed that when making the pr 👍

[opaque vessel]

(:

[void rivet]

Done! All tests passed 🤠

[woeful rain]

https://github.com/discordjs/discord.js/blob/main/packages/discord.js/src/util/Util.js#L14

Why extends null?

[outer raven]

boop

[solemn oyster]

So it doesn't inherit methods, and also forbids constructing it, at least 'til ECMA gets their stuff sorted out:

https://github.com/tc39/ecma262/issues/1036
https://github.com/tc39/ecma262/pull/1321

[lofty beacon]

hey 👋 my pr i just made has CI failing due to tests that i havent touched. https://github.com/discordjs/discord.js/runs/5072673078?check_suite_focus=true
any ideas?

[real jetty]

Should be a known issue by Crawl

You can ignore, it was made by a recent failing commit

[lofty beacon]

Alright thanks

[opaque vessel]

This can be specifically useful when working with moving threads between 2 channels.
You can move threads between channels?

[outer raven]

And how would a channel url be useful in that scenario?

[nova kestrel]

any particular reason https://github.com/discordjs/discord.js/blob/main/packages/discord.js/src/managers/GuildManager.js#L262 isnt defensive about invalid options being passed. eg nullish values will try and get read, resulting in an unfriendly error

[opaque vessel]

Why are you passing it in the first place?

[nova kestrel]

im not, but someone i was just helping had an issue where null was being passed

makes it quite confusing to debug

had to read the source to figure it out

[opaque vessel]

Are you telling me one doesn't check their input values

That seems to be the bigger issue

[nova kestrel]

it obviously wasnt intentional

they were passing in a slash command ID arg but made a mistake with it, resulting in null unexpectedly

so is this just a case of "not my data not my problem, check your inputs"? or is it going to get changed to be useful

[opaque vessel]

What was the error in the first place? (That was not useful)

[nova kestrel]

TypeError: Cannot read properties of null (reading 'guild')
    at GuildManager.fetch (node_modules\discord.js\src\managers\GuildManager.js:280:66)
    at Object.module.exports.run {user code}
    at Object.module.exports.run {user code}
    at processTicksAndRejections (node:internal/process/task_queues:96:5)

[opaque vessel]

It should be clear that the third line there you're doing something wrong, I wouldn't say that is not clear

The problem is that, if we add checks for nullish behaviour, might as well add it across the whole library... and that just ends up bloating it

[nova kestrel]

its clear that something went wrong internally sure

[opaque vessel]

Incorrect, something went wrong with what the user did

That error is only happening because you passed something that was not documented nor typed. That's on you

[ornate topaz]

does that mean that we have to remove every other input check now?

[nova kestrel]

"nor typed" in.. javascript.. a dynamically typed language

consistency would sure be nice

[opaque vessel]

I feel like this conversation has happened before

[nova kestrel]

probably has

[ornate topaz]

wasn't it about checking stuff passed directly to API though?

and not stuff that library blindly tries to access?

[nova kestrel]

both are pretty annoying to deal with

dapi returns pretty nonsensical errors at times

in this specific case, if djs threw an error saying "Malformed options passed", that would go a LONG way to helping people debug things

luckily the lib failed quite early, but if this object had made it more internal it would have been a nightmare to debug

[opaque vessel]

Maybe I see things differently but that looks braindead easy to see what went wrong

[nova kestrel]

to you and i perhaps

[visual hornet]

isn't this the kind of thing typescript helps with

[nova kestrel]

but we both know djs is used by much less experienced developers

[nova kestrel]

bold to assume the majority of djs users know what TS is and / or its benefits

but yes, TS would have stopped this

[visual hornet]

when did i assume the majority know...?

[ornate topaz]

if TS would have stopped this, why does the lib need any input checks at all then

[nova kestrel]

the.. the lib isnt written in TS

and can be used by JS

JS obviously has no static typing

[visual hornet]

typescript prevents bugs, if someone doesn't want to debug they can use ts for that to prevent them, if they don't know how to debug then maybe they should learn that first

[nova kestrel]

"maybe people should learn JS before writing a bot", we all know this is not the case, and we can all hope however unfortunately a lot of new developers try djs because it looks easy

also the sort of people i see who know nothing about programming arent interested in using TS

[visual hornet]

are the changes you're proposing even going to benefit them if they know that little

[ornate topaz]

do the 23 instances of d.js throwing INVALID_TYPE on input do that?

[nova kestrel]

it would benefit the people they come complaining to with a vague error message

[nova kestrel]

do what

[ornate topaz]

benefit them

[nova kestrel]

i would say so

i see countless people posting about the intents error / the invalid token error

makes helping so much easier

also all the embed checks

[visual hornet]

even if there was another custom error message for input validation, it'd have to use the proper terminology to be beneficial to the developers that understand them, and then complete beginners would still say they were vague.

i see countless people posting about the intents error / the invalid token error
...which means it isn't helping then? it makes you help them easier, because you know of the error, or if you don't, how to find it
to more experienced people, more abstract errors can be more useful, like the error you mentioned

TypeError: Cannot read properties of null (reading 'guild')

with experience you can know where to look or what to look for to debug.

[nova kestrel]

they are helped, be that through their own research or through someone else

wrt the abstract error, i literally had to read the source to figure out what was going wrong since the person i was helping was obviously too new to debug it themselves

[ornate topaz]

https://github.com/discordjs/discord.js/blob/12ffa069aa8b247e945fef16a543f41c2c391bf1/packages/discord.js/src/util/LimitedCollection.js#L22-L23
this here even checks if you passed an object

[nova kestrel]

ok and? why not do this everywhere then?

make a util and use it

[visual hornet]

uh, i mean, if you want it why not you do it everywhere?
you can just make a pull request

[nova kestrel]

i can

[ornate topaz]

and it started with a question about it

[nova kestrel]

i dont know enough about djs internals or design, nor am in particularly interested. i was just interested to see if there was any motive for this design and / or plans to change it

and tbh i assumed it would get rejected so i didnt bother

[void rivet]

according to my tests
i had an account with the MANAGE_THREADS permission but not the send messages in threads permission
and the account couldn't unarchive archived threads
so i'd say this code is ok
but the way it wouldn't let me unarchive was weird

[void rivet]

yes and how is that not correct? you tested it as well

so what, you're suggesting this.archived && this.sendable && (this.locked ? this.manageable :true)

is it like this for you too when you try to unarchive a thread?

[visual hornet]

the ternary could be removed with this.archived && this.sendable && (!this.locked || this.managable)

[void rivet]

reading the dapi docs, even that's so unclear, but regardless i think you need send messages in threads to unarchive in general

[visual hornet]

although it still could be a UI-specific issue, api tests would probably be better

[loud jay]

#7406 in discordjs/discord.js by PlavorSeol opened <t:1644069908:R> (review required)
Always require being sendable to be unarchivable
📥 npm i PlavorSeol/discord.js#pull-request

[sullen idol]

isCategory() method? 👀

[opaque vessel]

What about it

[sullen idol]

I'm not seeing one, how do I request to add?

[opaque vessel]

I see it here
https://discord.js.org/#/docs/discord.js/main/class/Channel?scrollTo=isCategory

[sullen idol]

Weird, alr thnx

[opaque vessel]

In future, it would be better to ask your actual question first

[sullen idol]

Wdym?

[raven wind]

means isCategory() method? isn't a clear question and could mean anything, so ask directly next time

[brittle cosmos]

he forgot the a

[ornate topaz]

no, not quite. and this isn't really for this channel either

[jovial smelt]

how long djs cache the last message id when the last message deleted?

[outer raven]

Iirc it deletes messages from cache if they’re deleted

[jovial smelt]

nope, it keep save the last message id for a while, but idk how long is it

[outer raven]

That’s sent by discord, so it only changes when a message is sent

[visual hornet]

it's not cached, it's a property that discord sends
so i guess it stays "cached" as long as discord doesn't send data to overwrite it?

[jovial smelt]

cause when my bot deleted the old message embed that has been edited, and send a new message with same command and trying to edit that message, and wallaaa it crash cause the my bot edited the last message that has been deleted

so i wonder how long the cache is stored and deletd from the memory

[visual hornet]

that seems like it would be a logic issue, between using channel.lastMessage.edit/channel.messages.cache.first().edit/interaction.editReply

[analog oyster]

discord.js updates the lastMessageId when a new message is received

[jovial smelt]

hmmm

[unique axle]

#lastMessageId is not evicted
#lastMessage is a getter from #lastMessageId and directly depends on the cache
we evict messages from cache when we receive a websocket delete event

[visual hornet]

wait, "evicted" is used in this context?

[unique axle]

delete, remove, yeet, pummel, obliterate, nuke, call it what you want

[jovial smelt]

like this

when the new command has been run, the bot trying to edit this embed that has been deleted

[outer raven]

you should catch that then, not really a topic for this channel anyway

[jovial smelt]

yep it's not the right channel ma bad and sorry, but i tried to catch it and it keep got error, let's move to #archive-djs-help-v13 / #djs-help-v14

[vernal rose]

Djs uses null everywhere but in some places undefined will make sense. An example is CommandInteractionOptionResolver#getX, here if a string named name isn't present that means its undefined which doesn't exists. But null is a value which exists but its null. So in these places why null is used instead of undefined?

[inland lotus]

null means it doesnt exist

[vernal rose]

a null is an object. null means a variable is declared but no value is assigned. But in this case, the string isn't defined, it doesn't exist means undefined

[real jetty]

Using undefined in a lib can get confusing quickly, because js also regularly throws undefined if a method doesn't exist

[rustic bough]

Documentation suggestion for @vernal rose:
<:_:818272565419573308> null
The value null represents the intentional absence of any object value. It is one of JavaScript's primitive values and is treated as falsy for boolean operations.

[analog oyster]

read the first phrase

[drifting knot]

null shouldn't even be in the language, this does a good job summarizing why https://github.com/sindresorhus/meta/discussions/7

[wild flax]

Meh

[golden mortar]

that doesn't explain why at all

[copper laurel]

Yeah that's just an opinion which states a list of reasons but doesn't really justify any of them.

1. Developers being inconsistent is because they're bad developers.
2. Supporting both doesn't complicate input validation because they have distinct use cases, its a symptom of the above, bad developers.
3. "Newer JS features like default parameters only work with undefined" yes, because undefined is fit for purpose. Passing null should be intentional and not default. This is #2, again a symptom of #1.
4. "I strongly believe it was a mistake to have two have both null and undefined." - purely subjective
5. "Using null makes TypeScript types more verbose" - good. Verbosity is a good thing. Stop trying to make code shorter, and usually worse, for no good reason.

This whole things reads as an instance of #1, a developer who used them inconsistently, didn't understand what they were both for, and just wants to eliminate one

typeof null being object is the only remotely valid point but they proposed changing it once and it was a disaster

That's a problem with JavaScript being backwards compatible to the dawn of JavaScript

[solemn oyster]

Why is this in #archive-library-discussion, and why is such a 40 IQ discussion even brought up?

Also, good luck removing nicknames or timeouts without null in Discord

[strange igloo]

Yeah that's a little bit complicated to get rid of it
null and undefined don't have the same meaning and both are useful when used accordingly

And for people who don't like (I include myself in this) writing | null all the time they can simply disable the strictNullChecks rule in their TS config instead of blindly enabling all strict options

[vernal rose]

From where it was started and now 😂

[strange igloo]

undefined means "no idea" while null is a concrete lack of value
Which is likely why these methods return null when no data can be retrieved

From undefined you can't guess much, but from null you can determine that what you are looking for is definitely not where you asked

Which is probably why (I don't hold the truth) methods in option resolver return null when they can't return an actual value

[solemn oyster]

When setting values, undefined means no behaviour or unset, while null is a reset signal

[real jetty]

If #7023 (modals pr) is ready to be merged before v14 is ready to be released, would a release be held off until v14 or would you guys do another semver minor in v13?

[solemn oyster]

We'll have an answer to that in the upcoming hours, deciding internally what we'll do

[copper laurel]

That PR would never have been v13 compatible, I've just drafted one which hopefully will be

[raven wind]

https://imageing.eu/shared/Ta1zZn1.png

[gaunt lotus]

Oh my bad I didn't read the category's name

[woeful rain]

why discordjs@rest 0.3.0 is not supported? (latest tag)

[visual hornet]

a newer (dev) version of @discordjs/rest is being targeted now i guess?
isn't the latest tag automatic? that would be why latest points to a deprecated version if that were the case, i'd guess

[fiery ocean]

Came here to second this question. I can't even find the repo for it on github. Is this a new issue?

[visual hornet]

https://github.com/discordjs/discord.js/tree/main/packages/rest?

[fiery ocean]

Not... sure how I missed that, thanks

[void rivet]

asking again 😢
about the setEmoji methods, now only taking an object instead of an EmojiResolvable, is that intended behavior? last i heard someone said it may not be but didn't get any follow-up to that

also should the documentation for the methods list the API defaults as it's own default value?
like https://discord.js.org/#/docs/discord.js/stable/typedef/FetchGuildsOptions this lists limit's default as 200, in the code it doesn't set a default value, it's actually the api default that's mentioned here

[quiet viper]

Is there a event/way (yes, know the invalidRequest ones...), in order to get informed about 40X errors?
Having one for metrics would be great

[tacit crypt]

we have apiResponse events iirc that you can tap into

[copper laurel]

@woeful rain @visual hornet @fiery ocean There was an error in deprecating old versions of those packages. All @discordjs/<package> are still supported - the source code have all been moved to the monorepo

[wintry pendant]

I'm running into an issue while making my pr (https://github.com/discordjs/discord.js/pull/7445):

I have modified GuildAuditLogsResolvable and GuildAuditLogsFetchOptions to:

export type GuildAuditLogsResolvable = AuditLogEvent | null;

export interface GuildAuditLogsFetchOptions<T extends GuildAuditLogsResolvable> {
  /* ... */
  type?: T;
} 

The issue is that typescript still allows for T to extend any arbitrary number even if its not a value of AuditLogEvent. This causes the following test case to fail:

expectType<Promise<User | undefined>>(
  // @ts-expect-error Invalid audit log ID
  guild.fetchAuditLogs({ type: 2000 }).then(al => al.entries.first()?.target),
);

Am I doing something incorrectly / is there anyway to make sure T can only extend AuditLogEvent properly?

[woeful rain]

Okay 👍

[cerulean pumice]

when are modals expected to be available on here?

[opaque vessel]

#769862166131245066

[cerulean pumice]

👍

[outer raven]

hey crawl, just realized we can't actually commit like that, can you add it to the config?

[tacit crypt]

Add it in the message body not the title

[outer raven]

Crawl told me to put it in the title

[wintry pendant]

so typescript is just stupid: https://github.com/microsoft/TypeScript/issues/30629, so I guess I will just remove the test case

[tacit crypt]

Crawl told you you can also add it in the title

[outer raven]

Which it seems like I can’t, that’s why I asked him to add it or I can pr too

[dawn merlin]

@vernal rose the src folder is published in the package

[vernal rose]

ic but why like u r publishing the same thing twice right? inside dist src will be in js and again src in ts.

[dawn merlin]

Well compiled js isn’t the same as the source js

[vernal rose]

but when we'll import anything from djs it'll come from compiled one and not source one right? as main is dist/src

[dawn merlin]

D.ts files don’t get compiled

[vernal rose]

i'm not saying that. you said src will be published means all **/*.js and **/*.ts files inside of src will be published right? or i've misunderstood maybe?

[dawn merlin]

Yeah and the typings folder is published too

[vernal rose]

ok so i was saying that dist/src is what everyone will use and src will be published as a root folder but except for typings this src folder is useless means no one will be able to use it. So I think you can add all src/**/*.js files in npm ignore and src/**/*.ts will be published only for typings. Means these source js files don't have any use in npm only compiled will be used

Also, I just realized that if you don't include typings folder in tsconfig it still works as it's a d.ts file that won't go under compilation so you can remove it from include and it'll work fine.

Here's a working example
https://github.com/imranbarbhuiya/pagination.djs/blob/43f3ee804b91db639abd57bb82d3e331290b0d29/tsconfig.json#L19

[dawn merlin]

I think you can’t import from the src folder in the d.ts if it’s not included in the ts project

[vernal rose]

ok i didn't tested it xD lemme test it.

I'm not seeing any errors when importing in vsc and this file isn't included in the build so idk how can I test it

[vernal rose]

@dawn merlinok I was wrong I was confused with typing files position. This means what I was thinking as typings will be outside of dist and src is inside and when u said src will be published I thought ./src will be published along with dist/src. But now I realized typing will be present inside of dist and only dist/src will be published.

[strange crane]

When the Shard tries to send a heartbeat but it didn’t receive the last ack
The shard is suppose to reconnect but when looking at the code it only calls WebSocketShard#destroy and does nothing else (see here: https://github.com/discordjs/discord.js/blob/861f0e2134662ab64a11d313130aff58b413d495/packages/discord.js/src/client/websocket/WebSocketShard.js#L570)

Isn’t it suppose to call the identify method 🤔

[ruby terrace]

The webscoket code is pretty complex, if you look into it you'll see it does attempt a resume, its just not the way you think its supposed to happen

[outer raven]

Just a question, why is https://github.com/discordjs/discord.js/pull/7449 semver major? All options that had their defaults removed are still optional and will behave the same way if the user leaves the parameter out

[visual hornet]

well, the behavior would change if the defaults changed in the api, so i'd guess that could be why?

[unique axle]

you literally noted it as a breaking change in the commit notation with the !

[visual hornet]

hmm that was after the label was added though

[outer raven]

yeah ^^ I initially thought it'd be minor

[visual hornet]

well it's making the behavior rely on the api instead of the library, and whether the behavior changes at all would be out of djs' control, so yeah i'd say that could possibly be a breaking change

[outer raven]

aight

On this typedef, the default for failIfNotExists comes from the default client options, which can be misleading because a user might be expecting this to default to true even if they set the client option as false. Should it be removed from the typedef?

[tacit crypt]

If it defaults to the client, you could also just do failIfImLazy=this.client.options.failIfImLazy

[outer raven]

that's probably a better move

also nice name

[woeful rain]

Why close https://github.com/discordjs/discord.js/pull/7439 ?

[solemn oyster]

Internal discussion

Basically we want the TypeScript rewrite to be done from the ground up, instead of translating everything with the flaws and worrying about the rest later

And our docsgen parses either JavaScript or TypeScript, not both together

[lilac tulip]

https://github.com/discordjs/builders/issues/36 is this being considered? it's been open for quite a while now

[copper laurel]

I'm not against it, but I don't quite understand the difference between 10 "addSubcommand" function calls, or an array with 10 builders in it

[lilac tulip]

imo it would look "cleaner", but I guess that's something everyone has to define for themselves at some point. I'm sure there's people that would argue that it's not cleaner in any way

[void rivet]

the thing is to make it consistent with the other methods, you'd need to use rest parameter
which brings me to a bigger question which monbrey and i talked about already i think, what the usage of addSingular(obj) now? when they're basically just addPlurals(obj) with one parameter
example: addField(field) is just addFields(field)

[tacit crypt]

Pretty sure most singular adds are called by the plurals

[unique axle]

still pretty useless, if we decide to go with rest parameters

[void rivet]

yeah there are also some that call plurals internally
it's a mix in the implementation

but not the point, i'm asking about the user's side, is it really useful to keep the singulars?

[unique axle]

making that all one method seems more streamlined, if the usage is the exact same, but with one more character

[void rivet]

soo, should we remove them?

[outer raven]

Getting this error with the v13 modals PR installed but I can't seem to understand why it's happening, is it related to that PR?

[void rivet]

version mishap i guess

[outer raven]

the only version of discord-api-types I have is 0.26.1

Also this might be more insightful but I'm still clueless

[tacit crypt]

mismatched dapi versions

[copper laurel]

Yeah I didn't touch those afaik

[warped crater]

yeah that's what the errors look like when you miss match versions

means djs or whatever package is using a different version than the one you're explicitly using

[outer raven]

it's weird bc I don't have discord-api-types as an explicit dependency in my project

but alright

[tacit crypt]

If you npm ls discord-api-types what does it show

Also interesting, you have a deduped -types and then d.js has its own -types

[outer raven]

it was showing 0.26.1 and djs had 0.26.0. I just updated djs and its fine now

[tacit crypt]

Probably needed a reinstall of deps yeah

[outer raven]

probably got messed up from changing between monorepo and v13 branches

[manic cairn]

i'd make a PR for this, but it's feeling pretty crazy pills that nobody else has this problem:

• i try to create a guild emoji, using a web URL
• resolveImage checks it, see it's not a data:
  and passes it to resolveFile
• resolveFile performs a node-fetch res = await fetch
  then returns res.body
• the body, per node-fetch v2.x.x docs,
  conforms to node's Readable Stream spec.
  it identifies in Console Log as a PassThrough object
• so, what resolveFile has returned, is a stream. not a buffer.
• the results of resolveFile are fed into resolveBase64
• resolveBase64's only check is "is this a Buffer" (it is not)
• resolveBase64 gives up, returns the readable stream as-is
• that object is stuck, as-is, into the API create emoji request
• this fails, obviously

DiscordAPIError[50035]: Invalid Form Body
image[BINARY_TYPE_INVALID_DATA_URI]: Only data uri strings can be used.
[...]
requestBody: {
    files: undefined,
    json: { image: [PassThrough], name: 'MyEmojiName' }
  }

a minimal repro in a clean dir, under current node 16, takes a few lines and fails. but the code looks like this in stable too. it's an easy fix, but i am sure i am doing something wrong here

otherwise it would be broken for everyone right???

[visual hornet]

do you have said minimal repro?

[manic cairn]

$ node -v
v16.14.0
$ npm install discord.js@dev

const { Client } = require("discord.js");

const client = new Client({ intents: ["Guilds"] });

client.once("ready", async () => {
  await client.guilds
    .resolve("7777777777777777777777")
    .emojis.create(
      "https://www.bungie.net/common/destiny2_content/icons/eaa17a0ce79c7caa171ce1852ef27569.jpg",
      "myTestEmoji"
    );
});

[golden mortar]

is there something preventing https://github.com/discordjs/discord.js/pull/7200 from being merged? even the v13 backport has been, just wondering

[dawn merlin]

Honestly the author hasn’t been too active on maintaining the PR

CC @errant brook

[errant brook]

well ive been busy

also its been approved by so many people, what's holding it?

[dawn merlin]

All good sorry if that came off a bit blunt

[errant brook]

nah that's fine

ok I just saw the comments monbrey left, seems like he was asking for a second opinion

[outer raven]

nah he's right, you can never have an APIAttachment so you dont need the type

[zenith oracle]

https://github.com/discordjs/discord.js/blob/988a51b7641f8b33cc9387664605ddc02134859d/src/util/DataResolver.js#L69 is where the conversion happens. It calls resolveFileAsBuffer which create a Buffer from a Stream if it's not already a buffer. Not sure how you're encountering this error

[manic cairn]

https://github.com/discordjs/discord.js/blob/main/packages/discord.js/src/util/DataResolver.js

doesn't exist in main

[visual hornet]

im getting the same error as well as using the docs example and an emoji link; honestly the reason it hasn't been brought up much is probably just this isn't really utilized

[manic cairn]

i'm surprised, i assumed emoji from urls would be more of a newb's path of least resistance

thank you though @zenith oracle, that explains the difference between stable

i'll drop a pr RQ i guess

[errant brook]

ok ill do the changes monbrey requested then

wait what should cache type reducer be changed to then? collection?

[zenith oracle]

Ohh yeah sorry, I didn't realize it wasn't present in main. It seems a bug introduced in the Dev version so, probably that's why no one has reported it yet

[outer raven]

just plain collection and messageattachment

[errant brook]

Collection<Cached, MessageAttachment> ?

[outer raven]

no, Collection<Snowflake, MessageAttachment>

[errant brook]

ok makes sense

what exactly is cached tho

[dawn merlin]

Cached is the generic parameter on the interaction class

[analog oyster]

the pr has a lot of unrelated style changes on the typings file

and the attachment option interface is not correct

[clever crypt]

when passing stringified json as the body to REST#post, i get the (python) error Only dictionaries may be used in a DictType from Discord. i'd argue typing RequestData#body as unknown is somewhat misleading because how was i supposed to know you can't pass through a string

[outer raven]

yeah you should probably re-run prettier on that, that should fix it @errant brook

[errant brook]

is there a script for it?

[outer raven]

just format with prettier

but i think there is

[analog oyster]

it should just run when you commit

don't know how that even happened in the first place?

[outer raven]

yarn format

[outer raven]

typescript formatter does that

[errant brook]

no idea i tried fixing with yarn format

[errant brook]

i already did this

[analog oyster]

typescript formatter?

[outer raven]

yes

the default

[analog oyster]

whats that

[errant brook]

inbuilt vscode formatter

[outer raven]

its the default formatter that comes with vscode

[errant brook]

yarn format didnt fix anything

[outer raven]

is your branch up to date? That script runs prettier and eslint

[analog oyster]

given there are not a lot of changes you could also just get away with rolling back the file and adding the changes back

[hollow stirrup]

I heard that discord.js will soon go modular

[errant brook]

yeah

[hollow stirrup]

At what major semver exactly?

[dawn merlin]

V14 I assume

[errant brook]

guys i need help with git

[hollow stirrup]

But that is monorepo, does mono repo means modular?

Or I missed something

[errant brook]

if you set it up to be modular then yes?

[dawn merlin]

Essentially yeah many large parts of djs have been split up

[hollow stirrup]

Hmm

I see

[outer raven]

can someone PR an update to discord.js's version of discord-api-types please? I don't really know how to update the deps on the monorepo so I'm scared I might mess it up

[wild flax]

It is updated

It’s on 0.27

[vernal rose]

You've all the dependencies installed? It should format the file when committing or it won't let u commit. But if you don't install them it'll allow u to commit without running git hook

[errant brook]

it does run the hooks and stuff

but it didn't fix the formatting

[vernal rose]

If your code is updated with main branch then default formatter shouldn't work as it's disabled in .vscode

[vernal rose]

Git revert? Ig you can just use reset --hard to delete previous commit right?

[outer raven]

oh yeah it just hasn't been released on dev, my bad

could you release one maybe? Since the attachment option PR was merged

[wild flax]

Only on builders

The one on djs is still open

[outer raven]

oh right ye

why did that PR bump deps then

for all projects

[wild flax]

I did to fix some stuff

So i just bumped all of them to align them

[outer raven]

Ah alright

[outer raven]

@zenith oracle how does using optional chaining fix that issue?

[zenith oracle]

Whoops I commented in the wrong PR lol sorry

[outer raven]

[solemn oyster]

@clever summit tests are failing

At __tests__/components/actionRow.test.ts:93:53

[clever summit]

two tests actually, should be fixed now

[dawn merlin]

Thannnnksss

[woeful rain]

Which style guide does discord.js follow ?

[golden mortar]

is it possible for someone to rerun the publish dev action? https://github.com/discordjs/discord.js/actions/runs/1840892663

[woeful rain]

https://github.com/discord/discord-api-docs/discussions/4510

[oak quail]

is the plan for api v10 to be used in v14?

[ruby terrace]

tis my hope

[vivid field]

I don't see why it shouldn't

[ruby terrace]

especially since it'll release so close to v9 deprecation date

[lilac tulip]

will v14 be released before the message content deprecation day?

30th april I think

because if so, and api v10 is used, wouldn‘t that mean that bots that dont have access to the message content will be restricted as soon as they update to v14?

[wild flax]

Don’t see how it matters?

v13 is on v9 which is not decommissions but only deprecated.

You can still run on that

Whether v14 is out or not

[lilac tulip]

yeah it won‘t be a problem, just something worth noting rather since people might be wondering after updating why they dont have access to the message content anymore even though it‘s before april 30th

[solemn oyster]

We're not doing those things for last day because it can result on many users realising they need the message content intent, which can lead to a large influx of requests for it, which... at the last day, is far from ideal

By doing so now, they'll notice and start applying over time, that way it makes the transition easier for both us (since they'll have an alternative) and for Discord (since the intent requests will be more spread out)

[lilac tulip]

okay sounds good

[outer raven]

I can already imagine the response to this but I’d honestly like to discuss this
Why don’t we undo the switch to builders?
Yes, maintenance overload by having to maintain the same thing in two places, fair, but discord.js will always need classes for every discord structure because it was built like that. Having those specific ones depend on a different package that uses snake_case has caused way more hassle than it could’ve solved (judging by the sheer amount of fix PRs that have been made since). If we just stick to one style (discord.js one) and keep the old classes, all this hassle would be gone and we could focus development on those classes only and not need to worry about complicated fixes for them. I personally don’t see a point in having builders at all but that’s personal and I won’t get into that. I think discord.js would gain a lot more by keeping its classes streamlined than having things all over the place with quick and messy fixups, but I’d like to know your thoughts

[wild flax]

You think too much of discord.js and less in sub-packages

[outer raven]

what about sub-packages? The hassle so far has only been in discord.js, the other ones are fine as they are

[wild flax]

That’s been our sole goal for the last 6+ months

Having sub packages that make up discord.js

And we won’t go back

[golden mortar]

Why don’t we undo the switch to builders?

I think the major problem with builders is that it's almost entirely useless. Embeds, Formatters, etc. are exported from discord.js so there is next to no reason to even use builders in the first place. Add on slow zod validation and it's pretty bad for now.

[outer raven]

that goal seems reasonable but I think you should look at the hassle that that has become. There are ways to do that properly, having all these inconsistencies and dirty patches is surely not one of them, so something should be changed IMO to improve maintenance overload while keeping functionality. It seems like the only goal you wanted to achieve with this has been the only thing this didn't achieve

[wild flax]

I don’t even know what hacky or dirty things you talk about

[copper laurel]

But the ones exported from discord.js are the builders

[dawn merlin]

They can be used outside the djs core for packages like rest or any other rest wrapper for that matter

[outer raven]

to name a few
https://github.com/discordjs/discord.js/pull/7424
https://github.com/discordjs/discord.js/pull/7422
https://github.com/discordjs/discord.js/pull/7387

[wild flax]

Yeah that’s what I mean with most of you think too much in “discord.js”

[outer raven]

Because that's the package we use and that's where the issue lies

[golden mortar]

Not in v14, Embed is a wrapper for builders' Embed and Formatters are identical.

[wild flax]

Unsafe is fair, the getters were a good improvement after we’ve seen some usage

And allowing camelcase was always planned

So..?

[dawn merlin]

We don’t reexport formatters in djs

I can’t remember if we use them

[copper laurel]

Wrapper for builder's Embed means its the builder's Embed

The wrapper only exists to add back camelCase support, maybe .equals() too

[golden mortar]

https://discord.js.org/#/docs/discord.js/stable/class/Formatters

[dawn merlin]

Tbh that should be replaced with the builders formatters

[copper laurel]

As much as I do agree with your initial point because I personally don't like builders, I dont see what's wrong with these PRs. Theyre just iterative development

[outer raven]

What I'm trying to say is that there's been those 3 PRs so far to fix stuff, now there's also https://github.com/discordjs/discord.js/pull/7464 open which brings 2 more deps and another hack. Not to mention you currently have an extended class from builders on every component & embed, which by itself doesn't ease much of the maintenance cost

[copper laurel]

Those aren't fixes

None of them are fixes

[outer raven]

don't get me wrong, I'm not saying those PRs I'm bad, I'm simply referring to the fact that we needed those 3 and now a fourth one that is open to get things working like they were before, with a bunch of hacks

[copper laurel]

None of them are hacks either

[golden mortar]

that was my point 😛, by re-exporting them with minimal changes it makes having builders almost pointless.

[copper laurel]

Then your point is bad

[outer raven]

they're fixes disguised as refactors, all of those PRs would be fine with the fix scope lol

[golden mortar]

of course something like equals could be handled by the user as well by extending the class

[outer raven]

no it shouldn't

[copper laurel]

This is a very disingenuous argument, and makes an assumption that the builder was "broken" in its original state

[golden mortar]

why not? genuinely curious

[dawn merlin]

No it doesn’t because builders still can be used without djs at all, which is a valid use case for any other js lib that isn’t djs.

[outer raven]

it wasn't broken if it was meant to be used alone. If it's meant to be used in discord.js then yeah, it was very much broken

[copper laurel]

Its just iterative development

[golden mortar]

does another lib use djs builders?

[copper laurel]

Build something that can work alone, add additional features to bring up to parity

This happens all the time in the enterprise world where the 2.0 version of something won't have feature parity in it's initial release, but what it does have is done better

[outer raven]

Because quite a lot of other properties have those methods and the point of semver is to only have to adapt your code to work properly on semver major changes. If you have an equals method in your code you will have to update it on every minor version that adds a new prop

[outer raven]

thing is this isn't done better at all