#archive-library-discussion

25085 messages · Page 16 of 26

clever spoke

yeah, thats what djs follows tho

visual hornet

we all know that, why do you think we referenced it?
semver major introduces breaking changes, that’s what it means to be semver major

kindred lagoon
visual hornet

for consistency, managers were introduced in v12 to manage data structures more easily

kindred lagoon
visual hornet

well, yes, if you read the update guide, permissionOverwrites were just moved to the manager
stuff is constantly changing, and we have to change to fit those
stuff can be overlooked and noticed and suggested, to try to keep improving the library further
and improving it majorly can’t come without a major change

kindred lagoon
visual hornet

and they aren’t required to, but they do it to improve the library

clever spoke
void rivet

if you're complaining about major versions having major changes
new major versions are the place to implement the major changes, which is what you mentioned here

lucid pasture
kindred lagoon
visual hornet

…because that’s what semver major is for? if you don’t want to have to update your code then don’t update to a major version

vivid field

it also happens that we find an idea to have a more organized lib design only after a few major versions, like e.g. managers broke a lot of stuff but after implementing them the methods that belong together are actually grouped together

and these breaking changes all get collected first so that there's not a new major version with every change, e.g. v13 would've come later too if it wasn't for discord bumping their api version

kindred lagoon
vernal adder

Because that is what updating is.

You cannot expect everything to be static.

Things change, and you sometimes need to change what's already there for new changes.

visual hornet

if it adds features then why would you expect them to be usable without changing anything?
when you update to a major version, you're making a bargain, you accept new features or better practices in exchange for changed features or removed methods

kindred lagoon
vernal adder Because that is what updating is.

Who told you that this is the update Do all the packages in the world make drastic changes and expect the user to change their idea in terms of specific points in one day and modify all projects to use a new feature?

vivid field

Other packages get new major versions too, yeah

vernal adder

Yes.

lucid pasture

As I said earlier no one expects you to update your code in 1 day

vernal adder

That is exactly what other packages do as well.

vivid field

but here it needs to happen more frequently because discord.js depends on another api

kindred lagoon
visual hornet

if you want an example for major drastic changes to other modules, just look at node-fetch, they dropped support for an entire module system with v3

kindred lagoon

😂😍

visual hornet
visual hornet
kindred lagoon
copper laurel

This can stop

You aren't providing any constructive criticism, nor are you accepting the answers given

This channel is not for you to rant and whine about small inconveniences in rewriting code

kindred lagoon
copper laurel

Yes, its been discussed. Read the rest of what I said

kindred lagoon
unique axle

Maybe you mistake some of the breaking changes that happen during development with released versions?
If not, I can just repeat what crawl and I said here (reply and down)

copper laurel

We hear this all the time, you aren't saying anything new, nor are you accepting any of the responses as to why it was necessary for us to make major version increases

kindred lagoon
copper laurel

v11 was release 5 years ago. v12 was 2 years ago. 13.0 was two months ago.

Quite large windows and well within the range you seem to be requesting from us

kindred lagoon
unique axle

The next major version (v14) will very likely re-structure some of the API in a breaking way to address problems we have found during development in the current version (be that actual bugs, unsupported use cases or "just" design decisions). That is, after all, what major versions are for.
The difference to other services (like the quoted node-fetch above) is, that this wraps another API we depend on (the Discord bot API). Meaning, while you can still use very old versions of node-fetch, you will eventually have to update discord.js, as Discord stops supporting older versions of their API. Discord also introduces new features only in new API versions, which forces the user to apply major version jumps and all the changes that come with it

kindred lagoon
wild flax

What a joke

wet sail

hey, with Discord JS, it was always hard to write test with its internal structure and the need for the client. But now with all classes having private constructors, ist much harder. Whats the point of making this even more harder?

outer raven
visual hornet

the constructors for most structures were never meant to be used, you should be getting most stuff from cache or from fetching or from other structures

wet sail
outer raven

Why would you need to test a package that isn't yours?

wet sail

we dont want to

we want to test our code

visual hornet

can't you just use the actual djs structures then

outer raven

well then test your code, not discord.js's?

tacit crypt
outer magnet

just a question, I was looking around and in the APIRouter.js file in rest folder and it has target in the parameters of the function but doesn't use it. Is this intentional or?

also in the deleted property in the Message class just defaults to false without any checks. Is this also intentional?

opaque vessel
opaque vessel

If you were to remove the target parameter there, clients would never connect to the gateway :P

outer magnet
opaque vessel

If a message is deleted then the client fetches it... well that won't happen, it can't fetch a deleted message. Also it's not a case whether or not you're using the event, the library handles the incoming event

outer magnet
wild flax

that makes no sense

you won't even get a message if the client isn't connected

outer magnet

oh yea mb, i just realised that

dawn merlin

on component collectors can we assume that the component interactions we get back are from a bot that's within a guild?

opaque vessel

What about DM channels?

I guess since those methods can only be used from discord.js's structures, you can at least assume that it's not from a guild that the bot is not in (and cached in DM channels)

distant prism

hi everyone 🙂 is it possible to mute my music bot for all users in a channel except one ? example --> I want to listen music, but the others users don't necessarily want to hear. can i make my bot play music only for me ?

visual hornet
split elm

A fetchRules function to fetch the guild’s rules ? (Suggestion)

outer raven

if you mean the rules channel, that exists as well

split elm
outer raven

yeah the welcome screen, it exists

rustic boughBOT
split elm

No, welcome screen = the screen that appears when you join a server

opaque vessel

That's membership gating. There isn't anything for it other than a few properties and flags

There is no method to fetch that information from a guild

split elm

Rules = the menu that you have to accept if you want to write

Okay so I suggest this function ^^

Because I saw that we can look if the rules are activated, but we can’t look at these rules

opaque vessel
split elm

Too bad…

Thx 🙏 !

gusty carbon

Why do some some properties in discord.js-modules use the javascript private properties (prefixed with #), while others are just marked as private in TypeScript? Imo it would make more sense to be consistent and use the hash prefix everywhere since the privacy of those fields is also actually enforced by Javascript itself

dawn merlin
gusty carbon

yeah theres benefits and disadvantages to both, it just feels kind of odd to me that the accessors arent consistent across the project

outer raven

I think I remember seeing some discussion about narrowing the type of the value of a collection with the filter or find methods, did that ever go through or is that possible to do?

I'm asking this because I'm trying to find a channel in the channels cache and I'm sure it's a category channel, so I wanna narrow it's type down and there's no method for that. Could we created methods for checking each type of channel like we did for interactions?

visual hornet

or just the type property?

outer raven

I guess, but using the methods like the Interaction ones is more useful to me at least

wild flax

How so

outer raven

well it's easier to just return if the method returns false than it is to import the class and do an instanceof check

visual hornet

oh you mean as typeguards

outer raven

yes

these

icy bronze

I'm not sure it's normal that Guild.available is sometimes undefined, it leads to an incorrect amount of unavailable guilds (i'll switch from !guild.available to guild.available === false to make sure I have to correct amount but just reporting)

loud jayBOT

pr_open #6769 in discordjs/discord.js by cycloptux opened <t:1633470939:R> (approved)
fix(Guild): guild.available is never set on new joins
📥 npm i cycloptux/discord.js#fix-guild-available-not-set

icy bronze

ohh thank you

wild flax

If importing another one is such a big problem

outer raven

Why can’t we implement them in the library?

wild flax

Because it’s not the same as the one you linked above

outer raven

How is it not?

wild flax

The others aren’t just instanceof checks lol

outer raven

Different channel types have different methods and properties, I don’t see how that’s any different from interactions

Yeah I know?

wild flax

Yeah

There you go

outer raven

So again, why can’t we implement them into the library

dawn merlin
outer raven these

As I explained in the PR these type guards are better than instanceof guards because they make all applicable properties on the object the cached type. Instanceof doesn’t do that

outer raven

The interaction checks just check for the type

outer raven
wild flax

If you don’t want to make your own

Make a pr

You’ll get your feedback on there if it’s something people want or not

outer raven

That’s what I was asking, I thought you were against me even making a pr lol

wild flax

I’m never. Just know that it could be denied that’s all

outer raven

Yeah I’m fine with that

outer raven

on second thought, I'd rather do that in a semver major because the current isVoice and isText check for text and voice based channels, and not the text and voice types, so that'd be confusing

unless it was split up and the other methods were added now and those renamed later

wild flax

its semver-major later too

you can break either way

whether you do it now, or later

visual hornet

isText vs isTextBased would probably confuse some people but i could see that being useful.

wild flax

could probably make it isStrictText, jest does that for some things

and then in semver major break it

but then, we might as well do isTextBased and isText

and just have an info block

outer raven

and if we were to add that we wouldn't need to break it later on

but then again it's a bit inconsistent and it feels weird to have isStage and isVoice which do different things (for example)

so to avoid confusion I think it's best to do this all in one go in a semver major change

grand eagle

Is this an error?

I copied this from another bot I had made which was running 13.1. 13.2 seems to have introduced this in the TypeScript typings.

opaque vessel

What is ActivityType?

grand eagle
outer raven

you need to cast the type because we changed it to not accept CUSTOM

so it can't be any string

grand eagle

I'm not using CUSTOM. In the screenshot, I am using Game.

outer raven

yeah but you still have to cast the type because it's a property in an object and not a string

why don't you just write "PLAYING"? (I think that's what you want)

grand eagle

You can see that it worked in 13.1

outer raven

yes I know

I'm telling you you need to explicitly say what the type of ActivityType.Game is

grand eagle

It's an enum? I am not sure what to say

I am not sure if I can actually cast it to a string.

So basically, we just do not accept from discord-api-types anymore? I found out that there is Constants.ActivityTypes.GAME

outer raven

yeah that's probably better. It may be a bug though, since it should know the type of that enum

not sure if it's fixable though due to how enums work in ts

maybe @dawn merlin knows?

copper laurel

That does seem a little odd, I'd agree that as a developer even though that field excludes a particular property of the enum, you'd think TypeScript would still let you use the other properties from the base enum

outer raven

it should, but TS is weird with number enums

so it doesn't know that ActivityType.Game is 0 but if you type 0 in it works

vernal adder

I have a similar issue with my option types, leading me to casting them.
type: ApplicationCommandOptionType.Subcommand as number

copper laurel

Thonk Isnt this exactly why we provide Constants?

Those are the real values and TS enums kinda dont exist in JS code

outer raven

yeah how are those imported? 🤔

copper laurel

uhh, import { Constants } from "discord.js"?

dawn merlin
grand eagle Is this an error?

The reason that this doesn't work is that TS doesn't let you use 'alternative' enums in place of the enum the that is expected. Basically just because one enum has the same values, doesn't imply it's strictly equivalent to another enum.

outer raven
vernal adder

Mine?

outer raven

like how do they translate to js

vernal adder

Looks like this in my dist.

Just the number (and a comment)

dawn merlin
outer raven

I don't really see a point in that tbh

i mean at that point might as well use the djs stuff

dawn merlin

I mean I would

like if it's djs, use djs types

outer raven

because if we were to do that for the entire lib it would be quite a lot of work for something that can be easily be fixed by using the djs package lol

outer raven
dawn merlin
vernal adder

Using discord.js' enums works as normal.

outer raven

well there's your solution

vernal adder

It's not a problem for me. Just thought it'd add to the convo.

dawn merlin

wait can enums be used during runtime now? I remember a PR was merged to do that

opaque vessel
dawn merlin

yeah

They said you only need Constants now if you wanna do indexed enum access but accessing an enum directly now works (in runtime) without Constants

dim steppe

The values selected, if the component which was interacted with was a select menu
Isn't the last bit kinda redundant, since it's in the SelectMenuInteraction class?

Also, I've been wondering why DJS calls _patch() at the end of some constructors - why does it do this, as opposed to assigning the values in the constructor?

copper laurel

_patch() is also called for updates to the structure

From events etc

When we dont want to construct new ones

dim steppe

Oh, so anything that could update, gets placed in the patch

copper laurel

yeah, which is why it also does checks on the existence of properties

opaque vessel

Hi @vagrant holly! For your event removals, I think the typings need to be updated (old event names still in there)? :P

vagrant holly

Did I not remove them? think

Oh huh I did not

vagrant holly

All fixed tick

outer raven
dawn merlin

Oh for DM's?

outer raven

nah, regular guilds

but if your bot is in the guild then the member options will be GuildMember objects and never API objects

dawn merlin

yeah assuming, the guild member in that option is cached, it should be GuildMember | null, I'm kinda busy rn, but if I don't get to it today you can submit a PR

outer raven

ill look into it and if it doesn't look too complicated I'll submit a PR

outer raven
copper laurel

You'd have to extend all the generic cached typings right down into the resolver

opaque vessel
dawn merlin
opaque vessel

Yep. With strict mode disabled, the else block from .inGuild() return the interaction as never. It doesn't do that for me anymore

dawn merlin

Ah ok

opaque vessel

So it seems this issue is now solved :P

dawn merlin

Genuinely curious why that PR resolved it

paper furnace

I'm encountering a typing issue moving from v12 to v13, when I need to setup the default activity type using env var, I'm making a validator which reflect the type in use which is ExcludeEnum<typeof ActivityTypes, 'CUSTOM'>, but ExcludeEnum type is not exported then I had to duplicate it from my side. Can you consider exporting custom types too ?

opaque vessel

ExcludeEnum is already exported, it's just not in a released version

paper furnace

so the next release will fix it 🥳

opaque vessel

Ideally, yes (:

paper furnace

there's a fix aboutn const enums too ?

i don't understand why all enums are declared as const, but it's not really useful

currently due to export const enum ActivityTypes... we cannot use those enum to validate values and compare them against data from any type using Object.values like Object.values(ActivityTypes).includes(input). Maybe this choice is motivated, but currently I don't get the point

outer raven

But typescript enums are not javascript enums, so you can’t really use them in code like that

They’re exported as consts so that they can be replaced by their values in compiled code

outer raven
paper furnace

when exporting a const enum TS don't generate the lookup table, so we cannot iterate on the values, I found the PR with the changes, but frankly I don't understand which errors are solved 😅

I'll check for the constants export

paper furnace

options are

export interface ActivityOptions {
  name?: string;
  url?: string;
  type?: ExcludeEnum<typeof ActivityTypes, 'CUSTOM'>;
  shardId?: number | readonly number[];
}
outer raven
outer raven
paper furnace

I see

then now it's fixed, i'll just wait for the next release to remove my ExcludeEnum copy

dawn merlin

I wonder what would happen if the user enabled —preserveConstEnums?

paper furnace

I'd prefer avoid using flags, they can update/remove them easily 😅

wild flax

Just like random strings then

Except that flags usually get updated to the new value, while a raw string doesn’t

So if anything using flags is a good thing

And that one surely won’t disappear anytime soon

It’s an integral compiler feature lol

dawn merlin

^

Also tsconfig.json

paper furnace

anyway i'm using constants which are made for it

is that normal to not being able to compare Message.channel.type to Constants.ChannelTypes ?

the first contains the API keywords as value, the second is numeric, why DJS don't use the same values everywhere?

tacit crypt

the ChannelTypes object is more of an ENUM (so we can go from number -> string and string -> number)

paper furnace

I don't see a mention about moving Guild.region info in the migrating doc, currently the value is not defined and I don't find it back 🤔

raven juniper

iirc regions are on voice channels now

paper furnace

oh ok, so each channel can have it's own region? that's pretty cool

anyway, v13 is awesome, seeing true typing is a real pleasure, I'm in love, tyvm for your awesome work ❤️

outer raven

@wild flax you commented on https://github.com/discordjs/discord.js/pull/6278 saying you weren't sure about it. The upstream PR is now merged and they said that default_auto_archive_duration is only intented to be used by clients and not the API so do you agree with defaulting autoArchiveDuration to that or should it be closed?

lapis echo

why does d.js use const enum over enum?

afaik const enum only exists now for legacy support.

outer raven
lapis echo
dawn merlin
lapis echo
dawn merlin

yeah because you can't import regular enums without you getting an import exception during runtime

lapis echo

I could've sworn TypeScript enum can be used to manipulate as an object, unless I'm missing something?

dawn merlin
lapis echo

oh wait, so the typings do not overlap for d.js' js constants?

outer raven
outer raven
lapis echo

damn, that kinda sucks

that's why i see so many people confused about it.

dawn merlin

yeah basically without const enums we were mapping enums to values that didn't actually exist, hence the changes

lapis echo

aha

outer raven
lapis echo

that makes a lot more sense why

dawn merlin

I mean unless you want reverse mapping you don't actually have to use Constants

lapis echo

Then my only question is why doesn't the typings just map over the Constants that are actually being exported to avoid confusion?

outer raven

The enums are literally put in a separate file to avoid that confusion

They’re not meant to be imported

You should just be using Constants, it’s the exact same

dawn merlin
lapis echo

Then why does the enums exist if there not meant to be imported? Are they just private typings?

dawn merlin

they're meant to be used in the type declarations

lapis echo

Let me take a look at src code rq

outer raven

I really don’t understand why there’s so much confusion on this topic

dawn merlin

thats relatively loose typing compared to using enums

lapis echo

In typings/index.d.ts couldn't you just use a namespace? e.g.```ts
export namespace Constants {

// objects

export interface Package {
name: string;
version: string;
description: string;
// ...
}

// enums

export enum ActivityTypes {
PLAYING = 0,
STREAMING = 1,
LISTENING = 2,
WATCHING = 3,
CUSTOM = 4,
COMPETING = 5
}
}```

dawn merlin

honestly the might work, but as I said earlier most people are just going to not use Constants bc they don't need reverse mapping. But if that code above actually works during runtime I'd say that's a definite improvement.

lapis echo

I'd have to do some testing for sure, but I think that could definitely clean up that section of typings.

dawn merlin

also small nit: namespaces are deprecated in favor of modules now

lapis echo

Yeah, but this is just typings, if it were a full TypeScript project I would totally go with that approach. This approach shouldn't make a difference afaik.

dawn merlin

im pretty sure ts/eslint won't let you, but you can try

lapis echo

dang, well then I'll probably just leave it then, because tbh whoever wrote that section like that probably tried a namespace approach. I'd rather not go against elint rules.

dawn merlin

why not just use module instead?

lapis echo

Could give me an example of what that looks like?

dawn merlin

it's pretty much just like namespaces but you replace 'namespace' with 'module'

lapis echo

oh, hmm

I'll do some testing later then. If it works, I'll probably make a PR for that improvement.

dawn merlin
lapis echo

that's awesome!

lapis echo
dawn merlin
lapis echo

awesome, ty!

i'll make it tonight then.

dawn merlin
lapis echo i'll make it tonight then.

For reference this is what the syntax should look like:

export declare module Constants {
  enum ChannelTypes {
    GUILD_TEXT = 0,
    DM = 1,
    GUILD_VOICE = 2,
    ...
  }

  ...
}
lapis echo

ah, ok then, no need to export inside, that's nice.

outer raven

@dawn merlin my only question about #6840 is how can you be sure that user is never null on partial guild members, specially on the guild member remove event?

dawn merlin

in the ticket, @opaque vessel stated the ID is always present on all three of the events, so wouldn't it have to be non-null?

lapis echo

I'm not going to make the PR for that because most of the Constants contain property value types that are named the same as their key so I can't really do anything to avoid it except to put the type in their manually. It adds too many lines. The only way it would work properly is if we did more files so I could rename types exported from the file when importing them, but I'm not sure if that's something we'd want to do as there is a minimal amount of declaration files.

opaque vessel
dawn merlin

in that case my PR should probably be inversed

opaque vessel

I'm also unsure when the .user is possibly null. No idea when that may be

dawn merlin

so basically, it can be null, so therefore id should also be?

opaque vessel

Maybe? Maybe not. I don't know

dawn merlin

for what it's worth its documented to give a user object back

opaque vessel

All I know is that the typings are wrong. The id cannot be always there yet the user be possibly null

outer raven

Yeah I think the bug here is that id should be nullable

But more testing should be done on this cuz from what I understood, Jiralite’s issue was only covering types and not actual return values

dawn merlin

yeah I misread it, I had assumed that was runtime types

opaque vessel
dawn merlin
opaque vessel
dawn merlin

Hmm ok, I'm going to assume that, that diff is a regression

opaque vessel

I guess so

dawn merlin

@oak quail I thought bots didn’t get command C/U/D events?

oak quail

they dont anymore, correct

this updates cache when u use the c/u/d methods on command manager

dawn merlin

Ahh ok

tender field

in #6838, why not override channel to be GuildTextBasedChannels too?

dawn merlin

@tacit crypt what is the policy on empty interfaces that may or may not have properties added later in the typings. Should they just be type aliases or remain as empty interfaces?

tacit crypt

shrug

I'd say keep them as interfaces

dawn merlin

Ok

dawn merlin
outer raven

Could we maybe have an object in every message that includes all the text in between formatting tags (so all the italic text, all the bold text, all the spoilers, etc)? I just wouldn’t be sure what to call this but it would work similarly to mentions

unique axle

why? that seems like a lot of additional memory, unless you want to make them getters which just throw a regex against the content and even then i can only think of edge use cases

outer raven
unique axle why? that seems like a lot of additional memory, unless you want to make them ge...

well I have a very specific edge case so I'll explain that
there's a bug in discord that allows you to hide text in a message (mentions included) by spamming a lot of empty spoilers and then writing after them. I don't know what the exact amount is but I'd like to delete these messages automatically and that would help, but it's obviously possible without that
If it were to be a getter, do you suggest one getter for each type of formatting or one getter that returns an object with all?

unique axle

i think that that's a good use case, but something very easily solved on your end and does not need a library solution

visual hornet

couldn't you just use (or make i guess) an external library to parse the Message#content

outer raven

well yeah what I can do is simply check if the message has an empty spoiler and delete it, but I thought this feature could possibly help others

unique axle

it's a super specific and super narrow use case to support in the base library

outer raven
outer raven
visual hornet
outer raven

ah I see
well still, that could just be done with a series of regexes

bitter peak

Could something like this be useful?

dawn merlin

Is there a reason that toJSON on structures don't return the discord api types, but instead return the djs object with camelcased keys?

real jetty
dawn merlin Is there a reason that `toJSON` on structures don't return the discord api types...

The toJSON() methods existing on the structures only return a JSON-serializable (JSON spec complaint) object of the original object so it can be consistent and spec complaint, for example it converts bigint literals to strings as the JSON spec does not have a way to serialize them, and to not get users confused when they store the object as a JSON, so the properties and method names would be consistent, but only for now; the return values of the methods might be changed later

analog oyster
solemn oyster
dawn merlin

idk if it's just me, but I can get @Typescript/eslint to successfully parse and report linting errors on .d.ts files 👀

outer raven
real jetty
outer raven

It is there tho

real jetty

what line?

also, tslint and eslint are different so that would be a // eslint-ignore line

outer raven
outer raven

Is there anything in discord-api-types that holds all the rtcRegion ids?

dawn merlin

I was under the assumption that eslint didn’t parse declaration files, that’s why tslint was still used

outer raven
analog oyster

dtslint uses tslint, which has been deprecated for a while. so yeah, switching to eslint would be great

wild flax

Good luck replicating the dtslint rules

opaque vessel
outer raven

One thing I’m not sure about is if we should put deprecated regions in there because my guild still has it’s region set to central-europe but this doesn’t appear in the endpoint

wild flax

If theres no publicly known list, we don't put it

outer raven

and it's reliable enough that we can put it in the docs and types and update it when/if it does update

wild flax

Not really

That list could be dynamically updated

outer raven
ornate topaz

that's not exactly what "dynamic" meant here

wild flax

If its not on the docs, I don't see why it would be documented

What is that argument here?

Flags and new errors get documented

outer raven

and kyra seemed to like the idea too

wild flax

And why would you randomly or consistently, 1x a week, call an endpoint and check?

lol

We implement the API based on the docs not the information returned from the api via endpoints

outer raven

why not? I mean saying that we can pass any string to that is not exactly true so having some sort of suggestion would be nice

ornate topaz

can you document content of last message in a channel?

wild flax

Because if something is not documented, its not meant to be

opaque vessel

At the very least you should think about documenting it at Discord's API documentation before here

outer raven

Guess I’ll try that then

dawn merlin

Anyone know the answer to this? 😅

is there any circumstances where the message is sent in a guild but the guild happens to not be cached?

wild flax

Not if you don't mess with the client cache

which is unsupported

dawn merlin

Ok ty

I believe if you don’t have the message intent, you don’t receive message events at all

visual hornet

if you don't have GUILDS you won't recieve messages from that guild either
even with GUILD_MESSAGES

so the only thing should be messing with cache

outer raven

Does anyone know why we export the api types as RawX?

Wouldn’t the name from dapi types be just fine

dawn merlin

It might be because Dapi types aren’t a dep on djs, and the only way to expose them would be to reexport them? But I’m not sure

Yup

ruby terrace
outer raven
outer raven
ruby terrace
  1. I don't think its "a lot" by any sense
  2. As I already said, consistency
  3. Ease of maintaining it, it's a lot easier to add to / adjust a union in one place than all over the place.
  4. As far as I'm aware, vsc doesn't even ever surface the "raw" types, instead displaying the underlying type / union
outer raven
ruby terrace

Its not all raw API types, its specifically the types that are used as data when constructing a structure

outer raven

@ruby terrace just a question: is there really no other way to do this?

like having 2 properties documented that do the same

if anything that should be mentioned on builders jsdocs right?

also you missed the types on your PR

dawn merlin
outer raven

ah aight

ruby terrace
outer raven
ruby terrace
outer raven

Seems more logical

ruby terrace

I debated that originally, decided to do it that way

(comment that on the pr, I'm interested in opinions)

outer raven

Aight, will do

opaque vessel
opaque vessel

*Technically it resolves ReactionEmoji and GuildEmoji no matter where it came from before it attempts to look into the cache, so the warning block here isn't accurate.

Anyway, it will work without being restricted to the guild as the URL is uploaded, so there's no need for restrictions here, so...

outer raven

and also the client only shows you emojis from that guild when applying a role icon

I assume it wouldn't be accepted now if it wasn't accepted when I suggested that on the main PR

lemme find the discussion

none of the maintainers replied tho so idk

opaque vessel

Okay, well, the warning is a lil' inaccurate - only the id needs to belong to the guild, other resolvables don't need to be

outer raven

Not sure what u wanna change here

opaque vessel

The description says the "same guild"

outer raven
opaque vessel

I said this already :P but no, because resolver methods return if it meets an instance, which means you can pass in an emoji from another guild

outer raven

the warning is completely accurate in my eyes

copper laurel

only the id needs to belong to the guild, other resolvables don't need to be
This. Not every type that is part of EmojiResolvable needs to belong to the guild, only string

outer raven

ah I see what you mean

eh I guess it could be changed but doesn't make that big of a difference

opaque vessel

A provided Snowflake must belong to the same guild as the role.
This is a better description of how it's handled internally

outer raven

Could we start adding deprecation messages to the types?

vagrant holly
gentle folio

Canvas is not compatible with v13?

raven juniper

If is, please see #djs-help-v14 if you have specific questions about the lib. You can also search in the channels, someone just asked about canvas on node 16

outer raven
vagrant holly

Sure, if there's an associated jsdoc deprecation notice that has a message, I guess it makes sense to also have it in the typings deprecation flag

outer raven

ideally we'd have all jsdoc comments in the typings file so they show up in the editor. but that's way too much work

vagrant holly

get an IDE that actually reads the source and so knows about the jsdoc comments, as well as the typings

opaque vessel

The one I just did has one too. Managed to make it a cute lil' link

outer raven
opaque vessel

It does, doesn't it for you?

outer raven

i had no idea

ok will pr to add it to all the other deprecation tags

only one I will not add is MessageEmbed#type since idrk what to say and the docs don't have a description for that either

wait most deprecation tags have descriptions and there's only 8 Kappa

well nvm i guess

dawn merlin

Wait should we be deprecating types that we remove in favor of other types?

Bc I have not been doing that 😅

outer raven

he's deprecating that because it will be removed in a semver major update

breaking type changes usually aren't classed as semver major, only if the actual code changes in a breaking way

dawn merlin

Yeah ik, but I didn’t know if we needed to offer deprecation notices for breaking type changes, while they’re not semver major, they can still break ts projects

outer raven
outer raven

@dawn merlin if you’re gonna open a new pr to move the types you can probably just repurpose that one right?

Just gotta undo the changes in the non ts files

dawn merlin

no I'm gonna do it in 6867

it's small enough to tack on

outer raven

Wait which one is that

loud jayBOT
outer raven

Ah gotcha

real jetty

Could we get some type narrowing for interaction#member so we don't have to do this?

dawn merlin

also ironic you and daro have very similar pfp's

real jetty
paper furnace

Is DJS 13 compatible with ES2020 ? I need BigInt support for my bot but when I'm setting up the project I'm always encountering this issue from DJS files:

ERROR :  C:\Wumpus\discordjs-bot\node_modules\discord.js\typings\tests.ts:1
import {
^^^^^^

SyntaxError: Cannot use import statement outside a module

When I'm creating a whole new project without deps it works well, once DJS is added to the project this error appears. IDK if opening an issue for it is relevant, 'cause I don't know if it's really related to DJS source code, or Jest which currently seem encountering some issues with Node 16 🤔

vernal adder

Yes, it is fully compatible with ES2020.

dawn merlin
paper furnace

@dawn merlin in fact I'm mainly asking about a potential issue related to a dep you're using for your tests, to see if it can be relevant to point it out in a new issue or not

vernal adder

I am using ES2021 with discord.js v13.

But I am not using ts-node.

dawn merlin
paper furnace

ok, so moving to node16 lead my setup to this error, pretty weird, then i'll continue to dig into

dawn merlin

@tacit crypt do you wanna steal #205 in dapi-types from me, I feel like it would be faster, because I don’t know where some of the types would go

tacit crypt

Whats #205 again?

dawn merlin

Ig anyone familiar with the types can take it over

tacit crypt

I mean the PR looks fine, you just have to fix your compile errors

and the inconsistency between AutoComplete and Autocomplete

split elm

Hey, plz add a function to "reset" a select menu (that is to say, the select menu returns to the form it had before selection)

instead of having to edit the message to reset the select menu

ruby terrace

That seems like a feature request for Discord, not djs

outer raven
split elm
outer raven

that's just resetting the default property on the option

split elm

that is to say ?

copper laurel

Editing the message is the correct way to do it though. Are you proposing a method like Message#resetComponents() that would just do a no-change edit for you?

Most of the time you want it to be part of an interaction response though

split elm
copper laurel

So it would really be more of a SelectMenuInteraction#reset() method, which would behave like an update with no changes

split elm

Yes 👍

fallen pasture

can bots send and receive messages between eachother in DMs?

unique axle

no

fallen pasture
dawn merlin

What’s the purpose of having bots dm each other lol

fallen pasture

lmao it’s a very interesting reason

there’s a website that has stats for this game called DayZ, and there’s a discord bot connected to it and a command will display your stats, i wanted to have my bot send the command and display the response on a webpage

the dude that runs the site doesn’t wanna take the time to add a “daily stats” function so i’m doing it myself lmao 😭

he put a lot of work into it tho i’m not complaining. just a qol i want

slate nacelle

We do not filter any members out.
You might not have had all those members cached at point in time of kicking.

severe sun 
member.roles.add(["901232121149018112"]) // This is an invalid role id

On v12 this code gives an TypeError [INVALID_TYPE]: Supplied roles is not an Array or Collection of Roles or Snowflakes. error
however on v13 it doesnt throw any errors ?

opaque vessel

On version 12, an actual role is attempted to be resolved. On version 13, the id is only extracted. So, this is expected behaviour. It's a bit strange you would pass an invalid id in the first instance

severe sun

its an invalid id because the role was deleted

slate nacelle

I'd expect this to error on Discord's end however.

outer raven

Are you guys aware of any bugs with creating channels with positions? I'm currently trying to create multiple category channels and sometimes the rawPosition and position properties are different and, on Discord's side, there are 2 or more categories with the same position until one of them is manually moved through the client

slate nacelle

I only remember Discord ignoring the position if it's 0.

outer raven

weird because that's not the case, I have two rawPosition 4 and no rawPosition 5 then a category at rawPosition 6 sometimes

and this is weird because on the client they don't even show up in those positions (the duplicated ones)

slate nacelle

rawPositions can be duplicated, since category channels even more

outer raven

but should they be duplicated among categories? I know they are different from categories to regular channels and voice channels but im exclusively working with raw positions here

slate nacelle

If Discord wants it to be that way, then sure I guess.
If they are equal, compare the ids instead.

outer raven

So do I have to make subsequent calls to update the positions after I create the channels? That sounds like a Discord bug imo

slate nacelle

One, since you can bulk-update positions, can't you? Might be a bug though, since the client doesn't allow you to create a channel on a specific position it might have gone unnoticed.

tacit crypt

Positions as a whole are a headache

outer raven

there is channel.setPosition tho

slate nacelle
outer raven

oooohhh I thought it would make sense for that to be on the manager

will have a look, thanks

slate nacelle

I too think so, probably an oversight, since those are very niche methods.

vernal adder

For it to be in guild#channels#setPositions would be better, in my opinion.

outer raven

could be something for v14, not worth the effort atm to duplicate the method

tacit crypt

you wouldn't duplicate, move the core logic in the manager and keep/deprecate the guild method with making it call the manager one

vernal adder

That is a status. These things:

visual hornet

(except the "custom status", that's also an activity)

vernal adder
outer raven
outer raven

Since Discord sends the timestamp of when a rate limit will expire in the x-ratelimit-reset header, could we use that to provide a date object in the RateLimitData object received in the event?

manic thistle

hey it seems like there's a problem with creating stickers.
it looks like it defaults to treating it as a jpg file, which makes discord throw an Invalid Asset error.
shouldn't passing stream or buffer data without the file name for context be disallowed to avoid this issue?

loud jayBOT
wild flax

Why though lol

outer raven

why is readfileSync being promisified if readFile exists hmmmmm

dawn merlin

Gonna work on making docgen accept TS types at some point, I think typescript-eslint/estree-parser would come in handy here

outer raven

What would be really awesome would be to move all jsdocs to the typings and have docgen read that instead so that we can get jsdoc comments in the IDE

dawn merlin

I think the ts rewrite will be a better time for that

primal prairie

hey, not discord.js but, does anyone know a npm package that can fetch a whole channel and send it as an html transcript? all of the packages i found are able to fetch 100 messages only, but i know this is possible cuz ive seen it done on ticketing bots.

outer raven
primal prairie
vernal adder

This channel is for developing discord.js, not asking about downloading a channel

outer raven

yeah you should've stopped at the "not discord.js"

primal prairie
patent widget

Slash commands add limit must be increase

ornate topaz

nothing is a "must", and since we are not discord, this is not in our control.

outer raven
patent widget
real jetty

Is there a possibility for the library to support SlashCommandBuilder objects for command creation (guild and global)?
Currently, it is not possible to create a slash command with the builders (discordjs library) because of the incompatibility of the types.

dawn merlin
real jetty

I didn't see it in the patch note, I have to check it then

dawn merlin
real jetty

Hummmm, this is not fixed...

"discord.js": "^13.2.0",
"@discordjs/builders": "^0.7.0"
dawn merlin

can you provide the error message it's giving you?

real jetty

Maybe we should move in #archive-builders if this is a development problem on my side :/

outer raven

Don’t you just need a non-null assertion?

dawn merlin
dawn merlin
tacit crypt

You're just encountering a version mismatch between -types in d.js and /builders

real jetty

Ho yes, discordjs use 0.23.1 and builders use 0.24.0. But the result is that the two modules are incompatible and it's a pity especially since they are made by the same organization 😂 I will create an issue in few minutes

outer raven

Solution: bump on djs

idk why no one bumped it yet

usually crawl does on release but if someone prs they can have that on the dev versions

dawn merlin
tacit crypt

and lock file

dawn merlin

Tru run npm i before pushing 😅

outer raven

speaking of lock file, why doesnt that have the "name" property on the first object? I always get that when running npm i for some reason

vivid field

a recent pr added that I think

yeah, #6809

outer raven

oh really

ah i see it now

didn't even notice that but finally lol

vivid field

(you're supposed to run npm ci anyway)

outer raven

when there's no node_modules yeah I do that, otherwise npm i which is the recommended

woeful rain
  • why tsukabi when you can use the built-in module.
outer raven
woeful rain
outer raven

Commits are done in bulk so I don’t think so but it should definitely be improved on because commits were happening much more often when this was first added

outer raven
woeful rain okay

Stuff might be a lot different with readFile tho since you gotta pass a callback to that

woeful rain

done

visual hornet

can't you just use fs/promises
that already has a promisified version

woeful rain
visual hornet

i guess?
i haven't been following what you're trying to do

dawn basalt

How long is the ETA for Autocomplete to land in D.js?

copper garden
dawn basalt
slate nacelle
dawn basalt

ThumbsUpSmile

uneven rain
zenith oracle

The documentation is the same for all the send methods related to a TextBasedChannel

It's actually just one. The one from GuildMember inherits the documentation from TextBasedChannel#send

vernal adder

It's just a short-hand for sending to their DM-channel. It still sends a message to a channel, but I can see it being somewhat weird.

vernal adder
outer raven

@vernal adder did you see #6875?

loud jayBOT

pr_open #6875 in discordjs/discord.js by ImRodry opened <t:1635031757:R> (approved)
refactor: move Guild setPositions methods to managers
📥 npm i ImRodry/discord.js#refactor-managers-setpositions

vernal adder

Not yet, will look soon

Looks good! 👍

outer raven

Noice

outer raven
wild flax

I flipped the important parts:

    "exports": {
        "import": "./dist/index.js",
        "require": "./dist/index.mjs"
    },

👴

woeful rain
wild flax

Because the docgen runs at node 8+

and it was written when it was running like... 6+?

and tsubaki automatically falls back to using promisify anyway if you are on 8+

Oh and the other reason this happens is because the CI uses npm i and not npm ci

so it automatically installs higher patch versions lol

its fixed though

no need for me to re-release the docgen this way

lofty birch

I think I found a bug with the library. When the client is first added to a server after being logged in, Guild#available is undefined, when it should instead be true. This behaviour did not exist in v12.x

It's also documented to be a boolean, not boolean|undefined

opaque vessel
wild flax

not yet released though

lofty birch

Ah

This caused me several hours of pain, wish I knew that sigh

outer raven

can we make a PR about a new discord feature if the dapi docs pr isn't up yet?

there's a thing that started rolling out but no one bothered to PR and i dont really like doing PRs to the docs but I'd be comfortable with the djs pr

well yeah i know that

wild flax

setPremiumProgressBarEnabled is a really mouthful name for a method

tacit crypt

It's also a big property, so I don't see an issue with it

wild flax

doesn't a boolean indicate a state already?

setPremiumProgressBar should be ok here for the method no?

tacit crypt

Removing the enabled can mean many things - set it to a number, to a goal, to something

wild flax

I agree that we should stick for properties with the API naming

but since we wrap things, I don't see why we would need to keep the explicit naming for methods

Well yes, but for that documentation exists too

tacit crypt
wild flax

hmm

tacit crypt
wild flax

I guess its whatever

But for v14 id def like to get rid of some of that ugly naming we have for those things

tacit crypt

And change it to..what

just remove utility functions like these no, someone will kill me for daring to bring that idea up

wild flax

not in this specific case, but we could also go with togglePremiumProgressBar()

if you supply a bool, its getting sent, if not, it flips the current state

tacit crypt

Hmm..we could, sure

wild flax

im ok with leaving it like this for now

but we have a lot of those ops, where we could name them toggle and flip states

tacit crypt

Definitely something for v14, although i know people will rage due to it 🙄

wild flax

I wanna build a good wrapper though and not be a people pleaser :P

tacit crypt

I want a mix of both meguFace

Although if it was after me I wouldn't even add these shortcuts because it's just extra code that adds little benefit other than a shortcut

wild flax

I think its important and makes code more idiomatic

instead of always calling edit() with a different partial object

tacit crypt

GuildEditBuilder

I suppose that's true, anywho

outer raven
wild flax

not really no

those helpers aren't really meant to represent the edit values 1:1

but rather have them being used as a more idiomatic way

outer raven

So what else would you name them?

tacit crypt
woeful rain

Now Tsukabi is completely useless in docgen, because docgen requires a minimum version of nodejs 8.

outer raven
tacit crypt .

but I'm talking about methods that don't take boolean values, those would still have to be setX right?

dawn merlin

Can’t you use the js “set” modifier for those?

outer raven

Because then it would be
this.nsfwLevel
set nsfwLevel

For example

dawn merlin
outer raven
dawn merlin

yes you can have getters and setters of the same name

dawn merlin
outer raven

hmmm would have to be discussed then, I'd rather see a PR with that later on so that I can judge better

and of course, would also like to see what the maintainers say

vernal adder

Sounds unnecessary to make every private prop a getter/setter.

Don't see the point of that.

dawn merlin
vernal adder Don't see the point of that.

Not every value needs to be private. Ones that can be edited without side effects, don’t need to be. However I was pointing out if we want truly private storage get/set is the only way to go about doing that, otherwise you can’t access it. Otherwise you just have props on an object that can be edited without using the proper setters. I’m just throwing in my 2 cents, we’ve been doing pseudo-private props so it’s up to the maintainers if they want to continue that or use another method of private storage.

vernal adder

IMO it's the end-user's fault if they mess with private properties. If they're marked as private you should not mess with them. If you do, that's your fault.

solemn oyster

Problem with true privates is that they're incredibly inefficient, both CPU and memory wise

Since it relies on WeakMap

dawn merlin

that is a good point

outer raven

Was looking at the code of GuildMember#manageable and this seems wrong: if the logged in user is the user in question and is the owner, why should this be false?

slate nacelle

I don't think it made much sense to declare the owner being above the owner, looking at the description of the getter.

outer raven

from what I can see the third if should be moved to the top right?

slate nacelle

No, that'd make GuildMember#kickable return true for the owner if the bot is the owner. You still can't kick yourself however.
(Same for GuildMember#bannable)

outer raven

maybe the issue is those getters depending on this one then, because as-is, this will return false when checking for the bot's guild member in a guild they own

slate nacelle

This behavior sounds about right, when looking at the description.

outer raven
slate nacelle

What else is there, aside from the two above, which would break here?

outer raven
slate nacelle

The certain methods you mentioned.

outer raven

oh no they wouldn't break, I'm just saying that if you have, for example, a function that relies on member.manageable to be ran and the value turns out to be inaccurate it won't complete

slate nacelle

It's accurate however.

outer raven

say for example member.setNickname

tacit crypt

and the setNickname has a different endpoint for the bot user vs normal members
technically space we were tied

slate nacelle

Setting your own nickname is, in the api, something else than setting another member's nickname.
too slow again

outer raven

well but the method is the same on djs isn't it

vernal adder
outer raven

well yeah it is

talking about the method, not the endpoint

tacit crypt

Makes a difference how

setNickname calls edit

edit calls the function Drango just linked

outer raven

yeah exactly

wild flax

Pretty much everything calls edit OMEGALUL

outer raven

but say you have a generic function to edit any member's nickname, and that relies on member.manageable (so you don't have to catch every time), that will not run when the member in question is the client user, but it should

tacit crypt

But it would make every other check inaccurate or outright wrong

And even then, manageable doesn't even get into the permissions side of things

outer raven

oh yeah I know I'd also have to check for MANAGE_NICKNAMES here, but both checks would need to be ran

is there a better way to do what I'm doing without using manageable?

tacit crypt

Except its not MANAGE_NICKNAMES for own member

outer raven

yeah ik

tacit crypt

Then what are you trying to get at??

Manageable is strictly role hierarchy wise

I mean we can guesstimate that if user.id === client.user.id, its manageable, but it's not 100% accurate

outer raven

hmm I guess this breaks more than what it fixes so it's better to leave it as-is ig

will do my own checks for the client user

tacit crypt

I mean.. cogwheels are spinning... wouldn't moving the client.user.id === guild.ownerId above then checking the user.id === client.user.id and returning true... that sounds like it'd make sense but either way we're still not 100% accurate

You might have a point, hold on I need to check things now

tacit crypt
tacit crypt
outer raven
tacit crypt

It's correct the way it is rn

outer raven

aight thumbs

hollow crest

Could anyone tell me why the MessageComponentCollectorOptions type in index.d.ts omits the message option from InteractionCollectorOptions? Passing a message option to createMessageComponentCollector is extremely useful, I have been doing so for a while as I find it far more elegant that using the filter option to filter interactions that came from a specific message.

It is possible in JavaScript so I am confused as to why it is being omitted from the type for typescript.
These are the relevant lines from index.d.ts:

copper laurel

uhhh, createMessageComponentCollector is a method you call on a Message instance (or TextChannel). It binds to the Message/Channel you're calling it on, not the one you pass in

If you want to construct a generic MessageComponentCollector you should be able to with those options though, so it does seem odd that it would exclude them.
@dawn merlin is this one of your typings?

outer raven

doesn't seem to be his, at least not after the rewrite

copper laurel

Oh I see why

Yeah, MessageComponentCollector isnt a class. Either you're constructing a generic InteractionCollector in which case those options exist and are entirely valid in InteractionCollectorOptions, or you're using <Message|TextChannel>#createMessageComponentCollector in which case it binds to the instance on which it's called, not the passed options

dawn merlin
uneven rain

GuildStickerManager#create should throw a error saying You didn't provide any tags or something like that when you didn't provide any tags

hollow crest
raven wind

what exactly is your mission here?

hollow crest
opaque vessel

Seems some files import @discordjs/collection without respect to the import/order rule from ESLint, which isn't being picked up from npm test. Example of this is at src/structures/interfaces/Collector.js on line 4:

outer raven
outer raven
uneven rain
outer raven

which error lol

uneven rain

saying cant read property name of undefined

outer raven

ah

then yeah sure

will look into doing that

outer raven
outer raven nope, thats an issue in your IDE, it is correct

to elaborate on this: VSCode and more specifically eslint comes with a bundled node version that is set to node 14 by default and thus your IDE sees a "node:X" import as a regular package and not a built-in module. If you update the node path to use your node installation this will be fixed. Also you wouldn't even be able to commit that because eslint --fix reverts the change

@wild flax I saw that you bumped collection to use node 16, but why not 16.6? The PR that adds #at needs 16.6 to apply the suggestion given by vlad

dawn merlin
wild flax

Like lol

Nothings released yet

outer raven

but you specified 16.0.0 and not >=16

wild flax

Again that doesn’t matter

You needed the CI bump

outer raven

aight if you say so

ill rebase the PR now

opaque vessel

I'm experiencing an issue ~~caused by https://github.com/discordjs/discord.js/pull/6084~~. This pull request allowed APIMessage to be passed to the interaction collector so it could be used via webhooks.

When you create an interaction collector on a message that's within a thread and delete the parent channel, the collector still has bound events (it continues collecting), but the thread no longer exists. If you do not pass a time, this is indefinite.

The interaction collector only caches the ids and allows raw messages to be put in. Since an APIMessage can be passed, I don't see a way to link it to the thread's parent channel. Before this pull request, this is easily doable.

Anyone got an idea of how to detect that if a channel is deleted to stop the interaction collector on the message within a thread? Right now, it's the same issue as https://github.com/discordjs/discord.js/issues/6895 and can cause possible event leaks

outer raven
opaque vessel

I don't think so. That would be the thread's id, and this implementation allows bots that aren't in the guild to use this collector so it wouldn't be in their cache

outer raven

ah good point

not sure then, that might be hard

opaque vessel

A fix can be made for the reaction collector and the message collector, but I don't think the interaction collector can be fixed

outer raven

Maybe @ruby terrace knows?

ruby terrace

I don't do collectors, sorry

outer raven

who does? I forgot

wild flax

Maybe just open an issue so it’s on the radar, detailing that

opaque vessel

Alrighty (:

wild flax

We could think about an info block

Detailing that long running collectors with that type of structure is a bad idea because of the implications it brings

And that one should set a timer

opaque vessel

Ya, that sounds ideal for now

opaque vessel
wild flax

no need for the ! but sure

[...] or manual cancellation

visual hornet

what about the idle option?

opaque vessel
visual hornet what about the `idle` option?

That works too, so I've amended it to this:

Interaction collectors that do not specify time or idle may be prone to always running. Ensure your interaction collectors end via either of these options or manual cancellation.

wild flax

hey @dawn merlin , iirc you wrote most of the <"cached"> stuff right?

dawn merlin
wild flax

Could you take a look after the most recent commit

into the test.ts file with all the assertions

im not sure if typescript in my editor is tripping

or if the types or tests don't properly grip

gives me like

Argument of type 'MessageComponentInteraction<CacheType> | MessageComponentInteraction<"cached"> | SelectMenuInteraction<"cached"> | ButtonInteraction<...>' is not assignable to parameter of type 'ButtonInteraction<"cached">'.
  Type 'MessageComponentInteraction<CacheType>' is not assignable to type 'ButtonInteraction<"cached">'.
    Types of property 'componentType' are incompatible.
      Type '"BUTTON" | "SELECT_MENU"' is not assignable to type '"BUTTON"'.
        Type '"SELECT_MENU"' is not assignable to type '"BUTTON"'.ts(2345)

since I extended and made the rules more strict in the tsconfig.json

in help of maybe catching some issues

if its some hackery we do, we can just // @ts-ignore them with a comment too I guess

dawn merlin

thats really strange, I just tsc'd and it compiles fine on my end, what version does tsc --version say?

wild flax

We don’t use tsc on the test file

Only on the index one to see if it even compiles

So only the editor shows those errors

Or you tsc the test file specifically

My editor uses ts 4.5 though

The repo uses 4.4

dawn merlin

Yeah I ran tsc manually on the test file, and my editor doesn't show any errors, I'm on 4.4 so maybe try switching to 4.4?

wild flax

huh ok you are right

4.5 seems to introduce some big breaking changes MonkaGiga

lets hope its not gonna end up that way

actually maybe not

after switching back to 4.5 they are gone

oh well

OMEGALUL

dawn merlin

#justTSthings

dawn merlin
wild flax

I feel like we did rename it though 🤔

dawn merlin

what's strange is it's not on just the cache types, it's affecting a bunch of other types as well

dawn merlin
outer raven

@wild flax sorry for another ping but was rebasing a pr and noticed you removed .vscode from gitignore. Is this intentional?

its pr #6666 btw, in which i modified it to exclude every file from .vscode except for the extensions.json file

real jetty

is this going to be addded?

thats the context menu?

hasty bluff

Yes

real jetty

oh

i see

outer raven

Do we support an endpoint for fetching the invite background?

analog oyster

aka the splash url

no, and theres no specific endpoint for fetching it

outer raven

It’s odd that the name doesn’t include invite

analog oyster

¯_(ツ)_/¯

opaque vessel

It's called splash on Discord's documentation too

analog oyster

the guild feature for it is called INVITE_SPLASH, but the property itself doesnt mention invite at all

opaque vessel

Best leave it, and you can document that it's the invite splash if you really want to

analog oyster

it has been like that for years, don't think it matters that much

outer raven
opaque vessel

Then we're all good!

real jetty

It’d be nice to have a shortcut for getting the client’s role on RoleManager

maybe call it clientRole?

dawn merlin

I don’t think perm shortcuts are really being considered anymore in the main djs lib, but I might be wrong

wild flax

yeah we dont like it as much

try to keep it low on those

not even a huge friend of like the whole TS checks

but they are absolutely necessary sadly

real jetty

I see, fair enough

dawn merlin
wild flax

yeee

I wish we didn't have to do them PepeHands

dawn merlin

Yeah this is what happens when instanceof can’t be used with interfaces or types

vivid field
outer raven

Could you guys enable reviews on the #858855149643497493 webhook? It currently only posts comments not approvals/changes requested/comments

vernal adder

Should guild#vanityURLCode and guild#vanityURLUses be moved or otherwise have a presence in guild#invites?
Maybe as a guild#invites#vanityInvite?

visual hornet

invites#vanityCode?

wait, hasn't this been discussed before lol

vernal adder

I don't know

Didn't check

jade creek

how do i shard my bot?

vernal adder
tacit crypt
outer raven
tacit crypt

That's our property/getter tho

While vanity data is on the guild object itself

Idk

vernal adder

Then something like that might be an option

Just found it odd that there is no "easy" way to get the vanity invite url

tacit crypt
vernal adder

Yes, but we have role#toString()

even thought it's just <@&id>

I think there should be a getter for the invite, but 🤷‍♂️

opaque vessel
vernal adder

If a getter were to be added it wouldn't affect InviteGuild and AnonymousGuild

But I see the point, yes

loud jayBOT
dawn basalt

Why was this closed?

dawn merlin
outer raven

What's the most common reason for prs to be ignored? There are quite a few atm and some are quite interesting

but prs are being reviewed daily and then there's some that are 20 days old that haven't gotten an update since

wild flax

most likely because they are very low in the priority list

we usually don't look at PRs if they arent on a milestone thats soon to be released

outer raven

You moved this to 13.4 just now, I was just wondering why

wild flax

Because in the timeframe we wanted to release 13.3 there was no resolution

And this PR comes with a huge perf hit

outer raven
wild flax And this PR comes with a huge perf hit

I understand, but the initial concerns were replied to and then nothing else was said, I believe a solution could’ve been worked out or a decision could’ve been made (since the only alternative to that PR without the performance hit would be semver major)

wild flax

IMO it was pretty clear that the perf hit is pretty much a no-go

so yeah

opaque vessel
wild flax

does not, is mislabeled

opaque vessel

Thank you for correcting it (':

outer raven
wild flax

the perf and memory hit is not acceptable honestly

for such a niche fix

like

the fix would be for <20% of people probably

but affect 100% of people

outer raven

So instead of removing userupdate events completely (or almost completely) it would be better to have them both work

I understand that the performance hit is not acceptable, but if feedback was given maybe other solutions could be found

wild flax

I honestly think over overestimate the ability for us to give feedback

certain things cant be magically fixed with some sort of solution honestly

outer raven

Either way saying nothing at all doesn’t seem right, specially for a “first” time contributor

wild flax

If theres nothing to say, im unsure what to elaborate on

outer raven

Close maybe? Idk I think it can’t stay open forever right

But if its closed it should be explained to the author maybe

outer raven

Apparently the collection repo says the latest release was 0.2.4 even though there's a 0.3.2 tag, could someone update this? (maybe even release a new version for the new method)

outer raven

Any reason why the return type of MessageEmbed#toJSON is set to unknown? Pretty sure this could be safely set to APIEmbed right?

opaque vessel

I think it's because nearly all others type toJSON() the same. It should be fine

outer raven

other toJSON methods don't have interfaces but this one does, so it should be used imo

unknown types are annoying to work with

and the docs even document it that way

dawn merlin
outer raven

I don't think so, it was just done to all because the other toJSON methods can't have the API types

so we'd have to be making tons of interfaces

unless there is a way to make an interface out of a class in TS

dawn merlin
outer raven

we'd still have to declare them all though, I'm talking about like a type or something that turns a class into an interface

tacit crypt

iirc

because it has our camelCased properties in footer/author

outer raven

it doesn't, I tested

didn't test with all fields, but the ones I did did have snake_case

will test with all actually

tacit crypt

From an embed you made or you got from the api?

outer raven

from an embed in a message that I called toJSON on

tacit crypt

Ah

We made toJSON act like the old _apITransform

outer raven

apparently yeah

tacit crypt

then yes that type could be corrected

outer raven

some props are snake cased some are camelCased

uwot

tacit crypt

toJSON returns api data

outer raven

nope, not for image for example

tacit crypt

then that should be fixed

wait

it doesn't even matter

proxyURL cannot be set by the end user

and the API ignores it

outer raven

so is it fine to keep it like this?

I mean others like author have their icon_url typed in snake_case so guess thats fine

tacit crypt

I think you can safely type it as APIEmbed, tho it will mean that proxy_url won't be set

outer raven

if it doesn't get set by us nor the API I guess it's fine right

tacit crypt

the API sets it derp, but it ignores it if we provide it

which is what I said

outer raven

aight then that's fine as it is

dawn merlin

@tacit crypt is this what you're suggesting in the builders PR?

    public setAutocomplete(autocomplete: false): Omit<this, 'setAutocomplete'>;
    public setAutocomplete(autocomplete: boolean): Omit<this, 'addChoice' | 'addChoices' | 'setAutocomplete'>
tacit crypt

.....no

Stop Omitting anything if autocomplete is false

and definitely don't omit setAutocomplete for a general boolean set

dawn merlin

So have the overload return this? and remove the setAutocomplete omission from the general boolean method?

tacit crypt

The overload should return the original class

without omissions

dawn merlin

ohh ok, now I see my misunderstanding, this is mutated whenever an Omit is returned, it's not static

hmm, the only workaround I can think of for this would be to do something similar to SharedSlashCommandOptions<ShouldOmitSubcommandFunctions = true>, and add a SharedSlashCommandOptions<ShouldOmitSubcommandFunctions = true, ShouldOmitChoiceFunctions = false>

tacit crypt

that won't help at all

the omit is done already

Just return the original class

or just don't omit it if this makes it so complex

dawn merlin

but how do you return the original class without a polymorphic this? If you change an overload to return the original class without using this it's an error:

do you mean return this & { addChoices(), addChoice() }

tacit crypt

you shouldn't add it there..

In the setAutocomplete base class

this & Pick<CommandOptionBaseWithAutoCompleteOrWhateverItIsCalled, 'addChoice' | 'addChoices'>

dawn merlin

Ok, so this?

public setAutocomplete(autocomplete: false): this & Pick<ApplicationCommandOptionWithChoicesBase, 'addChoice' | 'addChoices'>;
public setAutocomplete(autocomplete: boolean): Omit<this, 'addChoice' | 'addChoices'> { ... }
tacit crypt

yes

Probably, test it

make sure everything builds and works

dawn merlin

got it to work with conditional types:

public setAutocomplete<A extends boolean>(
  autocomplete: A,
): If<
  A,
  Omit<this, 'addChoice' | 'addChoices'>,
  this & Pick<ApplicationCommandOptionWithChoicesBase<T>, 'addChoice' | 'addChoices'>
>;
public setAutocomplete(autocomplete: boolean): Omit<this, 'addChoice' | 'addChoices'> { ... }
tacit crypt

That's..why

That's overkill

Makes the second overload useless

dawn merlin

Because if you use just the top overload, you have to cast the return to any ie return this as any

dawn merlin
burnt cradle
dawn merlin

Return this as any

burnt cradle

its not making me

I can use it fine without casting

dawn merlin
wild flax

can anyone test running npx tsc typings/tests.ts --noEmit

not sure if its TS thats being unreasonable here (again)

ruby terrace

uh....this seems weird

dawn merlin
wild flax

different file

ruby terrace

Does the tsconfig somehow not include tests.ts?

dawn merlin

oh I see

wild flax

yeah I found it by accident

ruby terrace

uh, why is skipLibCheck enabled again?

dawn merlin

I wanna say that there is a misconfiguration somewhere, because the typings work when djs is used in another project, and within tests.ts my intellisense is accurate

wild flax

it was never disabled

🤔

yeah but I dunno if I wanna trust the editor over the cli

ruby terrace

The editor doesn't show errors because those files aren't being included properly it seems

wild flax

also eslint errors

have to add them to an eslintignore file too

ruby terrace

ts will still error over eslint, but yeah, we'd need the typescript parser for eslint to not error

ruby terrace
dawn merlin

ok I included the typings dir and the editor now shows the corresponding errors, these are legitimate errors

ruby terrace
opaque vessel
ruby terrace

it keeps changing

That reaction from space feels appropriate

wild flax

feel free to disable it again