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

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

Merged
merged 1 commit into from
Apr 14, 2021

Conversation

seratch
Copy link
Member

@seratch seratch commented Apr 14, 2021

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

codecov bot commented Apr 14, 2021

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.

@@ -571,14 +571,16 @@ export default class App {
this.listeners.push([onlyCommands, matchCommandName(commandName), ...listeners] as Middleware<AnyMiddlewareArgs>[]);
}

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

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.

Copy link
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
seratch added a commit to seratch/bolt-js that referenced this pull request May 17, 2021
seratch added a commit to seratch/bolt-js that referenced this pull request May 17, 2021
seratch added a commit to seratch/bolt-js that referenced this pull request May 17, 2021
seratch added a commit that referenced this pull request May 19, 2021
* Add a type test verifying #922 is resolved by #878

* Update types-tests/options.test-d.ts
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 semver:minor TypeScript-specific
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ack(options) does not compile in TypeScript
2 participants