Please add API event & errors enums (feature request)

[matanlb]

• I've read and understood the Contributing guidelines and have done my best effort to follow them.
• I've read and agree to the Code of Conduct.
• I've searched for any related issues and avoided creating a duplicate issue.

Description

Currently event names and errors are not included in the SDK, applications using the SDK must hardcode them in the application.
This makes application using the SDK more fragile (changes or addition of error or events can be mediated by a simple package upgrade), and requires constant browsing of the online API.

A simple exported object like the following can go a long way simplifying usage of the SDK:

module.exports.Events = {
  VerificationChallenge: 'url_verification',
  MessageSent: 'message',
.
.
.
}

module.exports.Errors = {
  ChannelNotFound: 'channel_not_found',
  InvalidArgName: 'invalid_arg_name',
.
.
.
}

[aoberoi]

@matanlb i definitely understand your use case and the value these enum exports would provide.

until now, these have been purposely left out in order to allow for flexibility. if we were to start encoding these into enum types as you suggested, the chance for platform changes to introduce new types that aren't met with an SDK release at the exact same time grows. in my opinion, it would be worse for your application code to mix the use of hard-coded strings and SDK defined enums than just using strings for your enum values. in fact, using strings for enums is a common JavaScript pattern in many libraries (and probably will remain until we all start using Symbol more pervasively).

I think the solution to this use case will come from an effort we have internally to generate machine-readable descriptions of the Slack API. if and when that happens, we should have a source of information that always at parity with the platform. i can't tell you today exactly how this will turn into a solution for the need, but i'm confident that that would be a step in the right direction.

if you feel differently, or have a use case you think is super-important that i'm not seeing, id be happy to hear about it!

[matanlb]

@aoberoi thanks for the replay.
I have to say I not sure I'm following to way would the SDK fall back behind the platform when it comes to simple enums. But even if we assume that it does, why is a temporary partial definition is worst then a full user-application side implementation.

Assuming that an enum value had be changed or edited, a re-deployment of to the user-application is required, regardless of whether the events enum is defined in the user code or the SDK.

If the enums are defined in the SDK, most changes to these can be released as a minor or even a patch. Assuming most applications set there update policy to auto-update minor & patch, the user-application will not require any code changes, just a redeployment of the application reinstalling the packages.
In cases when the update to the enum cannot be considered a minor or a patch (can u give an example when this is the case?) user-application can for a short while define them in there own code (which they will have to do anyway in the other scenario) until the SDK catch up. This way most application can enjoy ease of mind when it comes to keeping up with the changing API. (I say most seance I assume changes that require a major update relate to bleeding edge features, so applications using them need to keep track either way).

The alternative to that, where all enums are defined in the user-application code, I think is more fragile. Each change to the enum will require all users to do update the application code.
And while the string-enum pattern is very common, many developers still hard-code these kind values in there code. I agree that this is a bad habit, but aside from the fact then it's still in use, it can provide stability to application how do use enums such as that.

• When would an update the these enum require a major update?
• In what way a temporary user-application patch of the missing enums values is worst?

[aoberoi]

@matanlb i appreciate your thoughtfully explained argument, but i still have some reservations about this proposal.

to start, we're moving in a direction to make the APIs more strongly typed. enums can actually support that goal! but in particular, we're planning to write the next major version of the SDK in TypeScript (#352). that should largely remain an implementation detail and all users of the SDK should think of it as a JavaScript API. but users of TypeScript will enjoy additional benefits when it comes to tooling and static analysis. but what this means is we have to try to express these enumerated types as strong types the way TypeScript considers them.

in typescript, there is a built-in enum keyword for making enumerated types, but it is limited to only number values, not strings as we would need. the general recommendation is to use string literal types to represent a string value enum (see here). this also looks great! but what happens when a new property is added and the SDK doesn't have that property yet. because its not defined as part of the type in the origin definition, the user-application code cannot add a new value to the type. now you are stuck. in the best case, the SDK is updated, a new version is published, and the application just updates (you are right that it would be a minor or patch). but what about the worst case? the worst case is that you are using some undocumented properties, something we never plan to add as an option in the SDK. now you are completely stuck. i want to avoid this.

yes, typescript would impose a limitation on the normal javascript capabilities here. but i actually think this limitation is a good one. it results in APIs that are more static and easier to reason about.

but to be honest, im not a typescript expert, so let me pull in @kwonoj to see if he has any thoughts or opinions.

[kwonoj]

it is limited to only number values, not strings as we would need. the general recommendation is to use string literal types to represent a string value enum

I may need to re-read thread again to understand crux of issue, but for above specific only - thanksfully 2.4RC now support long-waited string based enum (https://github.com/Microsoft/TypeScript/wiki/What's-new-in-TypeScript#string-enums) and probably once sdk starts actual migration, it'll be available in public release.

[matanlb]

@aoberoi thank you for taking the time to read in and explain a bit about TypeScript limitations (I work mainly in JS), but I'm not sure how this is relevant to the issue here.

1. Are you planning to completely deprecate the JS library?
   Yes, TS is transpiled to JS, but in my experience the packages end up being a more difficult to use in a vanilla JS application.
   Should I take from this that this repo is to forsaken (or turn completely TS) in the near future?

2. Why would you want to restrict yourself to a tightly defined enum-type?
   I completely agree with your reasoning regarding the problems of using strongly typed TS enums for this. But since tight-coupling is what we are trying to avoid here, why not just use the classic JS-like "enum" as we've talked earlier? It still solves all of the issues raised here and is completely doable in TS, assuming the methods describe a simple string parameter instead of the typed enum. Only difference is users can choose to either rally on the package defined literals for stability if they want to, and temporarily add missing values between versions. Users how want to define the literals in there own code can still do just that.

I think we can all agree that using string literals directly in the code is bad, especially when used in more then one place. Savvy users of this package are proboblsy already maintaining a version of these enums in there own applications, so why not simply move the implementation inward?

[aoberoi]

@kwonoj as a user of a library, is it possible to "mask" the definition of a type with your own definition? let's say we use the string-based enums in TS 2.4, and the library's enum definition is incomplete, can i as a user tell the typechecker that i have a defintion of that type i'd rather use?

[kwonoj]

@aoberoi for case of interfaces there's way so called augumentation, but that doesn't work with enums so far I know. I have to check once 2.4 released to ensure it though.

aside from possibility, I'd generally not consider as good practices to push forward. While there are some debates around managing versions of types, but it is generally safe type changes are also part of semver changes. If we really want to ship incomplete definition as aligned with current releases, it'd better to make type check loosened instead of strong typed one and let user customize it. i.e,

type messageType = 'type1' | 'type2';
//instead of strong type constraint to `messageType`
const interfaceA = (mType: messageType, ...args: Array<any>);

//loosened to accept string. This doesn't provide strict type checking to block value 
//other than specific type, but `messageType` is already incomplete
//once `messageType` is completes and next time bump up major semver, interface can be narrowed down to `typeof interfaceA`
const interfaceB = (mType: messageType | string, ...args: Array<any>);

[aoberoi]

@matanlb

Regarding 1:

No we are not deprecating this library at all. And I am only willing to accept a move to authoring in TypeScript so long as the APIs are just as usable and idiomatic in JavaScript. I consider the JavaScript API as the first-class citizen of this library now and in the foreseeable future. We will begin to work on the TypeScript authored version in a new branch (and continue to ship 3.x versions that are authored in JS until that branch and other 4.x features are ready). I would love JavaScript developers like you to give us feedback on that work, and I'll make sure there's at least one RC release to make it as easy to try out as possible. I'm not willing to downgrade the experience of a JS-only developer.

Regarding 2:

Now that I've started thinking about this in more detail, and with @kwonoj's feedback, i'm not super confident that a tightly defined type makes sense either. Would you mind helping me understand what you're after a little better? It sounds like the main thing you want to avoid is running back to the documentation website to know which events are available or which errors are possible.

The events are only really exposed in this module using the RtmClient (we have @slack/events-api that deals with the Events API, but that's not what we are discussing). There's already an enum in the package here. these are available in the package as require('@slack/cleint').RTM_EVENTS and require('@slack/client').RTM_MESSAGE_SUBTYPES. does this satisfy what you are looking for regarding events?

The error types are not enumerated at all, but I would assume that if you were to receive an error, you'd be at a point where you are debugging and looking these up in the documentation is the safest place to get the most up to date information regarding that error. How would having the error types enumerated help you? I feel like fully describing the Slack platform's error hierarchy in JavaScript is a lot of effort for a rather small return.

[kwonoj]

So after I read through issue itself, I came to realize this actually needs to be separated with a discussion of TS migration. For this specific matter, typescript has nothing to related to original feature request. It only becomes relevant if we're going to use exported enum list into some of constraint for interfaces, but as we're nowhere to close to export any type definition yet, it's not current scope of discussion. if it's non-runtime type export (i.e, type only or either interface only) then it's something diverge between TS users and JS user that need to be reconsidered. (I'd personally think this is more feasible way to go, but that's personal opinion and should be discussed separately).

Back to original issue, I'm getting impression that request is export run-time-accessible list of types being used in sdk. I can't say if it's useful or not in terms of purpose of sdk and I believe @aoberoi can fullfil this subject. To clarify, if it's going to be runtime values to be exported, either JS or TS will have same access cause TS is just javascript. (Especially to javascript user, there should not be any differences).

[matanlb]

@aoberoi

My primary use case for this is reuse/centralization of this string literals.
Having these declared at the application is an unneeded duplication and makes the integration with the SDK more fragile.
Having them declared just in the example you gave is great. As long as the SDK keeps pace with the backend people can simply upgrade the package version to deal with changes, and in the chance the SDK falls back users needing these new/changed literals can simply add/edit those in the application side (as they need to do now).

Regarding the errors, the use case for that is not for debug, This can definitely be done by digging in the docs. What I'm more concern about is error handling, for places where different behaviors are needed for different errors.
For example for looking for information about a group. If a group is not found, in one of my use cases this is a non issue, so looking at the error message I can suppress it while logging and bubbling all other errors.
And so instead of defining these error message in the application side, it be better to be able to import them from the SDK.
A better but a more development heavy approach would be to create a different Error extending type so it could be leveraged by libraries like bluebird that offer catch error type filtering.

[aoberoi]

I've marked this issue as a discussion so we can continue to refine until we get to one or more specifications for enhancements (which we can turn into new issues).

it sounds like the need for runtime-available event types is already being met to your satisfaction.

i like the idea of Error extending types for some coarse-level error categories (ones which you might want to switch on in a catch). the more fine-level error types could be specified in a code property of the error. I think this is more "node-friendly". in that case, having all codes available in an enumeration would be possible. if this seems to fit your need, i'm happy to create a new enhancement issue to track it.

lastly, i just wanted to say that since i last participated in this discussion, string enums are now available in typescript 2.4. 🎉

[aoberoi]

there's one more dimension to consider here, which are "lifecycle" events (not the events emitted from Slack, but events that have to do with the objects themselves). there's already a dedicated issue to address enumerating types for that: #371.

[aoberoi]

now that v4 is out, it makes sense to kick off this discussion again.

is the new API meeting expectations for Error types? All errors in the SDK now have a code property, and there is a top-level export called ErrorCode, which can be used as a dictionary to compare for available error types.

we haven't solved for event types in an enum, but there is some kind of a precident set that this could be something we support, especially since all the methods argument type information is being codified. we haven't done any typing of responses or events yet, but i'd like to hear if this is something that is important to the community.

[aoberoi]

#509 is relevant to this discussion.

[aoberoi]

closing for lack of feedback.