-
-
Notifications
You must be signed in to change notification settings - Fork 4.1k
refactor!: remove AttachmentBuilder and support new file body encodables #11278
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
497294e
2207fa3
bf43681
77241ad
147dac5
3335298
7dbe98f
0eb9fb3
e6aef1e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
This file was deleted.
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -88,7 +88,7 @@ class TextBasedChannel { | |||||||||||||||||||||||||
| * @property {Array<(EmbedBuilder|Embed|APIEmbed)>} [embeds] The embeds for the message | ||||||||||||||||||||||||||
| * @property {MessageMentionOptions} [allowedMentions] Which mentions should be parsed from the message content | ||||||||||||||||||||||||||
| * (see {@link https://discord.com/developers/docs/resources/message#allowed-mentions-object here} for more details) | ||||||||||||||||||||||||||
| * @property {Array<(AttachmentBuilder|Attachment|AttachmentPayload|BufferResolvable)>} [files] | ||||||||||||||||||||||||||
| * @property {Array<(Attachment|AttachmentPayload|BufferResolvable)>} [files] | ||||||||||||||||||||||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'd prefer if there was still a way to pass an AttachmentBuilder into here, albeit not sure what type would make sense to put for that here. Another helper type like
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The main issue I have with this is that AttachmentBuilder includes data tied to.. well, the What do we do when the user sets that data and they also provide
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The same we did before: discord.js/packages/discord.js/src/structures/MessagePayload.js Lines 193 to 204 in 02fc101
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That is absolutely awful in terms of library behavior. That snippet effectively means that if you pass both In essence, this renders the
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. But that's what AttachmentBuilder does too... it hides the same things the same way.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No. This isn't about hiding things, it's allowing the user to easily pass garbage. AttachmentBuilder on its own is fundamentally different from what you want, it removes your granular control of the files/attachments fields, it ties them together completely. There is no way when using it to get non-sense miss-matched arrays. All the more so when you use MessageBuilder
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Circling back on this, the only way I'd accept it is making it a union type that forbids
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thing is that all the other types allowed for And you got it backwards: it's an accepted type for the
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sure. I'm okay with designing the API that way, but we do also have to acknowledge the complexity it adds |
||||||||||||||||||||||||||
| * The files to send with the message. | ||||||||||||||||||||||||||
| * @property {Array<(ActionRowBuilder|MessageTopLevelComponent|APIMessageTopLevelComponent)>} [components] | ||||||||||||||||||||||||||
| * Action rows containing interactive components for the message (buttons, select menus) and other | ||||||||||||||||||||||||||
|
|
@@ -156,7 +156,7 @@ class TextBasedChannel { | |||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||||
| * Sends a message to this channel. | ||||||||||||||||||||||||||
| * | ||||||||||||||||||||||||||
| * @param {string|MessagePayload|MessageCreateOptions} options The options to provide | ||||||||||||||||||||||||||
| * @param {string|MessagePayload|MessageCreateOptions|JSONEncodable<RESTPostAPIChannelMessageJSONBody>|FileBodyEncodable<RESTPostAPIChannelMessageJSONBody>} options The options to provide | ||||||||||||||||||||||||||
| * @returns {Promise<Message>} | ||||||||||||||||||||||||||
| * @example | ||||||||||||||||||||||||||
| * // Send a basic message | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
Uh oh!
There was an error while loading. Please reload this page.