Skip to content

Add into_* methods to Components#3467

Open
calum4 wants to merge 246 commits intoserenity-rs:nextfrom
calum4:component-ergonomics
Open

Add into_* methods to Components#3467
calum4 wants to merge 246 commits intoserenity-rs:nextfrom
calum4:component-ergonomics

Conversation

@calum4
Copy link
Contributor

@calum4 calum4 commented Jan 4, 2026

No description provided.

GnomedDev and others added 30 commits October 6, 2025 16:24
…erenity-rs#2646)

This avoids having to allocate to store fixed length (replaced with normal
array) or fixed capacity (replaced with `ArrayVec`) collections as vectors for
the purposes of putting them through the `Request` plumbing.

Slight behavioral change - before, setting `params` to `Some(vec![])`
would still append a question mark to the end of the url. Now, we check
if the params array `is_empty` instead of `is_some`, so the question
mark won't be appended if the params list is empty.

Co-authored-by: Michael Krasnitski <42564254+mkrasnitski@users.noreply.github.com>
These are unnecessary. Accepting `impl Into<Arc<T>>` allows passing either `T` or `Arc<T>`.
This trades a heap allocation for messages sent along with thread
creation for `Message`'s inline size dropping from 1176 bytes to 760
bytes,
…l models (serenity-rs#2656)

This shrinks type sizes by a lot; however, it makes the user experience slightly
different:

- `FixedString` must be converted to String with `.into()` or `.into_string()`
  before it can be pushed to, but dereferences to `&str` as is.
- `FixedArray` must be converted to `Vec` with `.into()` or `.into_vec()`
  before it can be pushed to, but dereferences to `&[T]` as is.

The crate of these types is currently a Git dependency, but this is fine for
the `next` branch. It needs some basic testing, which Serenity is perfect for,
before a release will be made to crates.io.
…enity-rs#2668)

This commit:

- switches from `u64` to `i64` in `CreateCommandOption::min_int_value` and
`CreateCommandOption::max_int_value` to accommodate negative integers in
Discord's integer range (between -2^53 and 2^53). Values outside this
range will cause Discord's API to return an error.
- switches from `i32` to `i64` in `CreateCommandOption::add_int_choice` and
`CreateCommandOption::add_int_choice_localized` to accommodate Discord's
complete integer range (between -2^53 and 2^53). Values outside this
range will cause Discord's API to return an error.
This cache was just duplicating information already present in `Guild::members`
and therefore should be removed.

This saves around 700 MBs for my bot (pre-`FixedString`).

This has to refactor `utils::content_safe` to always take a `Guild` instead
of`Cache`, but in practice it was mostly pulling from the guild cache anyway
and this means it is more likely to respect nicknames and other information,
while losing the ability to clean mentions from DMs, which do not matter.
`Embed::fields` previously had to stay as a `Vec` due to `CreateEmbed` wrapping
around it, but by implementing `Serialize` manually we can overwrite the
`Embed::fields` with a normal `Vec`, for a small performance hit on
serialization while saving some space for all stored `Embed`s.
Simply missed these when finding and replacing.
This uses the `bool_to_bitflags` macro to remove boolean (and optional boolean)
fields from structs and pack them into a bitflags invocation, so a struct with
many bools will only use one or two bytes, instead of a byte per bool as is.

This requires using getters and setters for the boolean fields, which changes
user experience and is hard to document, which is a significant downside, but
is such a nice change and will just become more and more efficient as time goes
on.
…rs#2681)

This swaps fields that store `Option<Int>` for `Option<NonMaxInt>` where the
maximum value would be ludicrous. Since `nonmax` uses `NonZero` internally,
this gives us niche optimisations, so model sizes can drop some more.

I have had to include a workaround for [serenity-rs#17] in `optional_string` by making my
own `TryFrom<u64>`, so that should be removable once that issue is fixed.

[serenity-rs#17]: LPGhatguy/nonmax#17
A couple of clippy bugs have been fixed and I have shrunk model
sizes enough to make `clippy::large_enum_variant` go away.
A discord bot library should not be using the tools reserved for low
level OS interaction/data structure libraries.
Discord seems to internally default Ids to 0, which is a bug whenever
exposed, but this makes ID parsing more resilient. I also took the
liberty to remove the `From<NonZero*>` implementations, to prevent future
headaches, as it was impossible to not break public API as we exposed
`NonZero` in `*Id::parse`.
…nity-rs#2694)

This,
1. shrinks the size of Request, when copied around, as it doesn't have
to store the max capacity at all times
2. shrinks llvm-lines (compile time metric) for my bot in debug from
`1,153,519` to `1,131,480` as no monomorphisation has to be performed
for `MAX_PARAMS`.
Follow-up to serenity-rs#2694.

When `Request::params` was made into an ArrayVec, the `Option` around it
was removed in order to avoid having to add a turbofish on `None` to
specify the value of `MAX_PARAMS`. Also, `Request::new` also needed to
be changed so that the value of `MAX_PARAMS` could be inferred. Now that
the field is a slice again, we can wrap it in `Option` again (at no cost
to size, thanks to niche opts).

We ensure we never store the redundant `Some(&[])` by checking for an
empty slice and storing `None` instead. This way, we ensure we never
read an empty slice out of the `Some` variant.
The instrument macros generate 2% of Serenity's release mode llvm-lines,
and are proc-macros so hurt compile time in that way, so this limits
them to opt-in. This commit also fixes the issues that the instrument macro
was hiding, such as results that didn't ever error and missing
documentation.
This signature is hard to use as `None` cannot infer the type of the
generic. I also replaced `Option<u8>` with `Option<NonMaxU8>` as it's
more efficient and will make the user think of the maximum value.
…nity-rs#2698)

This removes inefficient `IntoIterator` generics and instead takes what is
actually required. I also reworked `reorder_channels` to allow for keeping the
generic, as it actually does only just need iterator semantics.
Previously, someone assumed that `Ratelimiter` was going to be cloned, so
put a ton of `Arc`s everywhere. This was unneeded, and before dashmap,
so the buckets were also stored massively inefficiently. This fixes all
that.

I had to shuffle around the `Ratelimit` methods a little bit to return
their sleep time instead of sleeping themselves, so I didn't have to
hold a dashmap lock over an `.await`.
This removes multiple error variants and overall cleans up the codebase
by moving overflow checks into two `ModelError` variants.
Shrinks `size_of::<Error>` from 136 bytes to 64 bytes, while removing unused
variants. This will improve performance for any method returning
`Result<T>` where `T` is less than the size of `Error` as both `Result`'s
`Ok` and `Err` have to be allocated stack space.
The compiler knows best as inlining is quite complicated. This should
help with compile times, significantly.
mkrasnitski and others added 19 commits October 7, 2025 12:37
This improves the ergonomics of the CI workflows and cuts down on CI
runtime by adding a new `check` job for feature flags which only need to
be tested using `cargo check` rather than `cargo test`. I removed the
dedicated `nightly` job by including it in the `test` job.

Also added `temp_cache` to the list of features to check; for some
reason it wasn't included in CI before.
…ity-rs#3412)

Back in September 2024, Discord added [Soundboard][soundboard] APIs that allow
a bot to create a new sound by uploading raw mp3 or ogg data.

This data is specified as part of the JSON rather than a multipart upload (an
odd decision IMO); however, that means that the semantics of `ImageData` are
wrong as it makes too many assumptions about its contents. In particular, we
assume that the base64 data will always be an image, and the
`CreateAttachment::encode` method always assigns the `image/png` mimetype.

This commit removes these assumptions by renaming the struct to simply
`DataUri`, and now requiring the user to specify the mimetype when calling
`CreateAttachment::encode`.

[soundboard]: https://discord.com/developers/docs/resources/soundboard#soundboard-resource
…matting the link (serenity-rs#3415)

Users can still convert it to a `String` using `ToString::to_string`, but if they happen to pass it to a formatting macro such as `format!`, they will avoid a heap allocation that would have otherwise occurred for the link, which is the motivation for this change.
The documentation for the `ReactionRemoveEmoji` event was identical to that of
`ReactionRemoveAll`. This commit tweaks it to match its function more closely.
Change the `image` parameter of `create_application_emoji` to `DataUri`
instead of `&str`, similar to `GuildId::create_emoji`.
…#3431)

Rather than having the `ClientBuilder` accept `impl Into<Arc<H>> where H:
HandlerTrait`, directly accept the corresponding `Arc<dyn HandlerTrait>`.

This allows accepting non-concrete handler types (f.e. if constructed by a
function that returns `Arc<dyn HandlerTrait>` already) and fixes type inference
failures when passing an `Arc<H>` rather than `H`.

This affects the setters for `EventHandler`, `RawEventHandler`, `Framework`,
and `VoiceGatewayManager`.

`Framework` is in a bit of a weird spot because it is only put in an `Arc`
after `Framework::init` has been called, so a `&mut` can be taken to it for
that call, so this PR has `ClientBuilder` accept `Box<dyn Framework>` for it.

Most of the commit diff is related to fixing examples.
Labels are a new top-level component only supported in modals that can
hold a Text Input, Select Menu, or File Upload (another new component)
inside it. Oddly enough, the label and description are removed from the
label payload when sent back as part of a modal response. See the
[docs](https://discord.com/developers/docs/components/reference#label).

Text Inputs inside Action Rows are deprecated, therefore I removed the
`InputText` variant for `ActionRowComponent`. Also, at the moment modals
can include Action Rows, Text Displays, and Labels as top-level
components, so I changed modal responses to store `Component` instead of
`ActionRow` and added a `text` method to quick modals. I thought about
adding select menu support to quick modals but the methods felt a little
too verbose.
Follow-up to serenity-rs#3430. Adds an optional method to add a description to any
input texts in quick modals. Changing the signature of `field` to take
an `Option<impl Into<Cow<'a, str>>>` meant that passing `None` would
require a turbofish, so adding a new method makes more sense. Also fixes
extraneous testing code that got accidentally left in.
…erenity-rs#3453)

Provide better information about exactly what went wrong with an unknown
event. Sometimes it isn't actually a new event but rather an existing
event that failed deserialization for some reason or another. Keeping
the error message lets us log it for better debugability. Also, it makes
sense to preserve the full original payload rather than parsing it only
a little bit.
Only certain components can appear at the top level in messages. This changes
the definition of `Component` to reflect that by removing variants which cannot
appear at the top level and must be included inside another component, and
aligns the `Component` type with the `CreateComponent` builder.

Also, Sections and Containers can only contain a subset of component types, so
added a dedicated enum for those. The corresponding builders already exist.

Similarly for modal forms, only Labels and Text Displays can appear at the top
level (Action Rows are deprecated). So I added a `ModalComponent` enum which
reflects that, and also a `CreateModalComponent` builder.
Separate out virtual events from the rest of the full event logic to make it
clear which virtual events are possible and when.

Also get rid of `&mut self` in CacheUpdate as raw events do not ever need to
carry any mutable state.
@github-actions github-actions bot added the model Related to the `model` module. label Jan 4, 2026
@mkrasnitski
Copy link
Collaborator

Could you describe your usecase for these functions? You could definitely just match directly on the enum variant if you need to destructure a Component. Either way, similar to the destructuring methods on Channel, these should return owned types rather than references, and drop the as_ prefix. This way, any further operations do not have to go through an immutable reference, which might require extra clones.

@calum4
Copy link
Contributor Author

calum4 commented Jan 5, 2026

Could you describe your usecase for these functions? You could definitely just match directly on the enum variant if you need to destructure a Component.

Yeah absolutely, it's just unnecessarily verbose in my opinion. For an example of my use case:

fn foo(interaction: &ModalInteraction) {
    let mut components_iter = interaction.data.components.iter();

    // AS_*

    let _channel_id = match components_iter
        .next()
        .and_then(|c| c.as_label())
        .and_then(|c| c.component.as_select_menu())
        .and_then(|c| c.values.first())
        .map(|s| GenericChannelId::from_str(s))
    {
        Some(Ok(channel_id)) => channel_id,
        _ => todo!(),
    };

    // LET ELSE

    let Some(Component::Label(label)) = components_iter.next() else {
        todo!()
    };

    let LabelComponent::SelectMenu(select_menu) = &label.component else {
        todo!()
    };

    let Some(Ok(_channel_id)) = select_menu.values.first().map(|s| GenericChannelId::from_str(s)) else {
        todo!()
    };

    // MATCH

    let _channel_id = match components_iter.next() {
        Some(Component::Label(label)) => {
            match &label.component {
                LabelComponent::SelectMenu(select_menu) => {
                    match select_menu.values.first().map(|s| GenericChannelId::from_str(s)) {
                        Some(Ok(channel_id)) => channel_id,
                        _ => todo!(),
                    }
                }
                _ => todo!(),
            }
        }
        _ => todo!(),
    };
}

Either way, similar to the destructuring methods on Channel, these should return owned types rather than references, and drop the as_ prefix.

I agree with this criticism, expect another commit to implement this. In my use case I am working with a reference to said ModalInteraction and the thought hadn't crossed my mind.

@calum4 calum4 force-pushed the component-ergonomics branch from 0ecd5c3 to 4a71548 Compare January 5, 2026 14:44
@calum4 calum4 changed the title Add as_* methods to Components Add into_* methods to Components Jan 5, 2026
@calum4
Copy link
Contributor Author

calum4 commented Jan 5, 2026

these should return owned types rather than references, and drop the as_ prefix.

Note, rather than simply omitting the as_ prefix, I followed the convention outlined here and used into_.

If this contravenes an overriding convention for this crate, I'll drop the into_ prefix altogether.

Thanks!

@mkrasnitski
Copy link
Collaborator

One way of match you've left out is using let-chains, which does make the code less verbose:

fn foo(interaction: &ModalInteraction) {
    if let Some(Component::Label(label)) = interaction.data.components.iter().next()
        && let LabelComponent::SelectMenu(select_menu) = &label.component
        && let Some(value) = select_menu.values.first()
        && let Ok(channel_id) = GenericChannelId::from_str(value)
    {
        todo!()
    } else {
        todo!()
    }
}

In fact, I'd say I prefer this to chaining .and_then together. Ideally the additional (single) level of indentation this introduces would go away if the let-else and let-chain features could be combined, but in this case you could simply return channel_id from the block like you already do in your example.

@calum4
Copy link
Contributor Author

calum4 commented Jan 5, 2026

I had no idea let-chains existed! That's certainly significantly better than my let-else and match examples.

I'll leave this PR open regardless, but thanks again!

@mkrasnitski
Copy link
Collaborator

I had no idea let-chains existed!

They were only stabilized in Rust 1.88 in June, so have only been usable on stable for a little while.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

model Related to the `model` module.

Projects

None yet

Development

Successfully merging this pull request may close these issues.