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

Missing exported types (Channel, etc) #1747

Closed mlynch closed 3 months ago

mlynch commented 7 months ago

(Describe your issue and goal here)

Packages:

Select all that apply:

I'm using @slack/web-api 7.0.1 (which is pulling in @slack/types@2.11.0). I've noticed many types returned in web api response objects are not exported, making it impossible/difficult to reference. For example, Channel is not exported, so storing a reference to the channels returned from client.conversations.list isn't clean:

  const channelsInfo = await client.conversations.list({
    types
  });

channelsInfo.channels is Channel[] but Channel is not properly exported.

There are many other examples of this in the library but I can't recall them at the moment.

hello-ashleyintech commented 7 months ago

Hi, @mlynch! Thanks for submitting this issue! 🙇

Apologies for the inconveniences you're running into with types! It looks like we haven't released a new @slack/types package since Dec 20th of last year, so perhaps it's time to take a look at it once again and also do an audit to make sure that all the needed types are properly exported for use. I will assign myself to this and start working on it hopefully within the next few weeks or so! ✨

mlynch commented 7 months ago

Thanks @hello-ashleyintech. I'm also noticing another typing issue with @slack/types 2.11.0 and @slack/web-api 7.0.2. Basically many API calls that don't need to take any arguments now require this options argument. Passing an empty object seems to work but also breaks a lot of existing example code:

image

hello-ashleyintech commented 6 months ago

hi, @mlynch! 🙌 I should be able to start work on this within the next few weeks. To help with these efforts, I wanted to see if you could provide the previous version of @slack/web-api (and consequently @slack/types) that you were using prior to upgrading to 7.0.1!

filmaj commented 6 months ago

@mlynch the requirement for an options object argument - even empty - is indeed a new change in v7. We may be able to adjust that on a per-method basis for methods where it makes sense.

As for changes to response types, this is a more delicate task than adjusting arguments, as response shapes vary greatly depending on the Slack workspace operating against (the plan the workspace is on, the administration settings the workspace has set, and so on). If you have specific observations on what is missing or what could be improved (such as your Channel observation), then we will certainly take a look.

mlynch commented 6 months ago

Makes sense. I think the issue was more the documentation and some examples were out of date, but I'm sure that was a work in progress

github-actions[bot] commented 5 months ago

👋 It looks like this issue has been open for 30 days with no activity. We'll mark this as stale for now, and wait 10 days for an update or for further comment before closing this issue out. If you think this issue needs to be prioritized, please comment to get the thread going again! Maintainers also review issues marked as stale on a regular basis and comment or adjust status if the issue needs to be reprioritized.

github-actions[bot] commented 5 months ago

As this issue has been inactive for more than one month, we will be closing it. Thank you to all the participants! If you would like to raise a related issue, please create a new issue which includes your specific details and references this issue number.

filmaj commented 3 months ago

FYI the issue around requiring an empty object for methods that require no arguments has been addressed in v7.1.0 of web-api.

filmaj commented 3 months ago

I see now what you meant by "not exported"; indeed the Channels type, while it is exported from its source file, we don't roll up that export back to the top-level of the web-api package. I think this is OK given that the response types are generated from automation, and unfortunately have to be something like the "lowest common denominator" of response types; that is, they are a big bag of optional properties, that may or may not apply to Slack customers' specific context (workspace and plan type).

If you're curious how that generation works, we basically shell out to the quicktype package to generate these sources from sample .json files of API responses we store in the Java SDK. All of this generation stuff is revisioned here: https://github.com/slackapi/node-slack-sdk/tree/main/scripts

If you really want the Channel interface, you can dip into the @slack/web-api package to get it from its source file directly:

import type { Channel } from '@slack/web-api/dist/types/response/ConversationsListResponse'

We could consider rolling up the exports from these within-response-type-source up to the top-level of the web-api package, but I am very hesitant to do so. The quicktype-generated types are volatile and change drastically from seemingly subtle changes in the underlying JSON response payloads, so I would rather not export them at the top level, as they could break easily. I would cautiously take my suggestion of pulling in the Channel type directly from the source due to this reason, too.

In the mean time I will close this issue but feel free to re-open or continue the conversation here if folks have strong opinions here.