Skip to content

Fix #720 ack(options) does not compile in TypeScript#878

Merged
seratch merged 1 commit intoslackapi:mainfrom
seratch:issue-720-options
Apr 14, 2021
Merged

Fix #720 ack(options) does not compile in TypeScript#878
seratch merged 1 commit intoslackapi:mainfrom
seratch:issue-720-options

Conversation

@seratch
Copy link
Copy Markdown
Contributor

@seratch seratch commented Apr 14, 2021 •
edited
Loading

Summary

This pull request resolves #720

Most developers (perhaps 100%) handle only block_suggestion requests in app.options listeners. Thus, this pull request set the default Source to block_suggestion to ensure better developer experience in TypeScript.

If a developer would like to handle either interactive_message (user actions on attachments) or dialog_suggestion (the ones in legacy dialogs), the developer can have an explicit type parameter as below:

app.options<'interactive_message'>({ callback_id: 'a' }, async ({ ack }) => {
  await ack();
});

app.options<'dialog_suggestion'>({ callback_id: 'a' }, async ({ ack }) => {
  await ack();
});

Requirements (place an x in each [ ])

@seratch seratch added bug M-T: confirmed bug report. Issues are confirmed when the reproduction steps are documented semver:minor TypeScript-specific labels Apr 14, 2021
@seratch seratch added this to the 3.4.0 milestone Apr 14, 2021
@seratch seratch self-assigned this Apr 14, 2021
@codecov
Copy link
Copy Markdown

codecov bot commented Apr 14, 2021 •
edited
Loading

Codecov Report

Merging #878 (5054be8) into main (23b4f80) will not change coverage.
The diff coverage is n/a.

❗ Current head 5054be8 differs from pull request most recent head 3e98504. Consider uploading reports for the commit 3e98504 to get more accurate results
Impacted file tree graph

@@           Coverage Diff           @@
##             main     #878   +/-   ##
=======================================
  Coverage   66.08%   66.08%           
=======================================
  Files          13       13           
  Lines        1200     1200           
  Branches      353      353           
=======================================
  Hits          793      793           
  Misses        338      338           
  Partials       69       69
Impacted Files Coverage Δ
src/App.ts 82.76% <ø> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 23b4f80...3e98504. Read the comment docs.

seratch
seratch commented Apr 14, 2021
View reviewed changes
src/App.ts
}

public options<Source extends OptionsSource = OptionsSource>(
public options<Source extends OptionsSource = 'block_suggestion'>(
Copy link
Copy Markdown
Contributor Author

@seratch seratch Apr 14, 2021

Choose a reason for hiding this comment

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

This is the only change in the main code.

mwbrooks
mwbrooks approved these changes Apr 14, 2021
View reviewed changes
Copy link
Copy Markdown
Member

@mwbrooks mwbrooks left a comment

Choose a reason for hiding this comment

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

Nice to see this land. I remembering running into this issue the first time I wanted to use ack with options.

@seratch seratch merged commit 0f736c6 into slackapi:main Apr 14, 2021
@seratch seratch deleted the issue-720-options branch April 14, 2021 21:08
This was referenced May 17, 2021
Merged
Closed
seratch added a commit that referenced this pull request May 19, 2021
@seratch
Add a type test verifying #922 is resolved by #878 (#923)
2c1a1e9 
* Add a type test verifying #922 is resolved by #878

* Update types-tests/options.test-d.ts
@zimeg zimeg mentioned this pull request Mar 31, 2023
Merged
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mwbrooks mwbrooks mwbrooks approved these changes

@stevengill stevengill Awaiting requested review from stevengill

@misscoded misscoded Awaiting requested review from misscoded

Assignees

@seratch seratch

Labels

bug M-T: confirmed bug report. Issues are confirmed when the reproduction steps are documented semver:minor TypeScript-specific

Projects

None yet

Milestone

3.4.0

Development

Successfully merging this pull request may close these issues.

ack(options) does not compile in TypeScript

2 participants

@seratch @mwbrooks