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.

Sign up for GitHub

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

Jump to bottom

ESLint Config Migration: Disable the only occurrences of no-nested-ternary rule violation #1055

Merged
merged 3 commits into from
Aug 12, 2021
Merged

ESLint Config Migration: Disable the only occurrences of no-nested-ternary rule violation #1055

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
09e48d0
Disable the only occurrences of no-nested-ternary - but is this the r…
filmaj Aug 9, 2021
0a348f3
Attempt at rewriting nested ternary payload assignment into a switch.
filmaj Aug 10, 2021
1b5a162
Specify type of payload when processing events
filmaj Aug 11, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
60 changes: 42 additions & 18 deletions src/App.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,14 @@ import {
Receiver,
ReceiverEvent,
RespondArguments,
DialogSubmitAction,
BlockElementAction,
InteractiveAction,
ViewOutput,
KnownOptionsPayloadFromType,
KnownEventFromType,
SlashCommand,
WorkflowStepEdit,
} from './types';
import { IncomingEventType, getTypeAndConversation, assertNever } from './helpers';
import { CodedError, asCodedError, AppInitializationError, MultipleListenerError } from './errors';
Expand Down Expand Up @@ -719,7 +727,39 @@ export default class App {
};
};

// Set body and payload (this value will eventually conform to AnyMiddlewareArgs)
// Set body and payload
// TODO: this value should eventually conform to AnyMiddlewareArgs
let payload: DialogSubmitAction | WorkflowStepEdit | SlackShortcut | KnownEventFromType<string> | SlashCommand
| KnownOptionsPayloadFromType<string> | BlockElementAction | ViewOutput | InteractiveAction;
switch (type) {
case IncomingEventType.Event:
payload = (bodyArg as SlackEventMiddlewareArgs['body']).event;
break;
case IncomingEventType.ViewAction:
payload = (bodyArg as SlackViewMiddlewareArgs['body']).view;
break;
case IncomingEventType.Shortcut:
payload = (bodyArg as SlackShortcutMiddlewareArgs['body']);
break;
// eslint-disable-next-line @typescript-eslint/ban-ts-comment
// @ts-ignore: Fallthrough case in switch
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We have noFallthroughCasesInSwitch enabled in tsconfig.json, which I think is a sensible default. However, I think this is one situation where a fallthrough in a case in a switch actually works to preserve the logic.

What does everyone think? Too complicated? Too many linter / compiler ignore comments to go along with it?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

// If above conditional does not hit, fall through to fallback payload in default block below

This is reasonable enough, so that I'm happy to ignore the rule here.

case IncomingEventType.Action:
if (isBlockActionOrInteractiveMessageBody(bodyArg as SlackActionMiddlewareArgs['body'])) {
const { actions } = (bodyArg as SlackActionMiddlewareArgs<BlockAction | InteractiveMessage>['body']);
[payload] = actions;
break;
}
// If above conditional does not hit, fall through to fallback payload in default block below
default:
payload = (bodyArg as (
| Exclude<
AnyMiddlewareArgs,
SlackEventMiddlewareArgs | SlackActionMiddlewareArgs | SlackViewMiddlewareArgs
>
| SlackActionMiddlewareArgs<Exclude<SlackAction, BlockAction | InteractiveMessage>>
)['body']);
break;
}
// NOTE: the following doesn't work because... distributive?
// const listenerArgs: Partial<AnyMiddlewareArgs> = {
const listenerArgs: Pick<AnyMiddlewareArgs, 'body' | 'payload'> & {
Expand All @@ -731,23 +771,7 @@ export default class App {
ack?: AckFn<any>;
} = {
body: bodyArg,
payload:
type === IncomingEventType.Event
? (bodyArg as SlackEventMiddlewareArgs['body']).event
: type === IncomingEventType.ViewAction
? (bodyArg as SlackViewMiddlewareArgs['body']).view
: type === IncomingEventType.Shortcut
? (bodyArg as SlackShortcutMiddlewareArgs['body'])
: type === IncomingEventType.Action &&
isBlockActionOrInteractiveMessageBody(bodyArg as SlackActionMiddlewareArgs['body'])
? (bodyArg as SlackActionMiddlewareArgs<BlockAction | InteractiveMessage>['body']).actions[0]
: (bodyArg as (
| Exclude<
AnyMiddlewareArgs,
SlackEventMiddlewareArgs | SlackActionMiddlewareArgs | SlackViewMiddlewareArgs
>
| SlackActionMiddlewareArgs<Exclude<SlackAction, BlockAction | InteractiveMessage>>
)['body']),
payload,
};

// Set aliases
Expand Down
6 changes: 4 additions & 2 deletions src/types/events/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,8 +60,10 @@ interface Authorization {
* When the string matches known event(s) from the `SlackEvent` union, only those types are returned (also as a union).
* Otherwise, the `BasicSlackEvent<T>` type is returned.
*/
type EventFromType<T extends string> = KnownEventFromType<T> extends never ? BasicSlackEvent<T> : KnownEventFromType<T>;
type KnownEventFromType<T extends string> = Extract<SlackEvent, { type: T }>;
export type EventFromType<T extends string> = KnownEventFromType<T> extends never ?
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Exporting these types should be fine.

BasicSlackEvent<T> :
KnownEventFromType<T>;
export type KnownEventFromType<T extends string> = Extract<SlackEvent, { type: T }>;

/**
* Type function which tests whether or not the given `Event` contains a channel ID context for where the event
Expand Down
4 changes: 2 additions & 2 deletions src/types/options/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,11 +25,11 @@ export interface BasicOptionsPayload<Type extends string = string> {
value: string;
}

type OptionsPayloadFromType<T extends string> = KnownOptionsPayloadFromType<T> extends never
export type OptionsPayloadFromType<T extends string> = KnownOptionsPayloadFromType<T> extends never
? BasicOptionsPayload<T>
: KnownOptionsPayloadFromType<T>;

type KnownOptionsPayloadFromType<T extends string> = Extract<SlackOptions, { type: T }>;
export type KnownOptionsPayloadFromType<T extends string> = Extract<SlackOptions, { type: T }>;

/**
* external data source in blocks
Expand Down