slackapi / node-slack-sdk

Slack Developer Kit for Node.js
https://slack.dev/node-slack-sdk
MIT License
3.27k stars 662 forks source link

web-api: Consider removing need to provide empty object as argument to zero-argument methods #1769

Closed kujiy closed 3 months ago

kujiy commented 6 months ago

Packages:

Select all that apply:

Reproducible in:

The Slack SDK version

    "@slack/web-api": "^7.0.2",

Node.js runtime version

$ node --version
v20.11.1

OS info

$ sw_vers && uname -v
ProductName:            macOS
ProductVersion:         13.6.3
BuildVersion:           22G436
Darwin Kernel Version 22.6.0: Tue Nov  7 21:40:08 PST 2023; root:xnu-8796.141.3.702.9~2/RELEASE_ARM64_T6000

Steps to reproduce:

Create a WebClient instance and call .auth.test() without argument.

import { WebClient } from "@slack/web-api";

const api = new WebClient(
            slackBotToken
        );

const botUserInfo: any = await api.auth.test();

Expected result:

// Works without errors
const botUserInfo: any = await api.auth.test();

.auth.test() returns a userinfo of the bot.

Actual result:

It requires an empty dict as an argument.

// Error
const botUserInfo: any = await api.auth.test();  //  error TS2554: Expected 1 arguments, but got 0.

// Works with an empty dict
const botUserInfo: any = await api.auth.test({});

Requirements

I read the requirements and agreed with the rules.

seratch commented 6 months ago

Hi @kujiy, thank you for taking the time to provide this feedback!

Indeed, the method does work even without passing an argument, but it is a rare case among the Slack web APIs. For this reason, we generally recommend passing an empty argument object to any methods, which is why the TS typing requires an arugment object.

That being said, we are open to making minor modifications to exceptions such as the api.test and auth.test API methods. I will leave this issue open for potential future enhancements.

filmaj commented 6 months ago

This change was introduced in web-api v7, and is explicitly detailed in the migration guide for this method, FYI.

That said I agree with @seratch - perhaps we can look into additional code to create an exception for zero-argument methods.