bolt-js icon indicating copy to clipboard operation
bolt-js copied to clipboard

Cannot access the user id in the "message_changed" event with TypeScript

Open okovpashko opened this issue 1 year ago • 3 comments

Reproducible in:

The Slack SDK version

3.13.1

Node.js runtime version

v16.15.1

OS info

ProductName: macOS ProductVersion: 12.6.5 BuildVersion: 21G531 Darwin Kernel Version 21.6.0: Thu Mar 9 20:08:59 PST 2023; root:xnu-8020.240.18.700.8~1/RELEASE_X86_64

Steps to reproduce:

The Slack docs say that there should be the message.user key in the message_changed event containing the Slack user id.

The TypeScript interface for the MessageChangedEvent refers to the MessageEvent interface in the message and the previous_message properties that make TypeScript assume that accessing event.message.user is not allowed because not every subtype of the message event contains a user id.

It seems like the reference to the MessageEvent is incorrect because (I suppose) not every message can be edited and moreover it makes the circular dependency in types when the message_changed event can reference to the message_changed event and so on.

Expected result:

  • the message property of the message_changed event has the valid type according to the Slack docs
  • no TypeScript errors when accessing event.message.user for the message_changed event.

Actual result:

TS2339: Property 'user' does not exist on type 'MessageEvent'.

okovpashko avatar May 12 '23 18:05 okovpashko

Hi @okovpashko, thanks for taking the time to report this issue with details!

I agree that the message/ previous_message in message_changed subtype message event type definition should be corrected as you pointed out. We will resolve it in the next patch version. If you're happy to make a pull request for it, I'd love to have your contribution 🙌

seratch avatar May 12 '23 19:05 seratch

Hi @seratch Thank you for the rapid response!

I will be happy to submit a PR with the fix. The only thing I need to understand beforehand is if there's already an existing type for the object I need or I should introduce a new interface for that. Could you give me a hint please?

okovpashko avatar May 12 '23 19:05 okovpashko

The MessageEvent is a union type that consists of all the possible message event types, so we can define a new union type with less subtype patterns. I think removing at least message_changed, message_deleted should be safe enough. But, for the rest, I cannot confidently say anything without thorough tests with real payloads.

seratch avatar May 12 '23 20:05 seratch