Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
okovpashko opened this issue May 12, 2023 · 3 comments
Open
Assignees
Labels
bug M-T: confirmed bug report. Issues are confirmed when the reproduction steps are documented TypeScript-specific
Milestone

Comments

@okovpashko
Copy link

okovpashko commented May 12, 2023

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'.
@seratch seratch added bug M-T: confirmed bug report. Issues are confirmed when the reproduction steps are documented TypeScript-specific and removed untriaged labels May 12, 2023
@seratch seratch added this to the 3.13.2 milestone May 12, 2023
@seratch seratch self-assigned this May 12, 2023
@seratch
Copy link
Member

seratch commented May 12, 2023

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 🙌

@okovpashko
Copy link
Author

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?

@seratch
Copy link
Member

seratch commented May 12, 2023

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 seratch modified the milestones: 3.13.2, 3.x Jul 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug M-T: confirmed bug report. Issues are confirmed when the reproduction steps are documented TypeScript-specific
Projects
None yet
Development

No branches or pull requests

2 participants