#archive-library-discussion

25085 messages · Page 11 of 26

outer raven

not really cuz there are multiple options and only one subcommand and group

tacit crypt

How is that moving the burden of checking from the user, when it now makes it twice as annoying because it throws an error

bitter peak

It still serves to validate the user's command data against their runtime code

If anything, I think it should be a getter

If not a function

outer raven

i don't see any advantage in that

tacit crypt

it should be a property

and let users check themselves

there's no reason for it to be a glorified getter

bitter peak

But none of the other methods do that, for the sole reason of ensuring the user doesn't have mismatched command definitions and code

tacit crypt

the other options are specifically of a type (roles, users, channels, etc)

subcommands and subcommand groups are strings and nothing more

All you do right now is throw an INCONSISTENT error when someone gets the subcommand / subcommand group

wild flax

I think you guys make this out to be a way bigger deal than it is

lol

tacit crypt

??

wild flax

the whole point of us transforming the options is so the user doesnt need to check at all

so I agree with shino there

otherwise you can just use the get() method

tacit crypt

I am strongly disagreeing with glorified getters for sub command (group) but shrug

wild flax

which does none of this

Yeah I don't think its too big of a deal

tacit crypt

I don't see how that's better than just having users check if interaction.options.subCommand === 'name'

Especially since I bet you there will be someone doing if/else chains of getSubCommand() instead of caching the variable

wild flax

I don't think we ever tried to pre-optimize what users end up doing

So that is out of scope for this and rather belongs into the guide or a programming guide in general

Calling a function that returns the same value over and over, and how that isnt optimal, is not something we need to teach people.

gaunt lotus

Not sure if this is where I should be asking this but I can't seem to import SlashCommandBuilder, I have:
import { SlashCommandBuilder } from '@discordjs/builders';

Says Module '"@discordjs/builders"' has no exported member 'SlashCommandBuilder'. I have @discordjs/builders@0.2.0 installed

wild flax

Its not yet published

Ill let you know when it is

gaunt lotus

Oh ok my bad, thank you

oak quail

hm

for my stickers pr should i actually make it work for both stickers and sticker_items

so it works for old messages and new messages

stickers is technically deprecated but its the only way of seeing what stickers old msgs have

oak quail

also, couldnt the Base class implement createdAt and createdTimestamp? since it says its for data models that have snowflakes (although looks like some non-snowflake objects incorrectly extend it)

warped crater

yeah thats primarily the reason why it doesnt annoyingly enough

i had a look at it once but its fairly obnoxious to refactor

slate nacelle

We could also update the docstring to reflect what it is.

wild flax

is it deprecated in v9 still?

bitter peak

Maybe Base can be refactored into Base and SnowflakeBase extends Base?

zenith oracle

Why?

tacit crypt

@wild flax @bitter peak why don't we make getSubCommandGroup do the exact same as getSubCommand? (aka throw if missing, return otherwise)?

bitter peak

I guess yeah that does make sense. What do you think, Crawl?

bitter peak

@wild flax

wild flax

sure

next time make sure to ping me directly instead of waiting :P

bitter peak
oak quail
wild flax

man fuck discord

ruby terrace

aj did say no more breaking changes for v9 right

oak quail

i think someone said theyre removing guild.region in v9 but that doesnt sound right

the client still uses it tho bloblul

dry valve

there doesn't seem to be any documentation about how context works in broadcastEval, I've spent the past hour searching and the only thing I've found is where the context should be placed, but not how to access it

passing a second argument seems to be the solution to this, but why is it not documented?

analog oyster

here are a few oddities with GuildInviteManager

  • #cache is typed as Collection<Snowflake, Invite>, while it is actually Collection<string, Invite>

  • it has #resolveId (should perhaps be called resolveCode?), and it just returns back whatever you pass at it as long as you pass a string

    • e.g. if you pass discord.gg/djs, it returns discord.gg/djs, if you pass djs, it returns djs

  • #resolve should support invite urls as suggested by InviteResolvable, but it doesn't. it only supports invite codes

gaunt lotus

Why do I need the MANAGE_ROLES permission to edit the permissionOverwrites of a channel when I can create a channel with initial permissionOverwrites without it?

frank turret
tacit crypt
frank turret

noted, the second part of the point still stands

loud jayBOT
upper moss

isnt this pr a major semver as it renames some of the properties

analog oyster

yes, but technically interactions haven't been released yet so shouldn't it be semver: N/A instead

tacit crypt

it's still semver major i think meguFace

gusty carbon

its labeled as semver minor tho

oak quail
tacit crypt
oak quail

they probably won't but I can ask

tacit crypt

They should

So please do ask 🙏

ruby terrace

I would guess they won't, based on the fact that old crossposts no longer return guild_id which is supposed to be guaranteed

rough glacier

current CommandInteractionOptionResolver implementation leaves subcommand options as a Collection and a tiny bit fuzzy to access them. Shouldn't they be adapted similarly?

bitter peak

getSubCommand() is just a getter for the name, which is resolved at construction time

junior pumice

I've got an idea:
What if you could insert a filter function in the cachefactory to tell it that it should only cache what matches the filter

So (c) => c.isText() to only cache text based channels

short crescent
copper laurel

message.edit({ embeds: [embed] }) for example - you dont want this to change the content (which is undefined)

null is the correct way to specify something as intentionally having no value, hence editing out the content. Undefined is only the lack of its presence in the object

rough glacier
bitter peak What do you mean?

well right now you can only access subcommand options as <interaction>.options.get(subcommandName).options if im not mistaken, and you get Collection<Snowflake, CommandInteractionOptionData[]> instead of another CommandInteractionOptionResolver. Maybe an interaction.options.getSubCommandOptions() is too much, but different types between first and second options layer is weird

bitter peak
rough glacier well right now you can only access subcommand options as `<interaction>.options....

That's intended.
CommandInteractionOptionResolver#get() is intended to return unparsed option data. It's up to you to parse the subcommand options if you do it that way. The preferred way to use it is like this:

declare const interaction: CommandInteraction;

const subcommand = interaction.options.getSubCommand();
switch (subcommand) {
  case 'foo': {//   /command foo
    await interaction.reply('foo!');
    break;
  }
  case 'bar': {//   /command bar <baz:string>
    const baz = interaction.options.getString('baz', true);
    await interaction.reply(`baz: ${baz.toUpperCase()}`);
    break;
  }
  default: throw new Error(`Unknown subcommand: ${subcommand}`);
}

CommandInteractionOptionResolver#getSubCommand() returns the subcommand name as a string

vivid field
copper laurel
rough glacier
bitter peak

Since the purpose of get() is to return "raw" data

bitter peak
vivid field

\👍

junior pumice
copper laurel

No I mean, it doesnt need a PR

junior pumice

Uh

Oh hmmmm

Can I?

I thought the cache classes are just object names

copper laurel 
const FilteredCollection extends Collection {
  set(key, value) {
    if(filter) {
      super.set(key, value);
    } else {
      // idk, something
    }
  }
}

const client = new Client({
  makeCache: (manager) => {
    if (manager.name === 'ManagerToFilter') return new FilteredCollection(filter);
  }
});```

Thats a really rough example of what you could do

junior pumice

Hmmm

I thought about something like this

[...].makeCache({
MessagesManager: {limit: 200, (f) => !f.author.bot},
EmojiManager: {limit: 1000, (f) => f.animated}
})```

Idk the exact names

mighty matrix
bitter peak

wtf

copper laurel

Nobody online can right now

real jetty
zenith oracle

Wdym?

real jetty
zenith oracle Wdym?

In reply options you have a property named fetchReply to fetch the reply immediately :

// First way
const reply = interaction.reply({ content: "test", fetchReply: true })

// Second way
interaction.reply({ content: "test" })
const reply = await interaction.fetchReply()
zenith oracle

Ah yeah, you're right, this option doesn't seem to be in the docs

bitter peak

regarding #6143, I'm not too sure what a good solution there would be; interaction.options.options definitely doesn't sound too great. While their issue could be solved with a loop/helper function in their code, it seems like a use-case that would be useful to support. Maybe CommandInteraction#optionsArray: CommandInteractionOption[]?

real jetty

Another thing that can be cool is to add the same thing for ClientApplication that for Client, something like :

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

export class ClientApplication<Fetched extends boolean = boolean> extends Application {
  ...
  public owner: If<Fetched, User | Team>;
  public isFetched(): this is ClientApplication<true>
  ...
}
bitter peak

@wild flax @tacit crypt thoughts on the issue above ^?

loud jayBOT
gaunt lotus

Would it be possible to have getSubCommandGroup() return null/undefined instead of throwing an error when the command issued doesn't have a sub command group? When checking if a received command interaction has one I would do:

if(<CommandInteractionOptionResolver>.getSubCommandGroup()) {
    (...)
}

instead of something like:

let subCommandGroup: string;
try {
    subCommandGroup = <CommandInteractionOptionResolver>.getSubCommandGroup();
}
catch {}

if(subCommandGroup) {
    (...)
}

I'm using this to run certain code for specific sub command groups

wild flax

That isn’t really the intended use case for it

Also there’s always the generic get method that doesn’t throw

zenith oracle

I think the option resolver should have a public collection property, so that users can still use their own ways to handle options, like the issue mentioned above. What if I want to filter or map the options? I actually can't. It's like a collection with a really advanced get methods...

gaunt lotus
tacit crypt
loud jayBOT
bitter peak

@tacit crypt

tacit crypt

Could just turn _options into a Collection like before

wait, right, options.options

uhhhh

ruby terrace

.raw?

opaque vessel

Might I just suggest .data or .raw?

.data as in, the data supplied to us
Or .raw if you really wanna go that route!

bitter peak

I was thinking like interaction.optionsArray

tacit crypt

It's not really raw tho

It's already processed by us

ruby terrace

eh, true

real jetty

Or instead name it like a property we could name it like a function / getter :
interaction.options.toCollection -> return _options into a Collection like before
interaction.options.toArray -> return _options into an Array like now

We will avoid interaction.options._options and give the possibility to DJs user to use what they want

zenith oracle

I agree, it would be really useful

bitter peak

I like the idea of toArray() since we could return a copy of the array

opaque vessel

I also wanna suggest a rename of the methods? Right now, they are .get(), .getBoolean(), .getChannel() etc. but imo this feels kinda weird since discord.js relies a lot on Collections and using .get() returns undefined but .get() here returns null. I know they're not the same but it's still... weird to me.

What about renaming the methods to.resolve(), .resolveBoolean(), .resolveChannel() etc.? The managers across discord.js use .resolve() and also return null which is what the current methods do on CommandInteractionOptionResolver. Also there's "resolver" in the name so it kinda matches up.

Thoughts?

wild flax

Not a friend

It’s not a collection

ornate topaz

it's the Collections that are odd ones here and return undefined

and it's like that because Collections extend Maps by adding Array methods, and both Maps and Arrays return undefined. We just mimic those in Collections

wild flax

Right ok theres one issue @bitter peak

bitter peak

👀

wild flax

Def need a way to access all the options before they get transformed or anything

I used to do something along the lines of const args = [...interaction.options.values()];

which is obviously not possible right now

I can do _options for now I guess

But since its private, it will obv throw in TS

bitter peak

okay, which do you prefer:

  1. interaction.options.toArray(): CommandInteractionOption[]
  2. interaction.optionsArray: ReadonlyArray<CommandInteractionOption>

or?

wild flax

just make options public

probably

bitter peak

but that looks bad

interaction.options.options

disgust

wild flax

call it data then

and we are good

bitter peak
wild flax

yeah but you know what im trying to say lol

bitter peak

ye ye

wild flax

theres just no real way yet

id be fine with interaction.options.data

bitter peak

alright I'll add that as a ReadonlyArray

wild flax

since data will probably be our key moving forward for raw data on structures

real jetty

In DataManager class we have this :

export abstract class DataManager<K, Holds, R> extends BaseManager {
  public constructor(client: Client, holds: Constructable<Holds>);
  public readonly holds: Constructable<Holds>;
  public readonly cache: Collection<K, Holds>;
  public resolve(resolvable: Holds): Holds;
  public resolve(resolvable: R): Holds | null;
  public resolveId(resolvable: Holds): K;
  public resolveId(resolvable: R): K | null;
  public valueOf(): Collection<K, Holds>;
}

But when you pass a snowflake to resolveId (for example with ChannelManager), TS consider that you are on this function: public resolveId(resolvable: R): K | null; instead of public resolveId(resolvable: Holds): K;... so TS consider that the result can be null. So I wonder if it would be possible to change the typing to include the snowflake type in the first function like this : public resolveId(resolvable: Holds | K): K; so resolveId(<Snowflake>) return Snowflake and not Snowflake | null

solemn oyster

It should be changed tots public resolveId(resolvable: K | Holds): K; public resolveId(resolvable: R): K | null;
Although we'll have to check for the extended managers, specifically emoji ones

real jetty
oak quail

@tacit crypt they said to use (data.sticker_items ?? data.stickers)

tacit crypt

CC @wild flax

also Bruh

wild flax

for real?

wait

but it isnt backwards compatible is it?

oak quail

it is

wild flax

so you dont even need to change anything but do that?

oak quail

sticker_items has partial sticker objects

yeah

wild flax

then sure

@oak quail did you add the new error codes?

oak quail

doing rn

wild flax

👍

oak quail

and i'll do the docs link thing

wild flax

@oak quail youll need to like rebase or something

the lock file is out of sync

actually hold on

oak quail
wild flax

yeah

oak quail

i dont have the new npm ver

wild flax

I should have thought first

done

oak quail

🎉

wild flax

really close to release now

real jetty
oak quail

why are deferred, ephemeral, replied, and webhook on CommandInteraction and MessageComponentInteraction instead of on Interaction 🤔

copper laurel

Future proofing, there might be a type of interaction that doesn't use those response types

loud jayBOT
real jetty

I hope I didn't make a mistake and did it right XD

winged dust

#archive-voice from v0.5.1 to 0.5.4 newState.channelId changed to newState.channelID (Capital D) which is the opposite of what happened to discord V13. Is this right?

tacit crypt

Boop @fringe temple

fringe temple

In JoinConfig, everything is cased as Id

real jetty

In Sticker.js we have :

  async fetchUser() {
    if (this.partial) await this.fetch();
    if (!this.guildID) throw new Error('NOT_GUILD_STICKER');

    const data = await this.client.api.guilds(this.guildId).stickers(this.id).get();
    this._patch(data);
    return this.user;
  }

!this.guildID will always be true because the variable has got renamed to guildId no ?

winged dust
fringe temple

That's unrelated to the new voice library, @tacit crypt ^

tacit crypt

ah

wild flax
real jetty
wild flax

Sure

loud jayBOT
honest barn

my first pr here, tell me if i have to change anything

winged dust
real jetty
honest barn

just found it, it's just one file so i can change that quickly

one sec

nvm its better to just make a new pr

honest barn
fringe temple

@honest barn if there are other places in the voice repo to make this change, feel free to do it in the same PR 🙂

i will review it later today

fringe temple
honest barn

i searched for id on the repo to see any mentions of ID

the only one was in Networking.js other than in the examples

honest barn
loud jayBOT

pr_open #156 in discordjs/voice by SuperchupuDev opened <t:1626786134:R> (review required)
refactor(ConnectionOptions): change xID to xId
📥 npm i SuperchupuDev/voice#refactor-connectionoptions

bitter peak

Will ReadonlyArray<T> work in a JSDoc comment?

diff --git a/src/structures/CommandInteractionOptionResolver.js b/src/structures/CommandInteractionOptionResolver.js
index ca0b1587..7d93b6cd 100644
--- a/src/structures/CommandInteractionOptionResolver.js
+++ b/src/structures/CommandInteractionOptionResolver.js
@@ -45,6 +45,15 @@ class CommandInteractionOptionResolver {
     }
   }
 
+  /**
+   * The array of `CommandInteractionOptions` managed by this resolver.
+   * @type {ReadonlyArray<CommandInteractionOption>}
+   * @readonly
+   */
+  get data() {
+    return this._options;
+  }
+
   /**
    * Gets an option by its name.
    * @param {string} name The name of the option.

And, should this be return this._options.slice()? Or assigned in the constructor?

zenith oracle

Why readonlyarray?

You already have the readonly tag

winged dust
strange igloo
honest barn

on my last pr in voice this workflow failed, do i need to change anything?

strange igloo
honest barn on my last pr in voice this workflow failed, do i need to change anything?

You can look at what went wrong with the Details

src/VoiceConnection.ts:337:5 - error TS2345: Argument of type '{ endpoint: string; serverID: `${bigint}`; token: string; sessionID: string; userID: `${bigint}`; }' is not assignable to parameter of type 'ConnectionOptions'.
  Object literal may only specify known properties, but 'serverID' does not exist in type 'ConnectionOptions'. Did you mean to write 'serverId'?

337     serverID: server.guild_id,
honest barn

oh im on mobile i didnt see it

ill fix it later fixed

winged dust
bitter peak
bitter peak
zenith oracle

What if we clone the array instead?

Like return [...this._options]

honest barn

fixed

epic

zenith oracle

👍

bitter peak
zenith oracle

Yeah

real jetty

The typing for the function Interaction.inGuild() ins't perfect because it doesn't change the typing for the channel...
Instead of :

public inGuild(): this is this & { guildId: Snowflake; member: GuildMember | APIInteractionGuildMember };

We should have :

public inGuild(): this is this & { guildId: Snowflake; member: GuildMember | APIInteractionGuildMember; channel: TextChannel | NewsChannel | ThreadChannel | null };

Are you agree ? (it will remove DMChannel and PartialDMChannel on possible types for channel)

clever crypt

should the set functions from Embed (builders) allow to pass undefined?

clever crypt
real jetty
tacit crypt
clever crypt

and just allowing set* to accept undefined would work fine

and of course all the field values are already public so a user could just do embed.description = '' so if anything, it makes sense to allow undefined or do nothing at all

tacit crypt

undefined is ew. Maybe null

bitter peak

Maybe we need a ?undefined tag at this point tbh

wild flax

no people just need to learn js

copper laurel

We've had this discussion several times over the past few days lol

real jetty
wild flax

I don't know about providing the complete raw data @copper laurel @bitter peak

where else do we do this

copper laurel

Well we dont, but whats the point of this otherwise

wild flax

Also its hilariously painful to work with

copper laurel

We were providing "all" the data in the Collection too

It had just been parsed into a different format

wild flax

What is the point of what?

copper laurel

Okay let me rephrase - what exactly are we intending to provide in interaction.options.data

If its not the raw data, then what it is?

wild flax

whatever _options is now

and whatever it was before where you just called options.values()

its perfectly parsed data, thats not entirely raw, but very easy to work with

and gives you all the freedom you want

if you want raw data, use the raw event

copper laurel

Right, I didnt see that we were still transforming the options, but still

Before the resolver PR, it was a Collection<string, CommandInteractionOption> of all parsed options, with subcommand data un-hoisted.
Now, in the current master, _options is CommandInteractionOption[], still parsed, no longer a Collection, and with the subcommand data hoisted up, since those have moved

So ignore the fact that I said raw - should it be with or without the sub-command hoisting is the question

wild flax

maybe I just accounted for that when parsing it myself, but I noticed no difference in the data given

so I'm not sure where hoisting or not hoisting is a problem here

You can always check the type

copper laurel

The type is the same either way

wild flax

Oh right, looks like my parsing just ignores it, since its hoisted now

bitter peak

You can also still get the subcommand using try/catch, if nothing else

copper laurel

Should it be

data = [{
  type: "SUB_COMMAND",
  name: "sub1",
  options: [{
    type: "STRING",
    name: "string",
    value: "foo"
  }]
}]

OR

data = [{
  type: "STRING",
  name: "string",
  value: "foo"
}]```
And make people use getSubCommand() to check subcommands
wild flax

Well I don't see a problem with that, it makes it so you don't have to write recursion

That seems like a win to me

copper laurel

The latter?

bitter peak

I vote for the latter

copper laurel

k

Thats probably fine then

I have no sympathy for people who cry about an error being thrown when they do something erroneous

wild flax

aaaactually

it did break my parser

I just didn't seem to notice

bitter peak

What part broke?

wild flax

Yeah thats what I'm about to find out

Ah yep

its the subcommand part

copper laurel

yeet it

wild flax

the easiest way would be going back to the nested approach

bitter peak

I already closed the discussion, we're sticking with the PR as is, right?

Kinda getting both signals here

wild flax

Considering my whole parser exploded from the hoisting

making it impossible to detect if its a subcommand/subcommandgroup without using the resolver

its a bit of a problem

presume you have something like

restrict
 -> mute
 -> embed

interaction.commandName will be restrict

but you will never find out which subcommand it is

with using interaction.options.data

bitter peak

Well, we kinda figured above that you can just use try { commandName += ${i.options.getSubCommand()}; } catch {} or something a bit more elegant

oops formatting

wild flax

using .options.data should allow you to completely opt out using the resolver methods

bitter peak

So in that case you agree with the former of the two suggestions monbrey posted?

wild flax

Yes, basically reverting to the format it had before it was removed/modified from being a collection

bitter peak

Alright sounds good

Also, it should be an array yeah?

Not a collection

wild flax

So basically the format it had when it was a collection before the resolver format, but as an array, correct

that should alleviate all our problems then

bitter peak

Alright, I'll change that this evening

I'll make .data a getter maybe

wild flax

are we really scared people will modify it?

I don't think we should

oak quail

imo options.getMentionable is pretty weird, since it has so many return types

meanwhile there are 2 methods just for user types

copper laurel

Yeah thats what I meant, I didnt realise we were still transforming options when I said "raw"

copper laurel
steady salmon

are we still waiting for threads to be finalized before releasing v13? it seems that they aren't gonna be fully coming out anytime soon from dapi

outer night

the last thing we want is to release another major version because discord decided on changes to their threads api

oak quail

why are getSubCommand and getSubCommandGroup functions 🤔

they just return a property, cant the property just be exposed

wild flax

We’ve been over this so many times now

smoky bridge

Is the 13.0.0-dev version collection dependency going to be updated?

clever crypt

would it be possible to get a build branch on builders similar to some sapphire repos?

tacit crypt

It's probably possible to setup an @dev / @next like d.js/-types

bitter peak

@wild flax What if I made getSubCommand() and getSubCommandGroup() have (required=true) so people can do whatever they want if they opt-out

I think that keeps the best of both worlds

wild flax

I explained why thats a problem

Theres situations you don't want to/can't use it

where you wanna work off the parsed data

bitter peak

It would be in addition to having the .data property tho

wild flax

all youd need to do is de-hoist the sub commands and we should be good

bitter peak

Right but I'm asking if I can do both

wild flax

I dont wanna bring in anther change thats unrelated to this pr

bitter peak

alright

wild flax

if you feel its a good change, maybe make a second pr

bitter peak

👍

bitter peak
sacred river

idk if its a discord thing or discord.js thing

but

maybe add a button style called like

WARNING or somethin

and its yellow

copper laurel

Discord

So no

We can only give you the styles they provide

sacred river

ah

i figured

im prob one of the very few thats asked that

oak quail
sacred river

Ah

Ok

opaque vessel

Hi, I notice discord.js doesn't allow us to edit messages with empty strings. Why is that? Bots can edit messages with an empty string if there is an embed

clever spoke

because you should be editing content to null, no?

opaque vessel
raven juniper

works fine for me

opaque vessel

I've tried it a dozen times on latest master and I get greeted with errors all the time (:

Simply removing the check discord.js has on Util.js on line 407 makes the edit

But that's not the fix either T_T

raven juniper

To be fair, I'm not on the latest commit, I'm still pre Id change, but I don't think the content has been changed since then

opaque vessel

Okay so I learnt that you have to pass null instead of an empty string so it'll edit, and that'll make allowEmpty true, happy days

Should it not be "string or null" or something here then

oak quail

it ideally should accept an empty string too

it used to work but broke a while ago

discord accepts empty strings tho

opaque vessel

Yeah it looks like discord.js' utility method for verifying a string will yell at you for supplying an empty string, and it is used for checking the content when editing a message and sending a message. Looking closely, it seems that if we pass null to editing content, discord.js makes the content an empty string and skips validation of if it's empty. So that's why shrugs

We're sending across empty strings, just it's transformed from null to one

wild flax
opaque vessel
wild flax

There’s no reason for it not to be an array

And data.options doesn’t return a collection

opaque vessel

Huh, it says it does

I haven't tried it yet, was just reading the new documentation on it (': Will test it out to see

Ah yeah it returns an array but docs say a Collection, odd. In that case I'm fine with it I guess, but I guess docs should be updated. Typings seem to be done already

loud jayBOT

pr_open #6169 in discordjs/discord.js by Jiralite opened <t:1626952120:R> (review required)
docs(CommandInteraction): Correct type definition of CommandInteractionOption
📥 npm i Jiralite/discord.js#patch-1

bitter peak

@tacit crypt the overloads are reachable — I specifically wrote a test case for them in index.ts

tacit crypt

huh?

bitter peak

In the option resolver PR

tacit crypt

Just saw, 👍

zenith oracle

Can the defer method return a Message? Because on the docs options don't include fetchReply but typings do...

strange igloo
zenith oracle

Mmh, but it's only a defer... if I use fetchReply, what Message will be returned?

wild flax

thats because uh

sometimes you need to do:

await interaction.defer();
const message = await interaction.fetchReply();

with the option you can do:

const message = await interaction.defer({ fetchReply: true });
strange igloo

Getting this from a defer with fetchReply - so it appears to be working

wild flax

its basically opt-in auto-fetch

zenith oracle

Ok, so it's only an error in the docs

Thanks!

oak oracle

what is license of discord.js?

strange igloo
bitter peak
tacit crypt

It's something our docs support, iirc its just <info>OwO</info>

bitter peak

Or would that go in the method description not the param

tacit crypt

That looks right but i don't know if we DO need to document that... @wild flax thoughts?

wild flax

Ye

I wouldn’t

Whatever really

bitter peak

ok that's even easier

resolved

btw are there plans to go through and close all the old now-invalid issues?

wild flax

Which?

Lol

bitter peak
wild flax

First one is def not fixed

bitter peak
wild flax

Maybe, we got no feedback

I’ll prolly close some of them for sure

But it’s not too needed rn

We aren’t in a position of 2 months ago or so with 90 open issues lol

bitter peak

fair enough

strange igloo

@cerulean swan this here's why it may return null
See ChannelManager#_add behavior when Channel.create returns undefined

supple current

will msg.channel.send("hi") be deprecated soon?(talking about on how u send messages)

clever spoke

not afaik

copper laurel

why would it

sharp wigeon

does message.channel.post sound better?

I guess since message.channel.send is so embedded now, it wouldn't make sense for a change

wild flax

post OMEGALUL

copper laurel

No, it does not sound better

real jetty 
(node:13788) DeprecationWarning: The message event is deprecated. Use messageCreate instead
(Use `node --trace-deprecation ...` to show where the warning was created)

how does it detect where I used the message or interaction events instead of the messageCreate or interactionCreate events?

oak quail
strange igloo

If you don't understand the magic behind, EventEmitter#emit returns true if there were listeners attached to the event, false otherwise

oak quail

assumed so, ty for confirming

haughty flame

hello, so guild.owner is nullable and have to fetch it manually right ?
why dont djs automaticly fetch the ownerId and return fetched owner object to guild.owner ?

raven juniper

the library should not be making api calls for the user like that. you should be the one initiating it

haughty flame

why not ?

is it cause it takes 2 api request and could be a spam ?

strange igloo

Because you probably don't need to know who is the guild owner
Would be useless to fetch this data if you don't care

slate nacelle

This has been addressed by replacing that getter with Guild#fetchOwner, which will fetch them from Discord if necessary.

haughty flame

aaa okay

zenith oracle

The ClientUser class is missing the readonly presence getter, which is documented and it relies on Client#presence

unique axle
zenith oracle

Right

It should be readded in the ClientUser class now, so?

unique axle

why?

zenith oracle

Because it exists actually? 😂

copper laurel

I'd say it should be removed if anything

Not sure what its supposed to be doing

zenith oracle

Yeah, it's actually just a shortcut to the Client#presence property

unique axle

yeah, imo that getter on ClientUser should go away

because Client#presence is quite something different than the guild propagated GuildMember#presence

also... isn't that a ClientPresence vs. a Presence anyways? nkoHmm right, but doesn't really matter, because that extends Presence... weird spaghetti there

zenith oracle

Wait, why is Client#presence private? Thonkang

copper laurel

because you should be using guild.me.presence

vivid field

But you can also have a bot that isn't in any guilds, in that case you can't access it from guild.me

copper garden

Internal sharding allows a client to yield multiple shards with different presences

uneven rain

so everytime the client starts, the lib caches a amount of users, do u guys fetch a random amount of users to do that or wut because i have a array of user ids and i want to fetch each user but that'll get me rate limited if i use a loop

unique axle

structures are cached whenever we receive them via websocket payloads. users are included in a bunch of payloads that might cause a user to be cached

uneven rain

hm so is there anyway to fetch a array of user ids without getting rate limited

haughty flame

why dont you guys defaultly set button style to PRIMARY if there is no Button.setStyle setted ?

wild flax

why would we

haughty flame

i think its better that way

wait..
if it defaultly setted to primary
someone would think style cant be changed

nvm then

upper moss

hello i found a bug with TextChannel#bulkDelete but it is very unconsistant and i replicated it once(50 tries in same code and only 1 times) https://sentry.io/share/issue/f588648f85a54b439a6cbfb7dfcbeb72/#exception i can give you breadcumbs of sentry(all http requests node procces has made) if you want too i didnt want to make an github issue because as its so unconsistant it can be a code issue as well

wild flax

That doesnt really tell us anything

Also this seems a rather odd... usage:

        await message.util.send("abi ğ", true);
        await message.channel.bulkDelete(4, true);
        await message.channel.bulkDelete(1, true);

what do you expect here to happen? lol

sharp wigeon

I’m kinda leaning towards the idea of setting that by default but idk

upper moss
copper laurel

@haughty flame @sharp wigeon its not the job of discord.js to decide what the "default" style of a button should be. The API doesn't have a default, neither do we

sharp wigeon

it should be alright

gaunt lotus

Not sure if this is where I should be asking this but, if I understood correctly and <Collection>.get() returns a reference to the object in the <Collection>, and every operation I do with that object applies to it in the <Collection> aswell, how can I create a "copy" of a <Collection>'s object, to which I can perform operations without affecting the original object in it?

sharp wigeon

Collection.clone() ? iirc, but I am probably wrong

unique axle

well, if the question is how to make a copy of a single entry element, yes, that's wrong

sharp wigeon

yep

unique axle

copying structures isn't really a topic for a library development channel
whether or not an entry is stored as value or reference depends on if it's primitive or not, non-primitive values would need to be deep-cloned if you want to not modify the stored structure (sometimes via Object#assign, sometimes preferably by using the structure's constructor, largely depends on the structure)

gaunt lotus

Okay thank you and sorry for asking it in the wrong channel

copper laurel
sharp wigeon ~~it should be~~ alright

No, it shouldn't. Libraries should not be opinionated and make these decisions on behalf of developers. We correctly reflect Discord's behaviour, thats all.

sharp wigeon

It was a joke 😦 but yeah I understand

copper laurel

Then I'm going to warn you again to keep this channel on topic. Right there in the topic, "serious discussions"

sharp wigeon

si senor

wild flax

@oak quail I disagree with you on the options

Making a new property object with arbitrary values on it got resolved “targets” with no resolver is way worse

Context menus are just slash commands with no args

They still give you resolved values though

bitter peak

If they add more options in the future, that'll probably be a major semver anyways, so I don't think we need to account for that rn

wild flax

It also splits up parsing logic lol

copper laurel

It wont be major with the current implementation though in theory

oak quail
copper laurel

Cant access that link, so no idea what youre saying

bitter peak

same

wild flax

Yeah I don’t see how that’s relevant

What does that have to do with options?

bitter peak

crol can you repost

wild flax

Why does it matter?

No I can’t

oak quail

well they said that they might put those in the options field

wild flax

I don’t know what the availability of this info is, so I can’t

oak quail

so wouldnt that conflict with the fake options

wild flax

No why?

You’d just write logic to support that additionally

Since that feature won’t only be on context menus

Thatd be kinda silly

bitter peak

Aren't underscores invalid in option names? If so, I see no reason why not to use a fake option with an underscore

wild flax

The naming restrictions might get lifted anyway

Also options with an underscore don’t make too much sense

bitter peak

wdym?

oak quail

i just dont like the idea of making fake options

discord gives it as a separate property

wouldnt this basically be like storing channel metadata in fake messages

bitter peak

maybe interaction.options.getTarget()?

To match the rest of that class

wild flax

Target what

bitter peak

hm

wild flax

User and Member is still separate

bitter peak

so getTargetMember/User/Message?

wild flax

Terrible

Just terrible

Also it’s our job the make this bad interface discord gives us in a good one

oak quail

that would be much better than getMessage('message')/getUser('user') and getMember('user') imo

wild flax

And if that means “fake” options, I’ll take it

How so

smoky bridge

Are context menus kinda like buttons but for use on members and users rather than messages?

bitter peak
wild flax

Bruh, it’s a context menu

What’s there to explain

You right click

smoky bridge

Yeah but there are many types of context menus

oak quail
smoky bridge

Ones on messages, guilds, members on member bar, on user profile, channels, categories

wild flax

Yes there’s currently ones for users and messages

smoky bridge

Yeah that’s what I meant

bitter peak

Will this be handled by a ContextMenuInteraction class or reuse the command one?

smoky bridge

If there are no arguments like how buttons work, why not treat them like buttons?

oak quail
smoky bridge

Instead of additional fake options

oak quail

so like interaction.message?

bitter peak

Maybe if we had it as a separate class this would be easier. We could have interaction.targetType = member | user | message etc. and interaction.target

wild flax

No

That’s awful still

It’s not member OR user

It’s member & user OR user

smoky bridge

Yeah that makes sense

wild flax

That’s just type fuckery then

copper laurel
smoky bridge

But you could do member.user

copper laurel

Thats literally why drafts arent ready for review

Its going to be subclassed

smoky bridge

Yeah

wild flax
copper laurel
smoky bridge

Oh ok true

copper laurel

Everyone making opinions based on code that is going to change anyway

smoky bridge

Forgot for a sec that GuildMember was its own thing

wild flax

I think we have a pretty good idea how to move forward with it ourselves

bitter peak

I'm just struggling to think of ways to implement this, I only skimmed the code in the PR

wild flax

And it will 100% involve the resolver

We aren’t going to not use it

bitter peak

oh yeah sorry that PR for the resolver was so messy

copper laurel

The resolver is fine tbh

smoky bridge

Yeah all good it looks like you have a clear idea anyways from what you were discussing. I’m just confused what the fake options thing was about in the discussion though

oak quail

the resolver is good except for the MENTIONABLE handling imo

bitter peak

What's the issue with mentionable?

oak quail
  • getUser: User
  • getMember: GuildMember
  • getMentionable: User | GuildMember | Role
bitter peak

Isn't that literally what the docs say tho

wild flax

mentionable gets resolved into one of those anyway

They don’t put it under resolved.mentionable

Lol

copper laurel

I think the argument is that getUser() should work on mentionable too

bitter peak

On a role?

wild flax

Wat

bitter peak
  • getMentionable: User | GuildMember | Role

I think the argument is that getUser() should work on mentionable too

oak quail

for USER args its like "use the method for the object you want" and for MENTIONABLE args its like "fuck what you want, i'll just give you something"

wild flax

I mean I guess I get it. You can’t know what it is, so you need manual checks anyway

Not sure what’s there to improve

copper laurel

well no, not for role

getRole() should exist for that

or something

wild flax

If you call getUser you are kinda fucked

bitter peak

But how do you know what they put?

wild flax

You don’t mmLol

oak quail

check which resolved object its in?

copper laurel

you dont, but you never had an issue with resolvers returning null/throwing before now

wild flax

Advaith

How do you as the dev know

You give them a choice

You have no idea without checking yourself

You can’t just call getUser and know they supplied a user lol

They might as well supplied a role

bitter peak

The way it is rn is 100% typesafe, any change would also need to be as such

copper laurel

The only real usecase I can think of for MENTIONABLE is something like permission overwrites, which means it doesn't matter which of the two it is

Otherwise the dev should have asked for a User anyway

wild flax

X

Souji has some weird commands that use mentionable

And based on the type do different things

copper laurel 
getUser(name, required = false) {
  const option = this._getTypedOption(name, ['USER', 'MENTIONABLE'], ['user'], required);
  return option?.user ?? null;
}
``` I think this is the suggestion right?
bitter peak

Didn't that array get converted to a string

wild flax

We can allow that, but I don’t think it’s the best approach honestly

oak quail

does souji use getMentionable?

copper laurel

This isnt the real code, its a mockup of the suggestion

wild flax

It feels like this will backfire on a lot of people

copper laurel

Which is why its an array

bitter peak

That would also be confusing to document IMO

oak quail

so how exactly would you use getMentionable

instanceof checks?

bitter peak

most likely

oak quail

just using get() and then checking the presence of .role or .user would probably be cleaner tbh

bitter peak

I don't think you can check for key presence in typescript

oak quail

what

why wouldnt you be able to

bitter peak

oh nvm

I remember it erroring with something like Property "username" cannot be used on type User | Role, Property "username" does not exist on type Role for example

but I guess it doesn't

copper laurel

Thats different, where one type definitely cant have the property, while on a resolved option both are just nullable

honest barn

shouldn't this pr be in the done section?

solemn oyster
honest barn

this one is in the in progress category too but it should be moved to review in progress

solemn oyster

That's automatic

honest barn

i see

vernal atlas

if interaction.options.getMember(...) can be a APIInteractionDataResolvedGuildMember how are you meant to get its ID, since there is no id property on that type 🤔

wild flax

probably by grabbing the user

or does that not have an id on it either

vernal atlas

probably not, lemme check

ah well the user does return a full user object

so that would have an id

should the internals assign the ID to raw member data though? for convienence

wild flax

no

lol

vernal atlas

🤷‍♂️

outer raven

Would it make sense to have a MessageEmbed.setFields() method? I saw someone talking abt this in a support channel earlier today and it would be relatively easy to make and really helpful as well

Could also add a shortcut for setting 1 field called .setField that would work like addField but would replace all existing fields

raven juniper

MessageEmbed#spliceFields should already cover this, no?

outer raven

well yes and no, that applies to when you wanna remove a specific field or replace it (although you can extend this to all fields). These methods would completely overwrite and seem easier to use

copper laurel

splice is just a more powerful version of that

outer raven

yeah but there's a set method for every embed property, why not fields too?

raven juniper

setDescription, etc is because there's only one of those per embed, not so for fields

outer raven

Yeah but I feel like if you wanted to overwrite all your fields this would be an easier way

ill make the PR and leave it up for the reviewers to decide

copper laurel

okay then ¯_(ツ)_/¯

void rivet

I thought about it too, like a resetFields or something, but like you can use Embed.fields = [] or assign it your array of fields?

copper laurel

embed.spliceFields(0, embed.fields.length, newFields)

bitter peak

Yeah, would be best to maintain the reference to the array with .splice

outer raven
bitter peak
outer raven

but there's no difference if you wanna overwrite all fields right?

bitter peak 
const fields = embed.fields;
embed.removeAllFields(); // this.fields = [];

fields.push({ /* ... */ }); // fields is now not the same variable
unique axle

nkoHmm embed.fields.length = 0 done
why would we need an api for that

copper laurel

souji what

why would you overwrite the length property of an array

kindred yoke

lmao

wild flax

Because thats how you remove all elements from an array

MessageEmbed.prototype.resetFields = function() {
  this.fields.length = 0;
  return this;
}

Here, make some discord.js-utils package

mmLol

copper laurel

awful awful language

outer raven

discord-api-types 0.18.1 has no deprecation message, is this intentional?

copper laurel

Im not sure what you mean exactly

Where/why are you suggesting it needs one

wild flax

Just bother vlad about it

oak quail

is this intentional?

could use interaction.channelId but its annoying that you can't just do interaction.channel

copper laurel

Seems intentional to me, even if Channel was made stricter you still wont be able to pass PartialDMChannel to that method

tacit crypt
outer raven

lmao aight
btw it should probably be bumped on discordjs/builders since that's where i got that warning from

tacit crypt

Pretty sure I bumped it already, just that @wild flax didn't publish yet

tacit crypt
outer raven

yeah I see, just hasn't had a release in almost a month, ty!

tacit crypt

I was referring to the deprecation messages

outer raven

ah ye i see that too, noice

grizzled rover

I was looking at Message.embeds in master and I saw that it was an array but in almost any other array-like property it is used as a collection -- is this intentionally an array or just not decided?

slate nacelle

Intentional, since there is nothing to key them by.

bitter peak

mock unitintegration testing

real jetty

I don't know if it's could be useful but maybe we can Improve typing by adding something like that :

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

export abstract class Application<Fetched extends boolean = boolean> extends Base {
  public constructor(client: Client, data: unknown);
  public readonly createdAt: Date;
  public readonly createdTimestamp: number;
  public description: If<Fetched, string>;
  public icon: string | null;
  public id: Snowflake;
  public name: If<Fetched, string>;
  public coverURL(options?: StaticImageURLOptions): string | null;
  public fetchAssets(): Promise<ApplicationAsset[]>;
  public iconURL(options?: StaticImageURLOptions): string | null;
  public toJSON(): unknown;
  public toString(): string | null;
}

export class ClientApplication<Fetched extends boolean = boolean> extends Application<Fetched> {
  public botPublic: If<Fetched, boolean>;
  public botRequireCodeGrant: If<Fetched, boolean>;
  public commands: ApplicationCommandManager;
  public cover: string | null;
  public flags: Readonly<ApplicationFlags>;
  public owner: If<Fetched, User | Team>;
  public readonly partial: boolean;
  public rpcOrigins: string[];
  public fetch(): Promise<ClientApplication<true>>;
}```
wild flax

Didn't we already have this discussion a few times lol

bitter peak

For Client

But not for application

real jetty

Ha maybe but I don't remember the answer XD

vernal atlas

because they are only ever used as a type

outer raven
ruby terrace

Yeah, its an error on the site side, I mentioned it a bit ago. It doesn't like the two warn tags in the same description

outer raven

could it be because they're inline with the duration or is it unrelated?

oak quail

should we make ApplicationCommand#description nullable now, so there isnt a breaking change later?

oh nvm, i forgot they'll change it

oak quail

so is v13 gonna release requiring the CHANNEL partial for receiving DMs or is that gonna be changed?

copper laurel

I raised the same, but there isn't exactly a nice way to not require it without introducing the possibility that message.channel is a partial on every message

oak quail

is that so bad

copper laurel

The change would be something similar to how interactions handles it with interaction#channelId and get channel() I guess, however right now without the CHANNEL partial its just null

We could do that, and if the partial isnt enabled, the channel is null on message

Or drop it entirely, and just include that as a type I guess?

ornate topaz

according to guide, partial structures are to be assumed not working, so technically message.channel.send couldn't be used without fetching

copper laurel

Probably would work though since it has the id

vagrant holly

resolve isn't much better either, but does interface with the cache at least

copper laurel

It's not a helper at all, both are used quite a lot internally

vagrant holly

The reason I bring it up is because it came up in a convo I was having with someone where they were expecting it to be able to resolve way more in terms of types

copper laurel

Could argue for making them private _resolveId

Not sure what they thought it would do though

vagrant holly

nod That'd make sense to me if they're needed internally

They were expecting it to be able to resolve strings to snowflakes, but the typings don't allow that

In most managers, its literally either is a no-op on the snowflake you pass, or returns the snowflake for an object

copper laurel

I mean, I don't think it's documented to suggest it does that anywhere

Because it doesn't accept strings

vagrant holly

Its not, for sure, but when you see resolve one might expect it to able to resolve more types

Cause as an end user, it feels very pointless as a function in its current state

copper laurel

I mean sure, but if it feels pointless don't use it?

I don't think we really ever encourage it's used in guides or anything

vagrant holly

Oh yeah for sure, just somewhat feels like a trap to have it there given how little it does

Making it private feels sensible to me

unique axle

agree, #resolve is rather useful as .cache.get(..) alternative, but resolveId was even confusing when i first looked into it because it just forwards strings and such
private seems quite fitting for those, tho if not documented that might actually be private enough as well

opaque vessel

Hi hi! Quick question, I notice the documentation uses "subcommand" and "sub-command" in places and to save my sanity, I wish to make these consistent (the same) so... should we be saying "subcommand" or "sub-command"? It should also match the guide's preferred style, but I don't know if there is one yet (or a preview of one)

unique axle
opaque vessel

Nice nice, thank you! As a sidenote, now I'm gonna be haunted by this 1 small result (':

unique axle

time to PR to dapi i guess kek

opaque vessel
unique axle

oh, right, yes

tacit crypt

@wild flax @opaque vessel related to your PR, I don't know how to feel about the rename of SubCommand to Subcommand

wild flax

Its 1 word

tacit crypt

No it ain't

warped crater

sub isnt a word though

opaque vessel

Well, I also don't like seeing "sub-command" and "subcommand" being used in several places in the documentation :thinking:

warped crater

subcommand is a single word

wild flax

Yes, it is one word.

tacit crypt

sub command 02shrug

opaque vessel

We have to pick one, so uhm... which one do you all want to agree upon then?

wild flax
tacit crypt

Oh good more breaking changes in -types

warped crater

fuck

that said - this does make more sense to me so im for it nonetheless

unique axle

nkoHmm

tacit crypt

Jiralite, for free PR, discord-api-types, SubCommand -> Subcommand

Tho..

Discord types it as SUB_COMMAND

So

warped crater

not sure how thats ever been a concern?

other than properties that reflect raw data of course

wild flax

Don't care much about types, they can stay if need be, but the word itself

tacit crypt

Its a concern because usually converting from snake case to camel or even PascalCase, every immediate letter after a _ gets uppercased

wild flax

So for stuff like Routes.registerSubcommands we should change it

And yeah, I would just go with Enum.Subcommand

tacit crypt

But that's inconsistent with discord

wild flax

The enum doesn't even represent discords data

Its literally just enum convention for enums

warped crater
tacit crypt

Talking about their docs/clients, where they use upper_snake_case, we use PascalCase in -types so... I'd rather stay consistent with that there

In docs sure

wild flax

It's the same when using Permissions.UseSlashCommands not representing USE_APPLICATION_COMMANDS

lol

tacit crypt
wild flax

Yeah but I'm just saying

its just a name

It doesn't have to be a 1:1 mapping, esp not when its about proper english

opaque vessel

At least for the documentation, since Discord documents it publicly as one word, we should use one word there
For the internal methods, I don't think that really matters
For methods that are not private... guess that's the tricky one T_T

wild flax

I'd just go with Subcommand and call it a day

It's not gonna break anyones neck, nor confuse anyone

upper moss

isnt #6192 need to be a semver major as it changes parameters of the constructor or i am mistaken

outer raven

Any reason why a ThreadChannel doesn't have a fetchOwner method?

tacit crypt

Is there data that comes from the API that points to the owner?

outer raven

yeah there's ownerId

can be missing tho somehow

i don't think it should be marked as nullable

ruby terrace

its nullable

opaque vessel
ruby terrace

interactions are annoying in that (partial channels)

outer raven

but you shouldn't mark a property as nullable if it's only missing in partial channels

wild flax

Yeah you should

tacit crypt
outer raven
wild flax Yeah you should

why tho? Something being partial means that all of it's properties can be missing (talking about the docs btw, not types)

wild flax

Because not everyone uses typescript

Lol

You get a struct as a partial, its not marked nullable on the docs, you assume its not

Now your application explodes

opaque vessel
outer raven

fair

but going back to the main topic, should i add a fetchOwner method to threadchannel?

ruby terrace

you're trying to fetch the member object for the owner right, cause you can't fetch the thread member

outer raven

well I didn't know that but in that case yeah the member object

Since we can't fetch members, should a getter be added for the owner thread member object that gets it from cache?

loud jayBOT

pr_open #6207 in discordjs/discord.js by ImRodry opened <t:1627509823:R> (review required)
feat(ThreadChannel): add owner and fetchOwner()
📥 npm i ImRodry/discord.js#thread-fetchowner

outer raven

did it anyway

wild flax

We generally don't have an owner prop anymore

not even on guilds

vernal atlas
outer raven

I know but that's because you can fetch the guildmember object of an owner. I decided to add it here because you can't do the same for thread members

wild flax

Seems kinda useless though

You would STILL need to do

let owner = thread.owner;
if (!owner) owner = await thread.fetchOwner();

lol

Might as well fetch in the first place

Fetch doesn't fetch anyway if its in your cache

outer raven

that's not really good practice to have two different types in a single variable

wild flax
vagrant holly
wild flax

No

vernal atlas

well ts thinks that djs exports the enums

but it doesn't

wild flax

Only on structures

not on the manager fetch method

vagrant holly

ah right

vernal atlas

so it would make sense to export them only as types

wild flax

I mean if you can fix it, without breaking everything

vernal atlas

i ask because im unsure why that wasnt done in the first place

wild flax

And them still being exported to be used as a type

We broke typings a lot trying to "fix" them

and it was never worth in the end

vernal atlas

ah

copper laurel

@outer raven I get your point about different types, but having both an owner and fetchOwner() property that return different things isn't any better. If owner is null, you'd fetch expecting to get that type... and then not get it

outer raven
copper laurel

RoleManager#fetch for example can't fetch a single role either, it fetches them all then gets the one you asked for

outer raven

I thought of that but didn't know you'd want that to be done. I'll update the PR with that and remove the owner getter

ruby terrace

I don't like that because it requires members intent to fetch a single member, which isn't something people are used to

outer raven

we can add that as a warning

wild flax

We don't really have any warnings based on intents

Thatd be very out of place

copper laurel

I mean we could go with the nullable cache getter too and not fetch

I just think having both, with those naming conventions, returning different types is yuck

wild flax

yeah but then its dumb too

the owner may be null and then you have to manually go with members.fetch() lol

and considering we alr removed the owner prop on guild to avoid the annoyingness of that

wed just back at square 0

outer raven
wild flax

Dunno, I don't think its too feasible currently

Lot of work, especially with v13 on the horizon too

Prolly needs to be another component on the docs website too

So we can do <Intent></Intent>

so its displayed nicely

outer raven

could add that later but for now a warn tag works doesn't it?

wild flax

So you can update the whole codebase?

Lol

outer raven

wdym

wild flax

Well its either all or nothing imo

Adding it now in one place is not the way to go

outer raven

i guess you could open an exception since this is a privileged intent

or just no warning at all

i got the code working, just can't test it cuz i dont have threads

copper laurel

Stemming of that exported enums discussion, Im looking at the fact that although we do document the exported typedefs in the Constants file, we don't actually document Constants itself, which means nothing in the docs suggests that people can do things like Constants.MessageButtonStyles.PRIMARY for example

Which is the alternative JS way, since the enums arent exported

bitter peak

I mean users could always do with (Constants) { /s

copper laurel

So I figured I'd document the Constants object, but then I run into a different issue - differentiating between the enum-like typedefs, and the ones which are just string arrays, eg Constants.MessageTypes which isnt one

bitter peak

Maybe an Enums object?

Enums.MessageButtonStyles.PRIMARY

copper laurel

Yeah, or I just dont document the ones that arent used in an enum-like fashion?

loud jayBOT

pr_open #6208 in discordjs/discord.js by monbrey opened <t:1627513177:R> (review required)
docs(Constants): document the Constants object for enum-like usage
📥 npm i monbrey/discord.js#constants-typedef

vernal atlas

the soloution for the enums could simply just be export const enum ...

wild flax

cant do that

we tried

vernal atlas

whats the issue with it ?

i tested compiling i didn't find any errors

wild flax
vernal atlas

oh

hm this one is very puzzling

copper laurel

Which is why I've now does the PR to document the Constants object, since thats the JS way of handling this

outer raven

Why does the <ThreadChannel>.delete() method accept a reason?

copper laurel

Because the channel delete endpoint supports it?

outer raven

Does it? I don't even see a log when I do that

the only thread delete logs I have are only from when I delete it

oak quail

is there a reason we dont put descriptions in typedefs?

wild flax

Presumably because most don't need any

tacit crypt

Would be nice to have since vsc can pull them in

wild flax

Typdefs != types

strange igloo

Mm.. looks like we can fetch guild previews even if the bot isn't in the said guild
Wouldn't that be convenient to have something like GuildManager#fetchPreview (or Client#fetchGuildPreview, no idea)? As of right now we can only fetch it through an existing guild instance
And if so, would the logic code behind go to this and Guild#fetchPreview would just call that new method, or should both keep their separate code logic (even tho there would be no difference?)

I'm gonna get glasses soon (:

opaque vessel
oak quail

lol what

@unique axle ^

wild flax

nvm

opaque vessel

I just found that out :D Managed to get it like that as so, will try to correct it :eyes:

loud jayBOT
outer raven

Shouldn't the <Message>.startThread method accept an object like <ThreadManager>.create does? This could create some issues if more parameters are added in the future

raven cloak

Are we getting a v12.5.4 to allow bots to read messages from threads?

currently they wont see messages and therefore commands and text moderation are no longer working inside of threads

Basically, people saying n word in threads to bypass chat filtering

ornate topaz

not possible really

events for threads are only sent on API v9

raven cloak

Damn

ornate topaz

v12 still uses v8, and it's not really possible to move v8 to v9 in a patch or a minor

raven cloak

Yeah that's understandable

solemn oyster
ornate topaz

eh, doesn't change the answer

and v7 is v6 anyway :)

solemn oyster

Yes, both are deprecated versions

While v8 is current and v9 is latest

empty viper

And what v9 adds ?

wild flax

Threads

outer raven
raven juniper
outer raven

was waiting for the person who replies to do that

ornate topaz

then you do it so it exists already instead of it not existing until someone decides to answer you

outer raven

fine

Possibly change Message.startThread to accept an object instead of 3 parameters

tacit crypt

@vernal atlas removing the assignment to this.target will break it more

vernal atlas

no?

i meant not assigning the promise itself to this.target

tacit crypt

Yeah, that

since the build method awaits all entry.target

and now you have a dangling promise thats never properly awaited

vernal atlas

oh mb

will fix

tacit crypt

Thats why I asked you if this still works meguFace

test it

vernal atlas

works

outer raven
vernal atlas

yeah probably

outer raven

aight, will do that

unique axle

i still don't see much if any utility in that method existing at all

outer raven
unique axle

then you can still do embed.fields = or set the length to 0 to clear it and re-populate it

outer raven

if you wanna go that way then why do all of the MessageEmbed methods exist?

obviously not saying they should be removed, they're just utilities that makes everyone's lives easier when building embeds and this is just another one

honest barn

i don't get why setFields would be useful

just make a new embed

vernal atlas

if im being honest i dont see the use case for it either

outer raven
honest barn

thats a good point

outer raven
honest barn

then you can still do embed.fields = or set the length to 0 to clear it and re-populate it

outer raven

if you wanna go that way then why do all of the MessageEmbed methods exist?

honest barn

hmm makes sense

yeah youre right

real jetty
wild flax

No, its a good change.

tacit crypt

Also not library related

outer raven

Shouldn't DataManager#cache be readonly on the docs?

pulsar dirge

nevermind my question i just posted, i was being dumb

sage frigate
outer raven

that's exactly what it does. Splicefields is used when you want to remove some fields. This method is a shortcut if you want to replace all of them

and spliceFields is not intuitive and new users are usually afraid to use this from what ive noticed

copper laurel

I disagree that it's not intuitive. It's pretty much exactly a basic Array method from core JavaScript

New users need to learn

outer raven
ornate topaz

they also don't do that for arrays, but i don't remember any Array.setValues, maybe apart from fill

spiral fulcrum

I've been trying to use makeCache option to reduce guild member cache. But it seems I can't set a lower value. As soon as you hit the limit, it removes the first cached member even if the member is the client. At that point most commands breaks. (e.g. can't check if the client has X perms).

dawn basalt

Why does interaction.reply() not return with the reply Message?

copper laurel

because discord doesnt give us the message data back

dawn basalt
spiral fulcrum

I don't think that's a good idea. This option was given to reduce cache cacheWithLimits({ GuildMemberManager: 10 }) but you can't set any lower value at all.

copper laurel

Just because you can doesn't mean you always should

oak quail

it makes sense to be able to limit it without it removing the client user

wild flax
strange igloo

No offense to whoever wrote that feature, but is it really a good idea to let people play with everything's cache limit? Looks like people are trying to reduce it at most without really thinking to how useful (and important) caching is, thinking also they can save a gigabyte of RAM having only 10 members cached at most? Thonk

Or at least, put a big warning in the guide against how delicate this feature is, and that only "advanced" developers should be playing with it

outer night

people have developed tons of third party packages just for the sake of cache manipulation so we're just giving them what they want in the main lib shrug shouldn't be playing with fire if you don't know it burns

spiral fulcrum
spiral fulcrum
slender kettle

Does someone know why the buttoncolours are named so unsual?

solemn oyster

wdym so unusual

strange igloo

That's how Discord calls them, not a decided by discord.js

Think they mean primary, danger etc instead of blue grey or red

bitter peak

yarn outdated flags dev tag as outdated

winged dust

shouldn't it be interaction.author.id instead of interaction.user.id ?
like msg.author.id

lethal storm

I was also wondering that, but I'm not very familiar with the naming thoughts in the API. For now I just use this message.author?.tag || message.user.tag

thin obsidian

Idk

strange igloo
outer raven

I think it's important to do that before v13 arrives, if we do

wild flax

just make a PR then

outer raven

alright, sure

wild flax
spiral fulcrum

It will, thanks.

honest barn

should i update the example in the readme to a slash command?

strange igloo

probably copy the example from the website* yeah

nevermind it's the same lol
i would have sweared to have seen an interaction example

vivid field

yes, the one from the website used interactions

it was just removed completely

strange igloo

prob because people thought it'd work out of box without any slash command creation? thonk

wild flax

Yeah

And it’s too long otherwise lol

So i at least removed It from the front page for now

loud jayBOT
honest barn

do i update the example in the website too

strange igloo

no that's actually intended to have reverted to message example
this example should then include the deploy code, but that would be too long

because that won't work out of box for a freshly created application
however client.on("message" should be turned into client.on("messageCreate" (on the website at least, readme looks fine)

honest barn

also, message based commands won't work after april 2022 without enabling the priviliged intent, so it wouldn't work for a freshly created application

wild flax

imo the example usage needs to handle creating a command too

otherwise its worthless

honest barn

but then it would be using a text based command again for deploying

wild flax

no, why?

honest barn

at least in the guide it does that by making a !deploy command

wild flax

I highly doubt that

honest barn

and making it create one on ready would be potentially bad for ratelimits if the bot is restarted a lot

strange igloo

that's nitpicking but message.content can be null

wild flax

Can it?

Anyway, scroll down on the guide

strange igloo

if that question was for me, yes if you just send a file it will be null

honest barn

hmm, i see it's a separate script for deploying

outer raven

Hey @solemn oyster can you undo that commit on my PR? I don’t think that should be changed

solemn oyster

Talk about timing

The change was requested for 4 hours and you reply 2 minutes after I make the change

outer raven

I literally just woke up

wild flax

It does fail the CI too though

solemn oyster

Gonna revert with force-push