Web API Type Safety #1673

filmaj · 2023-10-05T18:06:23Z

This work builds off #1670. Web API arguments are worked on to make type safe, though responses were not closely validated as that would require a lot more work. I am also leaving notes on API arguments and responses as I test them out, with breadcrumbs on future improvements / breaking changes.

Once merged, this PR intends to fix #1323.

USEFUL WHEN ASSEMBLING CHANGELOG: For the list of breaking changes introduced, best to look at the src/methods.ts file in this commit and search for "TODO".

Open Questions

How deeply do we want to type the Manifest schema? The answer here determines changes to still apply to the apps.manifest.* APIs in this PR.
- Answer: somewhat reasonably deeply. Will do my best!

Deprecations being removed

While here, and since this PR is in prep for a new major release, also took the opportunity to remove methods that were marked as deprecated:

channels.*
groups.*
im.*
mpim.*

New Deprecations

We will deprecate the following method groups, given they are mentioned as such in their reference docs.

oauth.access (superseded by the new oauth.v2.* methods)
stars.* (docs specifically mention end-users cant interact with them in the client anymore, presumably will be replaced with the Later feature at some point)
workflows.* (legacy steps-from-apps related workflow methods)

Updates/Testing

Documenting, DRYing up and type-safety-ifying arguments for all the methods against the documentation. Also writing type tests for the arguments as I go.

admin.*:
- refactored request shapes argument w/ docs
- type tests
admin.analytics.getFile (need to get unblocked on specific enterprise org setup, should be able to get access to that Monday Nov 27):
- refactored request shapes argument w/ docs
- type tests
api.test
- refactored request shapes argument w/ docs
- type tests
apps.manifest.* (see open questions above)
- refactored request shapes argument w/ docs
- type tests
auth.*
- refactored request shapes argument w/ docs
- type tests
bookmarks.*
- refactored request shapes argument w/ docs
- type tests
calls.*
- refactored request shapes argument w/ docs
- type tests
chat.*
- refactored request shapes argument w/ docs
- type tests
conversations.*
- refactored request shapes argument w/ docs
- type tests
dialog.open
- refactored request shapes argument w/ docs
- type tests
dnd.*
- refactored request shapes argument w/ docs
- type tests
emoji.list
- refactored request shapes argument w/ docs
- type tests
files.*
- refactored request shapes argument w/ docs
- type tests
migration.exchange
- refactored request shapes argument w/ docs
- type tests
oauth.*
- refactored request shapes argument w/ docs
- type tests
openid.*
- refactored request shapes argument w/ docs
- type tests
pins.*
- refactored request shapes argument w/ docs
- type tests
reactions.*
- refactored request shapes argument w/ docs
- type tests
reminders.*
- refactored request shapes argument w/ docs
- type tests
rtm.*
- refactored request shapes argument w/ docs
- type tests
search.*
- refactored request shapes argument w/ docs
- type tests
stars.*
- refactored request shapes argument w/ docs
team.*
- refactored request shapes argument w/ docs
- type tests
tooling.*
- refactored request shapes argument w/ docs
- type tests
usergroups.*
- refactored request shapes argument w/ docs
- type tests
users.*
- refactored request shapes argument w/ docs
- type tests
views.*
- refactored request shapes argument w/ docs
- type tests
workflows.*
- refactored request shapes argument w/ docs

…g, unknown>`. Remove unneeded disabling of no-trailing-space rule. Replace use of `headers: any` in private `serializeApiCallOptions` method with `Record<string, string>`.

…allResult. Tweak fileUploadV2 return type as a result.

…lines up behaviour with WebClient.

…inor types within its response.

…reaking change enhancements.

…ts to reflect docs.

…an enterprise org with specific settings set up properly - TODO on Monday

… pass at typing the manifest.

seratch

I can't guarantee everything works as with before but I am confident that this PR is carefully crafted. Great work!

seratch · 2023-12-14T01:57:45Z

packages/web-api/src/WebClient.ts

    shouldStop?: PaginatePredicate,
    reduce?: PageReducer<A>,
  ): (Promise<A> | AsyncIterable<WebAPICallResult>) {
-    if (!cursorPaginationEnabledMethods.has(method)) {


Removing this check is not a big deal at all but did you have to do so due to some changes in this PR? Also, the pagnation method does not provide type-safety for arguments: https://slack.dev/node-slack-sdk/web-api#pagination Do you have any possible ideas to improve it? (not necessarily work on it in this PR)

Yes, I removed this as I removed the cursorPaginationEnabledMethods variable altogether. I did not find it gave much value (its only use was to issue the warning below). I believe I brought this up in an early discussion internally two months ago or so (search for "from:fil paginate" in our #team-devrel-tools channel internally).

As for ideas on how to improve it, perhaps the API could be changed so that each API method supporting pagination could optionally return an iterator? This way the developer doesn't have to provide the method name string as that would be known by the framework, and the type safety for method arguments provided by this PR would apply as well.

Thanks for clarifying. Makes sense!

seratch · 2023-12-14T01:58:02Z

packages/web-api/src/WebClient.ts

   * @param options
   */
-  public async filesUploadV2(options: FilesUploadV2Arguments): Promise<WebAPICallResult> {
+  public async filesUploadV2(options: FilesUploadV2Arguments): Promise<
+  WebAPICallResult & { files: FilesCompleteUploadExternalResponse[] }


good catch!

packages/web-api/src/types/request/bots.ts

seratch · 2023-12-14T02:10:17Z

packages/web-api/src/types/request/chat.ts

@@ -0,0 +1,224 @@
+import type { KnownBlock, Block, MessageAttachment, LinkUnfurls, MessageMetadata } from '@slack/types';


I am still not 100% confident that all the patterns here are correct ... but well done!

Yes I think this is a risky area.. all possible combinations of Block elements, etc, may be hard to verify... but I think as long as we are OK to tell developers that find a problem, "please use // @ts-expect-error as a workaround until we release a patch", it should be OK.

seratch · 2023-12-14T02:33:17Z

packages/web-api/src/types/request/manifest.ts

+  app_home?: ManifestAppHome;
+  /**
+   * @description A subgroup of settings that describe {@link https://api.slack.com/legacy/enabling-bot-users bot user} configuration.
+   * @see {@link https://api.slack.com/legacy/enabling-bot-users Legacy bots}.


I don't like linking this "legacy" document here but indeed there is no other document for this

I took this from the Manifest fields documentation here: https://api.slack.com/reference/manifests#features

The link originally links to api.slack.com/bot-user which redirects to the above link.

seratch · 2023-12-14T02:38:38Z

packages/web-api/src/types/response/AdminAnalyticsGetFileResponse.ts

@@ -24,6 +16,7 @@ export interface ResponseMetadata {

 export interface AdminAnalyticsMemberDetails {
  enterprise_id:                 string;
+  team_id:                       string;


seratch · 2023-12-14T02:39:45Z

packages/web-api/test/types/methods/admin.analytics.test-d.ts

@@ -0,0 +1,45 @@
+import { expectAssignable, expectError } from 'tsd';


Thanks for adding these tests!

seratch · 2023-12-14T02:40:16Z

scripts/code_generator.rb

@@ -5,7 +5,7 @@



Co-authored-by: Kazuhiro Sera <[email protected]>

…h message-posting APIs, added tests to ensure this remains the case

filmaj added semver:major enhancement M-T: A feature request for new functionality pkg:web-api applies to `@slack/web-api` draft labels Oct 5, 2023

filmaj added this to the [email protected] milestone Oct 5, 2023

filmaj self-assigned this Oct 5, 2023

filmaj mentioned this pull request Oct 5, 2023

@slack/web-api making request and response shapes type-safe, and chat.postMessage arguments prototype #1670

Closed

filmaj added 7 commits October 6, 2023 10:30

Remove WebAPICallOptions, replace where it is used with `Record<strin…

ec34ce6

…g, unknown>`. Remove unneeded disabling of no-trailing-space rule. Replace use of `headers: any` in private `serializeApiCallOptions` method with `Record<string, string>`.

@slack/web-api: Remove inclusion of arbitrary properties from WebAPIC…

79aaf6e

…allResult. Tweak fileUploadV2 return type as a result.

Remove optionality on abstract generic Method interface options arg, …

62ffc7f

…lines up behaviour with WebClient.

adjust return type for fileuploadv2 based on recent change.

3cb6c62

adding notes on future type improvements to getFile API args

16464e5

Remove note about getFile response type being generated. Tweak some m…

b4837a4

…inor types within its response.

Tweaking web-api arguments as I audit them, adding TODOs for future b…

e2d2faa

…reaking change enhancements.

filmaj force-pushed the web-api-type-safety branch from 8559522 to e2d2faa Compare October 6, 2023 14:31

filmaj added 15 commits October 6, 2023 13:48

Audit of admin.* methods complete, added TODOs and updating argumen…

bc25c1f

…ts to reflect docs.

audited api.test and apps.* method arguments.

41a0d79

bots and auth

931c06f

audit bookmarks.*

2e9f906

remove deprecated channels methods, audit chat.* methods.

3a4a7a2

conversations and dialogs methods

7e7b16c

dnd methods

4e0a739

files methods

a3cfd8b

remove deprecated groups.* methods.

124e5f6

remove deprecated im.* methods.

2bf4dc9

removed deprecated mpim.* methods.

b9c63c8

oauth methods

daed47d

pin methods

202a321

reaction methods

4fca944

audit reminders methods and factor out optional team id into own mixin.

d64c44d

filmaj added 17 commits November 24, 2023 13:17

stubbing out bits for admin.analytics.getFile API but need access to …

a3db517

…an enterprise org with specific settings set up properly - TODO on Monday

Refactor arguments and type tests for admin.apps.* APIs.

80c3d5e

Refactor arguments and type tests for admin.auth.* APIs.

d548bff

start of admin.barrier api rework

1d94a70

Refactor arguments and type tests for admin.barriers.* APIs.

3687c89

Refactor arguments and type tests for admin.analytics.getFile API.

94c7c57

part of the way there for admin.conversations APIs

08d1413

Finishing up admin.conversations.* APIs.

7d03359

Refactor arguments and type tests for admin.emoji.* APIs.

38dfcde

Refactor arguments and type tests for admin.functions.* APIs.

692792f

merge in latest main (support for functions.* APIs)

456c49b

Refactor arguments and type tests for functions.* APIs.

2f1f9da

fix bad reference

22efb5a

Refactor arguments and type tests for admin.inviteRequests.* APIs.

0bec7f6

Refactor arguments and type tests for admin.roles.* APIs.

0f59b96

Refactor arguments and type tests for admin.teams.* APIs.

fcebf0e

users.profile.set should accept an object for the profile argument.

b6f1a88

hello-ashleyintech mentioned this pull request Dec 11, 2023

Unable to set user status without a typescript error #1703

Closed

7 tasks

filmaj added 4 commits December 12, 2023 11:39

Refactor arguments and type tests for admin.usergroups.* APIs.

d0526bf

Refactor arguments and type tests for admin.users.* APIs.

260e242

Refactor arguments and type tests for admin.workflows.* APIs.

a193b6e

Refactor arguments and type tests for apps.manifest.* APIs. Add first…

f01e0e6

… pass at typing the manifest.

filmaj marked this pull request as ready for review December 13, 2023 20:45

seratch approved these changes Dec 14, 2023

View reviewed changes

filmaj and others added 3 commits December 14, 2023 08:10

Update packages/web-api/src/types/request/bots.ts

920c3fb

Co-authored-by: Kazuhiro Sera <[email protected]>

7.0.0-rc.0 (#1704)

6e2d515

Tweaks to some JSDocs and being extra careful to allow for text wit…

f340482

…h message-posting APIs, added tests to ensure this remains the case

filmaj removed the draft label Dec 14, 2023

filmaj merged commit d53ef02 into main Jan 15, 2024
15 checks passed

filmaj mentioned this pull request Apr 3, 2024

[FEATURE] Add proper types to clients slackapi/deno-slack-api#97

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Web API Type Safety #1673

Web API Type Safety #1673

filmaj commented Oct 5, 2023 •

edited

Loading

seratch left a comment

seratch Dec 14, 2023

filmaj Dec 14, 2023

seratch Dec 20, 2023

seratch Dec 14, 2023

seratch Dec 14, 2023

filmaj Dec 14, 2023

seratch Dec 14, 2023

filmaj Dec 14, 2023

seratch Dec 14, 2023

seratch Dec 14, 2023

seratch Dec 14, 2023

		@@ -0,0 +1,224 @@
		import type { KnownBlock, Block, MessageAttachment, LinkUnfurls, MessageMetadata } from '@slack/types';

		@@ -0,0 +1,45 @@
		import { expectAssignable, expectError } from 'tsd';

Web API Type Safety #1673

Web API Type Safety #1673

Conversation

filmaj commented Oct 5, 2023 • edited Loading

Open Questions

Deprecations being removed

New Deprecations

Updates/Testing

seratch left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

filmaj commented Oct 5, 2023 •

edited

Loading