-
Notifications
You must be signed in to change notification settings - Fork 662
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
web-api: re-export method argument types #1729
Conversation
…o an index file and import from that file to ensure future arguments are organized in a similar manner.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking solid to me but running into problems while testing this with slackapi/bolt-js. Going to look into this a bit more (not sure that it's caused by or should be solved with these changes) but here's what I'm finding:
Bolt JS build errors
$ npm run build
> @slack/[email protected] build
> tsc
src/App.ts:977:15 - error TS2322: Type '{ token: string | undefined; text: string; channel: string; } | { token: string | undefined; channel: string; metadata?: MessageMetadata | undefined; text?: string | undefined; ... 12 more ...; unfurl_media?: boolean | undefined; }' is not assignable to type 'ChatPostMessageArguments'.
Type '{ token: string | undefined; channel: string; metadata?: MessageMetadata | undefined; text?: string | undefined; mrkdwn?: boolean | undefined; thread_ts?: string | undefined; ... 10 more ...; unfurl_media?: boolean | undefined; }' is not assignable to type 'ChatPostMessageArguments'.
Type '{ token: string | undefined; channel: string; metadata?: MessageMetadata | undefined; text?: string | undefined; mrkdwn?: boolean | undefined; thread_ts?: string | undefined; ... 10 more ...; unfurl_media?: boolean | undefined; }' is not assignable to type 'TokenOverridable & ChannelAndAttachments & BroadcastedThreadReply & IconURL & Username & ... 4 more ... & { ...; }'.
Type '{ token: string | undefined; channel: string; metadata?: MessageMetadata | undefined; text?: string | undefined; mrkdwn?: boolean | undefined; thread_ts?: string | undefined; ... 10 more ...; unfurl_media?: boolean | undefined; }' is not assignable to type 'ChannelAndAttachments'.
Types of property 'attachments' are incompatible.
Type 'unknown' is not assignable to type 'MessageAttachment[]'.
977 const postMessageArguments: ChatPostMessageArguments = typeof message === 'string' ?
~~~~~~~~~~~~~~~~~~~~
src/App.ts:1594:11 - error TS2322: Type 'RespondArguments | { text: string; }' is not assignable to type 'RespondArguments'.
Type '{ text: string; }' is not assignable to type 'RespondArguments'.
Type '{ text: string; }' is missing the following properties from type 'Pick<ChatPostMessageArguments, "metadata" | "token" | "mrkdwn" | "thread_ts" | "blocks" | "attachments" | "reply_broadcast" | "as_user" | "link_names" | ... 5 more ... | "unfurl_media">': blocks, attachments, username
1594 const normalizedArgs: RespondArguments = typeof message === 'string' ? { text: message } : message;
~~~~~~~~~~~~~~
src/types/utilities.ts:26:59 - error TS2344: Type '"metadata" | "text" | "token" | "mrkdwn" | "thread_ts" | "blocks" | "attachments" | "reply_broadcast" | "as_user" | "link_names" | "parse" | "icon_emoji" | "icon_url" | "username" | "unfurl_links" | "unfurl_media"' does not satisfy the constraint '"channel" | "metadata" | "text" | "token" | "mrkdwn" | "thread_ts" | "reply_broadcast" | "as_user" | "link_names" | "parse" | "icon_emoji" | "icon_url" | "unfurl_links" | "unfurl_media"'.
Type '"blocks"' is not assignable to type '"channel" | "metadata" | "text" | "token" | "mrkdwn" | "thread_ts" | "reply_broadcast" | "as_user" | "link_names" | "parse" | "icon_emoji" | "icon_url" | "unfurl_links" | "unfurl_media"'.
26 export type SayArguments = Pick<ChatPostMessageArguments, Exclude<ChatPostMessageArgumentsKnownKeys, 'channel'>> & {
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
src/types/utilities.ts:34:63 - error TS2344: Type '"metadata" | "token" | "mrkdwn" | "thread_ts" | "blocks" | "attachments" | "reply_broadcast" | "as_user" | "link_names" | "parse" | "icon_emoji" | "icon_url" | "username" | "unfurl_links" | "unfurl_media"' does not satisfy the constraint '"channel" | "metadata" | "text" | "token" | "mrkdwn" | "thread_ts" | "reply_broadcast" | "as_user" | "link_names" | "parse" | "icon_emoji" | "icon_url" | "unfurl_links" | "unfurl_media"'.
Type '"blocks"' is not assignable to type '"channel" | "metadata" | "text" | "token" | "mrkdwn" | "thread_ts" | "reply_broadcast" | "as_user" | "link_names" | "parse" | "icon_emoji" | "icon_url" | "unfurl_links" | "unfurl_media"'.
34 export type RespondArguments = Pick<ChatPostMessageArguments, Exclude<ChatPostMessageArgumentsKnownKeys, 'channel' | 'text'>
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Found 4 errors in 2 files.
Errors Files
2 src/App.ts:977
2 src/types/utilities.ts:26
If you think this is expected, happy to approve and look into a fix on the Bolt side of things!
import type { AdminUsergroupsAddChannelsArguments, AdminUsergroupsAddTeamsArguments, AdminUsergroupsListChannelsArguments, AdminUsergroupsRemoveChannelsArguments } from './types/request/admin/usergroups'; | ||
import type { AdminUsersAssignArguments, AdminUsersInviteArguments, AdminUsersListArguments, AdminUsersRemoveArguments, AdminUsersSessionListArguments, AdminUsersSessionClearSettingsArguments, AdminUsersSessionGetSettingsArguments, AdminUsersSessionInvalidateArguments, AdminUsersSessionResetArguments, AdminUsersSessionResetBulkArguments, AdminUsersSessionSetSettingsArguments, AdminUsersSetAdminArguments, AdminUsersSetExpirationArguments, AdminUsersSetOwnerArguments, AdminUsersSetRegularArguments, AdminUsersUnsupportedVersionsExportArguments } from './types/request/admin/users'; | ||
import type { AdminWorkflowsCollaboratorsAddArguments, AdminWorkflowsCollaboratorsRemoveArguments, AdminWorkflowsPermissionsLookupArguments, AdminWorkflowsSearchArguments, AdminWorkflowsUnpublishArguments } from './types/request/admin/workflows'; | ||
import type { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Small personal preference to break the types up on individual lines, but not a blocker at all.
I am not surprised that bolt-js is out-of-the-box incompatible w/ v7. A lot of the method argument types got stricter, so if bolt-js is playing fast and loose with those, that is no longer acceptable. Need to dig in to figure out the details; happy to huddle through that if you think that will help. |
@filmaj Makes total sense! Love to see this being caught now 🙌 Going to send ya a quick ping, but I think it's fine that Bolt breaks with these changes. Just means some updates to come! |
I'm going to merge this and work on a separate PR to enforce the import style you mentioned (one per line). A separate PR will be better as I think that will affect all the packages in this repo (since we share eslint configs). |
My bad, I accidentally removed exporting all method argument types as part of the 7.0.0 release.
This PR adds them back, and also creates a singular
request/index.ts
file that is meant to export ALL method argument types.src/method.ts
imports from this index file to ensure that future method arguments are added in the same place (and automatically exported by the top-levelsrc/index.ts
.