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

Methods with no options do not call callback #618

Closed
4 of 9 tasks
gabegorelick opened this issue Aug 20, 2018 · 1 comment · Fixed by #631
Closed
4 of 9 tasks

Methods with no options do not call callback #618

gabegorelick opened this issue Aug 20, 2018 · 1 comment · Fixed by #631
Labels
docs M-T: Documentation work only

Comments

@gabegorelick
Copy link

Description

https://slackapi.github.io/node-slack-sdk/web_api#using-a-callback-instead-of-a-promise lists the following example:

web.channels.list((err, res) => {
  if (err) {
    return console.error(err);
  }

  // `res` contains information about the channels
  res.channels.forEach(c => console.log(c.name));
});

While, web.channels.list({}, callback) does work, when omitting the options parameter the callback will not be called due to

// Adapt the interface for callback-based execution or Promise-based execution
if (callback !== undefined) {
callbackify(implementation)(callback);
return;
}
return implementation();

When compiled to JS, apiCall has a signature of function (method, options, callback) {}. If options is not passed, callback will be undefined and a promise will be returned instead.

A potential solution would be to check typeof options === 'function' to see if options is actually the callback.

What type of issue is this?

  • bug
  • enhancement (feature request)
  • question
  • documentation related
  • testing related
  • discussion

Requirements (place an x in each of the [ ])

  • I've read and understood the Contributing guidelines and have done my best effort to follow them.
  • I've read and agree to the Code of Conduct.
  • I've searched for any related issues and avoided creating a duplicate issue.

Bug Report

Reproducible in:

@slack/client version: 4.4.0, also verified in 4.0.0

node version: 8.11.4

OS version(s): macOS High Sierra

Steps to reproduce:

  1. Run example in https://slackapi.github.io/node-slack-sdk/web_api#using-a-callback-instead-of-a-promise

Expected result:

Channels should be logged

Actual result:

No channels logged due to callback not being called.

@aoberoi
Copy link
Contributor

aoberoi commented Aug 20, 2018

ah, you're right! i overlooked the fact that methods with no arguments still require that parameter when writing the example in the documentation.

the intention of the API is to still require the options parameter as seen in the overload signature below:

public apiCall(method: string, options: WebAPICallOptions, callback: WebAPIResultCallback): void;

so i'd describe this as a documentation issue. if you'd like us to consider making options optional, then let's open a separate feature request issue for that.

@aoberoi aoberoi added the docs M-T: Documentation work only label Aug 20, 2018
aoberoi added a commit to aoberoi/node-slack-sdk that referenced this issue Sep 10, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs M-T: Documentation work only
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants