slack-go / slack

Slack API in Go, originally by @nlopes; Maintainers needed, contact @parsley42
https://pkg.go.dev/github.com/slack-go/slack
BSD 2-Clause "Simplified" License
4.63k stars 1.12k forks source link

feat: add 'EventsAPIInnerEventData' interface #1175

Closed smoya closed 1 year ago

smoya commented 1 year ago

Description

This PR adds a new interface EventsAPIInnerEventData that all inner events will fulfill. At this time, with only one method (EventType()) but open to grow according to https://api.slack.com/apis/connections/events-api#event_type_structure.

A very useful use case for this is reacting to those events from event handlers. I.e. :

type EventHandler func(slackevents.EventsAPIInnerEventData) error

Alternatives

There is no good alternatives for this. I tried using generics but the immaturity of it made me drop the idea since you cannot use interface with constraints as types (See the type parameters proposal). E.g. the following is not allowed:

type MessageEvent interface {
    slackevents.AppMentionEvent | slackevents.AppHomeOpenedEvent | slackevents.AppUninstalledEvent |
        slackevents.ChannelCreatedEvent | slackevents.ChannelDeletedEvent | slackevents.ChannelArchiveEvent |
        slackevents.ChannelUnarchiveEvent | slackevents.ChannelLeftEvent | slackevents.ChannelRenameEvent |
        slackevents.ChannelIDChangedEvent | slackevents.GroupDeletedEvent | slackevents.GroupArchiveEvent |
        slackevents.GroupUnarchiveEvent | slackevents.GroupLeftEvent | slackevents.GroupRenameEvent |
        slackevents.GridMigrationFinishedEvent | slackevents.GridMigrationStartedEvent | slackevents.LinkSharedEvent |
        slackevents.MessageEvent | slackevents.MemberJoinedChannelEvent | slackevents.MemberLeftChannelEvent |
        slackevents.PinAddedEvent | slackevents.PinRemovedEvent | slackevents.ReactionAddedEvent |
        slackevents.ReactionRemovedEvent | slackevents.TeamJoinEvent | slackevents.TokensRevokedEvent |
        slackevents.EmojiChangedEvent
}

type EventHandler func(MessageEvent) error // This is not possible with the current version of Go.
smoya commented 1 year ago

Hi @kanata2! Do you think this PR has a chance to be reviewed at any time soon? Thank you!

kanata2 commented 1 year ago

@smoya

Sorry for delay.

A very useful use case for this is reacting to those events from event handlers. I.e. :

Is it inconvenient to use slackevents.EventsAPIInnerEvent ? I still don't see much benefit to defining it.

kanata2 commented 1 year ago

@smoya

Sorry for delay.

A very useful use case for this is reacting to those events from event handlers. I.e. :

Is it inconvenient to use slackevents.EventsAPIInnerEvent ? I still don't see much benefit to defining it.

smoya commented 1 year ago

Is it inconvenient to use slackevents.EventsAPIInnerEvent ? I still don't see much benefit to defining it.

I think you are right. It is more than enough for that use case.

Thanks