MessageEvent type definition does not match event data when subtype is 'message_changed'

[ieaalto]

Description

Concerning TypeScript, there seems to be a mismatch between message event type definitions and the actual events from Slack. For instance the following example would cause unexpected errors due to message_changed events:

app.message(({ message }) => {
  const user: string = message.user
  if (user.match(/^U/)) {
    // ...
  }
})

The type definition does not allow message.user to be undefined, but in reality it may be when the subtype is message_changed. This issue is easy to work around by filtering by subtype, but these type safety breaking issues can be fiendishly difficult to detect.

What type of issue is this?

• bug
• enhancement (feature request)
• question
• documentation related
• testing related
• discussion

Requirements

• I've read and understood the Contributing guidelines and have done my best effort to follow them.
• I've read and agree to the Code of Conduct.
• I've searched for any related issues and avoided creating a duplicate issue.

[seratch]

Sorry for our belated response and thank you for taking the time to share this. I will check this issue.

[seratch]

Let me share my current understanding of this issue.

As pointed out here, the data structure of type: message events varies. Bolt already has all the types for possible event types but the dispatcher always injects MessageEvent type which is compatible only with those that don't have subtype.

Currently, types to be used for events is determined by only type in the payload. To support other message events, we need to have another interface to consider subtype as well.

I'm going to come up with a solution that doesn't bring any breaking changes to existing TypeScript apps, but I'm still not 100% sure if it's possible yet.

[aoberoi]

@seratch makes a good point here: it would be useful for the app.message() API to learn about subtypes. there's probably a few ways we could go about getting this. i'd be happy to explore them.

i think in the short term, the right thing to do is to make the user property of MessageEvent optional. conceptually, all message subtypes should conform to MessageEvent, so maybe we should consider making that more formal by making sure the individual subtype interfaces (e.g. MessageChangedEvent) are all defined with extends MessageEvent. an even higher fidelity solution would be to make MessageEvent generic on subtype, and vary the properties based on conditional checks of the subtype, which I think is what @seratch is suggesting.

i think we avoided this until now because we knew that some properties are different, and that app developers who knew what they were doing could simply cast to the more specific kind of event when they knew they were dealing with it. we even provided a built-in middleware, subtype(), to allow you to filter more easily. admittedly, we've not been able to model middleware chains well with types because even when you use it you still get a MessageEvent in your subsequent listeners.

the newer pattern in Bolt has been to avoid using middleware for filtering events when they can potentially change the type, and instead add properties to the Constraints. then we can flow the specific type of event (including subtype) to the listener.