discordjs / discord.js

A powerful JavaScript library for interacting with the Discord API
https://discord.js.org
Apache License 2.0
25.4k stars 3.97k forks source link

ActionRowBuilder Constructor improperly merging provided components into new componentbuilders #10307

Closed MatthewFallon closed 5 months ago

MatthewFallon commented 5 months ago

Which package is this bug report for?

discord.js

Issue description

Currently observing this in the Action Row Builder specifically but likely will need to diagnose if this is from the ComponentBuilder super class as well causing this. It is not merging but instead nesting the data inside of the Component builders inside the ActionRowBuilder, steps to reproduce:

  1. Construct a new ActionRowBuilder using an existing ActionRow from an API call (i.e. received from an interaction and attempting to modify in place)
  2. observe that the components inside the new action row builder have a double nested data object rather than a merged one. (i.e. ButtonBuilder { data: { type: 2, data: [Object], emoji: null } })

Code sample

client.on(Events.InteractionCreate, async interaction => {
    if (!interaction.isButton()) return
    const rowWithDisabledButton = new ActionRowBuilder<MessageActionRowComponentBuilder>(interaction.message.components[0])
    rowWithDisabledButton.components[0].setDisabled(true)
    await interaction.update({components: [initialRowDisabledPlayer]})

})

Versions

"discord.js": "^14.15.2" Node: v20.12.0

Partial Dockerfile dump for docker version info:

FROM node:lts-alpine AS deploy
WORKDIR /home/node/app
COPY package-lock.json package.json ./
RUN NODE_ENV="production" npm ci
COPY --from=builder /home/node/app/build /home/node/app/

Issue priority

Low (slightly annoying)

Which partials do you have configured?

Not applicable

Which gateway intents are you subscribing to?

Not applicable, Guilds

I have tested this issue on a development release

29a50bb47

Jiralite commented 5 months ago

data is the data you set for the button builder, and data.data is the immutable data received from the API, if at all. API data is not supposed to be mutable. Otherwise, the representation of the received button would be incorrect. This is not a bug.

MatthewFallon commented 5 months ago

@Jiralite Thank you for the update, perhaps I'm misunderstanding the intent of the class constructor then. I am using Typescript so I am seeing the typings allowed to the constructor. Why does the ButtonBuilder class constructor accept ButtonComponent (based on it being API compliant) if it then creates an action row builder that cannot resolve without manually setting every field again? My general expectation was that if I provide the mutable ButtonBuilder class an immutable ButtonComponent class it would essentially clone that class in some manner to allow me to edit a given button before sending it back to the API.

Jiralite commented 5 months ago

API data is passed into the constructor, so it accepts the JSON payload.

If you want to edit a given button before sending it back to the API, you would use ButtonBuilder#from():

import { type ButtonInteraction, ActionRowBuilder, ButtonBuilder } from "discord.js";

declare const buttonInteraction: ButtonInteraction;
const { component } = buttonInteraction;

// We want to disable the button that was interacted with.
// First, we need to create a new button builder from the immutable API data.
const button = ButtonBuilder.from(component);

// Proceed as normal.
button.setDisabled(true);

// Make a new action row to put the button in.
// To reuse the action row, follow the same semantics as above.
const actionRow = new ActionRowBuilder<ButtonBuilder>().setComponents(button);

// Respond to the interaction.
buttonInteraction.update({ components: [actionRow] });
MatthewFallon commented 5 months ago

@Jiralite thank youuuu! sorry about the invalid issue then, 😅 I definitely missed that helper function. that rationale makes sense on why it is made that way then.