WebClient arguments are not type safe in TypeScript

[75asa]

Description

When using chat.postMessage(), I was able to put a non-existent property in ChatPostMessageArguments and the process was executed without a compile error! (Specifically, I typo icon_url as icon_ur...) I got a response from @seratch , and it seems that this is largely due to the scope of WebAPICallOptions. In the future, I'd like to see compilations not go through here, too.

What type of issue is this? (place an x in one of the [ ])

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

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

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

---

Bug Report

Filling out the following details about bugs will help us solve your issue sooner.

Packages:

Select all that apply:

• [x] @slack/web-api
• [ ] @slack/events-api
• [ ] @slack/interactive-messages
• [ ] @slack/rtm-api
• [ ] @slack/webhooks
• [ ] @slack/oauth
• [ ] @slack/socket-mode
• [ ] I don't know

Reproducible in:

package version: 6.2.4

node version: v16.4.0

OS version(s): MacOS 11.5.2

Steps to reproduce:

1. import ChatPostMessageArguments from @slack/web-api
2. define a variable of type ChatPostMessageArguments
3. define a property that does not exist in ChatPostMessageArguments. like a below code

    const msgOption: ChatPostMessageArguments = {
      channel: Config.Slack.CHANNEL_NAME,
      text,
      foo: "foo", // non-existent property !!
      username: arg.user.name,
      icon_url: arg.user.avatarURL,
      unfurl_links: true,
      blocks: JSXSlack(Header(databaseName!, page)),
    };

Expected result:

What you expected to happen

Actual result:

Compilation will pass.

Attachments:

Logs, screenshots, screencast, sample project, funny gif, etc.

[seratch]

Hi @75asa, thanks for flagging this!

To other maintainers, let me share the context behind this. @75asa and I just had a chat about this topic on Twitter (in Japanese). That's why he mentioned me in the description.

As all the Web API method argument types inherit WebAPICallOptions in TypeScript, any property names can be passed to WebClient without any compilation phase validation.

export interface WebAPICallOptions {
  [argument: string]: unknown;
}

Indeed, the approach is good for us as the SDK maintainers. We don't need to release a new version when the platform adds a new parameter. But, from a library user perspective, it's not optimal.

At the same time, I know that we are unable to change this in minor releases. So, for better TypeScript developer experience, we can revisit this in the next major version (=v7). Also, we may want to revisit the inheritance of WebAPICallResult in the API responses. But the benefit of the response type changes may be relatively more minor compared to the improvement of the argument pain. https://github.com/slackapi/node-slack-sdk/blob/db952a5cd56ff5982cd5bb9da2710d2dd74743ff/packages/web-api/src/response/ChatPostMessageResponse.ts#L12

There is no doubt that the changes are valuable for developers maintaining bolt-js apps in TypeScript. Only the downside is that we, bolt-js maintainers, may need to do releases more frequently. But it's just worth it.

Let me know if you have different thoughts on this. If moving this forward in the next major is acceptable for everyone, I'm happy to take the initiative in this improvement!

[seratch]

👋 @filmaj @mwbrooks @stevengill @misscoded @srajiang Let me know if you have some thoughts on this. I'm thinking that the improvements I suggested above can be one of the major changes in the next major version.

[mwbrooks]

Thanks for summarizing your Twitter conversation!

I agree with your view @seratch that the SDK should be optimized for developer usage even if its a little more work for the maintainers (i.e. releasing more often). Also, new SDK releases when the API Platform changes can be helpful to developers, because it signals when they should update their SDK version.

So, I'm in favour of making these improvements in the next major version!

[misscoded]

Only the downside is that we, bolt-js maintainers, may need to do releases more frequently

+1 for going forward with this, despite the need for more frequent releases on our part. Well worth it for the benefit of the community. 🙂

[filmaj]

I've been working on type safety for the @slack/web-api package now for a couple of months, and the release candidate v7.0.0-rc.0 is up for those interested in trying it out! You can install it via npm i @slack/web-api@rc (it is under the rc npm dist-tag). Would love to hear feedback for those who are able to integrate it into their existing projects, especially TypeScript projects (the type safety changes in this release candidate should not impact JavaScript projects).

It doesn't address type safety for responses (as those are very contextual and can depend on Slack plan, administrator and organization settings) but it does address type safety for API arguments.

I am also actively working on a Migration Guide in the wiki: https://github.com/slackapi/node-slack-sdk/wiki/Migration-Guide-for-web%E2%80%90api-v7 - but many methods were changed, so this will be an ongoing effort.

[filmaj]

The above-mentioned changes to make web-api method arguments type-safe were merged into main. I plan to release the next major version of web-api, v7, today, which will include these changes.