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

web-api: re-export method argument types #1729

Merged
merged 1 commit into from Jan 18, 2024
Merged

web-api: re-export method argument types #1729

merged 1 commit into from Jan 18, 2024

Conversation

filmaj
Copy link
Contributor

@filmaj filmaj commented Jan 18, 2024

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-level src/index.ts.

…o an index file and import from that file to ensure future arguments are organized in a similar manner.
@filmaj filmaj added bug M-T: A confirmed bug report. Issues are confirmed when the reproduction steps are documented semver:patch area:typescript issues that specifically impact using the package from typescript projects pkg:web-api applies to `@slack/web-api` labels Jan 18, 2024
@filmaj filmaj added this to the web-api@7.0.1 milestone Jan 18, 2024
@filmaj filmaj requested a review from zimeg January 18, 2024 20:16
@filmaj filmaj self-assigned this Jan 18, 2024
Copy link
Member

@zimeg zimeg left a 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/bolt@3.17.1 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 {
Copy link
Member

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.

@filmaj
Copy link
Contributor Author

filmaj commented Jan 18, 2024

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.

@zimeg
Copy link
Member

zimeg commented Jan 18, 2024

@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!

@filmaj
Copy link
Contributor Author

filmaj commented Jan 18, 2024

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).

@filmaj filmaj merged commit d5c17d8 into main Jan 18, 2024
15 checks passed
@filmaj filmaj deleted the web-api-export-args branch January 18, 2024 21:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:typescript issues that specifically impact using the package from typescript projects bug M-T: A confirmed bug report. Issues are confirmed when the reproduction steps are documented pkg:web-api applies to `@slack/web-api` semver:patch
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants