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

Simplify reaction_added/removed event's item type #1845

Merged
merged 2 commits into from
May 19, 2023

Conversation

seratch
Copy link
Member

@seratch seratch commented May 18, 2023

Summary

This pull request improves TypeScript developer experience by updating "item" property type in reaction_added/removed event data types. WIth this change, developers can safely access event.item.channel in TS code. More importantly, they can use say() utlitity in listeners. Currently, say() can be resolved as never in TS as event.item.channel can be absent for file/file comment items while it never happens. Slack apps no longer receive reaction events for files and file comments. So, this change is safe enough to apply in a patch release.

Requirements (place an x in each [ ])

@seratch seratch added bug M-T: confirmed bug report. Issues are confirmed when the reproduction steps are documented TypeScript-specific labels May 18, 2023
@seratch seratch added this to the 3.13.2 milestone May 18, 2023
@seratch seratch self-assigned this May 18, 2023
@codecov
Copy link

codecov bot commented May 18, 2023

Codecov Report

Merging #1845 (817dc85) into main (6eaf5f8) will not change coverage.
The diff coverage is n/a.

❗ Current head 817dc85 differs from pull request most recent head 13ce4b2. Consider uploading reports for the commit 13ce4b2 to get more accurate results

@@           Coverage Diff           @@
##             main    #1845   +/-   ##
=======================================
  Coverage   82.15%   82.15%           
=======================================
  Files          18       18           
  Lines        1519     1519           
  Branches      435      435           
=======================================
  Hits         1248     1248           
  Misses        175      175           
  Partials       96       96           

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@seratch seratch requested review from filmaj and zimeg May 18, 2023 06:30
Copy link
Contributor

@filmaj filmaj left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Left one minor comment

src/types/events/base-events.ts Outdated Show resolved Hide resolved
Co-authored-by: Fil Maj <maj.fil@gmail.com>
@seratch seratch merged commit 1d9d1dd into slackapi:main May 19, 2023
5 checks passed
@seratch seratch deleted the reaction-events branch May 19, 2023 05:46
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

Successfully merging this pull request may close these issues.

None yet

2 participants