Fix #904 by improving app.message listener's TS compatibility while bringing breaking changes

[seratch]

Summary

As mentioned in #904, implementing app.message listeners in TypeScript can be confusing as the payload/message objects are union types, so developers must have if/else statements to determine the message payload. In most cases, developers just want to handle newly posted messages in the listener. Having to add if (!payload.subtype || payload.subtype === 'bot_message' || payload.subtype === 'file_share' || payload.subtype === 'thread_broadcast') { ... } in all app.message listeners is not a great developer experience.

app.message(async ({ payload }) => {
  // This compiles! But you no longer receive message_changed etc.
  assert.isNotNull(payload.channel);
  assert.isNotNull(payload.ts);
  assert.isNotNull(payload.text);
  assert.isNotNull(payload.blocks);
  assert.isNotNull(payload.attachments);
});

I propose improving the app.message listener types in this pull request. Most developers will be happy with the change because, with the changes in this PR, developers no longer need to have the if/else blocks. One downside of this approach is that other subtype patterns, such as message_changed, won't be delivered to app.message listeners anymore. This behavioral modification can be a breaking change to some of the existing apps. For those apps, I've added app.allMessageSubtypes listener, which is fully compatible with the current app.message listener's behavior.

app.allMessageSubtypes(async ({ payload }) => {
  // You can still receive all the subtype requests in this listener
  if ((!payload.subtype ||
    payload.subtype === 'bot_message' ||
    payload.subtype === 'file_share' ||
    payload.subtype === 'thread_broadcast')) {
    // This compiles
    assert.isNotNull(payload.channel);
    assert.isNotNull(payload.ts);
    assert.isNotNull(payload.text);
    assert.isNotNull(payload.blocks);
    assert.isNotNull(payload.attachments);
  }
});

I would love to hear what other maintainers and community members think abou this proposal! If it looks good to many, we may merge the change in a future major or minor release. In theory, we should have this change in the next major but it may take long. Is it worth considering to have it in a minor release? Or, should we plan releasing next major soon?

Requirements (place an x in each [ ])

• [x] I've read and understood the Contributing Guidelines and have done my best effort to follow them.
• [x] I've read and agree to the Code of Conduct.

[seratch]

Coverted to draft as it seems that this PR is not yet complete.

[seratch]

Resolved the issues with app.event listeners

[codecov[bot]]

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 82.07%. Comparing base (449eced) to head (22b5179).

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1801      +/-   ##
==========================================
+ Coverage   81.97%   82.07%   +0.10%     
==========================================
  Files          18       18              
  Lines        1531     1540       +9     
  Branches      440      443       +3     
==========================================
+ Hits         1255     1264       +9     
  Misses        178      178              
  Partials       98       98              

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

[zimeg]

Wow, this is sweet! These changes seem really helpful for improving the experience with TypeScript, particularly with the first Hello, <@${message.user}> example. I'm a fan of these changes and agree that they improve a very common development path, thank you for putting this together! 👏 🎉

The app.allMessageSubtypes listener already seems like a useful way to access messages with a subtype, but I'm wondering if specifying the subtype as a listener constraint for app.message might provide a more consistent experience, where these subtypes are more "opt-in"? I'm unsure of the most common use cases here, but I would hope that this could also improve typing when using specific subtypes (assuming that typing can be inferred from this constraint parameter).

But regardless of that last point (it's only a suggestion that you can totally ignore!) I think this is a great proposal and would love to see it in an upcoming release! I agree that because this can be a breaking change, it may be better for a major release, but also understand releasing it in a minor version to bring these improvements to the near-term. The minor release would just require good communication about any breakages. However, if there are more breaking changes on the horizon, then planning for a major release soon would make more sense to me.