RFC: Implement different select menus with a SelectMenuType enum

[archer-321]

Sorry if I missed this project's preferred RFC format. I searched CONTRIBUTING.md and previous RFCs but couldn't find anything. Instead, I went for a template that mostly matches (some parts of) Rust's RFC template.

Summary

Implement Discord's various select menu types (text, users, roles, mentionables, and channels) by moving type-specific data into a separate enum. This would move the options field into a new enum variant for text select menus, add a data field to the SelectMenu struct, and leave all other fields in the struct, as they apply to all kinds of select menus.

Explanation

This RFC introduces a new enum: SelectMenuData with the variants Text, User, Role, Mentionable, and Channel. This enum holds data specific to a single "type" of Discord select menu. Currently, this means it holds the options: Vec<SelectMenuOption> field of text select menus (which would be moved out of SelectMenuData) and channel_types: Option<Vec<ChannelType>> of channel select menus. All other select menus get a SelectMenuData variant without any fields.

To specify the type of select menu to create, the developer constructs the SelectMenu instance with the respective SelectMenuData variant. For variants without type-specific fields, this leads to the following code:

SelectMenu {
    custom_id: String::from("bar"),
    placeholder: Some(String::from("Select user")),
    min_values: None,
    max_values: None,
    disabled: false,
    data: SelectMenuData::User,
}

Existing code simply moves the options field into an instance of SelectMenuData::Text:

SelectMenu {
    custom_id: "foo".into(),
    placeholder: Some("Select text".into()),
    min_values: None,
    max_values: None,
    disabled: false,
    options: vec![
        SelectMenuOption {
            label: "Foo".into(),
            emoji: None,
            value: "foo".into(),
            default: false,
            description: Some("The foo".into()),
        },
        SelectMenuOption {
            label: "Bar".into(),
            emoji: None,
            value: "bar".into(),
            default: false,
            description: Some("The bar".into()),
        },
    ],
}

// Becomes...

SelectMenu {
    custom_id: "foo".into(),
    placeholder: Some("Select text".into()),
    min_values: None,
    max_values: None,
    disabled: false,
    data: SelectMenuData::Text {
        options: vec![
            SelectMenuOption {
                label: "Foo".into(),
                emoji: None,
                value: "foo".into(),
                default: false,
                description: Some("The foo".into()),
            },
            SelectMenuOption {
                label: "Bar".into(),
                emoji: None,
                value: "bar".into(),
                default: false,
                description: Some("The bar".into()),
            },
        ],
    },
}

Drawbacks

Especially when new to the library, users might look for a type (kind) field to "select" the type of select menu they're creating. With this design, they'll have to use the data field instead.

Moreover, users might look for individual structures like TextSelectMenu instead of a "partially flattened" SelectMenu considering the various select menu types have different type IDs (just like buttons have a different type ID).

Rationale and alternatives

The "partially flattened" approach outlined above combines the advantages of a single "flattened" structure with those of individual structures for the various select menu types:

• Fields present in all select menu types (currently, custom_id, placeholder, min_values, max_values, and disabled) don't get duplicated for every select menu type.
• Users can't accidentally access fields that aren't available for the current select menu type (e.g. users can't try to access options if they're implementing a user select menu).
• If Discord decides to "upgrade" the options field (or any other type-specific field) to contain type-specific data (e.g. allow options to contain a list of possible role IDs for role select menus), this doesn't require a breaking change to Twilight's API. Instead, the affected enum variant can contain the new field without affecting other variants already containing a field with the same name.

Alternative implementations include:

• having a separate struct for every select menu type. In addition to being more intuitive than the extracted data approach, this has the second and third advantages listed above. However, this approach duplicates fields, so it requires changes to all structs if the upstream API changes a common field.
• having a single "fully flattened" struct that implements all (possible) fields currently listed (by moving type-specific fields into an Option). This has the advantage that no fields are duplicated, but it's possible to (try to) access fields unrelated to the current kind of select menu. Moreover, this would require a type (kind) field to differentiate some types of select menus that have the same set of fields (e.g. user and role select menus). Finally, this doesn't provide the third advantage listed above (colliding fields with different data types) without requiring an even more customized (de-)serialization.
• having a flattened struct that moves some type-specific fields into a separate enum. For the most part, this has the same advantages and disadvantages as the "fully flattened" struct above, but this aims to protect against upstream field collisions by moving potentially colliding fields into an enum that, for now, only has a single variant. Moreover, when compared to this RFC's approach, this still doesn't prevent users from trying to access unrelated fields.

Prior art

Pull Request #2051 implements the separate struct approach mentioned above. Moreover, a review comment on this PR discusses the other possible implementations and their advantages. This comment also lists the Channel and Interaction types that were flattened for different reasons.

Finally, the current state of select menus is described in Discord's API docs.

Unresolved questions

As this RFC essentially makes data the main field that defines the select menu's type, the name kind or menu_type might be more appropriate. However, users might not expect this field to hold type-specific data, then.

Considering most data enum variants don't have any fields at all, it might be worth boxing the variants that have. Even though they currently only hold one field each, boxing them for future updates would be breaking change (for code that doesn't have to construct the variant's fields). However, this would require creating new structs for the individual variants.

The PR linked above implements the different select menu types in individual modules. Considering that the select menu module isn't particularly long, it might be more convenient to implement those types in the select_menu module directly instead of creating a new module and pub use for every new upstream type.

[laralove143]

i dont think this is limited to select menus, but itd define how we handle whenever some fields exist based on the kind field

[archer-321]

Do you mean this would define how we implement future message component variants (e.g. toggle buttons)? If so, I agree. It's best to implement all message components similarly.

[laralove143]

not even just those, for the most consistency i think we should apply this across all models, for example Channel, which used to be just an enum making it end up with many duplicate fields, this approach would remove/reduce the duplicate fields while also providing non-null safety

i also think it makes documentation easier since people don't have to skip through fields not available to them because of the channel type

[archer-321]

With #2219 merged, I believe we can close this RFC.

The PR linked above implemented the flattened approach for the reasons outlined in this comment.

[laralove143]

i still think we should make a more generic rfc defining the style for handling these cases across all types

[archer-321]

For now, I understand that the types throughout this library have only recently been switched over to flattened enums, so I wonder whether there's any demand for this kind of static enforcement.
In any case, a new, more generic RFC would be the way to go 👍
Unfortunately, I don't have the time to create one myself at the moment.

[laralove143]

i'll create one when i get around to it, and imo this has better benefits than compile time enforcements for example cleaner api surface