node-slack-sdk icon indicating copy to clipboard operation
node-slack-sdk copied to clipboard

Draft: Improve types

Open raycharius opened this issue 3 years ago • 2 comments

Summary

Draft pull request to discussion in this issue around improving @slack/types. See discussion here.

Requirements (place an x in each [ ])

raycharius avatar May 07 '21 12:05 raycharius

I've been playing around with this a bit, and you are totally right. For some reason, I always though of enums as a subset of union types, but it will not work with simple strings.

Naming the union types would definitely make the code more structured, but that would be a change that's more internal for your team as opposed to offering value for developers using the SDK.

There is one other possibility, to have those values available as imports – export an object literal instead of an enum and create union types that reference the values in those objects. Advantage would be having those available as constants instead of working with strings for those using the package, but not sure if that's in scope.

raycharius avatar May 13 '21 08:05 raycharius

What are thoughts on the discriminating union types for more strict type checking?

Advantages:

  • A much better and more type-safe experience with the package and SDK, such as with the WebClient.

Disadvantages:

  • Would require more swift updates to the package if API validation changes.
  • It could be a breaking change, depending on how breaking is defined. It could throw compiler errors for objects that don't comply with the API validation, but unless there are some cases that I'm not taking into consideration, it would readonly only be showing issues with current code that would probably be rejected by the API anyway.

raycharius avatar May 13 '21 08:05 raycharius